launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:series-urls into launchpad:master.

Commit message:
Add alternate <pillar>/+series/<name> URLs

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/385489

These may be used instead of <pillar>/<name> URLs.  For now they just redirect to the classic URLs, but that may change later.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:series-urls into launchpad:master.

diff --git a/lib/lp/registry/browser/distribution.py b/lib/lp/registry/browser/distribution.py
index ffabedb..29eab7a 100644
--- a/lib/lp/registry/browser/distribution.py
+++ b/lib/lp/registry/browser/distribution.py
@@ -121,7 +121,6 @@ from lp.services.webapp import (
     canonical_url,
     ContextMenu,
     enabled_with_permission,
-    GetitemNavigation,
     LaunchpadView,
     Link,
     Navigation,
@@ -140,7 +139,7 @@ from lp.soyuz.interfaces.archive import IArchiveSet
 
 
 class DistributionNavigation(
-    GetitemNavigation, BugTargetTraversalMixin, QuestionTargetTraversalMixin,
+    Navigation, BugTargetTraversalMixin, QuestionTargetTraversalMixin,
     FAQTargetNavigationMixin, StructuralSubscriptionTargetTraversalMixin,
     PillarNavigationMixin, TargetDefaultVCSNavigationMixin):
 
@@ -178,12 +177,24 @@ class DistributionNavigation(
     def traverse_archive(self, name):
         return self.context.getArchive(name)
 
-    def traverse(self, name):
+    def _resolveSeries(self, name):
         try:
-            return super(DistributionNavigation, self).traverse(name)
+            return self.context[name], False
         except NotFoundError:
             resolved = self.context.resolveSeriesAlias(name)
-            return self.redirectSubTree(canonical_url(resolved), status=303)
+            return resolved, True
+
+    @stepthrough('+series')
+    def traverse_series(self, name):
+        series, _ = self._resolveSeries(name)
+        return self.redirectSubTree(canonical_url(series), status=303)
+
+    def traverse(self, name):
+        series, is_alias = self._resolveSeries(name)
+        if is_alias:
+            return self.redirectSubTree(canonical_url(series), status=303)
+        else:
+            return series
 
 
 class DistributionSetNavigation(Navigation):
diff --git a/lib/lp/registry/browser/product.py b/lib/lp/registry/browser/product.py
index c059dbb..5c3ebce 100644
--- a/lib/lp/registry/browser/product.py
+++ b/lib/lp/registry/browser/product.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser views for products."""
@@ -275,6 +275,11 @@ class ProductNavigation(
     def traverse_commercialsubscription(self, name):
         return self.context.commercial_subscription
 
+    @stepthrough('+series')
+    def traverse_series(self, name):
+        series = self.context.getSeries(name)
+        return self.redirectSubTree(canonical_url(series), status=303)
+
     def traverse(self, name):
         return self.context.getSeries(name)
 
diff --git a/lib/lp/registry/browser/tests/test_distribution.py b/lib/lp/registry/browser/tests/test_distribution.py
index 0979ec2..f7f37fd 100644
--- a/lib/lp/registry/browser/tests/test_distribution.py
+++ b/lib/lp/registry/browser/tests/test_distribution.py
@@ -17,11 +17,13 @@ from testtools.matchers import (
     Not,
     )
 from zope.schema.vocabulary import SimpleVocabulary
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
 from lp.registry.enums import EXCLUSIVE_TEAM_POLICY
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.webapp import canonical_url
+from lp.services.webapp.publisher import RedirectionView
 from lp.testing import (
     admin_logged_in,
     BrowserTestCase,
@@ -30,15 +32,61 @@ from lp.testing import (
     record_two_runs,
     TestCaseWithFactory,
     )
-from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    ZopelessDatabaseLayer,
+    )
 from lp.testing.pages import (
     extract_text,
     find_tag_by_id,
     find_tags_by_class,
     )
+from lp.testing.publication import test_traverse
 from lp.testing.views import create_initialized_view
 
 
+class TestDistributionNavigation(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def assertRedirects(self, url, expected_url):
+        _, view, _ = test_traverse(url)
+        self.assertIsInstance(view, RedirectionView)
+        self.assertEqual(expected_url, removeSecurityProxy(view).target)
+
+    def test_classic_series_url(self):
+        distroseries = self.factory.makeDistroSeries()
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/%s"; % (
+                distroseries.distribution.name, distroseries.name))
+        self.assertEqual(distroseries, obj)
+
+    def test_classic_series_url_with_alias(self):
+        distroseries = self.factory.makeDistroSeries()
+        distroseries.distribution.development_series_alias = "devel"
+        self.assertRedirects(
+            "http://launchpad.test/%s/devel"; % distroseries.distribution.name,
+            "http://launchpad.test/%s/%s"; % (
+                distroseries.distribution.name, distroseries.name))
+
+    def test_new_series_url_redirects(self):
+        distroseries = self.factory.makeDistroSeries()
+        self.assertRedirects(
+            "http://launchpad.test/%s/+series/%s"; % (
+                distroseries.distribution.name, distroseries.name),
+            "http://launchpad.test/%s/%s"; % (
+                distroseries.distribution.name, distroseries.name))
+
+    def test_new_series_url_with_alias_redirects(self):
+        distroseries = self.factory.makeDistroSeries()
+        distroseries.distribution.development_series_alias = "devel"
+        self.assertRedirects(
+            "http://launchpad.test/%s/+series/devel"; % (
+                distroseries.distribution.name),
+            "http://launchpad.test/%s/%s"; % (
+                distroseries.distribution.name, distroseries.name))
+
+
 class TestDistributionPage(TestCaseWithFactory):
     """A TestCase for the distribution index page."""
 
diff --git a/lib/lp/registry/browser/tests/test_product.py b/lib/lp/registry/browser/tests/test_product.py
index 4271302..cbf03bc 100644
--- a/lib/lp/registry/browser/tests/test_product.py
+++ b/lib/lp/registry/browser/tests/test_product.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for product views."""
@@ -52,7 +52,10 @@ from lp.registry.interfaces.product import (
 from lp.registry.model.product import Product
 from lp.services.config import config
 from lp.services.database.interfaces import IStore
-from lp.services.webapp.publisher import canonical_url
+from lp.services.webapp.publisher import (
+    canonical_url,
+    RedirectionView,
+    )
 from lp.services.webapp.vhosts import allvhosts
 from lp.testing import (
     BrowserTestCase,
@@ -66,12 +69,14 @@ from lp.testing.fixture import DemoMode
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
+    ZopelessDatabaseLayer,
     )
 from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import (
     extract_text,
     find_tag_by_id,
     )
+from lp.testing.publication import test_traverse
 from lp.testing.service_usage_helpers import set_service_usage
 from lp.testing.views import (
     create_initialized_view,
@@ -79,6 +84,31 @@ from lp.testing.views import (
     )
 
 
+class TestProductNavigation(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def assertRedirects(self, url, expected_url):
+        _, view, _ = test_traverse(url)
+        self.assertIsInstance(view, RedirectionView)
+        self.assertEqual(expected_url, removeSecurityProxy(view).target)
+
+    def test_classic_series_url(self):
+        productseries = self.factory.makeProductSeries()
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/%s"; % (
+                productseries.product.name, productseries.name))
+        self.assertEqual(productseries, obj)
+
+    def test_new_series_url_redirects(self):
+        productseries = self.factory.makeProductSeries()
+        self.assertRedirects(
+            "http://launchpad.test/%s/+series/%s"; % (
+                productseries.product.name, productseries.name),
+            "http://launchpad.test/%s/%s"; % (
+                productseries.product.name, productseries.name))
+
+
 class TestProductConfiguration(BrowserTestCase):
     """Tests the configuration links and helpers."""