Add backfillmoderationdecision management command

[krysal]

Fixes

Fixes #3641 by @sarayourfriend

Description

This PR adds the command as described in the implementation plan section for Deprecating and removing report status. It takes as options the media type, the username of the moderator (in production it should be opener but in local we use deploy) and a flag for whether to run the command or just get the count of report to process.

Verify that the mapping is according to the specification.

Testing Instructions

To test manually, create some reports, mark them with a reason other than "pending" and run the command.

just dj backfillmoderationdecision [--dry-run | --no-dry-run] [--media-type {image,audio}] --moderator deploy

Checklist

• My pull request has a descriptive title (not a vague title likeUpdate index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.
• I ran the DAG documentation generator (just catalog/generate-docs for catalog
  PRs) or the media properties generator (just catalog/generate-docs media-props
  for the catalog or just api/generate-docs for the API) where applicable.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[sarayourfriend]

Left a comment with just some fiddly ORM details, but if the number of non-pending reports in production is low enough, it probably won't even matter.

[stacimc]

In my tests, for both Image and Audio reports with the status "mature_filtered" did not get their action set to "confirmed_sensitive" as I expected (both had an empty action) 🤔 Are you able to reproduce that, @krysal?

[krysal]

@stacimc I do get the "confirmed_sensitive" action for a "mature_filtered" report. What output for the command do you get?

[sarayourfriend]

@krysal I don't see any MediaDecisionThrough being created when I run this. The decisions are created, but not the through table records. I thought this would happen automatically, but it doesn't. What's more, it isn't possible to set it directly on the media, it needs to be handled through image.imagedecision_set on the media (or equivalent for audio), or on the decision with decision.media_objs.set(). But, we can also bulk create it anyway, if we have to do it manually, and this appears to work for me locally:

diff --git a/api/api/management/commands/backfillmoderationdecision.py b/api/api/management/commands/backfillmoderationdecision.py
index f88e40769..e9340604a 100644
--- a/api/api/management/commands/backfillmoderationdecision.py
+++ b/api/api/management/commands/backfillmoderationdecision.py
@@ -5,7 +5,7 @@ from django.contrib.auth import get_user_model
 from django_tqdm import BaseCommand
 
 from api.constants.moderation import DecisionAction
-from api.models import AudioDecision, AudioReport, ImageDecision, ImageReport
+from api.models import AudioDecision, AudioReport, ImageDecision, ImageReport, ImageDecisionThrough, AudioDecisionThrough
 from api.models.media import DMCA, MATURE_FILTERED, NO_ACTION, PENDING
 
 
@@ -43,9 +43,11 @@ class Command(BaseCommand):
 
         MediaReport = ImageReport
         MediaDecision = ImageDecision
+        MediaDecisionThrough = ImageDecisionThrough
         if media_type == "audio":
             MediaReport = AudioReport
             MediaDecision = AudioDecision
+            MediaDecisionThrough = AudioDecisionThrough
 
         non_pending_reports = MediaReport.objects.filter(decision=None).exclude(
             status=PENDING
@@ -83,6 +85,15 @@ class Command(BaseCommand):
             for report, decision in zip(reports_chunk, decisions):
                 report.decision = decision
             MediaReport.objects.bulk_update(reports_chunk, ["decision"])
+            MediaDecisionThrough.objects.bulk_create(
+                [
+                    MediaDecisionThrough(
+                        media_obj_id=report.media_obj_id,
+                        decision_id=report.decision_id,
+                    )
+                    for report in reports_chunk
+                ]
+            )
             t.update(1)
 
         t.info(

[sarayourfriend]

LGTM except for the issue with the decision through table, which I've described in a comment. Otherwise this is working fine when I test it, I did not have the issue Staci saw and explicitly made sure a decision was created in that case. However, I wonder if Staci is seeing an issue in Django admin caused by the lack of the decision through? Not sure what the downstream effects are currently of a missing decision through row for a media object.

[dhruvkb]

I'll hold off on reviewing this PR till you've had a chance to address @sarayourfriend's requests for changes.

The relationship between the media object and the decision can be created more succinctly using this

decision.media_objs.add(media_obj)

where decision is the ImageDecision/AudioDecision instance and media_obj is the Image/Audio instance respectively. There is code very similar to the one you've created in the media_report.py file (maybe there is scope to extract them both into a separate util, but that might be me prematurely DRY-ing)

openverse/api/api/admin/media_report.py

Lines 443 to 468 in 2da7be1

	decision = form.save(commit=False)
	decision.moderator = request.user
	decision.save()
	logger.info(
	"Decision created",
	decision=decision.id,
	action=decision.action,
	notes=decision.notes,
	moderator=request.user.get_username(),
	)
	decision.media_objs.add(media_obj)
	logger.info(
	"Media linked to decision",
	decision=decision.id,
	media_obj=media_obj.id,
	)
	reports = form.cleaned_data["reports"]
	count = reports.update(decision=decision)
	logger.info(
	"Decision recorded in reports",
	report_count=count,
	decision=decision.id,
	)

[sarayourfriend]

The problem with doing it through the decision is that you have to go one-by-one, there's no way to make it a bulk operation; hence why I suggested bulk_create within the existing bulk creation loop. The relationships here are always 1-to-1, because of the nature of the backfill.

In other cases, where a single decision is being created for multiple media objects, then yes, setting it on the decision (or media) instance is more straightforward and doesn't require manually creating the through table instances. In this case, though, we should use bulk create, in line with the rest of the code.

[krysal]

@sarayourfriend Good catch! I think the bulk updates of MediaDecision skip the MediaDecisionThrough table/model. Thanks for the handy diff, the changes have been applied.

[stacimc]

LGTM! No longer seeing issues with confimed_sensitive. I checked all action types and double checked the relationships are all set correctly, looks great 👍

[sarayourfriend]

LGTM!