diff --git a/api/api/admin/media_report.py b/api/api/admin/media_report.py index 7d5c719589d..50e4f96ad0f 100644 --- a/api/api/admin/media_report.py +++ b/api/api/admin/media_report.py @@ -1,9 +1,13 @@ +from functools import update_wrapper from typing import Sequence +from django import forms from django.conf import settings -from django.contrib import admin +from django.contrib import admin, messages from django.contrib.admin.views.main import ChangeList from django.db.models import Count, F, Min +from django.http import JsonResponse +from django.shortcuts import redirect from django.urls import reverse from django.utils.html import format_html from django.utils.safestring import mark_safe @@ -11,18 +15,19 @@ import structlog from elasticsearch import NotFoundError from elasticsearch_dsl import Search -from openverse_attribution.license import License from api.models import ( - PENDING, Audio, + AudioDecision, + AudioDecisionThrough, AudioReport, Image, + ImageDecision, + ImageDecisionThrough, ImageReport, ) -from api.models.audio import AudioDecision -from api.models.image import ImageDecision from api.models.media import AbstractDeletedMedia, AbstractSensitiveMedia +from api.utils.moderation_lock import LockManager logger = structlog.get_logger(__name__) @@ -41,10 +46,56 @@ def register(site): ]: site.register(klass, MediaSubreportAdmin) - # Temporary addition of model admin for decisions while this view gets built - if settings.ENVIRONMENT != "production": - site.register(ImageDecision, admin.ModelAdmin) - site.register(AudioDecision, admin.ModelAdmin) + site.register(ImageDecision, ImageDecisionAdmin) + site.register(AudioDecision, AudioDecisionAdmin) + + +class MultipleValueField(forms.MultipleChoiceField): + """ + This is a variant of ``MultipleChoiceField`` that does not validate + the individual values. + """ + + def valid_value(self, value): + return True + + +def get_decision_form(media_type: str): + decision_class, report_class = { + "image": (ImageDecision, ImageReport), + "audio": (AudioDecision, AudioReport), + }[media_type] + + class MediaDecisionForm(forms.ModelForm): + report_id = MultipleValueField() # not rendered using its widget + + class Meta: + model = decision_class + fields = ["action", "notes"] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + for field in self.fields.values(): + field.widget.attrs.update({"form": "decision-create"}) + + def clean_report_id(self): + report_ids = set(self.cleaned_data["report_id"]) + report_qs = report_class.objects.filter( + decision=None, + id__in=report_ids, + ) + retrieved_report_ids = set( + str(val) for val in report_qs.values_list("id", flat=True) + ) + if diff := (report_ids - retrieved_report_ids): + raise forms.ValidationError( + "No pending reports found for IDs %(value)s.", + params={"value": ", ".join(diff)}, + ) + self.cleaned_data["reports"] = report_qs + return report_ids + + return MediaDecisionForm def _production_deferred(*values: str) -> Sequence[str]: @@ -61,6 +112,20 @@ def _production_deferred(*values: str) -> Sequence[str]: return values +def _non_production_deferred(*values: str) -> Sequence[str]: + """ + Define a sequence in only the production environment. + + The raw ID field is perfectly suited for massive tables, and so enabling + that in production will often prevent performance hits or outages. This will + return the input values only the environment is production, and in all other + cases it will return an empty sequence. + """ + if settings.ENVIRONMENT != "production": + return () + return values + + class PredeterminedOrderChangelist(ChangeList): """ ChangeList class which does not apply any default ordering to the items. @@ -99,19 +164,92 @@ def queryset(self, request, queryset): class MediaListAdmin(admin.ModelAdmin): + media_type = None + + def __init__(self, *args, **kwargs): + self.lock_manager = LockManager(self.media_type) + + super().__init__(*args, **kwargs) + + def get_urls(self): + # Start of block lifted from Django source. + from django.urls import path + + def wrap(view): + def wrapper(*args, **kwargs): + return self.admin_site.admin_view(view)(*args, **kwargs) + + wrapper.model_admin = self + return update_wrapper(wrapper, view) + + app, model = self.opts.app_label, self.opts.model_name + # End of block lifted from Django source. + + urls = super().get_urls() + + # Using slice assignment (docs: + # https://docs.python.org/3/tutorial/introduction.html#lists), + # insert custom URLs at the penultimate position so that they + # appear just before the catch-all view. + urls[-1:-1] = [ + path( + "/moderate/", + wrap(self.moderate_view), + name=f"{app}_{model}_moderate", + ), + path( + "/lock/", + wrap(self.lock_view), + name=f"{app}_{model}_lock", + ), + ] + return urls + + @admin.display(description="Has sensitive text?", boolean=True) + def has_sensitive_text(self, obj): + """ + Determine if the item has sensitive text. + + If the item cannot be found in the filtered index, that means it + was filtered out due to text sensitivity. + + This is displayed both as a column in the list page as well as a + read-only field in the change page. + + :param obj: the item to check for presence of sensitive text + :return: whether the item has sensitive text + """ + + filtered_index = f"{settings.MEDIA_INDEX_MAPPING[self.media_type]}-filtered" + try: + search = ( + Search(index=filtered_index) + .query("term", identifier=obj.identifier) + .execute() + ) + if search.hits: + return False + except NotFoundError: + logger.error("Could not resolve index.", name=filtered_index) + return True + + ############# + # List view # + ############# + + change_list_template = "admin/api/media/change_list.html" list_display = ( "identifier", "total_report_count", "pending_report_count", "oldest_report_date", "pending_reports_links", + "has_sensitive_text", ) list_filter = (PendingRecordCountFilter,) - # Disable link display for images - list_display_links = None + list_display_links = ("identifier",) search_fields = _production_deferred("identifier") - media_type = None - # Ordering is not set here, see get_queryset + sortable_by = () # Ordering is defined in ``get_queryset``. def total_report_count(self, obj): return obj.total_report_count @@ -134,6 +272,154 @@ def pending_reports_links(self, obj): return mark_safe(", ".join(data)) + def changelist_view(self, request, extra_context=None): + extra_context = extra_context or {} + + extra_context["media_type"] = self.media_type + + valid_locks = self.lock_manager.prune() + locked_media = list( + int(item.replace(f"{self.media_type}:", "")) + for moderator, lock_set in valid_locks.items() + for item in lock_set + if self.media_type in item and moderator != request.user.get_username() + ) + extra_context["locked_media"] = locked_media + + return super().changelist_view(request, extra_context) + + ############### + # Change view # + ############### + + change_form_template = "admin/api/media/change_form.html" + readonly_fields = ( + "attribution", + "license_url", + "has_sensitive_text", + ) + + def change_view(self, request, object_id, form_url="", extra_context=None): + # Populate a warning message for locked items. + mods = self.lock_manager.moderator_set(object_id) + mods -= {request.user.get_username()} + if len(mods): + messages.warning( + request, + f"This {self.media_type} is also being viewed by {', '.join(mods)}.", + ) + + # Expand the context based on the template's needs. + extra_context = extra_context or {} + + extra_context["media_type"] = self.media_type + + media_obj = self.get_object(request, object_id) + if media_obj: + extra_context["media_obj"] = media_obj + + tags_by_provider = {} + if tags := media_obj.tags: + for tag in tags: + text = tag["name"] + if acc := tag.get("accuracy"): + text = f"{text} ({acc})" + tags_by_provider.setdefault(tag["provider"], []).append(text) + extra_context["tags"] = tags_by_provider + + manager = getattr(media_obj, f"{self.media_type}decisionthrough_set") + decision_throughs = manager.order_by("decision__created_on") + extra_context["decision_throughs"] = decision_throughs + + manager = getattr(media_obj, f"{self.media_type}_report") + reports = manager.order_by("-created_at") + extra_context["reports"] = reports + + pending_report_count = reports.filter(decision_id=None).count() + extra_context["pending_report_count"] = pending_report_count + + extra_context["mod_form"] = get_decision_form(self.media_type)() + + return super().change_view(request, object_id, form_url, extra_context) + + ############# + # Lock view # + ############# + + def lock_view(self, request, object_id): + """ + Softly lock the media object with the current user to notify + other moderators about a potential conflict. + """ + + if request.method == "POST": + expiration = self.lock_manager.add_locks( + request.user.get_username(), object_id + ) + return JsonResponse( + data={"expiration": expiration}, + status=503 if expiration == 0 else 200, + ) + + return redirect(f"admin:api_{self.media_type}_change", object_id) + + ################# + # Moderate view # + ################# + + def moderate_view(self, request, object_id): + """ + Create a decision for the media object and associate selected + reports referencing the media with this decision. + """ + + if request.method == "POST": + media_obj = self.get_object(request, object_id) + + form = get_decision_form(self.media_type)(request.POST) + if form.is_valid(): + 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, + ) + else: + logger.warning( + "Form is invalid", + **form.cleaned_data, + errors=form.errors, + ) + + return redirect(f"admin:api_{self.media_type}_change", object_id) + + ############# + # Overrides # + ############# + + # TODO: This construct breaks down if a decision is associated with + # a media item that does not have any reports. Such an item cannot + # be reached at the URL ``admin/api/{media_type}/{id}/change/``. def get_queryset(self, request): qs = super().get_queryset(request) # Return all available image if this is for an autocomplete request @@ -163,131 +449,154 @@ def get_changelist(self, request, **kwargs): class MediaReportAdmin(admin.ModelAdmin): - change_form_template = "admin/api/media_report/change_form.html" - list_display = ("id", "reason", "is_pending", "description", "created_at", "url") + media_type = None + + @admin.display(description="Is pending?", boolean=True) + def is_pending(self, obj): + """ + Set an explicit display type for the ``is_pending`` property. + + This is required so that the property, which otherwise renders + "True" or "False" strings, now renders as check/cross icons in + Django Admin. + """ + + return obj.is_pending + + ############# + # List view # + ############# + + list_display = ( + "id", + "created_at", + "reason", + "description", + "is_pending", + "media_id", # used because ``media_obj`` does not render a link + ) list_filter = ( - ("decision", admin.EmptyFieldListFilter), # ~status, i.e. pending or moderated "reason", + ("decision", admin.EmptyFieldListFilter), # ~is_pending ) - list_display_links = ("id",) list_select_related = ("media_obj",) - search_fields = _production_deferred("description", "media_obj__identifier") - autocomplete_fields = _production_deferred("media_obj") - actions = None - media_type = None + search_fields = ("description", *_production_deferred("media_obj__identifier")) - def get_fieldsets(self, request, obj=None): - if obj is None: - return [ - ( - "Report details", - {"fields": ["status", "decision", "reason", "description"]}, - ), - ("Media details", {"fields": ["media_obj"]}), - ] - return [ - ( - "Report details", - { - "fields": [ - "created_at", - "status", - "decision", - "reason", - "description", - "has_sensitive_text", - ], - }, - ), - ] + @admin.display(description="Media obj") + def media_id(self, obj): + path = reverse(f"admin:api_{self.media_type}_change", args=(obj.media_obj.id,)) + return format_html(f'{obj.media_obj}') - def get_exclude(self, request, obj=None): - # ``identifier`` cannot be edited on an existing report. - if request.path.endswith("/change/"): - return ["media_obj"] + ############### + # Change view # + ############### + + autocomplete_fields = ("decision", *_production_deferred("media_obj")) + raw_id_fields = _non_production_deferred("media_obj") + actions = None def get_readonly_fields(self, request, obj=None): - if obj is None: - return [] - readonly_fields = [ + if obj is None: # Create form + return () + # These fields only make sense after a report has been created. + # Hence they are only shown in the change form. + return ( "created_at", - "reason", - "description", - "has_sensitive_text", - "media_obj_id", - ] - # ``status`` cannot be changed on a finalised report. - if obj.status != PENDING: - readonly_fields.append("status") - return readonly_fields + "is_pending", + "media_obj", + ) - @admin.display(description="Has sensitive text") - def has_sensitive_text(self, obj): - """ - Return `True` if the item cannot be found in the filtered index - which means the item - was filtered out due to text sensitivity. - """ - if not self.media_type or not obj: - return None + def get_exclude(self, request, obj=None): + if obj is None: # Create form + # The decision will be linked to the report after it has + # been created, not during. + return ("decision",) + else: # Change form + # In the change form, we do not want to allow the media + # object to be changed. + return ("media_obj",) - filtered_index = f"{settings.MEDIA_INDEX_MAPPING[self.media_type]}-filtered" - try: - search = ( - Search(index=filtered_index) - .query("term", identifier=obj.media_obj.identifier) - .execute() + +class MediaDecisionAdmin(admin.ModelAdmin): + media_type = None + through_model = None + + ############# + # List view # + ############# + + list_display = ( + "id", + "created_on", + "moderator", + "action", + "notes", + "media_ids", + ) + list_filter = ("moderator", "action") + list_prefetch_related = ("media_objs",) + search_fields = ("notes", *_production_deferred("media_objs__identifier")) + + @admin.display(description="Media objs") + def media_ids(self, obj): + through_objs = getattr(obj, f"{self.media_type}decisionthrough_set").all() + text = [] + for obj in through_objs: + path = reverse( + f"admin:api_{self.media_type}_change", args=(obj.media_obj.id,) ) - if search.hits: - return False - except NotFoundError: - logger.error(f"Could not resolve index {filtered_index}") - return None - return True + text.append(f'• {obj.media_obj}') + return format_html("
".join(text)) - def get_other_reports(self, obj): - if not self.media_type or not obj: - return [] + ############### + # Change view # + ############### - reports = ( - self.model.objects.filter(media_obj__identifier=obj.media_obj.identifier) - .exclude(id=obj.id) - .order_by("created_at") + def get_readonly_fields(self, request, obj=None): + if obj is None: + return () + # These fields only make sense after a decision has been created. + # Moderator is set automatically and cannot be changed. + return ( + "created_on", + "moderator", ) - return reports - def _get_media_obj_data(self, obj): - tags_by_provider = {} - if obj.media_obj.tags: - for tag in obj.media_obj.tags: - tags_by_provider.setdefault(tag["provider"], []).append(tag["name"]) - additional_data = { - "other_reports": self.get_other_reports(obj), - "media_obj": obj.media_obj, - "license": License( - obj.media_obj.license, - obj.media_obj.license_version, - ).full_name, - "tags": tags_by_provider, - "description": obj.media_obj.meta_data.get("description", ""), - } - logger.info(f"Additional data: {additional_data}") - return additional_data + def get_exclude(self, request, obj=None): + if obj is None: # Create form + # Moderator is set automatically and cannot be changed. + return ("moderator",) + return () - def change_view(self, request, object_id, form_url="", extra_context=None): - extra_context = extra_context or {} - extra_context["media_type"] = self.media_type + def get_inlines(self, request, obj=None): + if obj is None: + # New decision, can make changes to the media objects. + is_mutable = True + else: + # Once created, media objects associated with decisions are + # immutable. + is_mutable = False - obj = self.get_object(request, object_id) - if obj and obj.media_obj: - additional_data = self._get_media_obj_data(obj) - extra_context = {**extra_context, **additional_data} + class MediaDecisionThroughAdmin(admin.TabularInline): + model = self.through_model + extra = 1 + autocomplete_fields = _production_deferred("media_obj") + raw_id_fields = _non_production_deferred("media_obj") - return super().change_view( - request, - object_id, - form_url, - extra_context=extra_context, - ) + def has_add_permission(self, request, obj=None): + return is_mutable and super().has_change_permission(request, obj) + + def has_change_permission(self, request, obj=None): + return is_mutable and super().has_change_permission(request, obj) + + def has_delete_permission(self, request, obj=None): + return is_mutable and super().has_delete_permission(request, obj) + + return (MediaDecisionThroughAdmin,) + + def save_model(self, request, obj, form, change): + obj.moderator = request.user + return super().save_model(request, obj, form, change) class ImageReportAdmin(MediaReportAdmin): @@ -306,6 +615,16 @@ class AudioListViewAdmin(MediaListAdmin): media_type = "audio" +class ImageDecisionAdmin(MediaDecisionAdmin): + media_type = "image" + through_model = ImageDecisionThrough + + +class AudioDecisionAdmin(MediaDecisionAdmin): + media_type = "audio" + through_model = AudioDecisionThrough + + class MediaSubreportAdmin(admin.ModelAdmin): exclude = ("media_obj",) search_fields = ("media_obj__identifier",) diff --git a/api/api/models/__init__.py b/api/api/models/__init__.py index fb1adcfaa16..bb99e0809b3 100644 --- a/api/api/models/__init__.py +++ b/api/api/models/__init__.py @@ -2,13 +2,23 @@ from api.models.audio import ( AltAudioFile, Audio, + AudioDecision, + AudioDecisionThrough, AudioList, AudioReport, AudioSet, DeletedAudio, SensitiveAudio, ) -from api.models.image import DeletedImage, Image, ImageList, ImageReport, SensitiveImage +from api.models.image import ( + DeletedImage, + Image, + ImageDecision, + ImageDecisionThrough, + ImageList, + ImageReport, + SensitiveImage, +) from api.models.media import ( DEINDEXED, DMCA, diff --git a/api/api/models/audio.py b/api/api/models/audio.py index d945e373473..f1863ea4060 100644 --- a/api/api/models/audio.py +++ b/api/api/models/audio.py @@ -251,6 +251,13 @@ def get_or_create_waveform(self): class Meta(AbstractMedia.Meta): db_table = "audio" + def get_absolute_url(self): + """Enable the "View on site" link in the Django Admin.""" + + from django.urls import reverse + + return reverse("audio-detail", args=[str(self.identifier)]) + class DeletedAudio(AbstractDeletedMedia): """ diff --git a/api/api/models/image.py b/api/api/models/image.py index 1e9fc3a8e77..3d361d473f7 100644 --- a/api/api/models/image.py +++ b/api/api/models/image.py @@ -54,6 +54,13 @@ class Image(ImageFileMixin, AbstractMedia): class Meta(AbstractMedia.Meta): db_table = "image" + def get_absolute_url(self): + """Enable the "View on site" link in the Django Admin.""" + + from django.urls import reverse + + return reverse("image-detail", args=[str(self.identifier)]) + @property def sensitive(self) -> bool: return hasattr(self, "sensitive_image") diff --git a/api/api/models/moderation.py b/api/api/models/moderation.py index 26bd67c18ac..5a6f30d8255 100644 --- a/api/api/models/moderation.py +++ b/api/api/models/moderation.py @@ -1,5 +1,7 @@ from django.conf import settings +from django.contrib.auth import get_user_model from django.db import models +from django.db.models import Q from django.db.models.signals import post_save from django.dispatch import receiver @@ -9,7 +11,7 @@ class UserPreferences(models.Model): preferences = models.JSONField(default=dict) def __str__(self): - return f"{self.user.username}'s preferences" + return f"{self.user.get_username()}'s preferences" @property def moderator(self): @@ -36,3 +38,17 @@ def create_or_update_user_profile(sender, instance, created, **kwargs): if created: UserPreferences.objects.create(user=instance) instance.userpreferences.save() + + +def get_moderators() -> models.QuerySet: + """ + Get all users who either are members of the "Content Moderators" + group or have superuser privileges. + + :return: a ``QuerySet`` of ``User``s who can perform moderation + """ + + User = get_user_model() + return User.objects.filter( + Q(groups__name="Content Moderators") | Q(is_superuser=True) + ) diff --git a/api/api/templates/admin/api/components/media_additional.html b/api/api/templates/admin/api/components/media_additional.html new file mode 100644 index 00000000000..7567726a182 --- /dev/null +++ b/api/api/templates/admin/api/components/media_additional.html @@ -0,0 +1,69 @@ +{% comment %} +Props: +- media_type +- media_obj +- tags +{% endcomment %} + +
+

