launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26420
[Merge] ~pappacena/launchpad:snap-pillar-subscribe-ui into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:snap-pillar-subscribe-ui into launchpad:master with ~pappacena/launchpad:snap-pillar-subscribe-removal-job as a prerequisite.
Commit message:
UI workflow for users to subscribe to snaps
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/398319
Screenshots:
- Snap page: https://private-fileshare.canonical.com/~pappacena/screenshots/private-snaps/snap-page.png
- Edit subscription: https://private-fileshare.canonical.com/~pappacena/screenshots/private-snaps/edit-subscription.png
- Subscribe someone else: https://private-fileshare.canonical.com/~pappacena/screenshots/private-snaps/subscribe-someone-else.png
- Subscribed success message: https://private-fileshare.canonical.com/~pappacena/screenshots/private-snaps/subscribed-msg.png
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:snap-pillar-subscribe-ui into launchpad:master.
diff --git a/lib/lp/registry/tests/test_personmerge.py b/lib/lp/registry/tests/test_personmerge.py
index 4c55cb2..6a89718 100644
--- a/lib/lp/registry/tests/test_personmerge.py
+++ b/lib/lp/registry/tests/test_personmerge.py
@@ -681,22 +681,22 @@ class TestMergePeople(TestCaseWithFactory, KarmaTestMixin):
login_admin()
# Owner should have being subscribed automatically on creation.
self.assertTrue(snap.visibleByUser(duplicate))
- self.assertThat(snap._getSubscription(duplicate), MatchesStructure(
+ self.assertThat(snap.getSubscription(duplicate), MatchesStructure(
snap=Equals(snap),
person=Equals(duplicate)
))
self.assertFalse(snap.visibleByUser(mergee))
- self.assertIsNone(snap._getSubscription(mergee))
+ self.assertIsNone(snap.getSubscription(mergee))
duplicate, mergee = self._do_merge(duplicate, mergee)
self.assertTrue(snap.visibleByUser(mergee))
- self.assertThat(snap._getSubscription(mergee), MatchesStructure(
+ self.assertThat(snap.getSubscription(mergee), MatchesStructure(
snap=Equals(snap),
person=Equals(mergee)
))
self.assertFalse(snap.visibleByUser(duplicate))
- self.assertIsNone(snap._getSubscription(duplicate))
+ self.assertIsNone(snap.getSubscription(duplicate))
def test_merge_moves_oci_recipes(self):
# When person/teams are merged, oci recipes owned by the from
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 35c8b8c..8192755 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.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).
"""Security policies for using content objects."""
@@ -214,6 +214,7 @@ from lp.snappy.interfaces.snappyseries import (
ISnappySeries,
ISnappySeriesSet,
)
+from lp.snappy.interfaces.snapsubscription import ISnapSubscription
from lp.soyuz.interfaces.archive import IArchive
from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthToken
from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
@@ -3298,17 +3299,18 @@ class ViewSnap(AuthorizationBase):
permission = 'launchpad.View'
usedfor = ISnap
- def checkUnauthenticated(self):
- return not self.obj.private
-
def checkAuthenticated(self, user):
- if not self.obj.private:
+ # Check user visibility first: public snaps should be visible to
+ # anyone immediately. Doing this check first can save us some
+ # queries done by the not-so-common cases checked below.
+ if self.obj.visibleByUser(user.person):
+ return True
+ if user.isOwner(self.obj) or user.in_commercial_admin or user.in_admin:
return True
+ return False
- return (
- user.isOwner(self.obj) or
- user.in_commercial_admin or
- user.in_admin)
+ def checkUnauthenticated(self):
+ return self.obj.visibleByUser(None)
class EditSnap(AuthorizationBase):
@@ -3339,6 +3341,30 @@ class AdminSnap(AuthorizationBase):
and EditSnap(self.obj).checkAuthenticated(user))
+class SnapSubscriptionEdit(AuthorizationBase):
+ permission = 'launchpad.Edit'
+ usedfor = ISnapSubscription
+
+ def checkAuthenticated(self, user):
+ """Is the user able to edit a Snap recipe subscription?
+
+ Any team member can edit a Snap recipe subscription for their
+ team.
+ Launchpad Admins can also edit any Snap recipe subscription.
+ The owner of the subscribed Snap can edit the subscription. If
+ the Snap owner is a team, then members of the team can edit
+ the subscription.
+ """
+ return (user.inTeam(self.obj.snap.owner) or
+ user.inTeam(self.obj.person) or
+ user.inTeam(self.obj.subscribed_by) or
+ user.in_admin)
+
+
+class SnapSubscriptionView(SnapSubscriptionEdit):
+ permission = 'launchpad.View'
+
+
class ViewSnapBuildRequest(DelegatedAuthorization):
permission = 'launchpad.View'
usedfor = ISnapBuildRequest
diff --git a/lib/lp/snappy/browser/configure.zcml b/lib/lp/snappy/browser/configure.zcml
index 9da248a..b11f797 100644
--- a/lib/lp/snappy/browser/configure.zcml
+++ b/lib/lp/snappy/browser/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2015-2020 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2015-2021 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -37,6 +37,45 @@
name="+portlet-privacy"
template="../templates/snap-portlet-privacy.pt"/>
<browser:page
+ for="lp.snappy.interfaces.snap.ISnap"
+ permission="launchpad.View"
+ name="+portlet-subscribers"
+ template="../templates/snap-portlet-subscribers.pt"/>
+ <browser:page
+ for="lp.snappy.interfaces.snap.ISnap"
+ class="lp.snappy.browser.snapsubscription.SnapPortletSubscribersContent"
+ permission="launchpad.View"
+ name="+snap-portlet-subscriber-content"
+ template="../templates/snap-portlet-subscribers-content.pt"/>
+
+ <browser:defaultView
+ for="lp.snappy.interfaces.snapsubscription.ISnapSubscription"
+ name="+index"/>
+ <browser:page
+ for="lp.snappy.interfaces.snapsubscription.ISnapSubscription"
+ class="lp.snappy.browser.snapsubscription.SnapSubscriptionEditView"
+ permission="launchpad.Edit"
+ name="+index"
+ template="../templates/snapsubscription-edit.pt"/>
+ <browser:page
+ for="lp.snappy.interfaces.snap.ISnap"
+ class="lp.snappy.browser.snapsubscription.SnapSubscriptionAddView"
+ permission="launchpad.AnyPerson"
+ name="+subscribe"
+ template="../../app/templates/generic-edit.pt"/>
+ <browser:page
+ for="lp.snappy.interfaces.snap.ISnap"
+ class="lp.snappy.browser.snapsubscription.SnapSubscriptionAddOtherView"
+ permission="launchpad.AnyPerson"
+ name="+addsubscriber"
+ template="../../app/templates/generic-edit.pt"/>
+ <browser:url
+ for="lp.snappy.interfaces.snapsubscription.ISnapSubscription"
+ path_expression="string:+subscription/${person/name}"
+ attribute_to_parent="snap"
+ rootsite="code"/>
+
+ <browser:page
for="lp.code.interfaces.branch.IBranch"
class="lp.snappy.browser.snap.SnapAddView"
permission="launchpad.AnyPerson"
diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index f434bbf..684fdc0 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -35,6 +35,7 @@ from zope.schema import (
List,
TextLine,
)
+from zope.security.interfaces import Unauthorized
from lp import _
from lp.app.browser.launchpadform import (
@@ -59,6 +60,7 @@ from lp.buildmaster.interfaces.processor import IProcessorSet
from lp.code.browser.widgets.gitref import GitRefWidget
from lp.code.interfaces.gitref import IGitRef
from lp.registry.enums import VCSType
+from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.features import getFeatureFlag
from lp.services.propertycache import cachedproperty
@@ -128,6 +130,13 @@ class SnapNavigation(WebhookTargetNavigationMixin, Navigation):
return None
return build
+ @stepthrough("+subscription")
+ def traverse_subscription(self, name):
+ """Traverses to an `ISnapSubscription`."""
+ person = getUtility(IPersonSet).getByName(name)
+ if person is not None:
+ return self.context.getSubscription(person)
+
class SnapBreadcrumb(NameBreadcrumb):
@@ -182,12 +191,31 @@ class SnapContextMenu(ContextMenu):
facet = 'overview'
- links = ('request_builds',)
+ links = ('request_builds', 'add_subscriber', 'subscription')
@enabled_with_permission('launchpad.Edit')
def request_builds(self):
return Link('+request-builds', 'Request builds', icon='add')
+ @enabled_with_permission("launchpad.AnyPerson")
+ def subscription(self):
+ if self.context.hasSubscription(self.user):
+ url = "+subscription/%s" % self.user.name
+ text = "Edit your subscription"
+ icon = "edit"
+ elif self.context.userCanBeSubscribed(self.user):
+ url = "+subscribe"
+ text = "Subscribe yourself"
+ icon = "add"
+ else:
+ return None
+ return Link(url, text, icon=icon)
+
+ @enabled_with_permission("launchpad.Edit")
+ def add_subscriber(self):
+ text = "Subscribe someone else"
+ return Link("+addsubscriber", text, icon="add")
+
class SnapView(LaunchpadView):
"""Default view of a Snap."""
@@ -222,6 +250,13 @@ class SnapView(LaunchpadView):
def store_channels(self):
return ', '.join(self.context.store_channels)
+ @property
+ def user_can_see_source(self):
+ try:
+ return self.context.source.visibleByUser(self.user)
+ except Unauthorized:
+ return False
+
def builds_and_requests_for_snap(snap):
"""A list of interesting builds and build requests.
diff --git a/lib/lp/snappy/browser/snapsubscription.py b/lib/lp/snappy/browser/snapsubscription.py
new file mode 100644
index 0000000..a11cbc2
--- /dev/null
+++ b/lib/lp/snappy/browser/snapsubscription.py
@@ -0,0 +1,180 @@
+# Copyright 2020-2021 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Snap subscription views."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+ 'SnapPortletSubscribersContent'
+]
+
+from zope.component._api import getUtility
+from zope.formlib.form import action
+from zope.security.interfaces import ForbiddenAttribute
+
+from lp.app.browser.launchpadform import (
+ LaunchpadEditFormView,
+ LaunchpadFormView,
+ )
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.webapp import (
+ canonical_url,
+ LaunchpadView,
+ )
+from lp.services.webapp.authorization import (
+ check_permission,
+ precache_permission_for_objects,
+ )
+from lp.snappy.interfaces.snapsubscription import ISnapSubscription
+
+
+class SnapPortletSubscribersContent(LaunchpadView):
+ """View for the contents for the subscribers portlet."""
+
+ def subscriptions(self):
+ """Return a decorated list of Snap recipe subscriptions."""
+
+ # Cache permissions so private subscribers can be rendered.
+ # The security adaptor will do the job also but we don't want or
+ # need the expense of running several complex SQL queries.
+ subscriptions = list(self.context.subscriptions)
+ person_ids = [sub.person.id for sub in subscriptions]
+ list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+ person_ids, need_validity=True))
+ if self.user is not None:
+ subscribers = [
+ subscription.person for subscription in subscriptions]
+ precache_permission_for_objects(
+ self.request, "launchpad.LimitedView", subscribers)
+
+ visible_subscriptions = [
+ subscription for subscription in subscriptions
+ if check_permission("launchpad.LimitedView", subscription.person)]
+ return sorted(
+ visible_subscriptions,
+ key=lambda subscription: subscription.person.displayname)
+
+
+class RedirectToSnapMixin:
+ @property
+ def next_url(self):
+ if self.snap.visibleByUser(self.user):
+ return canonical_url(self.snap)
+ # If the subscriber can no longer see the Snap recipe, tries to
+ # redirect to the project page.
+ try:
+ project = self.snap.project
+ if project is not None and project.userCanLimitedView(self.user):
+ return canonical_url(self.snap.project)
+ except ForbiddenAttribute:
+ pass
+ # If not possible, redirect user back to its own page.
+ return canonical_url(self.user)
+
+ cancel_url = next_url
+
+
+class SnapSubscriptionEditView(RedirectToSnapMixin, LaunchpadEditFormView):
+ """The view for editing Snap recipe subscriptions."""
+ schema = ISnapSubscription
+ field_names = []
+
+ @property
+ def page_title(self):
+ return (
+ "Edit subscription to Snap recipe %s" %
+ self.snap.displayname)
+
+ @property
+ def label(self):
+ return (
+ "Edit subscription to Snap recipe for %s" %
+ self.person.displayname)
+
+ def initialize(self):
+ self.snap = self.context.snap
+ self.person = self.context.person
+ super(SnapSubscriptionEditView, self).initialize()
+
+ @action("Unsubscribe", name="unsubscribe")
+ def unsubscribe_action(self, action, data):
+ """Unsubscribe the team from the Snap recipe."""
+ self.snap.unsubscribe(self.person, self.user)
+ self.request.response.addNotification(
+ "%s has been unsubscribed from this Snap recipe."
+ % self.person.displayname)
+
+
+class _SnapSubscriptionCreationView(RedirectToSnapMixin, LaunchpadFormView):
+ """Contains the common functionality of the Add and Edit views."""
+
+ schema = ISnapSubscription
+ field_names = []
+
+ def initialize(self):
+ self.snap = self.context
+ super(_SnapSubscriptionCreationView, self).initialize()
+
+
+class SnapSubscriptionAddView(_SnapSubscriptionCreationView):
+
+ page_title = label = "Subscribe to Snap recipe"
+
+ @action("Subscribe")
+ def subscribe(self, action, data):
+ # To catch the stale post problem, check that the user is not
+ # subscribed before continuing.
+ if self.context.hasSubscription(self.user):
+ self.request.response.addNotification(
+ "You are already subscribed to this Snap recipe.")
+ else:
+ self.context.subscribe(self.user, self.user)
+
+ self.request.response.addNotification(
+ "You have subscribed to this Snap recipe.")
+
+
+class SnapSubscriptionAddOtherView(_SnapSubscriptionCreationView):
+ """View used to subscribe someone other than the current user."""
+
+ field_names = ["person"]
+ for_input = True
+
+ # Since we are subscribing other people, the current user
+ # is never considered subscribed.
+ user_is_subscribed = False
+
+ page_title = label = "Subscribe to Snap recipe"
+
+ def validate(self, data):
+ if "person" in data:
+ person = data["person"]
+ subscription = self.context.getSubscription(person)
+ if (subscription is None
+ and not self.context.userCanBeSubscribed(person)):
+ self.setFieldError(
+ "person",
+ "Open and delegated teams cannot be subscribed to "
+ "private Snap recipes.")
+
+ @action("Subscribe", name="subscribe_action")
+ def subscribe_action(self, action, data):
+ """Subscribe the specified user to the Snap recipe.
+
+ The user must be a member of a team in order to subscribe that team
+ to the Snap recipe. Launchpad Admins are special and they can
+ subscribe any team.
+ """
+ person = data["person"]
+ subscription = self.context.getSubscription(person)
+ if subscription is None:
+ self.context.subscribe(person, self.user)
+ self.request.response.addNotification(
+ "%s has been subscribed to this Snap recipe." %
+ person.displayname)
+ else:
+ self.request.response.addNotification(
+ "%s was already subscribed to this Snap recipe with." %
+ person.displayname)
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index 7174460..5db72c5 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -1571,6 +1571,74 @@ class TestSnapView(BaseTestSnapView):
Primary Archive for Ubuntu Linux
""", self.getMainText(build.snap))
+ def test_index_for_subscriber_without_git_repo_access(self):
+ [ref] = self.factory.makeGitRefs(
+ owner=self.person, target=self.person, name="snap-repository",
+ paths=["refs/heads/master"],
+ information_type=InformationType.PRIVATESECURITY)
+ snap = self.makeSnap(git_ref=ref, private=True)
+ with admin_logged_in():
+ self.makeBuild(
+ snap=snap, status=BuildStatus.FULLYBUILT,
+ duration=timedelta(minutes=30))
+
+ subscriber = self.factory.makePerson()
+ with person_logged_in(self.person):
+ snap.subscribe(subscriber, self.person)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""\
+ Snap packages snap-name
+ .*
+ Snap package information
+ Owner: Test Person
+ Distribution series: Ubuntu Shiny
+ Source: <Redacted>
+ Build source tarball: No
+ Build schedule: \(\?\)
+ Built on request
+ Source archive for automatic builds:
+ Pocket for automatic builds:
+ Builds of this snap package are not automatically uploaded to
+ the store.
+ Latest builds
+ Status When complete Architecture Archive
+ Successfully built 30 minutes ago i386
+ Primary Archive for Ubuntu Linux
+ """, self.getMainText(snap, user=subscriber))
+
+ def test_index_for_subscriber_without_archive_access(self):
+ [ref] = self.factory.makeGitRefs(
+ owner=self.person, target=self.person, name="snap-repository",
+ paths=["refs/heads/master"],
+ information_type=InformationType.PRIVATESECURITY)
+ snap = self.makeSnap(git_ref=ref, private=True)
+ with admin_logged_in():
+ archive = self.factory.makeArchive(private=True)
+ self.makeBuild(
+ snap=snap, status=BuildStatus.FULLYBUILT, archive=archive,
+ duration=timedelta(minutes=30))
+
+ subscriber = self.factory.makePerson()
+ with person_logged_in(self.person):
+ snap.subscribe(subscriber, self.person)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""\
+ Snap packages snap-name
+ .*
+ Snap package information
+ Owner: Test Person
+ Distribution series: Ubuntu Shiny
+ Source: <Redacted>
+ Build source tarball: No
+ Build schedule: \(\?\)
+ Built on request
+ Source archive for automatic builds:
+ Pocket for automatic builds:
+ Builds of this snap package are not automatically uploaded to
+ the store.
+ Latest builds
+ Status When complete Architecture Archive
+ This snap package has not been built yet.
+ """, self.getMainText(snap, user=subscriber))
+
def test_index_git_url(self):
ref = self.factory.makeGitRefRemote(
repository_url="https://git.example.org/foo";,
diff --git a/lib/lp/snappy/browser/tests/test_snapsubscription.py b/lib/lp/snappy/browser/tests/test_snapsubscription.py
new file mode 100644
index 0000000..810af84
--- /dev/null
+++ b/lib/lp/snappy/browser/tests/test_snapsubscription.py
@@ -0,0 +1,260 @@
+# Copyright 2015-2021 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test snap package views."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+
+from fixtures import FakeLogger
+from zope.security.interfaces import Unauthorized
+
+from lp.app.enums import InformationType
+from lp.registry.enums import BranchSharingPolicy
+from lp.services.features.testing import FeatureFixture
+from lp.services.webapp import canonical_url
+from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
+from lp.testing import (
+ admin_logged_in,
+ BrowserTestCase,
+ person_logged_in,
+ )
+from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.pages import (
+ extract_text,
+ find_main_content,
+ find_tag_by_id,
+ find_tags_by_class,
+ )
+
+
+class BaseTestSnapView(BrowserTestCase):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(BaseTestSnapView, self).setUp()
+ self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+ self.useFixture(FakeLogger())
+ self.person = self.factory.makePerson(name='snap-owner')
+
+ def makeSnap(self, project=None, **kwargs):
+ [ref] = self.factory.makeGitRefs(
+ owner=self.person, target=self.person, name="snap-repository",
+ paths=["refs/heads/master"])
+ if project is None:
+ project = self.factory.makeProduct(
+ owner=self.person, registrant=self.person)
+ return self.factory.makeSnap(
+ registrant=self.person, owner=self.person, name="snap-name",
+ git_ref=ref, project=project, **kwargs)
+
+ def getSubscriptionPortletText(self, browser):
+ return extract_text(
+ find_tag_by_id(browser.contents, 'portlet-subscribers'))
+
+ def extractMainText(self, browser):
+ return extract_text(find_main_content(browser.contents))
+
+ def extractInfoMessageContent(self, browser):
+ return extract_text(
+ find_tags_by_class(browser.contents, 'informational message')[0])
+
+
+class TestPublicSnapSubscriptionViews(BaseTestSnapView):
+
+ def test_subscribe_self(self):
+ snap = self.makeSnap()
+ another_user = self.factory.makePerson(name="another-user")
+ browser = self.getViewBrowser(snap, user=another_user)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Subscribe yourself
+ Subscribers
+ Snap-owner
+ """, self.getSubscriptionPortletText(browser))
+
+ # Go to "subscribe myself" page, and click the button.
+ browser = self.getViewBrowser(
+ snap, view_name="+subscribe", user=another_user)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Subscribe to Snap recipe
+ Snap packages
+ snap-name
+ Subscribe to Snap recipe or Cancel
+ """, self.extractMainText(browser))
+ browser.getControl("Subscribe").click()
+
+ # We should be redirected back to snap page.
+ with admin_logged_in():
+ self.assertEqual(canonical_url(snap), browser.url)
+
+ # And the new user should be listed in the subscribers list.
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Edit your subscription
+ Subscribers
+ Another-user
+ Snap-owner
+ """, self.getSubscriptionPortletText(browser))
+
+ def test_unsubscribe_self(self):
+ snap = self.makeSnap()
+ another_user = self.factory.makePerson(name="another-user")
+ with person_logged_in(snap.owner):
+ snap.subscribe(another_user, snap.owner)
+ subscription = snap.getSubscription(another_user)
+ browser = self.getViewBrowser(subscription, user=another_user)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Edit subscription to Snap recipe for Another-user
+ Snap packages
+ snap-name
+ If you unsubscribe from a snap recipe it will no longer show up on
+ your personal pages. or Cancel
+ """, self.extractMainText(browser))
+ browser.getControl("Unsubscribe").click()
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Another-user has been unsubscribed from this Snap recipe.
+ """, self.extractInfoMessageContent(browser))
+ with person_logged_in(self.person):
+ self.assertIsNone(snap.getSubscription(another_user))
+
+ def test_subscribe_someone_else(self):
+ snap = self.makeSnap()
+ another_user = self.factory.makePerson(name="another-user")
+ browser = self.getViewBrowser(snap, user=snap.owner)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Edit your subscription
+ Subscribe someone else
+ Subscribers
+ Snap-owner
+ """, self.getSubscriptionPortletText(browser))
+
+ # Go to "subscribe" page, and click the button.
+ browser = self.getViewBrowser(
+ snap, view_name="+addsubscriber", user=another_user)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Subscribe to Snap recipe
+ Snap packages
+ snap-name
+ Subscribe to Snap recipe
+ Person:
+ .*
+ The person subscribed to the related snap recipe.
+ or
+ Cancel
+ """, self.extractMainText(browser))
+ browser.getControl(name="field.person").value = 'another-user'
+ browser.getControl("Subscribe").click()
+
+ # We should be redirected back to snap page.
+ with admin_logged_in():
+ self.assertEqual(canonical_url(snap), browser.url)
+
+ # And the new user should be listed in the subscribers list.
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Edit your subscription
+ Subscribers
+ Another-user
+ Snap-owner
+ """, self.getSubscriptionPortletText(browser))
+
+ def test_unsubscribe_someone_else(self):
+ snap = self.makeSnap()
+ another_user = self.factory.makePerson(name="another-user")
+ with person_logged_in(snap.owner):
+ snap.subscribe(another_user, snap.owner)
+
+ subscription = snap.getSubscription(another_user)
+ browser = self.getViewBrowser(subscription, user=snap.owner)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Edit subscription to Snap recipe for Another-user
+ Snap packages
+ snap-name
+ If you unsubscribe from a snap recipe it will no longer show up on
+ your personal pages. or Cancel
+ """, self.extractMainText(browser))
+ browser.getControl("Unsubscribe").click()
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Another-user has been unsubscribed from this Snap recipe.
+ """, self.extractInfoMessageContent(browser))
+ with person_logged_in(self.person):
+ self.assertIsNone(snap.getSubscription(another_user))
+
+
+class TestPrivateSnapSubscriptionViews(BaseTestSnapView):
+
+ def makePrivateSnap(self, **kwargs):
+ project = self.factory.makeProduct(
+ owner=self.person, registrant=self.person,
+ information_type=InformationType.PROPRIETARY,
+ branch_sharing_policy=BranchSharingPolicy.PROPRIETARY)
+ return self.makeSnap(
+ information_type=InformationType.PRIVATESECURITY,
+ project=project)
+
+ def test_cannot_subscribe_to_private_snap(self):
+ snap = self.makePrivateSnap()
+ another_user = self.factory.makePerson(name="another-user")
+ # Unsubscribed user should not see the snap page.
+ self.assertRaises(
+ Unauthorized, self.getViewBrowser, snap, user=another_user)
+ # Nor the subscribe pages.
+ self.assertRaises(
+ Unauthorized, self.getViewBrowser,
+ snap, view_name="+subscribe", user=another_user)
+ self.assertRaises(
+ Unauthorized, self.getViewBrowser,
+ snap, view_name="+addsubscriber", user=another_user)
+
+ def test_snap_owner_can_subscribe_someone_to_private_snap(self):
+ snap = self.makePrivateSnap()
+ another_user = self.factory.makePerson(name="another-user")
+
+ # Go to "subscribe" page, and click the button.
+ browser = self.getViewBrowser(
+ snap, view_name="+addsubscriber", user=self.person)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Subscribe to Snap recipe
+ Snap packages
+ snap-name
+ Subscribe to Snap recipe
+ Person:
+ .*
+ The person subscribed to the related snap recipe.
+ or
+ Cancel
+ """, self.extractMainText(browser))
+ browser.getControl(name="field.person").value = 'another-user'
+ browser.getControl("Subscribe").click()
+
+ # Now the new user should be listed in the subscribers list,
+ # and have access to the snap page.
+ browser = self.getViewBrowser(snap, user=another_user)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Edit your subscription
+ Subscribers
+ Another-user
+ Snap-owner
+ """, self.getSubscriptionPortletText(browser))
+
+ def test_unsubscribe_self(self):
+ snap = self.makePrivateSnap()
+ another_user = self.factory.makePerson(name="another-user")
+ with person_logged_in(self.person):
+ snap.subscribe(another_user, self.person)
+ subscription = snap.getSubscription(another_user)
+ browser = self.getViewBrowser(subscription, user=another_user)
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Edit subscription to Snap recipe for Another-user
+ Snap packages
+ snap-name
+ If you unsubscribe from a snap recipe it will no longer show up on
+ your personal pages. or Cancel
+ """, self.extractMainText(browser))
+ browser.getControl("Unsubscribe").click()
+ self.assertTextMatchesExpressionIgnoreWhitespace(r"""
+ Another-user has been unsubscribed from this Snap recipe.
+ """, self.extractInfoMessageContent(browser))
+ with person_logged_in(self.person):
+ self.assertIsNone(snap.getSubscription(another_user))
diff --git a/lib/lp/snappy/configure.zcml b/lib/lp/snappy/configure.zcml
index a16c664..05529f8 100644
--- a/lib/lp/snappy/configure.zcml
+++ b/lib/lp/snappy/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2015-2019 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2015-2021 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -42,6 +42,15 @@
<allow interface="lp.snappy.interfaces.snap.ISnapSet" />
</securedutility>
+ <!-- SnapSubscription -->
+
+ <class class="lp.snappy.model.snapsubscription.SnapSubscription">
+ <allow interface="lp.snappy.interfaces.snapsubscription.ISnapSubscription"/>
+ <require
+ permission="zope.Public"
+ set_schema="lp.snappy.interfaces.snapsubscription.ISnapSubscription"/>
+ </class>
+
<!-- SnapBuildRequest -->
<class class="lp.snappy.model.snap.SnapBuildRequest">
<require
diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
index b423263..c33ea82 100644
--- a/lib/lp/snappy/interfaces/snap.py
+++ b/lib/lp/snappy/interfaces/snap.py
@@ -571,10 +571,25 @@ class ISnapView(Interface):
# Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
value_type=Reference(schema=Interface), readonly=True)))
+ subscriptions = CollectionField(
+ title=_("SnapSubscriptions associated with this repository."),
+ readonly=True,
+ # Really IGitSubscription, patched in _schema_circular_imports.py.
+ value_type=Reference(Interface))
+
subscribers = CollectionField(
- title=_("Persons subscribed to this repository."),
+ title=_("Persons subscribed to this snap recipe."),
readonly=True, value_type=Reference(IPerson))
+ def getSubscription(person):
+ """Returns the person's snap subscription for this snap recipe."""
+
+ def hasSubscription(person):
+ """Is this person subscribed to the snap recipe?"""
+
+ def userCanBeSubscribed(person):
+ """Checks if the given person can be subscribed to this snap recipe."""
+
def visibleByUser(user):
"""Can the specified user see this snap recipe?"""
@@ -584,6 +599,9 @@ class ISnapView(Interface):
If the user is a Launchpad admin, any type is acceptable.
"""
+ def unsubscribe(person, unsubscribed_by):
+ """Unsubscribe a person to this snap recipe."""
+
class ISnapEdit(IWebhookTarget):
"""`ISnap` methods that require launchpad.Edit permission."""
@@ -887,9 +905,6 @@ class ISnapAdminAttributes(Interface):
def subscribe(person, subscribed_by):
"""Subscribe a person to this snap recipe."""
- def unsubscribe(person, unsubscribed_by):
- """Unsubscribe a person to this snap recipe."""
-
# XXX cjwatson 2015-07-17 bug=760849: "beta" is a lie to get WADL
# generation working. Individual attributes must set their version to
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index 488a8d2..129ba0e 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1138,6 +1138,18 @@ class Snap(Storm, WebhookTargetMixin):
order_by = Desc(SnapBuild.id)
return self._getBuilds(filter_term, order_by)
+ @property
+ def subscriptions(self):
+ return Store.of(self).find(
+ SnapSubscription, SnapSubscription.snap == self)
+
+ @property
+ def subscribers(self):
+ return Store.of(self).find(
+ Person,
+ SnapSubscription.person_id == Person.id,
+ SnapSubscription.snap == self)
+
def visibleByUser(self, user):
"""See `ISnap`."""
if self.information_type in PUBLIC_INFORMATION_TYPES:
@@ -1150,7 +1162,11 @@ class Snap(Storm, WebhookTargetMixin):
Snap.id == self.id,
get_snap_privacy_filter(user)).is_empty()
- def _getSubscription(self, person):
+ def hasSubscription(self, person):
+ """See `ISnap`."""
+ return self.getSubscription(person) is not None
+
+ def getSubscription(self, person):
"""Returns person's subscription to this snap recipe, or None if no
subscription is available.
"""
@@ -1161,7 +1177,7 @@ class Snap(Storm, WebhookTargetMixin):
SnapSubscription.person == person,
SnapSubscription.snap == self).one()
- def _userCanBeSubscribed(self, person):
+ def userCanBeSubscribed(self, person):
"""Checks if the given person can subscribe to this snap recipe."""
return not (
self.private and
@@ -1177,11 +1193,11 @@ class Snap(Storm, WebhookTargetMixin):
def subscribe(self, person, subscribed_by, ignore_permissions=False):
"""See `ISnap`."""
- if not self._userCanBeSubscribed(person):
+ if not self.userCanBeSubscribed(person):
raise SubscriptionPrivacyViolation(
"Open and delegated teams cannot be subscribed to private "
"snap recipes.")
- subscription = self._getSubscription(person)
+ subscription = self.getSubscription(person)
if subscription is None:
subscription = SnapSubscription(
person=person, snap=self, subscribed_by=subscribed_by)
@@ -1196,7 +1212,7 @@ class Snap(Storm, WebhookTargetMixin):
def unsubscribe(self, person, unsubscribed_by, ignore_permissions=False):
"""See `ISnap`."""
- subscription = self._getSubscription(person)
+ subscription = self.getSubscription(person)
if (not ignore_permissions
and not subscription.canBeUnsubscribedByUser(unsubscribed_by)):
raise UserCannotUnsubscribePerson(
diff --git a/lib/lp/snappy/templates/snap-index.pt b/lib/lp/snappy/templates/snap-index.pt
index 7d3083e..5c51a5d 100644
--- a/lib/lp/snappy/templates/snap-index.pt
+++ b/lib/lp/snappy/templates/snap-index.pt
@@ -32,6 +32,7 @@
<metal:side fill-slot="side">
<div tal:replace="structure context/@@+portlet-privacy" />
<div tal:replace="structure context/@@+global-actions"/>
+ <tal:subscribers replace="structure context/@@+portlet-subscribers" />
</metal:side>
<metal:heading fill-slot="heading">
@@ -58,10 +59,13 @@
<dl id="source"
tal:define="source context/source" tal:condition="source">
<dt>Source:</dt>
- <dd>
+ <dd tal:condition="view/user_can_see_source">
<a tal:replace="structure source/fmt:link"/>
<a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
</dd>
+ <dd tal:condition="not: view/user_can_see_source">
+ <span style="font-style: italic"><Redacted></span>
+ </dd>
</dl>
<dl id="build_source_tarball"
tal:define="build_source_tarball context/build_source_tarball">
diff --git a/lib/lp/snappy/templates/snap-portlet-subscribers-content.pt b/lib/lp/snappy/templates/snap-portlet-subscribers-content.pt
new file mode 100644
index 0000000..05495ff
--- /dev/null
+++ b/lib/lp/snappy/templates/snap-portlet-subscribers-content.pt
@@ -0,0 +1,31 @@
+<div
+ tal:omit-tag=""
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";>
+ <div class="section snap-subscribers">
+ <div
+ tal:condition="view/subscriptions"
+ tal:repeat="subscription view/subscriptions"
+ tal:attributes="id string:subscriber-${subscription/person/name}">
+ <a tal:condition="subscription/person/name|nothing"
+ tal:attributes="href subscription/person/fmt:url">
+
+ <tal:block replace="structure subscription/person/fmt:icon" />
+ <tal:block replace="subscription/person/fmt:displayname/fmt:shorten/20" />
+ </a>
+
+ <a tal:condition="subscription/required:launchpad.Edit"
+ tal:attributes="
+ href subscription/fmt:url;
+ title string:Edit subscription ${subscription/person/fmt:displayname};
+ id string:editsubscription-${subscription/person/name}">
+ <img class="editsub-icon" src="/@@/edit"
+ tal:attributes="id string:editsubscription-icon-${subscription/person/name}" />
+ </a>
+ </div>
+ <div id="none-subscribers" tal:condition="not:view/subscriptions">
+ No subscribers.
+ </div>
+ </div>
+</div>
diff --git a/lib/lp/snappy/templates/snap-portlet-subscribers.pt b/lib/lp/snappy/templates/snap-portlet-subscribers.pt
new file mode 100644
index 0000000..5f0dd60
--- /dev/null
+++ b/lib/lp/snappy/templates/snap-portlet-subscribers.pt
@@ -0,0 +1,29 @@
+<div
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ class="portlet" id="portlet-subscribers">
+ <div tal:define="context_menu view/context/menu:context">
+ <div>
+ <div class="section">
+ <div
+ tal:define="link context_menu/subscription"
+ tal:condition="link/enabled"
+ id="selfsubscriptioncontainer">
+ <a class="sprite add subscribe-self"
+ tal:attributes="href link/url"
+ tal:content="link/text" />
+ </div>
+ <div
+ tal:define="link context_menu/add_subscriber"
+ tal:condition="link/enabled"
+ tal:content="structure link/render" />
+ </div>
+ </div>
+
+ <h2>Subscribers</h2>
+ <div id="snap-subscribers-outer">
+ <div tal:replace="structure context/@@+snap-portlet-subscriber-content" />
+ </div>
+ </div>
+</div>
diff --git a/lib/lp/snappy/templates/snapsubscription-edit.pt b/lib/lp/snappy/templates/snapsubscription-edit.pt
new file mode 100644
index 0000000..f2d9d8c
--- /dev/null
+++ b/lib/lp/snappy/templates/snapsubscription-edit.pt
@@ -0,0 +1,25 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad"
+>
+ <body>
+
+<div metal:fill-slot="main">
+
+ <div metal:use-macro="context/@@launchpad_form/form" >
+ <metal:extra fill-slot="extra_info">
+ <p class="documentDescription">
+ If you unsubscribe from a snap recipe it will no longer show up on
+ your personal pages.
+ </p>
+ </metal:extra>
+ </div>
+
+</div>
+
+</body>
+</html>
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 869df79..e1a2871 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -1355,19 +1355,16 @@ class TestSnapVisibility(TestCaseWithFactory):
AccessArtifactGrant.abstract_artifact_id == AccessArtifact.id,
*conditions)
- def getSnapSubscription(self, snap, person):
- return removeSecurityProxy(snap)._getSubscription(person)
-
def test_only_owner_can_grant_access(self):
owner = self.factory.makePerson()
pillar = self.factory.makeProduct(owner=owner)
snap = self.factory.makeSnap(
registrant=owner, owner=owner, project=pillar, private=True)
other_person = self.factory.makePerson()
- with person_logged_in(owner):
- snap.subscribe(other_person, owner)
with person_logged_in(other_person):
self.assertRaises(Unauthorized, getattr, snap, 'subscribe')
+ with person_logged_in(owner):
+ snap.subscribe(other_person, owner)
def test_private_is_invisible_by_default(self):
owner = self.factory.makePerson()
@@ -1395,20 +1392,18 @@ class TestSnapVisibility(TestCaseWithFactory):
with person_logged_in(owner):
self.assertFalse(snap.visibleByUser(person))
snap.subscribe(person, snap.owner)
- self.assertThat(
- self.getSnapSubscription(snap, person),
- MatchesStructure(
- person=Equals(person),
- snap=Equals(snap),
- subscribed_by=Equals(snap.owner),
- date_created=IsInstance(datetime)))
+ self.assertThat(snap.getSubscription(person), MatchesStructure(
+ person=Equals(person),
+ snap=Equals(snap),
+ subscribed_by=Equals(snap.owner),
+ date_created=IsInstance(datetime)))
# Calling again should be a no-op.
snap.subscribe(person, snap.owner)
self.assertTrue(snap.visibleByUser(person))
snap.unsubscribe(person, snap.owner)
self.assertFalse(snap.visibleByUser(person))
- self.assertIsNone(self.getSnapSubscription(snap, person))
+ self.assertIsNone(snap.getSubscription(person))
def test_reconcile_set_public(self):
owner = self.factory.makePerson()
@@ -1419,7 +1414,7 @@ class TestSnapVisibility(TestCaseWithFactory):
snap.subscribe(another_user, snap.owner)
self.assertEqual(1, self.getSnapGrants(snap, another_user).count())
self.assertThat(
- self.getSnapSubscription(snap, another_user),
+ snap.getSubscription(another_user),
MatchesStructure(
person=Equals(another_user),
snap=Equals(snap),
@@ -1429,7 +1424,7 @@ class TestSnapVisibility(TestCaseWithFactory):
snap.information_type = InformationType.PUBLIC
self.assertEqual(0, self.getSnapGrants(snap, another_user).count())
self.assertThat(
- self.getSnapSubscription(snap, another_user),
+ snap.getSubscription(another_user),
MatchesStructure(
person=Equals(another_user),
snap=Equals(snap),
@@ -1459,7 +1454,7 @@ class TestSnapVisibility(TestCaseWithFactory):
self.assertTrue(snap.visibleByUser(another_person))
self.assertEqual(2, self.getSnapGrants(snap).count())
self.assertThat(
- self.getSnapSubscription(snap, another_person),
+ snap.getSubscription(another_person),
MatchesStructure(
person=Equals(another_person),
snap=Equals(snap),
@@ -1470,7 +1465,7 @@ class TestSnapVisibility(TestCaseWithFactory):
self.assertTrue(snap.visibleByUser(another_person))
self.assertEqual(2, self.getSnapGrants(snap).count())
self.assertThat(
- self.getSnapSubscription(snap, another_person),
+ snap.getSubscription(another_person),
MatchesStructure(
person=Equals(another_person),
snap=Equals(snap),
Follow ups
