Make media items the centre for all moderation activity #4386

dhruvkb · 2024-05-25T10:11:07Z

Fixes

Fixes #3638 by @sarayourfriend
Fixes #3639 by @sarayourfriend

Description

This PR

makes it possible for moderators to view the media model list and change pages, in addition to accessing decision pages and making changes to decision-media relationships
adds soft-locking and warning messages so that moderators can avoid potential conflicts
makes the media model (and not the media-report model) the root of all moderation activity
adds additional information to media model change page
provides comprehensive history of pending/reviewed reports and past decisions for any media item
adds actions for moderation into the media model change page

This PR also

breaks admin templates into smaller, more-manageable components
adheres more closely to Django idioms for site links in the admin
refactors and organises code in the extremely large model-admin classes

Please note that this PR is intentionally allowed to deviate from some permissions and access controls. These permissions will be tightened in #3640.

Testing Instructions

For the steps involving the Django admin, it is best to use two different profiles (or even browsers) so that you can see the admin from the perspective of a superuser ("deploy") and a moderator ("moderator").

Preparing the data

Take down services (with volumes), bring up the API and initialise.
```
just down -v
just api/up
just api/init
```

Get a few image and audio identifiers.

curl --silent 'http://localhost:50280/v1/images/?page_size=3' | jq '.results[].id'
curl --silent 'http://localhost:50280/v1/audio/?page_size=3' | jq '.results[].id'

File many different reports of many different reasons against these identifiers. You can use this cURL command as a reference.

curl \
  -X POST \
  -H 'content-type: application/json' \
  -H 'accept: application/json, */*;q=0.5' \
  -d '{"reason":"<mature|dmca|other>","description":"<any random message of 20+ chars>"}' \
  http://localhost:50280/v1/<image|audio>/<identifier>/report/

Testing

Access control

Open the Django Admin homepage at /admin.
Ensure that relevant models are visible. User "moderator" should see "Images" and "Audios" and some other selected models, whereas user "admin" should see all registered models.
Open a media item's detail page. User "moderator" should see most fields as read-only, whereas user "admin" should see editable fields. Both should be able to make a decision for pending reports (more info below).

Soft-locking

Open a media item's detail page from "moderator"'s session.
On the media list page in "admin"'s session, you should see that media item highlighted in the warning color.
If you visit the same media item's detail page in "admin"'s session, you will also see a warning message at the top stating that "moderator" is viewing this.
If you now refresh the media item's detail page in "moderator"'s session, you will see the same warning but referencing "admin" instead.
The soft-lock endpoint will be polled as long as the detail page is open. Close the detail page and the locks should be cleared in a few seconds.

Additional info

Open the media list view.
You should see the following additional information:
- column "Has sensitive text?" showing whether the media item has sensitive text
Open a media item's detail page.
You should see the following additional information:
- links to view the item on the API and Openverse.org in the top right corner
- the attribution as a read-only field below the media item's fields
- the license URL as a read-only field below the attribution
- whether the media item has sensitive text as a read-only field below the license URL
- a preview of the media item (which in the case of images, can be blurred and un-blurred by clicking the preview)
- a view of the tags grouped by provider and showing accuracy (if known)
- information about past decisions
- information about reports (pending and reviewed)
  - controls to take a new decision for pending reports

Moderation actions

Open a media item's detail page.
Go to the report's list at the bottom of the page.
Select any number of pending reports.
Choose an action and enter moderation notes (optional).
Click on the "Create decision" button. The page will reload (the form submits to the /moderate endpoint which redirects back to /change endpoint).
Verify the results.
- The new decision should appear under the decision section (with the selected reports shown in the last column).
- The selected reports should no longer be pending (with the checkbox gone and the decision shown in the last column).

Production deferred

Go to the page to create a new report /admin/api/imagereport/add/. You should see an autocomplete field for "Media obj".
Similarly go to the page to create a new decision /admin/api/audiodecision/add/. You should see an inline tabular admin with autocomplete field for "Media obj".
Now switch your current ENVIRONMENT to "production". These fields should be replaced with raw_id_fields that should offer considerably better performance.

Report admin pages (list, create, change)

The list of reports is shown in a table identical to that present inside the media item change page.
The page to change the report does not allow changing the "Media obj".
The page to create a new report hides away fields that do not make sense before and during filing of reports.

Decision admin pages (list, create, change)

The list of decisions is shown in a table identical to that present inside the media item change page.
The page to change the decision freezes the set of media-decision-through models and does not allow changing them.
The page to create a new decision hides away fields that do not make sense before and during filing of report, and populates the moderator from request.user automatically.

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
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 · 2024-05-27T23:22:11Z

