launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29399
[Merge] ~andrey-fedoseev/launchpad:edit-feature-rules into launchpad:master
Andrey Fedoseev has proposed merging ~andrey-fedoseev/launchpad:edit-feature-rules into launchpad:master.
Commit message:
Add redirects from old feature rules URLs
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/433363
/+feature-info -> /+feature-rules/info
/+feature-changelog -> /+feature-rules/changelog
Add `responseStatusCode` property to test browser classes
Update the existing tests to use the new property
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad:edit-feature-rules into launchpad:master.
diff --git a/lib/lp/charms/browser/tests/test_charmrecipe.py b/lib/lp/charms/browser/tests/test_charmrecipe.py
index 7db9c4c..f3ca37e 100644
--- a/lib/lp/charms/browser/tests/test_charmrecipe.py
+++ b/lib/lp/charms/browser/tests/test_charmrecipe.py
@@ -430,7 +430,7 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
]
),
)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
return_to_matcher = AfterPreprocessing(
urlsplit,
MatchesStructure(
@@ -716,7 +716,7 @@ class TestCharmRecipeEditView(BaseTestCharmRecipeView):
browser.getControl("Update charm recipe").click()
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
browser = self.getViewBrowser(recipe, user=self.person)
content = find_main_content(browser.contents)
@@ -792,7 +792,7 @@ class TestCharmRecipeEditView(BaseTestCharmRecipeView):
browser.getControl("Update charm recipe").click()
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
browser = self.getViewBrowser(recipe, user=self.person)
content = find_main_content(browser.contents)
@@ -1101,7 +1101,7 @@ class TestCharmRecipeAuthorizeView(BaseTestCharmRecipeView):
self.assertEqual(
{"root": root_macaroon_raw}, self.recipe.store_secrets
)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
return_to_matcher = AfterPreprocessing(
urlsplit,
MatchesStructure(
diff --git a/lib/lp/charms/tests/test_charmrecipebuild.py b/lib/lp/charms/tests/test_charmrecipebuild.py
index 1e7b069..6a949db 100644
--- a/lib/lp/charms/tests/test_charmrecipebuild.py
+++ b/lib/lp/charms/tests/test_charmrecipebuild.py
@@ -985,7 +985,7 @@ class TestCharmRecipeBuildWebservice(TestCaseWithFactory):
def assertCanOpenRedirectedUrl(self, browser, url):
browser.open(url)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
urlopen(browser.headers["Location"]).close()
def test_logs(self):
diff --git a/lib/lp/code/browser/tests/test_branch.py b/lib/lp/code/browser/tests/test_branch.py
index e8d914e..8d49a15 100644
--- a/lib/lp/code/browser/tests/test_branch.py
+++ b/lib/lp/code/browser/tests/test_branch.py
@@ -1413,7 +1413,7 @@ class TestBranchDiffView(BrowserTestCase):
browser = self.getUserBrowser()
browser.raiseHttpErrors = False
browser.open(branch_url + "/+diff/2/1")
- self.assertEqual(401, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(401, browser.responseStatusCode)
self.assertEqual(
"Proxying of branch diffs is disabled.\n", browser.contents
)
diff --git a/lib/lp/code/model/tests/test_cibuild.py b/lib/lp/code/model/tests/test_cibuild.py
index 8f555a1..755eb96 100644
--- a/lib/lp/code/model/tests/test_cibuild.py
+++ b/lib/lp/code/model/tests/test_cibuild.py
@@ -1457,7 +1457,7 @@ class TestCIBuildWebservice(TestCaseWithFactory):
def assertCanOpenRedirectedUrl(self, browser, url):
browser.open(url)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
urlopen(browser.headers["Location"]).close()
def test_logs(self):
diff --git a/lib/lp/code/model/tests/test_revisionstatus.py b/lib/lp/code/model/tests/test_revisionstatus.py
index afc7ff2..1ab7618 100644
--- a/lib/lp/code/model/tests/test_revisionstatus.py
+++ b/lib/lp/code/model/tests/test_revisionstatus.py
@@ -452,9 +452,7 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
# ensure the url works
browser = self.getNonRedirectingBrowser()
browser.open(log_url)
- self.assertEqual(
- 303, int(browser.headers["Status"].split(" ", 1)[0])
- )
+ self.assertEqual(303, browser.responseStatusCode)
self.assertEqual(
b"log_data", requests.get(browser.headers["Location"]).content
)
@@ -517,9 +515,7 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
# we should be redirected to librarian with authentication
browser = self.getNonRedirectingBrowser(user=requester)
browser.open(log_url)
- self.assertEqual(
- 303, int(browser.headers["Status"].split(" ", 1)[0])
- )
+ self.assertEqual(303, browser.responseStatusCode)
# Actually requesting files from the restricted librarian is
# cumbersome, but at least test that we're redirected to the
# restricted librarian with a suitable token.
diff --git a/lib/lp/oci/tests/test_ocirecipebuild.py b/lib/lp/oci/tests/test_ocirecipebuild.py
index 9dbc708..da54f9c 100644
--- a/lib/lp/oci/tests/test_ocirecipebuild.py
+++ b/lib/lp/oci/tests/test_ocirecipebuild.py
@@ -942,7 +942,7 @@ class TestOCIRecipeBuildWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
def assertCanOpenRedirectedUrl(self, browser, url):
browser.open(url)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
urlopen(browser.headers["Location"]).close()
def test_logs(self):
diff --git a/lib/lp/services/features/browser/configure.zcml b/lib/lp/services/features/browser/configure.zcml
index 344dbf8..268166b 100644
--- a/lib/lp/services/features/browser/configure.zcml
+++ b/lib/lp/services/features/browser/configure.zcml
@@ -35,10 +35,26 @@
template="../templates/feature-info.pt"/>
<browser:page
+ for="lp.services.webapp.interfaces.ILaunchpadRoot"
+ class="lp.services.features.browser.redirect.FeatureRulesRedirectView"
+ name="+feature-info"
+ permission="launchpad.Edit"
+ attribute="redirect_to_info"
+ />
+
+ <browser:page
for="lp.services.features.interfaces.IFeatureRules"
class="lp.services.features.browser.changelog.FeatureChangeLogView"
name="changelog"
permission="launchpad.View"
template="../templates/feature-changelog.pt"/>
+ <browser:page
+ for="lp.services.webapp.interfaces.ILaunchpadRoot"
+ class="lp.services.features.browser.redirect.FeatureRulesRedirectView"
+ name="+feature-changelog"
+ permission="launchpad.Edit"
+ attribute="redirect_to_changelog"
+ />
+
</configure>
diff --git a/lib/lp/services/features/browser/redirect.py b/lib/lp/services/features/browser/redirect.py
new file mode 100644
index 0000000..e4053eb
--- /dev/null
+++ b/lib/lp/services/features/browser/redirect.py
@@ -0,0 +1,35 @@
+# Copyright 2022 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Redirect to new feature rule URLs."""
+
+__all__ = [
+ "FeatureRulesRedirectView",
+]
+
+from zope.component import getUtility
+
+from lp.services.features.interfaces import IFeatureRules
+from lp.services.webapp.publisher import canonical_url
+
+
+class FeatureRulesRedirectView:
+ """Redirect to new feature rule URLs."""
+
+ def __init__(self, context, request):
+ self.context = context
+ self.request = request
+ self.feature_rules = getUtility(IFeatureRules)
+
+ def redirect_to_info(self):
+ """/+feature-info -> /+feature-rules/info"""
+ return self.request.response.redirect(
+ canonical_url(self.feature_rules, view_name="info"), status=301
+ )
+
+ def redirect_to_changelog(self):
+ """/+feature-changelog -> /+feature-rules/changelog"""
+ return self.request.response.redirect(
+ canonical_url(self.feature_rules, view_name="changelog"),
+ status=301,
+ )
diff --git a/lib/lp/services/features/browser/tests/test_redirect.py b/lib/lp/services/features/browser/tests/test_redirect.py
new file mode 100644
index 0000000..fa4cd9e
--- /dev/null
+++ b/lib/lp/services/features/browser/tests/test_redirect.py
@@ -0,0 +1,48 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for feature flag change log views."""
+from http.client import MOVED_PERMANENTLY
+
+from zope.component import getUtility
+
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.services.webapp import canonical_url
+from lp.services.webapp.interfaces import ILaunchpadRoot
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestRedirectView(TestCaseWithFactory):
+ """Test the feature flag ChangeLog view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self, **kwargs):
+ super().setUp(**kwargs)
+ self.root = getUtility(ILaunchpadRoot)
+ admin = self.factory.makePerson(
+ member_of=[getUtility(ILaunchpadCelebrities).admin]
+ )
+ self.admin_browser = self.getNonRedirectingBrowser(user=admin)
+
+ def test_redirect_to_info(self):
+ self.admin_browser.open(canonical_url(self.root) + "+feature-info")
+ self.assertEqual(
+ MOVED_PERMANENTLY,
+ self.admin_browser.responseStatusCode,
+ )
+ self.assertEndsWith(
+ self.admin_browser.headers["Location"], "/+feature-rules/info"
+ )
+
+ def test_redirect_changelog(self):
+ self.admin_browser.open(
+ canonical_url(self.root) + "+feature-changelog"
+ )
+ self.assertEqual(
+ self.admin_browser.responseStatusCode, MOVED_PERMANENTLY
+ )
+ self.assertEndsWith(
+ self.admin_browser.headers["Location"], "/+feature-rules/changelog"
+ )
diff --git a/lib/lp/services/webapp/tests/test_login.py b/lib/lp/services/webapp/tests/test_login.py
index e2f5f49..3647d12 100644
--- a/lib/lp/services/webapp/tests/test_login.py
+++ b/lib/lp/services/webapp/tests/test_login.py
@@ -765,15 +765,12 @@ class TestOpenIDReplayAttack(TestCaseWithFactory):
self.assertEqual("Login", browser.title)
fill_login_form_and_submit(browser, "test@xxxxxxxxxxxxx")
- self.assertEqual(
- http.client.FOUND, int(browser.headers["Status"].split(" ", 1)[0])
- )
+ self.assertEqual(http.client.FOUND, browser.responseStatusCode)
callback_url = browser.headers["Location"]
self.assertIn("+openid-callback", callback_url)
browser.open(callback_url)
self.assertEqual(
- http.client.TEMPORARY_REDIRECT,
- int(browser.headers["Status"].split(" ", 1)[0]),
+ http.client.TEMPORARY_REDIRECT, browser.responseStatusCode
)
browser.open(browser.headers["Location"])
login_status = extract_text(
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index 4089efc..c9eb67c 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -682,7 +682,7 @@ class TestSnapAddView(BaseTestSnapView):
self.assertEqual(
expected_body, json.loads(call.request.body.decode("UTF-8"))
)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
parsed_location = urlsplit(browser.headers["Location"])
self.assertEqual(
urlsplit(canonical_url(snap) + "/+authorize/+login")[:3],
@@ -1729,7 +1729,7 @@ class TestSnapEditView(BaseTestSnapView):
self.assertEqual(
expected_body, json.loads(call.request.body.decode("UTF-8"))
)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
parsed_location = urlsplit(browser.headers["Location"])
self.assertEqual(
urlsplit(canonical_url(snap) + "/+authorize/+login")[:3],
@@ -1815,7 +1815,7 @@ class TestSnapAuthorizeView(BaseTestSnapView):
self.assertEqual(
{"root": root_macaroon_raw}, self.snap.store_secrets
)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
self.assertEqual(
snap_url + "/+authorize/+login?macaroon_caveat_id=dummy&"
"discharge_macaroon_action=field.actions.complete&"
diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
index c1e0378..33a1d1d 100644
--- a/lib/lp/snappy/tests/test_snapbuild.py
+++ b/lib/lp/snappy/tests/test_snapbuild.py
@@ -983,7 +983,7 @@ class TestSnapBuildWebservice(TestCaseWithFactory):
def assertCanOpenRedirectedUrl(self, browser, url):
browser.open(url)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
urlopen(browser.headers["Location"]).close()
def test_logs(self):
diff --git a/lib/lp/soyuz/tests/test_livefsbuild.py b/lib/lp/soyuz/tests/test_livefsbuild.py
index b8f33b0..83110e0 100644
--- a/lib/lp/soyuz/tests/test_livefsbuild.py
+++ b/lib/lp/soyuz/tests/test_livefsbuild.py
@@ -610,7 +610,7 @@ class TestLiveFSBuildWebservice(TestCaseWithFactory):
def assertCanOpenRedirectedUrl(self, browser, url):
browser.open(url)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
urlopen(browser.headers["Location"]).close()
def test_logs(self):
diff --git a/lib/lp/soyuz/tests/test_packageupload.py b/lib/lp/soyuz/tests/test_packageupload.py
index 654a0d6..7e56c8b 100644
--- a/lib/lp/soyuz/tests/test_packageupload.py
+++ b/lib/lp/soyuz/tests/test_packageupload.py
@@ -1235,7 +1235,7 @@ class TestPackageUploadWebservice(TestCaseWithFactory):
def assertCanOpenRedirectedUrl(self, browser, url):
browser.open(url)
- self.assertEqual(303, int(browser.headers["Status"].split(" ", 1)[0]))
+ self.assertEqual(303, browser.responseStatusCode)
urlopen(browser.headers["Location"]).close()
def assertRequiresEdit(self, method_name, **kwargs):
diff --git a/lib/lp/testing/browser.py b/lib/lp/testing/browser.py
index 6a4a665..be59f2b 100644
--- a/lib/lp/testing/browser.py
+++ b/lib/lp/testing/browser.py
@@ -14,6 +14,7 @@ __all__ = [
]
import ssl
+from typing import Optional
import six
from lazr.uri import URI
@@ -63,6 +64,13 @@ class Browser(_Browser):
uri = URI(self.url)
return uri.path
+ @property
+ def responseStatusCode(self) -> Optional[int]:
+ try:
+ return int(self.headers["Status"].split(" ", 1)[0])
+ except (KeyError, IndexError, ValueError):
+ return None
+
def setUp(test):
"""Set up appserver tests."""
diff --git a/lib/lp/testing/pages.py b/lib/lp/testing/pages.py
index 632d4ec..0a032f4 100644
--- a/lib/lp/testing/pages.py
+++ b/lib/lp/testing/pages.py
@@ -23,6 +23,7 @@ from contextlib import contextmanager
from datetime import datetime
from io import BytesIO
from itertools import chain
+from typing import Optional
from urllib.parse import urljoin
import six
@@ -816,6 +817,13 @@ class Browser(_Browser):
return Link(elem, self, baseurl)
+ @property
+ def responseStatusCode(self) -> Optional[int]:
+ try:
+ return int(self.headers["Status"].split(" ", 1)[0])
+ except (KeyError, IndexError, ValueError):
+ return None
+
def setupBrowser(auth=None):
"""Create a testbrowser object for use in pagetests.