Additional information

+ +
+
+
+ + {% if media_type == 'image' %} +
+ Media Image +
+ + + + {% elif media_type == 'audio' %} + + {% endif %} +
+
+ {% if media_type == 'image' %} +
Click to show/hide content.
+ {% endif %} +
+
+
+ +
+
+
+ +
+
+ {% for provider, provider_tags in tags.items %} +
{{ provider }}:
+
{{ provider_tags|join:', ' }}
+ {% endfor %} +
+ +
+
+
+
+
diff --git a/api/api/templates/admin/api/components/media_decisions.html b/api/api/templates/admin/api/components/media_decisions.html new file mode 100644 index 00000000000..6ce5b6a126b --- /dev/null +++ b/api/api/templates/admin/api/components/media_decisions.html @@ -0,0 +1,55 @@ +{% comment %} +Props: +- media_type +- decision_throughs +{% endcomment %} + +{% load get_attr %} + +
+

Decisions

+ + + + + + + + + + + + + + + {% for decision_through in decision_throughs %} + {% with decision_through.decision as decision %} + + + + + + + + + + {% endwith %} + {% endfor %} + +
IDDateModeratorActionNotesReports
+ {{ decision.id }} + {{ decision.created_on }}{{ decision.moderator }}{{ decision.action }}{{ decision.notes }} + {% with media_type|add:'report_set' as attr_name %} + {% with decision|get_attr:attr_name as reports %} + {% for report in reports.all %} + {{ report.id }} ({{report.reason}})
+ {% endfor %} + {% endwith %} + {% endwith %} +
+ + +
diff --git a/api/api/templates/admin/api/components/media_reports.html b/api/api/templates/admin/api/components/media_reports.html new file mode 100644 index 00000000000..eb595388d84 --- /dev/null +++ b/api/api/templates/admin/api/components/media_reports.html @@ -0,0 +1,105 @@ +{% comment %} +Props: +- media_type +- reports +- pending_report_count +- mod_form +{% endcomment %} + +
+

Reports

+ + {% if pending_report_count %} +

+ You can take a decision for the pending reports by selecting one + or more of them and creating a decision. +

+ {% endif %} + + + + + {% if pending_report_count %}{% endif %} + + + + + + + + + + {% for report in reports %} + + + {% if pending_report_count %} + + {% endif %} + + + + + + + + {% endfor %} + +
SelectIDDateReasonDescriptionIs pending?Decision
+ {% if report.is_pending %} + + {% endif %} + + {{ report.id }} + {{ report.created_at }}{{ report.reason }}{{ report.description }} + {% if report.is_pending %} + False + {% else %} + False + {% endif %} + + {% if report.decision %} + {{ report.decision.id }} + ({{ report.decision.action }}) + {% else %} + - + {% endif %} +
+ + + {% if pending_report_count %} +
+
+
+ {{ mod_form.action.label_tag }} + {{ mod_form.action }} +
+
{{ mod_form.action.help_text }}
+
+
+ +
+
+
+ {{ mod_form.notes.label_tag }} + {{ mod_form.notes }} +
+
{{ mod_form.notes.help_text }}
+
+
+ +
+ +
+ + + {% endif %} +
diff --git a/api/api/templates/admin/api/media/change_form.html b/api/api/templates/admin/api/media/change_form.html new file mode 100644 index 00000000000..765eb8ecf34 --- /dev/null +++ b/api/api/templates/admin/api/media/change_form.html @@ -0,0 +1,60 @@ +{% extends "admin/change_form.html" %} + +{% block extrahead %}{{ block.super }} + + + + + +{% endblock %} + +{% block content %}{{ block.super }} + +{% if pending_report_count %} +
+ {% csrf_token %} +
+{% endif %} +{% endblock %} + +{% block object-tools-items %}{{ block.super }} +
  • View on openverse.org
  • +{% endblock %} + +{% block after_field_sets %} +{% include 'admin/api/components/media_additional.html' with media_type=media_type media_obj=media_obj tags=tags only %} +{% include 'admin/api/components/media_reports.html' with media_type=media_type reports=reports pending_report_count=pending_report_count mod_form=mod_form only %} +{% include 'admin/api/components/media_decisions.html' with media_type=media_type decision_throughs=decision_throughs only %} +{% endblock %} diff --git a/api/api/templates/admin/api/media/change_list.html b/api/api/templates/admin/api/media/change_list.html new file mode 100644 index 00000000000..7ef5365308f --- /dev/null +++ b/api/api/templates/admin/api/media/change_list.html @@ -0,0 +1,32 @@ +{% extends "admin/change_list.html" %} +{% load i18n admin_urls static admin_list %} + +{% block extrahead %}{{ block.super }} + + +{% endblock %} + +{% block extrastyle %}{{ block.super }} + +{% endblock %} + +{% block result_list %} +

    + Note: + Media items that are being moderated by another user are highlighted. +

    +{{ block.super }} +{% endblock %} diff --git a/api/api/templatetags/__init__.py b/api/api/templatetags/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/api/api/templatetags/get_attr.py b/api/api/templatetags/get_attr.py new file mode 100644 index 00000000000..aeec2e8cb0f --- /dev/null +++ b/api/api/templatetags/get_attr.py @@ -0,0 +1,23 @@ +import re + +from django import template + + +numeric_test = re.compile("^\d+$") +register = template.Library() + + +def get_attr(value, arg): + """Get an attribute of an object dynamically from a string name""" + + if hasattr(value, str(arg)): + return getattr(value, arg) + elif hasattr(value, "has_key") and value.has_key(arg): + return value[arg] + elif numeric_test.match(str(arg)) and len(value) > int(arg): + return value[int(arg)] + else: + return None + + +register.filter("get_attr", get_attr) diff --git a/api/api/utils/moderation_lock.py b/api/api/utils/moderation_lock.py new file mode 100644 index 00000000000..96214e82a01 --- /dev/null +++ b/api/api/utils/moderation_lock.py @@ -0,0 +1,129 @@ +import functools +import time + +import django_redis +import structlog +from redis.exceptions import ConnectionError + +from api.models.moderation import get_moderators + + +LOCK_PREFIX = "moderation_lock" +TTL = 10 # seconds + +logger = structlog.get_logger(__name__) + + +def handle_redis_exception(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except ConnectionError: + return None + + return wrapper + + +class LockManager: + """ + Kudos to this Google Group discussion for the solution using a + ranked-set: + https://web.archive.org/web/20211205091916/https://groups.google.com/g/redis-db/c/rXXMCLNkNSs + """ + + def __init__(self, media_type): + self.media_type = media_type + + @handle_redis_exception + def prune(self) -> dict[str, set[str]]: + """ + Delete all expired locks and get a mapping of usernames to + media items that have active locks. + + :return: a mapping of moderators to media items they are viewing + """ + + redis = django_redis.get_redis_connection("default") + valid_locks = {} + + now = int(time.time()) + pipe = redis.pipeline() + for username in get_moderators().values_list("username", flat=True): + key = f"{LOCK_PREFIX}:{username}" + for value, score in redis.zrange(key, 0, -1, withscores=True): + if score <= now: + logger.info("Deleting expired lock", key=key, value=value) + pipe.zrem(key, value) + else: + logger.info("Keeping valid lock", key=key, value=value) + valid_locks.setdefault(username, set()).add(value.decode()) + pipe.execute() + + return valid_locks + + @handle_redis_exception + def add_locks(self, username, object_id) -> int: + """ + Add a soft-lock for a given media item to the given moderator. + + :param username: the username of the moderator viewing a media item + :param object_id: the ID of the media item being viewed + """ + + redis = django_redis.get_redis_connection("default") + + object = f"{self.media_type}:{object_id}" + + expiration = int(time.time()) + TTL + logger.info("Adding lock", object=object, user=username, expiration=expiration) + redis.zadd(f"{LOCK_PREFIX}:{username}", {object: expiration}) + return expiration + + @handle_redis_exception + def remove_locks(self, username, object_id): + """ + Remove the soft-lock for a given media item from the given moderator. + + :param username: the username of the moderator not viewing a media item + :param object_id: the ID of the media item not being viewed + """ + + redis = django_redis.get_redis_connection("default") + + object = f"{self.media_type}:{object_id}" + + logger.info("Removing lock", object=object, user=username) + redis.zrem(f"{LOCK_PREFIX}:{username}", object) + + def moderator_set(self, object_id) -> set[str]: + """ + Get the list of moderators on a particular item. + + :param object_id: the ID of the media item being viewed + :return: the list of moderators on a particular item + """ + + valid_locks = self.prune() or {} + + object = f"{self.media_type}:{object_id}" + mods = {mod for mod, objects in valid_locks.items() if object in objects} + logger.info("Retrieved moderators", object=object, mods=mods) + return mods + + @handle_redis_exception + def score(self, username, object_id) -> int: + """ + Get the score of a particular moderator on a particular item. + + :param username: the username of the moderator viewing a media item + :param object_id: the ID of the media item being viewed + :return: the score of a particular moderator on a particular item + """ + + redis = django_redis.get_redis_connection("default") + + object = f"{self.media_type}:{object_id}" + score = redis.zscore(f"{LOCK_PREFIX}:{username}", object) + logger.info("Retrieved score", object=object, user=username, score=score) + return score diff --git a/api/test/unit/admin/test_media_report.py b/api/test/unit/admin/test_media_report.py index 1c027c312bc..cb4ca175c4c 100644 --- a/api/test/unit/admin/test_media_report.py +++ b/api/test/unit/admin/test_media_report.py @@ -2,20 +2,22 @@ import pytest -from api.admin.media_report import _production_deferred +from api.admin.media_report import _non_production_deferred, _production_deferred @pytest.mark.parametrize( - "values, environment, expected", + "values, environment, prod_expected, non_prod_expected", [ - ([1, 2, 3], "local", (1, 2, 3)), - ([1, 2, 3], "production", ()), - ([], "local", ()), - ([], "production", ()), + ([1, 2, 3], "local", (1, 2, 3), ()), + ([1, 2, 3], "production", (), (1, 2, 3)), + ([], "local", (), ()), + ([], "production", (), ()), ], ) -def test_production_deferred(values, environment, expected): +def test_production_deferred(values, environment, prod_expected, non_prod_expected): with mock.patch("api.admin.media_report.settings") as mock_settings: mock_settings.ENVIRONMENT = environment - actual = _production_deferred(*values) - assert actual == expected + prod_deferred = _production_deferred(*values) + non_prod_deferred = _non_production_deferred(*values) + assert prod_deferred == prod_expected + assert non_prod_deferred == non_prod_expected diff --git a/api/test/unit/utils/test_moderation_lock.py b/api/test/unit/utils/test_moderation_lock.py new file mode 100644 index 00000000000..db6be1ad865 --- /dev/null +++ b/api/test/unit/utils/test_moderation_lock.py @@ -0,0 +1,81 @@ +from datetime import datetime, timedelta + +from django.contrib.auth.models import Group + +import pytest +from freezegun import freeze_time + +from api.utils.moderation_lock import TTL, LockManager + + +pytestmark = pytest.mark.django_db + + +@pytest.fixture(autouse=True) +def moderators(django_user_model): + for username in ["one", "two"]: + user = django_user_model.objects.create(username=username, password=username) + group, _ = Group.objects.get_or_create(name="Content Moderators") + group.user_set.add(user) + + +@pytest.mark.parametrize( + "is_cache_reachable, cache_name", + [(True, "redis"), (False, "unreachable_redis")], +) +def test_lock_manager_handles_missing_redis(is_cache_reachable, cache_name, request): + request.getfixturevalue(cache_name) + + lm = LockManager("media_type") + expiration = lm.add_locks("one", 10) + + if is_cache_reachable: + assert expiration is not None + assert lm.prune() == {"one": {"media_type:10"}} + assert lm.moderator_set(10) == {"one"} + assert lm.score("one", 10) == expiration + else: + assert expiration is None + assert lm.prune() is None + assert lm.moderator_set(10) == set() + assert lm.score("one", 10) == expiration + + +def test_lock_manager_adds_and_removes_locks(): + lm = LockManager("media_type") + + lm.add_locks("one", 10) + assert lm.moderator_set(10) == {"one"} + lm.add_locks("two", 10) + assert lm.moderator_set(10) == {"one", "two"} + lm.remove_locks("two", 10) + assert lm.moderator_set(10) == {"one"} + + +def test_relocking_updates_score(redis): + lm = LockManager("media_type") + now = datetime.now() + + with freeze_time(now): + lm.add_locks("one", 10) + init_score = lm.score("one", 10) + + with freeze_time(now + timedelta(seconds=TTL / 2)): + lm.add_locks("one", 10) + updated_score = lm.score("one", 10) + + assert updated_score == init_score + TTL / 2 + + +def test_lock_manager_prunes_after_timeout(): + lm = LockManager("media_type") + now = datetime.now() + + with freeze_time(now): + lm.add_locks("one", 10) + + with freeze_time(now + timedelta(seconds=TTL - 1)): + assert lm.moderator_set(10) == {"one"} + + with freeze_time(now + timedelta(seconds=TTL + 1)): + assert lm.moderator_set(10) == set() diff --git a/load_sample_data.sh b/load_sample_data.sh index ff45ca64c80..7d7160514fc 100755 --- a/load_sample_data.sh +++ b/load_sample_data.sh @@ -81,15 +81,26 @@ for username in usernames: # Credit: https://stackoverflow.com/a/53733693 echo " from django.contrib.auth.models import User, Group, Permission -perms_to_add = ['view', 'add', 'change'] -models_to_affect = ['audio report', 'image report', 'sensitive audio', 'sensitive image'] +crud_perm_map = {'C': 'add', 'R': 'view', 'U': 'change', 'D': 'delete'} +model_perms_map = { + 'image': 'R', + 'audio': 'R', + 'image report': 'CRU', + 'audio report': 'CRU', + 'sensitive image': 'CRU', + 'sensitive audio': 'CRU', + 'image decision': 'CRU', + 'audio decision': 'CRU', + 'image decision through': 'CRUD', + 'audio decision through': 'CRUD', +} mod_group, created = Group.objects.get_or_create(name='Content Moderators') if created: print('Setting up Content Moderators group') - for model in models_to_affect: - for perm in perms_to_add: - name = f'Can {perm} {model}' + for model, perms in model_perms_map.items(): + for perm in perms: + name = f'Can {crud_perm_map[perm]} {model}' print(f'Adding permission to moderators group: {name}') model_add_perm = Permission.objects.get(name=name) mod_group.permissions.add(model_add_perm)