From 41ebd724b18c3a9898e16047e3593d74a6533d27 Mon Sep 17 00:00:00 2001 From: Gianfranco Rossi Date: Thu, 17 Jul 2025 12:24:32 -0500 Subject: [PATCH 1/5] feat(search.models): add ClusterRedirection model Solves #5983 - Creates a model to store deleted cluster ids and point them to existing cluster ids; useful to handle versioning, consolidation and deletion of duplicates without affecting users that may have saved links - Adds a decorator to handle opinion_page views that look for an OpinionCluster - Overrides OpinionClusterViewSet to handle API redirection - Adds tests for both cases --- cl/api/tests.py | 42 +++++++++++++++++++ cl/opinion_page/decorators.py | 41 ++++++++++++++++++ cl/opinion_page/tests.py | 36 ++++++++++++++++ cl/opinion_page/views.py | 11 ++++- cl/search/api_views.py | 36 +++++++++++++++- .../0041_add_cluster_redirection.py | 23 ++++++++++ .../0041_add_cluster_redirection.sql | 8 ++++ cl/search/models.py | 38 +++++++++++++++++ 8 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 cl/opinion_page/decorators.py create mode 100644 cl/search/migrations/0041_add_cluster_redirection.py create mode 100644 cl/search/migrations/0041_add_cluster_redirection.sql diff --git a/cl/api/tests.py b/cl/api/tests.py index d5f3c135f5..009c34e35f 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -22,6 +22,7 @@ from django.test.utils import CaptureQueriesContext from django.urls import reverse from django.utils.timezone import now +from rest_framework import status from rest_framework.exceptions import NotFound from rest_framework.pagination import Cursor, CursorPagination from rest_framework.request import Request @@ -96,11 +97,13 @@ CourtFactory, DocketFactory, OpinionClusterWithChildrenAndParentsFactory, + OpinionClusterWithParentsFactory, ) from cl.search.models import ( PRECEDENTIAL_STATUS, SEARCH_TYPES, SOURCES, + ClusterRedirection, Court, Docket, Opinion, @@ -3738,3 +3741,42 @@ def test_can_increment_exiting_events(self): event_record.refresh_from_db() # Assert that the value of the event record has been incremented by 1 self.assertEqual(event_record.value, 4) + + +class ClusterRedirectionTest(TestCase): + @classmethod + def setUpTestData(cls): + cls.deleted_cluster_id = 999999 + cls.redirect_to_cluster = OpinionClusterWithParentsFactory.create() + ClusterRedirection.objects.create( + deleted_cluster_id=cls.deleted_cluster_id, + cluster=cls.redirect_to_cluster, + reason=ClusterRedirection.VERSIONING, + ) + cls.user = UserProfileWithParentsFactory.create( + user__username="a-user", + user__password=make_password("password"), + ) + + async def test_opinion_cluster_redirection(self): + """Test that a deleted cluster redirects to an existing cluster""" + url = reverse( + "opinioncluster-detail", + kwargs={"version": "v4", "pk": self.deleted_cluster_id}, + ) + api_client = await sync_to_async(make_client)(self.user.user.pk) + response = await api_client.get(url) + self.assertEqual( + response.status_code, status.HTTP_301_MOVED_PERMANENTLY + ) + + redirect_response = await api_client.get(response.headers["Location"]) + data = json.loads(redirect_response.content) + self.assertEqual(data["id"], self.redirect_to_cluster.id) + + # Check that we don't get false redirections for non-existent record + url = reverse( + "opinioncluster-detail", kwargs={"version": "v4", "pk": 777777} + ) + response = await api_client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) diff --git a/cl/opinion_page/decorators.py b/cl/opinion_page/decorators.py new file mode 100644 index 0000000000..89a174aef6 --- /dev/null +++ b/cl/opinion_page/decorators.py @@ -0,0 +1,41 @@ +from functools import wraps + +from django.http import Http404 +from django.shortcuts import redirect + +from cl.search.models import ClusterRedirection + + +def handle_cluster_redirection(view_func): + """ + Redirect from deleted clusters to existing clusters + + Uses the ClusterRedirection table, and only changed the `pk` of the request + """ + + @wraps(view_func) + async def _wrapped_view(request, *args, **kwargs): + try: + response = await view_func(request, *args, **kwargs) + return response + except Http404 as exc: + try: + redirection = await ClusterRedirection.objects.aget( + deleted_cluster_id=kwargs["pk"] + ) + cluster_id = redirection.cluster_id + + if not cluster_id: + # TODO: should redirect to sealed page + raise exc + except ClusterRedirection.DoesNotExist: + raise exc + + # redirect to the same URL, only change the target PK + url_name = request.resolver_match.url_name + url_kwargs = kwargs.copy() + url_kwargs["pk"] = cluster_id + url_kwargs["permanent"] = True + return redirect(url_name, **url_kwargs) + + return _wrapped_view diff --git a/cl/opinion_page/tests.py b/cl/opinion_page/tests.py index b843558fd4..a10c5ef6e1 100644 --- a/cl/opinion_page/tests.py +++ b/cl/opinion_page/tests.py @@ -82,6 +82,7 @@ PRECEDENTIAL_STATUS, SEARCH_TYPES, Citation, + ClusterRedirection, Docket, DocketEntry, Opinion, @@ -1881,3 +1882,38 @@ def test_cache_view_docket_feed(self) -> None: self.assertEqual(response.headers["Cache-Control"], "max-age=300") self.assertIn("Expires", response.headers) self.assertIn(self.docket.case_name, response.content.decode()) + + +class ClusterRedirectionTest(TestCase): + @classmethod + def setUpTestData(cls): + cls.deleted_cluster_id = 99999999 + cls.redirected_cluster = OpinionClusterWithParentsFactory.create( + precedential_status=PRECEDENTIAL_STATUS.PUBLISHED, + citation_count=1, + date_filed=datetime.date.today(), + ) + ClusterRedirection.objects.create( + cluster=cls.redirected_cluster, + deleted_cluster_id=cls.deleted_cluster_id, + reason=ClusterRedirection.DUPLICATE, + ) + + def test_cluster_redirection(self): + """Can we permanently redirect a deleted cluster to an existing one?""" + deleted_cluster_url = reverse( + "view_case", kwargs={"pk": self.deleted_cluster_id, "_": "test"} + ) + + response = self.client.get(deleted_cluster_url) + + expected_redirect_url = reverse( + "view_case", + kwargs={"pk": self.redirected_cluster.pk, "_": "test"}, + ) + + self.assertRedirects( + response, + expected_redirect_url, + status_code=301, + ) diff --git a/cl/opinion_page/views.py b/cl/opinion_page/views.py index a315b6dfca..615bd2a3fe 100644 --- a/cl/opinion_page/views.py +++ b/cl/opinion_page/views.py @@ -58,7 +58,9 @@ ) from cl.lib.auth import group_required from cl.lib.bot_detector import is_og_bot -from cl.lib.decorators import cache_page_ignore_params +from cl.lib.decorators import ( + cache_page_ignore_params, +) from cl.lib.http import is_ajax from cl.lib.model_helpers import choices_to_csv from cl.lib.models import THUMBNAIL_STATUSES @@ -67,6 +69,7 @@ from cl.lib.string_utils import trunc from cl.lib.thumbnails import make_png_thumbnail_for_instance from cl.lib.url_utils import get_redirect_or_abort +from cl.opinion_page.decorators import handle_cluster_redirection from cl.opinion_page.feeds import DocketFeed from cl.opinion_page.forms import ( CitationRedirectorForm, @@ -975,6 +978,7 @@ async def update_opinion_tabs(request: HttpRequest, pk: int): @never_cache +@handle_cluster_redirection async def view_opinion(request: HttpRequest, pk: int, _: str) -> HttpResponse: """View Opinions @@ -989,6 +993,7 @@ async def view_opinion(request: HttpRequest, pk: int, _: str) -> HttpResponse: return await render_opinion_view(request, cluster, "opinions") +@handle_cluster_redirection async def view_opinion_pdf( request: HttpRequest, pk: int, _: str ) -> HttpResponse: @@ -1005,6 +1010,7 @@ async def view_opinion_pdf( return await render_opinion_view(request, cluster, "pdf") +@handle_cluster_redirection async def view_opinion_authorities( request: HttpRequest, pk: int, _: str ) -> HttpResponse: @@ -1027,6 +1033,7 @@ async def view_opinion_authorities( ) +@handle_cluster_redirection async def view_opinion_cited_by( request: HttpRequest, pk: int, _: str ) -> HttpResponse: @@ -1050,6 +1057,7 @@ async def view_opinion_cited_by( ) +@handle_cluster_redirection async def view_opinion_summaries( request: HttpRequest, pk: int, _: str ) -> HttpResponse: @@ -1088,6 +1096,7 @@ async def view_opinion_summaries( ) +@handle_cluster_redirection async def view_opinion_related_cases( request: HttpRequest, pk: int, _: str ) -> HttpResponse: diff --git a/cl/search/api_views.py b/cl/search/api_views.py index 14b009d38b..48e85ca07a 100644 --- a/cl/search/api_views.py +++ b/cl/search/api_views.py @@ -1,10 +1,13 @@ from http import HTTPStatus from django.db.models import Prefetch -from rest_framework import pagination, permissions, response, viewsets +from django.http.response import Http404 +from django.urls import reverse +from rest_framework import pagination, permissions, response, status, viewsets from rest_framework.exceptions import NotFound from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import DjangoModelPermissionsOrAnonReadOnly +from rest_framework.response import Response from cl.api.api_permissions import V3APIPermission from cl.api.pagination import ESCursorPagination @@ -55,6 +58,7 @@ from cl.search.forms import SearchForm from cl.search.models import ( SEARCH_TYPES, + ClusterRedirection, Court, Docket, DocketEntry, @@ -239,6 +243,36 @@ class OpinionClusterViewSet( "citations", ).order_by("-id") + def retrieve(self, request, pk=None, version=None): + try: + # First, try to get the object normally + instance = self.get_object() + serializer = self.get_serializer(instance) + return Response(serializer.data) + except Http404 as exc: + try: + redirection = ClusterRedirection.objects.get( + deleted_cluster_id=pk + ) + cluster_id = redirection.cluster_id + + if not cluster_id: + # TODO return sealed page + raise exc + + redirection_url = reverse( + "opinioncluster-detail", + kwargs={"version": version, "pk": cluster_id}, + ) + absolute_new_url = request.build_absolute_uri(redirection_url) + + return Response( + status=status.HTTP_301_MOVED_PERMANENTLY, + headers={"Location": absolute_new_url}, + ) + except (ClusterRedirection.DoesNotExist, Http404): + raise exc + class OpinionViewSet( LoggingMixin, NoFilterCacheListMixin, viewsets.ModelViewSet diff --git a/cl/search/migrations/0041_add_cluster_redirection.py b/cl/search/migrations/0041_add_cluster_redirection.py new file mode 100644 index 0000000000..6db9775f74 --- /dev/null +++ b/cl/search/migrations/0041_add_cluster_redirection.py @@ -0,0 +1,23 @@ +# Generated by Django 5.1.8 on 2025-07-17 16:49 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('search', '0040_add_opinion_main_version_field'), + ] + + operations = [ + migrations.CreateModel( + name='ClusterRedirection', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('deleted_cluster_id', models.IntegerField()), + ('reason', models.SmallIntegerField(choices=[(1, 'Opinion versions clusters were consolidated in a single one'), (2, 'Clusters were duplicated'), (3, 'Opinion types clusters were consolidated in a single one'), (4, 'Cluster was deleted due to sealing')], help_text='The reason why the old cluster was deleted')), + ('cluster', models.ForeignKey(help_text='The existing cluster the deleted clusters now point to', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='merged_clusters', to='search.opinioncluster')), + ], + ), + ] diff --git a/cl/search/migrations/0041_add_cluster_redirection.sql b/cl/search/migrations/0041_add_cluster_redirection.sql new file mode 100644 index 0000000000..2185c5b3b7 --- /dev/null +++ b/cl/search/migrations/0041_add_cluster_redirection.sql @@ -0,0 +1,8 @@ +BEGIN; +-- +-- Create model ClusterRedirection +-- +CREATE TABLE "search_clusterredirection" ("id" integer NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "deleted_cluster_id" integer NOT NULL, "reason" smallint NOT NULL, "cluster_id" integer NULL); +ALTER TABLE "search_clusterredirection" ADD CONSTRAINT "search_clusterredire_cluster_id_82cdb249_fk_search_op" FOREIGN KEY ("cluster_id") REFERENCES "search_opinioncluster" ("id") DEFERRABLE INITIALLY DEFERRED; +CREATE INDEX "search_clusterredirection_cluster_id_82cdb249" ON "search_clusterredirection" ("cluster_id"); +COMMIT; diff --git a/cl/search/models.py b/cl/search/models.py index dcc3ff60cd..10257193e1 100644 --- a/cl/search/models.py +++ b/cl/search/models.py @@ -3786,3 +3786,41 @@ class Meta: models.Index(fields=["date_created"]), ] verbose_name_plural = "Search Queries" + + +class ClusterRedirection(models.Model): + """Model to prevent dead /opinion/ links""" + + VERSIONING = 1 + DUPLICATE = 2 + CONSOLIDATION = 3 + SEALED = 4 + REDIRECTION_REASON = ( + ( + VERSIONING, + "Opinion versions clusters were consolidated in a single one", + ), + (DUPLICATE, "Clusters were duplicated"), + ( + CONSOLIDATION, + "Opinion types clusters were consolidated in a single one", + ), + (SEALED, "Cluster was deleted due to sealing"), + ) + + deleted_cluster_id = models.IntegerField() + cluster = models.ForeignKey( + OpinionCluster, + help_text="The existing cluster the deleted clusters now point to", + related_name="merged_clusters", + # if a cluster with `merged_cluster` is to be deleted, it will raise + # an IntegrityError. User should assign a different cluster before + # deleting the current cluster + on_delete=models.DO_NOTHING, + # need null values for SEALED opinions + null=True, + ) + reason = models.SmallIntegerField( + help_text="The reason why the old cluster was deleted", + choices=REDIRECTION_REASON, + ) From fa85a173896fc821254a89eb0655aed9ee94e4cb Mon Sep 17 00:00:00 2001 From: Gianfranco Rossi Date: Wed, 23 Jul 2025 15:47:55 -0500 Subject: [PATCH 2/5] fix(search.models): refactor ClusterRedirection model --- cl/api/tests.py | 2 +- cl/assets/templates/410.html | 17 +++++++++++++++++ cl/opinion_page/decorators.py | 10 ++++++++-- .../migrations/0041_add_cluster_redirection.py | 7 ++++--- .../migrations/0041_add_cluster_redirection.sql | 2 +- cl/search/models.py | 14 +++++++------- 6 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 cl/assets/templates/410.html diff --git a/cl/api/tests.py b/cl/api/tests.py index 009c34e35f..163ac70a82 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -3751,7 +3751,7 @@ def setUpTestData(cls): ClusterRedirection.objects.create( deleted_cluster_id=cls.deleted_cluster_id, cluster=cls.redirect_to_cluster, - reason=ClusterRedirection.VERSIONING, + reason=ClusterRedirection.VERSION, ) cls.user = UserProfileWithParentsFactory.create( user__username="a-user", diff --git a/cl/assets/templates/410.html b/cl/assets/templates/410.html new file mode 100644 index 0000000000..5d70fc1659 --- /dev/null +++ b/cl/assets/templates/410.html @@ -0,0 +1,17 @@ +{% extends "base.html" %} + +{% block privacy %}{% endblock %} +{% block sidebar %}{% endblock %} + +{% block title %}Gone (410) – CourtListener.com{% endblock %} + +{% block content %} + +
+

