[Launchpad-reviewers] [Merge] ~pappacena/launchpad:snap-pillar-product-url into launchpad:master
The proposal to merge ~pappacena/launchpad:snap-pillar-product-url into launchpad:master has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399025 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:snap-pillar-list-filters. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:snap-pillar-product-url into launchpad:master
The proposal to merge ~pappacena/launchpad:snap-pillar-product-url into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399025 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:snap-pillar-list-filters. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:snap-pillar-product-url into launchpad:master
Pushed the requested changes.
Diff comments:
> diff --git a/lib/lp/registry/browser/person.py
> b/lib/lp/registry/browser/person.py
> index 8311bd1..91da499 100644
> --- a/lib/lp/registry/browser/person.py
> +++ b/lib/lp/registry/browser/person.py
> @@ -646,7 +646,12 @@ class PersonNavigation(BranchTraversalMixin, Navigation):
> @stepthrough('+snap')
> def traverse_snap(self, name):
> """Traverse to this person's snap packages."""
> -return getUtility(ISnapSet).getByName(self.context, name)
> +snap = getUtility(ISnapSet).getByName(self.context, name)
> +# If it's a snap attached to a pillar, this is not the right place
> +# to traverse. The correct URL should be under IPersonProduct.
> +if snap.project:
> +raise NotFoundError(name)
Changing it (and doing the adjustments on `getByPillarAndName` method)
> +return snap
>
>
> class PersonSetNavigation(Navigation):
> diff --git a/lib/lp/snappy/browser/tests/test_snap.py
> b/lib/lp/snappy/browser/tests/test_snap.py
> index 7b8981b..25383d9 100644
> --- a/lib/lp/snappy/browser/tests/test_snap.py
> +++ b/lib/lp/snappy/browser/tests/test_snap.py
> @@ -1521,6 +1519,15 @@ class TestSnapView(BaseTestSnapView):
> "snap breadcrumb", "li",
> text=re.compile(r"\ssnap-name\s")
>
> +def test_snap_with_project_pillar_redirects(self):
Ok!
> +project = self.factory.makeProduct()
> +snap = self.factory.makeSnap(project=project)
> +browser = self.getViewBrowser(snap)
> +with admin_logged_in():
> +expected_url = 'http://launchpad.test/~{}/{}/+snap/{}'.format(
> +snap.owner.name, project.name, snap.name)
> +self.assertEqual(expected_url, browser.url)
> +
> def test_index_bzr(self):
> branch = self.factory.makePersonalBranch(
> owner=self.person, name="snap-branch")
> diff --git a/lib/lp/snappy/tests/test_snap.py
> b/lib/lp/snappy/tests/test_snap.py
> index 2a522f5..604aacc 100644
> --- a/lib/lp/snappy/tests/test_snap.py
> +++ b/lib/lp/snappy/tests/test_snap.py
> @@ -2887,9 +2891,13 @@ class TestSnapWebservice(TestCaseWithFactory):
> admin_webservice = webservice_for_person(
> admin, permission=OAuthPermission.WRITE_PRIVATE)
> admin_webservice.default_api_version = "devel"
> -response = admin_webservice.patch(
> -snap_url, "application/json",
> -json.dumps({"information_type": 'Public'}))
> +data = json.dumps({"information_type": 'Public'})
> +content_type = "application/json"
> +response = admin_webservice.patch(snap_url, content_type, data)
> +# If it's a redirect, try again.
> +if response.status == 301:
> +location = urlsplit(response.getheader('location')).path
> +response = admin_webservice.patch(location, content_type, data)
It shouldn't. Removing it.
> self.assertEqual(400, response.status)
> self.assertEqual(
> b"Snap recipe contains private information and cannot be
> public.",
> diff --git a/lib/lp/snappy/tests/test_snapbuild.py
> b/lib/lp/snappy/tests/test_snapbuild.py
> index 7acff54..e12eb61 100644
> --- a/lib/lp/snappy/tests/test_snapbuild.py
> +++ b/lib/lp/snappy/tests/test_snapbuild.py
> @@ -789,7 +790,11 @@ class TestSnapBuildWebservice(TestCaseWithFactory):
> self.factory.makePerson(),
> permission=OAuthPermission.WRITE_PUBLIC)
> unpriv_webservice.default_api_version = "devel"
> logout()
> -self.assertEqual(200, self.webservice.get(build_url).status)
> +response = self.webservice.get(build_url)
> +if response.status == 301:
> +location = urlsplit(response.getheader('location')).path
> +response = self.webservice.get(location)
It shouldn't. Removing it.
> +self.assertEqual(200, response.status)
> # 404 since we aren't allowed to know that the private team exists.
> self.assertEqual(404, unpriv_webservice.get(build_url).status)
>
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399025
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:snap-pillar-list-filters.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:snap-pillar-product-url into launchpad:master
Review: Approve
Diff comments:
> diff --git a/lib/lp/registry/browser/person.py
> b/lib/lp/registry/browser/person.py
> index 8311bd1..91da499 100644
> --- a/lib/lp/registry/browser/person.py
> +++ b/lib/lp/registry/browser/person.py
> @@ -646,7 +646,12 @@ class PersonNavigation(BranchTraversalMixin, Navigation):
> @stepthrough('+snap')
> def traverse_snap(self, name):
> """Traverse to this person's snap packages."""
> -return getUtility(ISnapSet).getByName(self.context, name)
> +snap = getUtility(ISnapSet).getByName(self.context, name)
> +# If it's a snap attached to a pillar, this is not the right place
> +# to traverse. The correct URL should be under IPersonProduct.
> +if snap.project:
> +raise NotFoundError(name)
I'd prefer something like `return
getUtility(ISnapSet).getByPillarAndName(self.context, None, name)` (which I
realize would need a bit of work in that method).
> +return snap
>
>
> class PersonSetNavigation(Navigation):
> diff --git a/lib/lp/snappy/browser/tests/test_snap.py
> b/lib/lp/snappy/browser/tests/test_snap.py
> index 7b8981b..25383d9 100644
> --- a/lib/lp/snappy/browser/tests/test_snap.py
> +++ b/lib/lp/snappy/browser/tests/test_snap.py
> @@ -1521,6 +1519,15 @@ class TestSnapView(BaseTestSnapView):
> "snap breadcrumb", "li",
> text=re.compile(r"\ssnap-name\s")
>
> +def test_snap_with_project_pillar_redirects(self):
This test name no longer seems quite right.
> +project = self.factory.makeProduct()
> +snap = self.factory.makeSnap(project=project)
> +browser = self.getViewBrowser(snap)
> +with admin_logged_in():
> +expected_url = 'http://launchpad.test/~{}/{}/+snap/{}'.format(
> +snap.owner.name, project.name, snap.name)
> +self.assertEqual(expected_url, browser.url)
> +
> def test_index_bzr(self):
> branch = self.factory.makePersonalBranch(
> owner=self.person, name="snap-branch")
> diff --git a/lib/lp/snappy/tests/test_snap.py
> b/lib/lp/snappy/tests/test_snap.py
> index 2a522f5..604aacc 100644
> --- a/lib/lp/snappy/tests/test_snap.py
> +++ b/lib/lp/snappy/tests/test_snap.py
> @@ -2887,9 +2891,13 @@ class TestSnapWebservice(TestCaseWithFactory):
> admin_webservice = webservice_for_person(
> admin, permission=OAuthPermission.WRITE_PRIVATE)
> admin_webservice.default_api_version = "devel"
> -response = admin_webservice.patch(
> -snap_url, "application/json",
> -json.dumps({"information_type": 'Public'}))
> +data = json.dumps({"information_type": 'Public'})
> +content_type = "application/json"
> +response = admin_webservice.patch(snap_url, content_type, data)
> +# If it's a redirect, try again.
> +if response.status == 301:
> +location = urlsplit(response.getheader('location')).path
> +response = admin_webservice.patch(location, content_type, data)
Is this still needed, now that you're no longer redirecting?
> self.assertEqual(400, response.status)
> self.assertEqual(
> b"Snap recipe contains private information and cannot be
> public.",
> diff --git a/lib/lp/snappy/tests/test_snapbuild.py
> b/lib/lp/snappy/tests/test_snapbuild.py
> index 7acff54..e12eb61 100644
> --- a/lib/lp/snappy/tests/test_snapbuild.py
> +++ b/lib/lp/snappy/tests/test_snapbuild.py
> @@ -789,7 +790,11 @@ class TestSnapBuildWebservice(TestCaseWithFactory):
> self.factory.makePerson(),
> permission=OAuthPermission.WRITE_PUBLIC)
> unpriv_webservice.default_api_version = "devel"
> logout()
> -self.assertEqual(200, self.webservice.get(build_url).status)
> +response = self.webservice.get(build_url)
> +if response.status == 301:
> +location = urlsplit(response.getheader('location')).path
> +response = self.webservice.get(location)
Is this still needed, now that you're no longer redirecting?
> +self.assertEqual(200, response.status)
> # 404 since we aren't allowed to know that the private team exists.
> self.assertEqual(404, unpriv_webservice.get(build_url).status)
>
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399025
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:snap-pillar-list-filters.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:snap-pillar-product-url into launchpad:master
Pushed the requested changes. This should be ready for another round of review.
Diff comments:
> diff --git a/lib/lp/registry/browser/person.py
> b/lib/lp/registry/browser/person.py
> index 31c29ed..f1bb302 100644
> --- a/lib/lp/registry/browser/person.py
> +++ b/lib/lp/registry/browser/person.py
> @@ -647,7 +648,17 @@ class PersonNavigation(BranchTraversalMixin, Navigation):
> @stepthrough('+snap')
> def traverse_snap(self, name):
> """Traverse to this person's snap packages."""
> -return getUtility(ISnapSet).getByName(self.context, name)
> +snap = getUtility(ISnapSet).getByName(self.context, name)
> +# If it's a snap attached to a pillar, redirect to the Snap
> +# URL under pillar's URL.
Ok! I'm changing definition for the Snap, which should make
this traverse to not be used when the snap has a pillar, but I'll raise a 404
here just in case.
> +if snap.project:
> +person_product = getUtility(IPersonProductFactory).create(
> +self.context, snap.project)
> +project_url = canonical_url(person_product)
> +snap_url = urljoin(project_url + '/', '+snap/')
> +snap_url = urljoin(snap_url, snap.name)
> +return self.redirectSubTree(snap_url)
> +return snap
>
>
> class PersonSetNavigation(Navigation):
> diff --git a/lib/lp/snappy/browser/tests/test_snap.py
> b/lib/lp/snappy/browser/tests/test_snap.py
> index 7b8981b..a969292 100644
> --- a/lib/lp/snappy/browser/tests/test_snap.py
> +++ b/lib/lp/snappy/browser/tests/test_snap.py
> @@ -58,6 +58,7 @@ from lp.code.tests.helpers import (
> )
> from lp.registry.enums import (
> BranchSharingPolicy,
> +BranchSharingPolicy,
Probably some merge confusion. Fixing it.
> PersonVisibility,
> TeamMembershipPolicy,
> )
> diff --git a/lib/lp/snappy/browser/tests/test_snapsubscription.py
> b/lib/lp/snappy/browser/tests/test_snapsubscription.py
> index ef021d9..b091e95 100644
> --- a/lib/lp/snappy/browser/tests/test_snapsubscription.py
> +++ b/lib/lp/snappy/browser/tests/test_snapsubscription.py
> @@ -9,6 +9,9 @@ __metaclass__ = type
>
>
> from fixtures import FakeLogger
> +from lp.registry.interfaces.personproduct import IPersonProductFactory
Ok!
> +from six.moves.urllib.parse import urljoin
> +from zope.component._api import getUtility
pycharm really likes importing this from _api... fixing it.
> from zope.security.interfaces import Unauthorized
>
> from lp.app.enums import InformationType
> @@ -65,6 +68,17 @@ class BaseTestSnapView(BrowserTestCase):
>
> class TestPublicSnapSubscriptionViews(BaseTestSnapView):
>
> +def getSnapURL(self, snap):
> +with admin_logged_in():
> +if snap.project:
> +person_product = getUtility(IPersonProductFactory).create(
> +snap.owner, snap.project)
> +project_url = canonical_url(person_product)
> +snap_url = urljoin(project_url + '/', '+snap/')
> +return urljoin(snap_url, snap.name)
I agree. Fixing the URL mapping, and removing this method in favor of raw
`canonical_url`.
> +else:
> +return canonical_url(snap)
> +
> def test_subscribe_self(self):
> snap = self.makeSnap()
> another_user = self.factory.makePerson(name="another-user")
> diff --git a/lib/lp/snappy/tests/test_snap.py
> b/lib/lp/snappy/tests/test_snap.py
> index 80bcaf4..528c0fa 100644
> --- a/lib/lp/snappy/tests/test_snap.py
> +++ b/lib/lp/snappy/tests/test_snap.py
> @@ -69,6 +69,7 @@ from lp.code.tests.helpers import (
> )
> from lp.registry.enums import (
> BranchSharingPolicy,
> +BranchSharingPolicy,
Ok!
> PersonVisibility,
> TeamMembershipPolicy,
> )
> diff --git a/lib/lp/snappy/tests/test_snapbuild.py
> b/lib/lp/snappy/tests/test_snapbuild.py
> index 7acff54..8404c86 100644
> --- a/lib/lp/snappy/tests/test_snapbuild.py
> +++ b/lib/lp/snappy/tests/test_snapbuild.py
> @@ -17,6 +17,7 @@ from pymacaroons import Macaroon
> import pytz
> import six
> from six.moves.urllib.request import urlopen
> +from six.moves.urllib.parse import urlsplit
Ok!
> from testtools.matchers import (
> ContainsDict,
> Equals,
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399025
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:snap-pillar-list-filters.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:snap-pillar-product-url into launchpad:master
Review: Needs Information
Diff comments:
> diff --git a/lib/lp/registry/browser/person.py
> b/lib/lp/registry/browser/person.py
> index 31c29ed..f1bb302 100644
> --- a/lib/lp/registry/browser/person.py
> +++ b/lib/lp/registry/browser/person.py
> @@ -647,7 +648,17 @@ class PersonNavigation(BranchTraversalMixin, Navigation):
> @stepthrough('+snap')
> def traverse_snap(self, name):
> """Traverse to this person's snap packages."""
> -return getUtility(ISnapSet).getByName(self.context, name)
> +snap = getUtility(ISnapSet).getByName(self.context, name)
> +# If it's a snap attached to a pillar, redirect to the Snap
> +# URL under pillar's URL.
The redirection is a bit unusual; we do it for some things like bug tasks and
questions that are commonly moved around, but this sort of thing would normally
be expected to stay where it's put. Can we just require Person:+snap/name to
traverse only to pillarless snaps?
> +if snap.project:
> +person_product = getUtility(IPersonProductFactory).create(
> +self.context, snap.project)
> +project_url = canonical_url(person_product)
> +snap_url = urljoin(project_url + '/', '+snap/')
> +snap_url = urljoin(snap_url, snap.name)
> +return self.redirectSubTree(snap_url)
> +return snap
>
>
> class PersonSetNavigation(Navigation):
> diff --git a/lib/lp/snappy/browser/tests/test_snap.py
> b/lib/lp/snappy/browser/tests/test_snap.py
> index 7b8981b..a969292 100644
> --- a/lib/lp/snappy/browser/tests/test_snap.py
> +++ b/lib/lp/snappy/browser/tests/test_snap.py
> @@ -58,6 +58,7 @@ from lp.code.tests.helpers import (
> )
> from lp.registry.enums import (
> BranchSharingPolicy,
> +BranchSharingPolicy,
Duplicate import.
> PersonVisibility,
> TeamMembershipPolicy,
> )
> diff --git a/lib/lp/snappy/browser/tests/test_snapsubscription.py
> b/lib/lp/snappy/browser/tests/test_snapsubscription.py
> index ef021d9..b091e95 100644
> --- a/lib/lp/snappy/browser/tests/test_snapsubscription.py
> +++ b/lib/lp/snappy/browser/tests/test_snapsubscription.py
> @@ -9,6 +9,9 @@ __metaclass__ = type
>
>
> from fixtures import FakeLogger
> +from lp.registry.interfaces.personproduct import IPersonProductFactory
Could use a format-imports here.
> +from six.moves.urllib.parse import urljoin
> +from zope.component._api import getUtility
Just zope.component.
> from zope.security.interfaces import Unauthorized
>
> from lp.app.enums import InformationType
> @@ -65,6 +68,17 @@ class BaseTestSnapView(BrowserTestCase):
>
> class TestPublicSnapSubscriptionViews(BaseTestSnapView):
>
> +def getSnapURL(self, snap):
> +with admin_logged_in():
> +if snap.project:
> +person_product = getUtility(IPersonProductFactory).create(
> +snap.owner, snap.project)
> +project_url = canonical_url(person_product)
> +snap_url = urljoin(project_url + '/', '+snap/')
> +return urljoin(snap_url, snap.name)
This seems quite circuitous; generally speaking canonical_url should just work,
and if it doesn't then we need to fix it.
> +else:
> +return canonical_url(snap)
> +
> def test_subscribe_self(self):
> snap = self.makeSnap()
> another_user = self.factory.makePerson(name="another-user")
> diff --git a/lib/lp/snappy/tests/test_snap.py
> b/lib/lp/snappy/tests/test_snap.py
> index 80bcaf4..528c0fa 100644
> --- a/lib/lp/snappy/tests/test_snap.py
> +++ b/lib/lp/snappy/tests/test_snap.py
> @@ -69,6 +69,7 @@ from lp.code.tests.helpers import (
> )
> from lp.registry.enums import (
> BranchSharingPolicy,
> +BranchSharingPolicy,
Duplicate import.
> PersonVisibility,
> TeamMembershipPolicy,
> )
> diff --git a/lib/lp/snappy/tests/test_snapbuild.py
> b/lib/lp/snappy/tests/test_snapbuild.py
> index 7acff54..8404c86 100644
> --- a/lib/lp/snappy/tests/test_snapbuild.py
> +++ b/lib/lp/snappy/tests/test_snapbuild.py
> @@ -17,6 +17,7 @@ from pymacaroons import Macaroon
> import pytz
> import six
> from six.moves.urllib.request import urlopen
> +from six.moves.urllib.parse import urlsplit
Could you sort these?
> from testtools.matchers import (
> ContainsDict,
> Equals,
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399025
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:snap-pillar-list-filters.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:snap-pillar-product-url into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:snap-pillar-product-url into launchpad:master with
~pappacena/launchpad:snap-pillar-list-filters as a prerequisite.
Commit message:
Redirecting snaps with pillar to stay under ~user/pillar URL
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399025
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:snap-pillar-product-url into launchpad:master.
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index 31c29ed..f1bb302 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Person-related view classes."""
@@ -69,6 +69,7 @@ import six
from six.moves.urllib.parse import (
quote,
urlencode,
+urljoin,
)
from storm.zope.interfaces import IResultSet
from zope.browserpage import ViewPageTemplateFile
@@ -647,7 +648,17 @@ class PersonNavigation(BranchTraversalMixin, Navigation):
@stepthrough('+snap')
def traverse_snap(self, name):
"""Traverse to this person's snap packages."""
-return getUtility(ISnapSet).getByName(self.context, name)
+snap = getUtility(ISnapSet).getByName(self.context, name)
+# If it's a snap attached to a pillar, redirect to the Snap
+# URL under pillar's URL.
+if snap.project:
+person_product = getUtility(IPersonProductFactory).create(
+self.context, snap.project)
+project_url = canonical_url(person_product)
+snap_url = urljoin(project_url + '/', '+snap/')
+snap_url = urljoin(snap_url, snap.name)
+return self.redirectSubTree(snap_url)
+return snap
class PersonSetNavigation(Navigation):
diff --git a/lib/lp/registry/browser/personproduct.py b/lib/lp/registry/browser/personproduct.py
index a249b27..7d850e3 100644
--- a/lib/lp/registry/browser/personproduct.py
+++ b/lib/lp/registry/browser/personproduct.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Views, menus and traversal related to PersonProducts."""
@@ -30,6 +30,7 @@ from lp.services.webapp import (
)
from lp.services.webapp.breadcrumb import Breadcrumb
from lp.services.webapp.interfaces import IMultiFacetedBreadcrumb
+from lp.snappy.interfaces.snap import ISnapSet
class PersonProductNavigation(PersonTargetDefaultVCSNavigationMixin,
@@ -53,6 +54,13 @@ class PersonProductNavigation(PersonTargetDefaultVCSNavigationMixin,
else:
return branch
+@stepthrough('+snap')
+def traverse_snap(self, name):
+return getUtility(ISnapSet).getByPillarAndName(
+owner=self.context.person,
+pillar=self.context.product,
+name=name)
+
@implementer(IMultiFacetedBreadcrumb)
class PersonProductBreadcrumb(Breadcrumb):
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index 61f514a..a8586a2 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -58,6 +58,7 @@ from lp.code.tests.helpers import (
)
from lp.registry.enums import (
BranchSharingPolicy,
+BranchSharingPolicy,
PersonVisibility,
TeamMembershipPolicy,
)
@@ -1521,6 +1522,15 @@ class TestSnapView(BaseTestSnapView):
"snap breadcrumb", "li",
text=re.compile(r"\ssnap-name\s")
+def test_snap_with_project_pillar_redirects(self):
+project = self.factory.makeProduct()
+snap = self.factory.makeSnap(project=project)
+browser = self.getViewBrowser(snap)
+with admin_logged_in():
+expected_url = 'http://launchpad.test/~{}/{}/+snap/{}'.format(
+snap.owner.name, project.name, snap.name)
+self.assertEqual(expected_url, browser.url)
+
def test_index_bzr(self):
branch = self.factory.makePersonalBranch(
owner=self.person, name="snap-branch")
diff --git a/lib/lp/snappy/browser/tests/test_snapsubscription.py b/lib/lp/snappy/browser/tests/test_snapsubscription.py
index ef021d9..b091e95 100644
--- a/lib/lp/snappy/browser/tests/test_snapsubscription.py
+++ b/lib/lp/snappy/browser/tests/test_snapsubscription.py
@@ -9,6 +9,9 @@ __metaclass__ = type
from fixtures import FakeLogger
+from lp.registry.interfaces.personproduct import IPersonProductFactory
+from six.moves.urllib.parse