api/api/utils/moderation_lock.py

+def handle_redis_exception(func):
+    @functools.wraps(func)
+    def wrapper(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except ConnectionError:
+            return None
+
+    return wrapper


Nice idea. I wish we could use something like this in other parts of the app that handle Redis connection failures to reduce all those try/except blocks.

sarayourfriend · 2024-05-27T23:43:23Z

Here's a one-liner for generating the reports using the commands you shared, Dhruv, for anyone else testing:

curl --silent 'http://localhost:50280/v1/audio/?page_size=3' | jq -r '.results[].id' | xargs -I{} curl \
  -X POST \
  -H 'content-type: application/json' \
  -H 'accept: application/json, */*;q=0.5' \
  -d '{"reason":"mature","description":"blah blah blah"}' \
  http://localhost:50280/v1/audio/\{\}/report/

Replace audio with images for images, etc.

sarayourfriend

@dhruvkb I think we can get rid of the logic for the media report view and make it just a static view of the media report contents, if that would simplify things. Actioning reports already works from the media view, so this wouldn't affect the ability to moderate reports in prod even though the rest of these moderation workflow features aren't fully finished.

Otherwise, this LGTM, none of my comments are blocking.

api/api/admin/media_report.py

api/api/utils/moderation_lock.py

api/api/admin/media_report.py

sarayourfriend · 2024-05-29T01:33:46Z

@dhruvkb deferring those two further changes sounds good to me, can you create issues for them so they are tracked?

Co-authored-by: sarayourfriend <[email protected]>

sarayourfriend · 2024-05-29T06:46:24Z

@dhruvkb I caused some nasty merge conflicts for you, sorry! I've got a rebased version of this branch I'll push up in a moment once I have it fully back to the working condition of this branch for you to use and not need to do the rebase yourself.

sarayourfriend · 2024-05-29T06:52:20Z

The branch media_admin-rebased is this one but rebased on main with tests passing.

dhruvkb · 2024-05-29T07:11:24Z

@sarayourfriend thank you very much for resolving the conflicts! I compared the diffs and apart from some reorganisation, the changes lined up. So I updated this branch with your code.

zackkrida · 2024-05-29T19:52:59Z

@dhruvkb I'll be reviewing this in the next 24hrs, apologies for the delay! I was so excited by this PR and following its progress that I actually failed to observe that I was one of the assigned reviewers.

…dia_admin

AetherUnbound

This is excellent Dhruv, thanks for your hard work on this! I've tested out a lot of the flows and added a few comments that should be addressed but don't block a merge.

I did try out viewing two media items with different accounts, and then closing one of the tabs. I wasn't able to see the message disappear from the other tab, even after waiting 10 seconds. I did see this error in the console when loading the image change pages:

Uncaught TypeError: document.getElementById(...) is null
    Actions http://localhost:50280/static/admin/js/actions.js:105
    <anonymous> http://localhost:50280/static/admin/js/actions.js:198
actions.js:105:18
    Actions http://localhost:50280/static/admin/js/actions.js:105
    <anonymous> http://localhost:50280/static/admin/js/actions.js:198

Not sure if that's related. Regardless, that can be fixed in a fast-follow IMO (or part of this PR, either way).

AetherUnbound · 2024-05-30T17:52:25Z

api/api/admin/media_report.py

+        # Start of block lifted from Django source.
+        from django.urls import path


Any reason to not have this import hoisted to the top of the file?

The only reason is because that is how it was in the Django source file I copied from. I can hoist it, it would not change the behaviour at all.

api/api/admin/media_report.py

api/api/models/audio.py

api/api/templates/admin/api/media/change_list.html

Co-authored-by: Madison Swain-Bowden <[email protected]>

openverse-bot added the 🕹 aspect: interface Concerns end-users' experience with the software label May 26, 2024

dhruvkb mentioned this pull request May 26, 2024

Enable soft-locking functionality for media reports to assist simultaneous moderators #4374

Closed

8 tasks

dhruvkb force-pushed the media_admin branch from 43e98d1 to 7983b53 Compare May 27, 2024 22:06

sarayourfriend reviewed May 27, 2024

View reviewed changes

dhruvkb marked this pull request as ready for review May 27, 2024 23:32

dhruvkb requested review from a team as code owners May 27, 2024 23:33

dhruvkb requested review from zackkrida and sarayourfriend May 27, 2024 23:33

sarayourfriend approved these changes May 28, 2024

View reviewed changes

dhruvkb added 12 commits May 29, 2024 16:38

Create template tag get_attr which will be useful later

b63525c

Define get_absolute_url for media models

ec9b28e

Make media change view accessible

12c4c91

Create form to create decisions for media

4e004ca

Section parts of huge ModelAdmin class

ecb005e

Create utility for soft-lock management

1e7310b

Create and save LockManager instance in MediaListAdmin

691a93d

Create lock endpoint

2fda662

Override change view with extra context

cb7f8fb

Add preview and tags to the media change view

9d9cd54

Document list of decisions for a media item

10bfa62

Define a form to handle creation of decisions

58e581a

dhruvkb and others added 4 commits May 29, 2024 16:45

Do not highlight a moderator's own locks in the list view

8e621fe

Co-authored-by: sarayourfriend <[email protected]>

Use .get_username() instead of .username

fd6c7fd

Credit the source of the ranked-set idea

5f61e2c

Co-authored-by: sarayourfriend <[email protected]>

Avoid use of ambiguous term "shadow"

83df8a4

Fix imports missed during rebase

b120145

dhruvkb force-pushed the media_admin branch from 50c420c to b120145 Compare May 29, 2024 08:10

dhruvkb added 2 commits May 29, 2024 12:13

Fix linting problem

2bb855f

Remove the decision creation form if there are no pending reports

4bf0117

dhruvkb added 🟧 priority: high Stalls work on the project or its dependents and removed 🟨 priority: medium Not blocking but should be addressed soon labels May 30, 2024

dhruvkb mentioned this pull request May 30, 2024

Include report filing in admin media view #4413

Closed

Merge branch 'main' of https://github.com/WordPress/openverse into me…

7d29cf8

…dia_admin

AetherUnbound approved these changes May 30, 2024

View reviewed changes

dhruvkb and others added 10 commits May 30, 2024 23:10

Move list of reports above list of decisions

e789172

Replace hardcoded labels with .label_tag and add help text

3766e06

Add colons to label for consistency

dcec561

Use model-form for code clarity

8c69e14

Change "complaining about' to "referencing"

a2851cf

Co-authored-by: Madison Swain-Bowden <[email protected]>

Use correct reverse URL for audio detail

d0d3b03

Co-authored-by: Madison Swain-Bowden <[email protected]>

Bulk update reports with new decision

9e5e3b2

Remove class causing errors in browser console

7c222b9

Explain the purpose of the script in a comment

e799814

Explain slice assignment syntax and purpose

90b9fbe

dhruvkb merged commit 1f24ba1 into main May 30, 2024
47 checks passed

dhruvkb deleted the media_admin branch May 30, 2024 20:53

AetherUnbound mentioned this pull request Jun 3, 2024

Prevent Django Admin default queries on primary media tables in production #4344

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make media items the centre for all moderation activity #4386

Make media items the centre for all moderation activity #4386

dhruvkb commented May 25, 2024 •

edited

Loading

sarayourfriend May 27, 2024

sarayourfriend commented May 27, 2024

sarayourfriend left a comment

sarayourfriend commented May 29, 2024

sarayourfriend commented May 29, 2024

sarayourfriend commented May 29, 2024 •

edited

Loading

dhruvkb commented May 29, 2024 •

edited

Loading

zackkrida commented May 29, 2024

AetherUnbound left a comment

AetherUnbound May 30, 2024

dhruvkb May 30, 2024

		# Start of block lifted from Django source.
		from django.urls import path

Make media items the centre for all moderation activity #4386

Make media items the centre for all moderation activity #4386

Conversation

dhruvkb commented May 25, 2024 • edited Loading

Fixes

Description

Testing Instructions

Preparing the data

Testing

Access control

Soft-locking

Additional info

Moderation actions

Production deferred

Report admin pages (list, create, change)

Decision admin pages (list, create, change)

Checklist

Developer Certificate of Origin

sarayourfriend May 27, 2024

Choose a reason for hiding this comment

sarayourfriend commented May 27, 2024

sarayourfriend left a comment

Choose a reason for hiding this comment

sarayourfriend commented May 29, 2024

sarayourfriend commented May 29, 2024

sarayourfriend commented May 29, 2024 • edited Loading

dhruvkb commented May 29, 2024 • edited Loading

zackkrida commented May 29, 2024

AetherUnbound left a comment

Choose a reason for hiding this comment

AetherUnbound May 30, 2024

Choose a reason for hiding this comment

dhruvkb May 30, 2024

Choose a reason for hiding this comment

dhruvkb commented May 25, 2024 •

edited

Loading

sarayourfriend commented May 29, 2024 •

edited

Loading

dhruvkb commented May 29, 2024 •

edited

Loading