launchpad-reviewers team mailing list archive

[Ines Almeida <mp+452572@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:change-traverse-archive-redirect-status into launchpad:master.

Commit message:
Update the redirection status code from 301 to 303 when traversing an archive
    
The 301 signals a permanent redirect, which means that these redirects get cached. This permanent redirection can lead to issues where the wrong redirection gets cached in a browser.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/452572
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:change-traverse-archive-redirect-status into launchpad:master.

diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index c359df7..3bd82ec 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -516,7 +516,8 @@ class PersonNavigation(BranchTraversalMixin, Navigation):
                     self.request.stepstogo.consume()
                 if redirect:
                     return self.redirectSubTree(
-                        canonical_url(ppa, request=self.request)
+                        canonical_url(ppa, request=self.request),
+                        status=303,
                     )
                 else:
                     return ppa
diff --git a/lib/lp/registry/browser/tests/test_person.py b/lib/lp/registry/browser/tests/test_person.py
index 098c2a6..7df31c3 100644
--- a/lib/lp/registry/browser/tests/test_person.py
+++ b/lib/lp/registry/browser/tests/test_person.py
@@ -111,13 +111,15 @@ from lp.testing.views import create_initialized_view, create_view
 class TestPersonNavigation(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer
 
-    def assertRedirect(self, path, redirect):
+    def assertRedirect(self, path, redirect, status_code=None):
         view = test_traverse(path)[1]
         self.assertIsInstance(view, RedirectionView)
         self.assertEqual(
             urljoin(allvhosts.configs["mainsite"].rooturl, redirect),
             removeSecurityProxy(view).target,
         )
+        if status_code:
+            self.assertEqual(status_code, removeSecurityProxy(view).status)
 
     def test_traverse_archive_distroful(self):
         archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
@@ -171,9 +173,13 @@ class TestPersonNavigation(TestCaseWithFactory):
             archive.distribution.name,
             archive.name,
         )
-        self.assertRedirect(in_suf, out_suf)
-        self.assertRedirect("/api/devel" + in_suf, "/api/devel" + out_suf)
-        self.assertRedirect("/api/1.0" + in_suf, "/api/1.0" + out_suf)
+        self.assertRedirect(in_suf, out_suf, status_code=303)
+        self.assertRedirect(
+            "/api/devel" + in_suf, "/api/devel" + out_suf, status_code=303
+        )
+        self.assertRedirect(
+            "/api/1.0" + in_suf, "/api/1.0" + out_suf, status_code=303
+        )
 
     def test_traverse_git_repository_project(self):
         project = self.factory.makeProduct()