-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop FK constraint on media_obj in MediaDecisionThrough, update backfillmoderationdecision command #4530
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 This fix makes sense... and actually gets rid of an unnecessary query too (passing report.media_obj
to the query set would load the media object in first, I think?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the very thorough testing instructions! This worked as expected locally, I'm glad the set of changes was so simple 🚀
5a83c4b
to
a082bce
Compare
This PR overlapped with mine (PR #4544), so I requested changes where we differed. |
This PR has migrations. Please rebase it before merging to ensure that conflicting migrations are not introduced. |
Agreed. That shouldn't be necessary for the backfill but I can go ahead and squash it into this migration 👍 |
Fixes
Fixes #4512 by @krysal
Description
The
backfillmoderationdecision
management command creates backfilled ModerationDecision objects for existing MediaReports that have had some action taken on them (ie, not pending). The command was failing when encountering images that were deleted as part of adeindexed
report, because the image no longer exists in theimage
table.This PR resolves this by:
media_obj
in theAudioDecisionThrough
andImageDecisionThrough
tables. A decision can still reference the record it acted upon by its identifier, even if that record no longer exists in the primary media table.backfillmoderationdecision
mgmt command to both access and set the plain value of the image identifier when creating the ImageDecisionThrough records, rather than attempting to use the image instance (which fails for deleted images).Testing Instructions
Run
just api/recreate
.Go to http://localhost:50280/admin/api/imagereport/ and create several new image reports:
status
deindexedstatus
no_actionstatus
mature_filteredstatus
pendingThere is an issue at the moment where DeletedMedia & SensitiveMedia objects won't be automatically created for these reports. I manually made them by running
just api/ipython
and:Verify in the admin that you can see the newly created DeletedImage/SensitiveImage object.
Now run the management command:
just api/dj backfillmoderationdecision --no-dry-run --media-type image --moderator deploy
You should see 3 moderation decisions were created. Check them out in the admin and make sure they look as expected, and that the reports/images/decisions are all linked together appropriately. The image that was deindexed will appear empty in the admin, since the image does not exist:
However if you run
just api/pgcli
and actually inspect the ImageDecisionThrough table, you can see that it is set appropriately:Checklist
Update index.md
).main
) or a parent feature branch../ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
./ov just catalog/generate-docs media-props
for the catalog or
./ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin