Story #2608 :: Task: Fix the formal-review importer and derive the Reviewer achievement from it

badges/sources.py

@@ -92,11 +92,24 @@ def _iter_code_commits():
         yield commit.author.user, commit
 
 
+def _iter_library_review():
+    """Yield (user, review) for every review submission with a linked user."""
+    from versions.models import Review
+
+    for review in Review.objects.prefetch_related("submitters__user").iterator(
+        chunk_size=500
+    ):
+        for commit_author in review.submitters.all():
+            if commit_author.user_id:
+                yield commit_author.user, review
+
+
 BACKFILL_ITERATORS = {
     AchievementSlug.LIBRARY_AUTHORING: _iter_library_authoring,
     AchievementSlug.LIBRARY_MAINTENANCE: _iter_library_maintenance,
     AchievementSlug.LIBRARY_VERSIONING: _iter_library_versioning,
     AchievementSlug.CODE_COMMITS: _iter_code_commits,
+    AchievementSlug.LIBRARY_REVIEW: _iter_library_review,
 }
 
 # Derived, so the CLI choices can never drift from the wired iterators.

badges/tests/test_admin.py

@@ -1179,6 +1179,21 @@ def test_reconcile_preview_refuses_an_empty_source(
     assert UserAchievement.objects.filter(user=plain_user).count() == 1
 
 
+def test_reconcile_preview_blocks_an_incomplete_catalogue(
+    client, super_user, plain_user, commit_by_someone_else, stale_commit_grant
+):
+    """An unseeded slug is what the command refuses outright, so say so first."""
+    Achievement.objects.filter(slug="library-review").delete()
+    client.force_login(super_user)
+
+    response = client.post(reverse(RECONCILE_URL))
+
+    body = response.content.decode()
+    assert "the catalogue is incomplete" in body
+    assert "Run migrations first" in body
+    assert 'name="apply"' not in body
+
+
 def test_reconcile_button_needs_more_than_the_change_permission(
     client, plain_user, commit_by_someone_else, stale_commit_grant
 ):

badges/tests/test_commands.py

@@ -121,6 +121,18 @@ def test_backfill_library_versioning(plain_user):
     )
 
 
+def test_backfill_library_review(plain_user):
+    """Backfill grants the reviewer achievement from linked review submitters."""
+    review = baker.make("versions.Review")
+    review.submitters.add(baker.make("libraries.CommitAuthor", user=plain_user))
+
+    call_command("backfill_achievements", "--source", "library-review")
+
+    assert UserBadge.objects.filter(
+        user=plain_user, badge__achievement__slug="library-review"
+    ).exists()
+
+
 def test_backfill_fails_loudly_on_an_explicit_unseeded_source(plain_user):
     """A named source with no Achievement row is a deploy bug, not a skip."""
     Achievement.objects.filter(slug=AchievementSlug.CODE_COMMITS).delete()

badges/tests/test_sources.py

@@ -104,3 +104,14 @@ def test_iter_code_commits_skips_unlinked(plain_user):
 
     pairs = list(sources._iter_code_commits())
     assert [u for u, _ in pairs] == [plain_user]
+
+
+def test_iter_library_review_skips_unlinked(plain_user):
+    """Review submitters without a linked user are skipped."""
+    review = baker.make("versions.Review")
+    review.submitters.add(
+        baker.make("libraries.CommitAuthor", user=plain_user),
+        baker.make("libraries.CommitAuthor", user=None),
+    )
+    pairs = list(sources._iter_library_review())
+    assert [u for u, _ in pairs] == [plain_user]

versions/admin.py

@@ -5,11 +5,68 @@
 from django.urls import path
 from django.utils.html import format_html, format_html_join
 
+from core.admin_buttons import TaskButton, TaskButtonAdminMixin
 from libraries.tasks import import_new_versions_tasks
 
 from . import models
 from .models import Version
-from .tasks import dispatch_whats_new
+from .tasks import dispatch_whats_new, import_reviews_task
+
+
+def _import_reviews_preview(_request, _value):
+    """Static warning for the destructive parts of review reconciliation."""
+    return {
+        "title": "Import reviews from boost.org",
+        "summary": (
+            "This re-scrapes the published formal-review results and updates the "
+            "stored reviews to match. Existing reviews with the same normalized "
+            "identity are updated instead of copied."
+        ),
+        "rows": (
+            {
+                "label": "Duplicate reviews",
+                "detail": (
+                    "Duplicate stored reviews are merged; the duplicate rows, "
+                    "their results, and achievements sourced from them are deleted."
+                ),
+                "warning": True,
+            },
+            {
+                "label": "Review results",
+                "detail": (
+                    "Published results are created or updated for every imported "
+                    "review."
+                ),
+                "warning": False,
+            },
+        ),
+        "warning": (
+            "Continue only if boost.org is the source you intend to reconcile "
+            "against."
+        ),
+        "can_apply": True,
+    }
+
+
+IMPORT_REVIEWS_BUTTON = TaskButton(
+    name="import_reviews",
+    label="Import Reviews from boost.org",
+    task=import_reviews_task,
+    success_message="Reviews are being imported from boost.org in the background.",
+    busy_message=(
+        "A review import is already queued or running; not starting another one."
+    ),
+    permission="versions.delete_review",
+    pass_actor=True,
+    confirm=_import_reviews_preview,
+    description=(
+        "Re-scrapes the formal-review results published on boost.org and updates "
+        "the reviews and results below, matching each against what is already "
+        "stored rather than adding a near-duplicate. Duplicate reviews, their "
+        "results, and achievements sourced from them may be deleted; Reviewer "
+        "achievements are brought into step afterwards."
+    ),
+)
 
 
 class VersionFileInline(admin.StackedInline):
@@ -134,16 +191,38 @@ class ResultInline(admin.StackedInline):
 
 
 @admin.register(models.Review)
-class ReviewAdmin(admin.ModelAdmin):
-    list_display = ["submission", "review_dates", "get_results"]
-    search_fields = ["submission"]
+class ReviewAdmin(TaskButtonAdminMixin, admin.ModelAdmin):
+    list_display = [
+        "id",
+        "submission",
+        "review_dates",
+        "get_results",
+        "get_review_manager",
+        "get_scraped_review_manager",
+    ]
+    ordering = ["-id"]
+    search_fields = ["submission", "review_manager_raw", "review_manager__name"]
     inlines = [ResultInline]
+    task_buttons = (IMPORT_REVIEWS_BUTTON,)
 
     def get_results(self, obj):
-        return " | ".join(obj.results.values_list("short_description", flat=True))
+        return " | ".join(result.short_description for result in obj.results.all())
+
+    @admin.display(description="Review manager", ordering="review_manager__name")
+    def get_review_manager(self, obj):
+        return obj.review_manager or ""
+
+    @admin.display(description="Scraped review manager", ordering="review_manager_raw")
+    def get_scraped_review_manager(self, obj):
+        return obj.review_manager_raw
 
     def get_queryset(self, request: HttpRequest) -> QuerySet:
-        return super().get_queryset(request).prefetch_related("results")
+        return (
+            super()
+            .get_queryset(request)
+            .select_related("review_manager")
+            .prefetch_related("results")
+        )
 
 
 @admin.register(models.ReviewResult)