The opinion you're looking for has been permanently removed.

+

Record Sealed by Court Order


+

You can normally find unsealed records in the Case Law database, RECAP Archive, Oral Arguments database, or Judge database.

+

We make decisions about the privacy of documents on a case by case basis. To learn more, please see our removal policy.

+
+ +{% endblock %} diff --git a/cl/opinion_page/decorators.py b/cl/opinion_page/decorators.py index 89a174aef6..7778ee9b52 100644 --- a/cl/opinion_page/decorators.py +++ b/cl/opinion_page/decorators.py @@ -1,7 +1,9 @@ from functools import wraps +from http import HTTPStatus from django.http import Http404 from django.shortcuts import redirect +from django.template.response import TemplateResponse from cl.search.models import ClusterRedirection @@ -24,10 +26,14 @@ async def _wrapped_view(request, *args, **kwargs): deleted_cluster_id=kwargs["pk"] ) cluster_id = redirection.cluster_id - if not cluster_id: - # TODO: should redirect to sealed page raise exc + + if redirection.reason == ClusterRedirection.SEALED: + return TemplateResponse( + request, "410.html", status=HTTPStatus.GONE + ) + except ClusterRedirection.DoesNotExist: raise exc diff --git a/cl/search/migrations/0041_add_cluster_redirection.py b/cl/search/migrations/0041_add_cluster_redirection.py index 6db9775f74..d6e12551e0 100644 --- a/cl/search/migrations/0041_add_cluster_redirection.py +++ b/cl/search/migrations/0041_add_cluster_redirection.py @@ -1,4 +1,4 @@ -# Generated by Django 5.1.8 on 2025-07-17 16:49 +# Generated by Django 5.1.8 on 2025-07-23 20:52 import django.db.models.deletion from django.db import migrations, models @@ -15,8 +15,9 @@ class Migration(migrations.Migration): name='ClusterRedirection', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('deleted_cluster_id', models.IntegerField()), - ('reason', models.SmallIntegerField(choices=[(1, 'Opinion versions clusters were consolidated in a single one'), (2, 'Clusters were duplicated'), (3, 'Opinion types clusters were consolidated in a single one'), (4, 'Cluster was deleted due to sealing')], help_text='The reason why the old cluster was deleted')), + ('date_created', models.DateTimeField(auto_now_add=True, help_text='Datetime when the record was created.')), + ('deleted_cluster_id', models.IntegerField(unique=True)), + ('reason', models.SmallIntegerField(choices=[(1, 'Cluster replaced by a newer version of the same opinion'), (2, 'Cluster removed as a duplicate'), (3, 'Spread opinions were consolidated in a single cluster (e.g., a dissent opinion added to a cluster containing the majority opinion)'), (4, 'Cluster removed by court order')], help_text='The reason why the old cluster was deleted')), ('cluster', models.ForeignKey(help_text='The existing cluster the deleted clusters now point to', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='merged_clusters', to='search.opinioncluster')), ], ), diff --git a/cl/search/migrations/0041_add_cluster_redirection.sql b/cl/search/migrations/0041_add_cluster_redirection.sql index 2185c5b3b7..9713373274 100644 --- a/cl/search/migrations/0041_add_cluster_redirection.sql +++ b/cl/search/migrations/0041_add_cluster_redirection.sql @@ -2,7 +2,7 @@ BEGIN; -- -- Create model ClusterRedirection -- -CREATE TABLE "search_clusterredirection" ("id" integer NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "deleted_cluster_id" integer NOT NULL, "reason" smallint NOT NULL, "cluster_id" integer NULL); +CREATE TABLE "search_clusterredirection" ("id" integer NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "date_created" timestamp with time zone NOT NULL, "deleted_cluster_id" integer NOT NULL UNIQUE, "reason" smallint NOT NULL, "cluster_id" integer NULL); ALTER TABLE "search_clusterredirection" ADD CONSTRAINT "search_clusterredire_cluster_id_82cdb249_fk_search_op" FOREIGN KEY ("cluster_id") REFERENCES "search_opinioncluster" ("id") DEFERRABLE INITIALLY DEFERRED; CREATE INDEX "search_clusterredirection_cluster_id_82cdb249" ON "search_clusterredirection" ("cluster_id"); COMMIT; diff --git a/cl/search/models.py b/cl/search/models.py index 10257193e1..14e84aa836 100644 --- a/cl/search/models.py +++ b/cl/search/models.py @@ -3791,24 +3791,24 @@ class Meta: class ClusterRedirection(models.Model): """Model to prevent dead /opinion/ links""" - VERSIONING = 1 + VERSION = 1 DUPLICATE = 2 CONSOLIDATION = 3 SEALED = 4 REDIRECTION_REASON = ( ( - VERSIONING, - "Opinion versions clusters were consolidated in a single one", + VERSION, + "Cluster replaced by a newer version of the same opinion", ), - (DUPLICATE, "Clusters were duplicated"), + (DUPLICATE, "Cluster removed as a duplicate"), ( CONSOLIDATION, - "Opinion types clusters were consolidated in a single one", + "Spread opinions were consolidated in a single cluster (e.g., a dissent opinion added to a cluster containing the majority opinion)", ), - (SEALED, "Cluster was deleted due to sealing"), + (SEALED, "Cluster removed by court order"), ) - deleted_cluster_id = models.IntegerField() + deleted_cluster_id = models.IntegerField(unique=True) cluster = models.ForeignKey( OpinionCluster, help_text="The existing cluster the deleted clusters now point to", From 1d9c61d74aa8c987e98e41846253b3c3eb76b529 Mon Sep 17 00:00:00 2001 From: Gianfranco Rossi Date: Thu, 24 Jul 2025 10:25:41 -0500 Subject: [PATCH 3/5] fix(search.models): add ClusterRedirection.date_created field --- cl/search/models.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cl/search/models.py b/cl/search/models.py index 14e84aa836..790e2b1eb9 100644 --- a/cl/search/models.py +++ b/cl/search/models.py @@ -3807,7 +3807,10 @@ class ClusterRedirection(models.Model): ), (SEALED, "Cluster removed by court order"), ) - + date_created = models.DateTimeField( + help_text="Datetime when the record was created.", + auto_now_add=True, + ) deleted_cluster_id = models.IntegerField(unique=True) cluster = models.ForeignKey( OpinionCluster, From 2e5d5ae958804ef3bd9789f2ae925ac345272fba Mon Sep 17 00:00:00 2001 From: Gianfranco Rossi Date: Thu, 24 Jul 2025 11:25:17 -0500 Subject: [PATCH 4/5] refactor(api.tests): use HTTPStatus class --- cl/api/tests.py | 7 ++----- cl/search/migrations/0041_add_cluster_redirection.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/cl/api/tests.py b/cl/api/tests.py index 163ac70a82..2a5af7b407 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -22,7 +22,6 @@ from django.test.utils import CaptureQueriesContext from django.urls import reverse from django.utils.timezone import now -from rest_framework import status from rest_framework.exceptions import NotFound from rest_framework.pagination import Cursor, CursorPagination from rest_framework.request import Request @@ -3766,9 +3765,7 @@ async def test_opinion_cluster_redirection(self): ) api_client = await sync_to_async(make_client)(self.user.user.pk) response = await api_client.get(url) - self.assertEqual( - response.status_code, status.HTTP_301_MOVED_PERMANENTLY - ) + self.assertEqual(response.status_code, HTTPStatus.MOVED_PERMANENTLY) redirect_response = await api_client.get(response.headers["Location"]) data = json.loads(redirect_response.content) @@ -3779,4 +3776,4 @@ async def test_opinion_cluster_redirection(self): "opinioncluster-detail", kwargs={"version": "v4", "pk": 777777} ) response = await api_client.get(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.status_code, HTTPStatus.NOT_FOUND) diff --git a/cl/search/migrations/0041_add_cluster_redirection.py b/cl/search/migrations/0041_add_cluster_redirection.py index d6e12551e0..085a714d00 100644 --- a/cl/search/migrations/0041_add_cluster_redirection.py +++ b/cl/search/migrations/0041_add_cluster_redirection.py @@ -1,4 +1,4 @@ -# Generated by Django 5.1.8 on 2025-07-23 20:52 +# Generated by Django 5.1.8 on 2025-07-24 16:21 import django.db.models.deletion from django.db import migrations, models From a435853f48e2521582f262d59e4558f6813b13b4 Mon Sep 17 00:00:00 2001 From: Gianfranco Rossi Date: Thu, 24 Jul 2025 18:12:10 -0500 Subject: [PATCH 5/5] fix(ClusterRedirection): handle SEALED status --- cl/api/tests.py | 18 ++++++++++++++++++ cl/opinion_page/decorators.py | 16 +++++++--------- cl/search/api_views.py | 34 ++++++++++++++++++---------------- 3 files changed, 43 insertions(+), 25 deletions(-) diff --git a/cl/api/tests.py b/cl/api/tests.py index 2a5af7b407..16509e04a7 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -3756,6 +3756,12 @@ def setUpTestData(cls): user__username="a-user", user__password=make_password("password"), ) + cls.sealed_cluster_id = 6666666 + ClusterRedirection.objects.create( + reason=ClusterRedirection.SEALED, + deleted_cluster_id=cls.sealed_cluster_id, + cluster=None, + ) async def test_opinion_cluster_redirection(self): """Test that a deleted cluster redirects to an existing cluster""" @@ -3777,3 +3783,15 @@ async def test_opinion_cluster_redirection(self): ) response = await api_client.get(url) self.assertEqual(response.status_code, HTTPStatus.NOT_FOUND) + + # Check that a sealed record returns proper code and message + message = dict(ClusterRedirection.REDIRECTION_REASON)[ + ClusterRedirection.SEALED + ] + url = reverse( + "opinioncluster-detail", + kwargs={"version": "v4", "pk": self.sealed_cluster_id}, + ) + response = await api_client.get(url) + self.assertEqual(response.json()["detail"], message) + self.assertEqual(response.status_code, HTTPStatus.GONE) diff --git a/cl/opinion_page/decorators.py b/cl/opinion_page/decorators.py index 7778ee9b52..3f42b86ee8 100644 --- a/cl/opinion_page/decorators.py +++ b/cl/opinion_page/decorators.py @@ -25,18 +25,16 @@ async def _wrapped_view(request, *args, **kwargs): redirection = await ClusterRedirection.objects.aget( deleted_cluster_id=kwargs["pk"] ) - cluster_id = redirection.cluster_id - if not cluster_id: - raise exc - - if redirection.reason == ClusterRedirection.SEALED: - return TemplateResponse( - request, "410.html", status=HTTPStatus.GONE - ) - except ClusterRedirection.DoesNotExist: raise exc + if redirection.reason == ClusterRedirection.SEALED: + return TemplateResponse( + request, "410.html", status=HTTPStatus.GONE + ) + + cluster_id = redirection.cluster_id + # redirect to the same URL, only change the target PK url_name = request.resolver_match.url_name url_kwargs = kwargs.copy() diff --git a/cl/search/api_views.py b/cl/search/api_views.py index b6efb46798..78247347b5 100644 --- a/cl/search/api_views.py +++ b/cl/search/api_views.py @@ -3,7 +3,7 @@ from django.db.models import Prefetch from django.http.response import Http404 from django.urls import reverse -from rest_framework import pagination, permissions, response, status, viewsets +from rest_framework import pagination, permissions, response, viewsets from rest_framework.exceptions import NotFound from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import DjangoModelPermissionsOrAnonReadOnly @@ -254,24 +254,26 @@ def retrieve(self, request, pk=None, version=None): redirection = ClusterRedirection.objects.get( deleted_cluster_id=pk ) - cluster_id = redirection.cluster_id + except ClusterRedirection.DoesNotExist: + raise exc - if not cluster_id: - # TODO return sealed page - raise exc + if redirection.reason == ClusterRedirection.SEALED: + message = dict(ClusterRedirection.REDIRECTION_REASON)[ + ClusterRedirection.SEALED + ] + return Response({"detail": message}, status=HTTPStatus.GONE) - redirection_url = reverse( - "opinioncluster-detail", - kwargs={"version": version, "pk": cluster_id}, - ) - absolute_new_url = request.build_absolute_uri(redirection_url) + cluster_id = redirection.cluster_id + redirection_url = reverse( + "opinioncluster-detail", + kwargs={"version": version, "pk": cluster_id}, + ) + absolute_new_url = request.build_absolute_uri(redirection_url) - return Response( - status=status.HTTP_301_MOVED_PERMANENTLY, - headers={"Location": absolute_new_url}, - ) - except (ClusterRedirection.DoesNotExist, Http404): - raise exc + return Response( + status=HTTPStatus.MOVED_PERMANENTLY, + headers={"Location": absolute_new_url}, + ) class OpinionViewSet(