launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24012
[Merge] ~cjwatson/launchpad:git-subscriptions-by-path into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:git-subscriptions-by-path into launchpad:master.
Commit message:
Allow subscribing to only particular reference paths within a Git repository
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/373730
This is needed as a soft prerequisite for beefing up subscriptions to send revision information, since users may very well want to e.g. subscribe to changes to master without subscribing to random other branches.
It's conceivable that people may want to have separate subscriptions to different ref patterns (e.g. "send me diffs for changes to master, but only tell me summary information elsewhere". I haven't attempted to solve that here (subscriptions are still unique up to person and repository), because it's not especially obvious how to do the UI. That can always be retrofitted later if there's demand.
This is essentially the same as https://code.launchpad.net/~cjwatson/launchpad/git-subscriptions-by-path/+merge/310471, converted to git and rebased on master.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:git-subscriptions-by-path into launchpad:master.
diff --git a/lib/lp/code/browser/branchsubscription.py b/lib/lp/code/browser/branchsubscription.py
index ca3833f..e30faaf 100644
--- a/lib/lp/code/browser/branchsubscription.py
+++ b/lib/lp/code/browser/branchsubscription.py
@@ -232,7 +232,7 @@ class BranchSubscriptionAddOtherView(_BranchSubscriptionView):
'%s was already subscribed to this branch with: '
% person.displayname,
subscription.notification_level, subscription.max_diff_lines,
- review_level)
+ subscription.review_level)
class BranchSubscriptionEditView(LaunchpadEditFormView):
diff --git a/lib/lp/code/browser/gitsubscription.py b/lib/lp/code/browser/gitsubscription.py
index 3eda78c..d533d29 100644
--- a/lib/lp/code/browser/gitsubscription.py
+++ b/lib/lp/code/browser/gitsubscription.py
@@ -11,7 +11,13 @@ __all__ = [
'GitSubscriptionEditView',
]
+from lazr.restful.interface import (
+ copy_field,
+ use_template,
+ )
from zope.component import getUtility
+from zope.formlib.textwidgets import TextWidget
+from zope.interface import Interface
from lp.app.browser.launchpadform import (
action,
@@ -60,11 +66,62 @@ class GitRepositoryPortletSubscribersContent(LaunchpadView):
key=lambda subscription: subscription.person.displayname)
+# XXX cjwatson 2016-11-09: We should just use IGitSubscription directly, but
+# we have to modify the description of `paths`: WADL generation demands that
+# the "*" characters be quoted as inline literals to avoid being treated as
+# mismatched emphasis characters, but "``" looks weird in HTML. Perhaps we
+# should arrange for forms to run descriptions through pydoc so that we
+# don't display unprocessed markup?
+# But in any case we need to adjust the description to say that the list is
+# space-separated, given our presentation of it.
+class IGitSubscriptionSchema(Interface):
+
+ use_template(IGitSubscription, include=[
+ "person",
+ "notification_level",
+ "max_diff_lines",
+ "review_level",
+ ])
+ paths = copy_field(IGitSubscription["paths"], description=(
+ u"A space-separated list of patterns matching subscribed reference "
+ u"paths. For example, 'refs/heads/master refs/heads/next' matches "
+ u"just those two branches, while 'refs/heads/releases/*' matches "
+ u"all branches under 'refs/heads/releases/'. Leave this empty to "
+ u"subscribe to the whole repository."))
+
+
+class SpaceDelimitedListWidget(TextWidget):
+ """A widget that represents a list as space-delimited text."""
+
+ def __init__(self, field, value_type, request):
+ # We don't use value_type.
+ super(SpaceDelimitedListWidget, self).__init__(field, request)
+
+ def _toFormValue(self, value):
+ """Convert a list to a space-separated string."""
+ if value == self.context.missing_value or value is None:
+ value = self._missing
+ else:
+ value = u" ".join(value)
+ return super(SpaceDelimitedListWidget, self)._toFormValue(value)
+
+ def _toFieldValue(self, value):
+ """Convert the input string into a list."""
+ value = super(SpaceDelimitedListWidget, self)._toFieldValue(value)
+ if value == self.context.missing_value:
+ return value
+ else:
+ return value.split()
+
+
class _GitSubscriptionView(LaunchpadFormView):
"""Contains the common functionality of the Add and Edit views."""
- schema = IGitSubscription
- field_names = ['notification_level', 'max_diff_lines', 'review_level']
+ schema = IGitSubscriptionSchema
+ field_names = [
+ 'paths', 'notification_level', 'max_diff_lines', 'review_level',
+ ]
+ custom_widget_paths = SpaceDelimitedListWidget
LEVELS_REQUIRING_LINES_SPECIFICATION = (
BranchSubscriptionNotificationLevel.DIFFSONLY,
@@ -83,17 +140,26 @@ class _GitSubscriptionView(LaunchpadFormView):
cancel_url = next_url
- def add_notification_message(self, initial, notification_level,
+ def add_notification_message(self, initial, paths, notification_level,
max_diff_lines, review_level):
+ items = []
+ if paths is not None:
+ items.append("Only paths matching %(paths)s.")
+ items.append("%(notification_level)s")
if notification_level in self.LEVELS_REQUIRING_LINES_SPECIFICATION:
- lines_message = "<li>%s</li>" % max_diff_lines.description
- else:
- lines_message = ""
-
- format_str = "%%s<ul><li>%%s</li>%s<li>%%s</li></ul>" % lines_message
+ items.append("%(max_diff_lines)s")
+ items.append("%(review_level)s")
+ format_str = (
+ "%(initial)s<ul>" +
+ "".join("<li>%s</li>" % item for item in items) + "</ul>")
message = structured(
- format_str, initial, notification_level.description,
- review_level.description)
+ format_str, initial=initial,
+ paths=(" ".join(paths) if paths is not None else None),
+ notification_level=notification_level.description,
+ max_diff_lines=(
+ max_diff_lines.description
+ if max_diff_lines is not None else None),
+ review_level=review_level.description)
self.request.response.addNotification(message)
def optional_max_diff_lines(self, notification_level, max_diff_lines):
@@ -115,6 +181,7 @@ class GitSubscriptionAddView(_GitSubscriptionView):
self.request.response.addNotification(
"You are already subscribed to this repository.")
else:
+ paths = data.get("paths")
notification_level = data["notification_level"]
max_diff_lines = self.optional_max_diff_lines(
notification_level, data["max_diff_lines"])
@@ -122,11 +189,11 @@ class GitSubscriptionAddView(_GitSubscriptionView):
self.context.subscribe(
self.user, notification_level, max_diff_lines, review_level,
- self.user)
+ self.user, paths=paths)
self.add_notification_message(
"You have subscribed to this repository with: ",
- notification_level, max_diff_lines, review_level)
+ paths, notification_level, max_diff_lines, review_level)
class GitSubscriptionEditOwnView(_GitSubscriptionView):
@@ -146,15 +213,19 @@ class GitSubscriptionEditOwnView(_GitSubscriptionView):
# This is the case of URL hacking or stale page.
return {}
else:
- return {"notification_level": subscription.notification_level,
- "max_diff_lines": subscription.max_diff_lines,
- "review_level": subscription.review_level}
+ return {
+ "paths": subscription.paths,
+ "notification_level": subscription.notification_level,
+ "max_diff_lines": subscription.max_diff_lines,
+ "review_level": subscription.review_level,
+ }
@action("Change")
def change_details(self, action, data):
# Be proactive in the checking to catch the stale post problem.
if self.context.hasSubscription(self.user):
subscription = self.context.getSubscription(self.user)
+ subscription.paths = data.get("paths")
subscription.notification_level = data["notification_level"]
subscription.max_diff_lines = self.optional_max_diff_lines(
subscription.notification_level,
@@ -163,6 +234,7 @@ class GitSubscriptionEditOwnView(_GitSubscriptionView):
self.add_notification_message(
"Subscription updated to: ",
+ subscription.paths,
subscription.notification_level,
subscription.max_diff_lines,
subscription.review_level)
@@ -186,7 +258,9 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
"""View used to subscribe someone other than the current user."""
field_names = [
- "person", "notification_level", "max_diff_lines", "review_level"]
+ "person", "paths", "notification_level", "max_diff_lines",
+ "review_level",
+ ]
for_input = True
# Since we are subscribing other people, the current user
@@ -214,6 +288,7 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
to the repository. Launchpad Admins are special and they can
subscribe any team.
"""
+ paths = data.get("paths")
notification_level = data["notification_level"]
max_diff_lines = self.optional_max_diff_lines(
notification_level, data["max_diff_lines"])
@@ -223,17 +298,17 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
if subscription is None:
self.context.subscribe(
person, notification_level, max_diff_lines, review_level,
- self.user)
+ self.user, paths=paths)
self.add_notification_message(
"%s has been subscribed to this repository with: "
- % person.displayname, notification_level, max_diff_lines,
- review_level)
+ % person.displayname,
+ paths, notification_level, max_diff_lines, review_level)
else:
self.add_notification_message(
"%s was already subscribed to this repository with: "
% person.displayname,
- subscription.notification_level, subscription.max_diff_lines,
- review_level)
+ subscription.paths, subscription.notification_level,
+ subscription.max_diff_lines, subscription.review_level)
class GitSubscriptionEditView(LaunchpadEditFormView):
@@ -243,8 +318,15 @@ class GitSubscriptionEditView(LaunchpadEditFormView):
through the repository action item to edit the user's own subscription.
This is the only current way to edit a team repository subscription.
"""
- schema = IGitSubscription
- field_names = ["notification_level", "max_diff_lines", "review_level"]
+ schema = IGitSubscriptionSchema
+ field_names = [
+ "paths", "notification_level", "max_diff_lines", "review_level",
+ ]
+
+ @property
+ def adapters(self):
+ """See `LaunchpadFormView`."""
+ return {IGitSubscriptionSchema: self.context}
@property
def page_title(self):
diff --git a/lib/lp/code/browser/tests/test_gitsubscription.py b/lib/lp/code/browser/tests/test_gitsubscription.py
index cb49310..7065930 100644
--- a/lib/lp/code/browser/tests/test_gitsubscription.py
+++ b/lib/lp/code/browser/tests/test_gitsubscription.py
@@ -116,7 +116,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
with person_logged_in(subscriber):
subscription = repository.getSubscription(subscriber)
self.assertThat(subscription, MatchesStructure.byEquality(
- person=subscriber, repository=repository,
+ person=subscriber, repository=repository, paths=None,
notification_level=(
BranchSubscriptionNotificationLevel.ATTRIBUTEONLY),
max_diff_lines=None,
@@ -147,6 +147,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
None, CodeReviewNotificationLevel.FULL, subscriber)
browser = self.getViewBrowser(repository, user=subscriber)
browser.getLink('Edit your subscription').click()
+ browser.getControl('Paths').value = 'refs/heads/master refs/heads/next'
browser.getControl('Notification Level').displayValue = [
'Branch attribute and revision notifications']
browser.getControl('Generated Diff Size Limit').displayValue = [
@@ -154,6 +155,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
browser.getControl('Change').click()
self.assertTextMatchesExpressionIgnoreWhitespace(
'Subscription updated to: '
+ 'Only paths matching refs/heads/master refs/heads/next. '
'Send notifications for both branch attribute updates and new '
'revisions added to the branch. '
'Limit the generated diff to 5000 lines. '
@@ -163,6 +165,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
subscription = repository.getSubscription(subscriber)
self.assertThat(subscription, MatchesStructure.byEquality(
person=subscriber, repository=repository,
+ paths=['refs/heads/master', 'refs/heads/next'],
notification_level=BranchSubscriptionNotificationLevel.FULL,
max_diff_lines=BranchSubscriptionDiffSize.FIVEKLINES,
review_level=CodeReviewNotificationLevel.FULL))
diff --git a/lib/lp/code/interfaces/gitref.py b/lib/lp/code/interfaces/gitref.py
index 16198eb..c1eca9b 100644
--- a/lib/lp/code/interfaces/gitref.py
+++ b/lib/lp/code/interfaces/gitref.py
@@ -197,7 +197,7 @@ class IGitRefView(IHasMergeProposals, IHasRecipes, IPrivacy, IInformationType):
"Persons subscribed to the repository containing this reference.")
def subscribe(person, notification_level, max_diff_lines,
- code_review_level, subscribed_by):
+ code_review_level, subscribed_by, paths=None):
"""Subscribe this person to the repository containing this reference.
:param person: The `Person` to subscribe.
@@ -209,6 +209,8 @@ class IGitRefView(IHasMergeProposals, IHasRecipes, IPrivacy, IInformationType):
cause notification.
:param subscribed_by: The person who is subscribing the subscriber.
Most often the subscriber themselves.
+ :param paths: An optional list of patterns matching reference paths
+ to subscribe to.
:return: A new or existing `GitSubscription`.
"""
diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index d03d25d..9b03412 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -432,14 +432,23 @@ class IGitRepositoryView(IHasRecipes):
vocabulary=BranchSubscriptionDiffSize),
code_review_level=Choice(
title=_("The level of code review notification emails."),
- vocabulary=CodeReviewNotificationLevel))
+ vocabulary=CodeReviewNotificationLevel),
+ paths=List(
+ title=_("Paths"), required=False,
+ description=_(
+ "A list of patterns matching reference paths to subscribe "
+ "to. For example, ``['refs/heads/master', "
+ "'refs/heads/next']`` matches just those two branches, while "
+ "``['refs/heads/releases/*']`` matches all branches under "
+ "``refs/heads/releases/``. Omit this parameter to subscribe "
+ "to the whole repository.")))
# Really IGitSubscription, patched in _schema_circular_imports.py.
@operation_returns_entry(Interface)
@call_with(subscribed_by=REQUEST_USER)
@export_write_operation()
@operation_for_version("devel")
def subscribe(person, notification_level, max_diff_lines,
- code_review_level, subscribed_by):
+ code_review_level, subscribed_by, paths=None):
"""Subscribe this person to the repository.
:param person: The `Person` to subscribe.
@@ -451,6 +460,8 @@ class IGitRepositoryView(IHasRecipes):
cause notification.
:param subscribed_by: The person who is subscribing the subscriber.
Most often the subscriber themselves.
+ :param paths: An optional list of patterns matching reference paths
+ to subscribe to.
:return: A new or existing `GitSubscription`.
"""
@@ -486,11 +497,14 @@ class IGitRepositoryView(IHasRecipes):
:return: A `ResultSet`.
"""
- def getNotificationRecipients():
+ def getNotificationRecipients(path=None):
"""Return a complete INotificationRecipientSet instance.
The INotificationRecipientSet instance contains the subscribers
and their subscriptions.
+
+ :param path: If not None, only consider subscriptions that match
+ this reference path.
"""
landing_targets = Attribute(
diff --git a/lib/lp/code/interfaces/gitsubscription.py b/lib/lp/code/interfaces/gitsubscription.py
index 94929db..37c3916 100644
--- a/lib/lp/code/interfaces/gitsubscription.py
+++ b/lib/lp/code/interfaces/gitsubscription.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Git repository subscription interfaces."""
@@ -22,6 +22,8 @@ from zope.interface import Interface
from zope.schema import (
Choice,
Int,
+ List,
+ TextLine,
)
from lp import _
@@ -58,6 +60,18 @@ class IGitSubscription(Interface):
Reference(
title=_("Repository ID"), required=True, readonly=True,
schema=IGitRepository))
+ paths = List(title=_("Paths"), value_type=TextLine(), required=False)
+ api_paths = exported(
+ List(
+ title=_("Paths"), value_type=TextLine(), required=True,
+ description=_(
+ "A list of patterns matching subscribed reference paths. "
+ "For example, ``['refs/heads/master', 'refs/heads/next']`` "
+ "matches just those two branches, while "
+ "``['refs/heads/releases/*']`` matches all branches under "
+ "``refs/heads/releases/``. Leave this empty to subscribe "
+ "to the whole repository.")),
+ exported_as="paths")
notification_level = exported(
Choice(
title=_("Notification Level"), required=True,
@@ -97,3 +111,6 @@ class IGitSubscription(Interface):
@operation_for_version("devel")
def canBeUnsubscribedByUser(user):
"""Can the user unsubscribe the subscriber from the repository?"""
+
+ def matchesPath(path):
+ """Does this subscription match this reference path?"""
diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
index 526b815..84968ce 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -212,11 +212,11 @@ class GitRefMixin:
return self.repository.subscribers
def subscribe(self, person, notification_level, max_diff_lines,
- code_review_level, subscribed_by):
+ code_review_level, subscribed_by, paths=None):
"""See `IGitRef`."""
return self.repository.subscribe(
person, notification_level, max_diff_lines, code_review_level,
- subscribed_by)
+ subscribed_by, paths=paths)
def getSubscription(self, person):
"""See `IGitRef`."""
@@ -228,7 +228,7 @@ class GitRefMixin:
def getNotificationRecipients(self):
"""See `IGitRef`."""
- return self.repository.getNotificationRecipients()
+ return self.repository.getNotificationRecipients(path=self.path)
@property
def landing_targets(self):
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index 5d4607a..aaf365c 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -854,23 +854,28 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
person.anyone_can_join())
def subscribe(self, person, notification_level, max_diff_lines,
- code_review_level, subscribed_by):
+ code_review_level, subscribed_by, paths=None):
"""See `IGitRepository`."""
if not self.userCanBeSubscribed(person):
raise SubscriptionPrivacyViolation(
"Open and delegated teams cannot be subscribed to private "
"repositories.")
+ if paths is not None and isinstance(paths, six.string_types):
+ raise TypeError(
+ "The paths argument must be a sequence of strings, not a "
+ "string.")
# If the person is already subscribed, update the subscription with
# the specified notification details.
subscription = self.getSubscription(person)
if subscription is None:
subscription = GitSubscription(
- person=person, repository=self,
+ person=person, repository=self, paths=paths,
notification_level=notification_level,
max_diff_lines=max_diff_lines, review_level=code_review_level,
subscribed_by=subscribed_by)
Store.of(subscription).flush()
else:
+ subscription.paths = paths
subscription.notification_level = notification_level
subscription.max_diff_lines = max_diff_lines
subscription.review_level = code_review_level
@@ -928,10 +933,12 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
artifact, [person])
store.flush()
- def getNotificationRecipients(self):
+ def getNotificationRecipients(self, path=None):
"""See `IGitRepository`."""
recipients = NotificationRecipientSet()
for subscription in self.subscriptions:
+ if path is not None and not subscription.matchesPath(path):
+ continue
if subscription.person.is_team:
rationale = 'Subscriber @%s' % subscription.person.name
else:
diff --git a/lib/lp/code/model/gitsubscription.py b/lib/lp/code/model/gitsubscription.py
index fe3a254..2240884 100644
--- a/lib/lp/code/model/gitsubscription.py
+++ b/lib/lp/code/model/gitsubscription.py
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -6,8 +6,11 @@ __all__ = [
'GitSubscription',
]
+import fnmatch
+
from storm.locals import (
Int,
+ JSON,
Reference,
)
from zope.interface import implementer
@@ -40,6 +43,8 @@ class GitSubscription(StormBase):
repository_id = Int(name='repository', allow_none=False)
repository = Reference(repository_id, 'GitRepository.id')
+ paths = JSON(name='paths', allow_none=True)
+
notification_level = EnumCol(
enum=BranchSubscriptionNotificationLevel, notNull=True,
default=DEFAULT)
@@ -52,19 +57,38 @@ class GitSubscription(StormBase):
name='subscribed_by', allow_none=False, validator=validate_person)
subscribed_by = Reference(subscribed_by_id, 'Person.id')
- def __init__(self, person, repository, notification_level, max_diff_lines,
- review_level, subscribed_by):
+ def __init__(self, person, repository, paths, notification_level,
+ max_diff_lines, review_level, subscribed_by):
super(GitSubscription, self).__init__()
self.person = person
self.repository = repository
+ self.paths = paths
self.notification_level = notification_level
self.max_diff_lines = max_diff_lines
self.review_level = review_level
self.subscribed_by = subscribed_by
def canBeUnsubscribedByUser(self, user):
- """See `IBranchSubscription`."""
+ """See `IGitSubscription`."""
if user is None:
return False
permission_check = GitSubscriptionEdit(self)
return permission_check.checkAuthenticated(IPersonRoles(user))
+
+ @property
+ def api_paths(self):
+ """See `IGitSubscription`.
+
+ lazr.restful can't represent the difference between a NULL
+ collection and an empty collection, so we simulate the former.
+ """
+ return ["*"] if self.paths is None else self.paths
+
+ def matchesPath(self, path):
+ """See `IGitSubscription`."""
+ if self.paths is None:
+ return True
+ for pattern in self.paths:
+ if fnmatch.fnmatch(path, pattern):
+ return True
+ return False
diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
index c899c10..8fa4e16 100644
--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
@@ -1020,6 +1020,34 @@ class TestMergeProposalNotificationGit(
source_ref=source, target_ref=target,
prerequisite_ref=prerequisite, **kwargs)
+ def test_getNotificationRecipients_path(self):
+ # If the subscription specifies path patterns, then one of them must
+ # match the reference.
+ bmp = self.makeBranchMergeProposal()
+ source_owner = bmp.merge_source.owner
+ target_owner = bmp.merge_target.owner
+ recipients = bmp.getNotificationRecipients(
+ CodeReviewNotificationLevel.STATUS)
+ subscriber_set = set([source_owner, target_owner])
+ self.assertEqual(subscriber_set, set(recipients.keys()))
+ # Subscribing somebody to a non-matching path has no effect.
+ subscriber = self.factory.makePerson()
+ bmp.merge_source.subscribe(
+ subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
+ CodeReviewNotificationLevel.FULL, subscriber, paths=["no-match"])
+ recipients = bmp.getNotificationRecipients(
+ CodeReviewNotificationLevel.STATUS)
+ self.assertEqual(subscriber_set, set(recipients.keys()))
+ # Subscribing somebody to a matching path is effective.
+ bmp.merge_source.subscribe(
+ subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
+ CodeReviewNotificationLevel.FULL, subscriber,
+ paths=[bmp.merge_source.path])
+ recipients = bmp.getNotificationRecipients(
+ CodeReviewNotificationLevel.STATUS)
+ subscriber_set.add(subscriber)
+ self.assertEqual(subscriber_set, set(recipients.keys()))
+
class TestMergeProposalWebhooksMixin:
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index e679142..e70d5f7 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -26,6 +26,7 @@ from storm.exceptions import LostObjectError
from storm.store import Store
from testtools.matchers import (
AnyMatch,
+ ContainsDict,
EndsWith,
Equals,
Is,
@@ -3572,9 +3573,34 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
with person_logged_in(ANONYMOUS):
subscription_db = repository_db.getSubscription(subscriber_db)
self.assertIsNotNone(subscription_db)
- self.assertThat(
- response.jsonBody()["self_link"],
- EndsWith(api_url(subscription_db)))
+ self.assertThat(response.jsonBody(), ContainsDict({
+ "self_link": EndsWith(api_url(subscription_db)),
+ "paths": Equals(["*"]),
+ }))
+
+ def test_subscribe_with_paths(self):
+ # A user can subscribe to some reference paths in a repository.
+ repository_db = self.factory.makeGitRepository()
+ subscriber_db = self.factory.makePerson()
+ webservice = webservice_for_person(
+ subscriber_db, permission=OAuthPermission.WRITE_PUBLIC)
+ webservice.default_api_version = "devel"
+ with person_logged_in(ANONYMOUS):
+ repository_url = api_url(repository_db)
+ subscriber_url = api_url(subscriber_db)
+ response = webservice.named_post(
+ repository_url, "subscribe", person=subscriber_url,
+ notification_level="Branch attribute notifications only",
+ max_diff_lines="Don't send diffs", code_review_level="No email",
+ paths=["refs/heads/master", "refs/heads/next"])
+ self.assertEqual(200, response.status)
+ with person_logged_in(ANONYMOUS):
+ subscription_db = repository_db.getSubscription(subscriber_db)
+ self.assertIsNotNone(subscription_db)
+ self.assertThat(response.jsonBody(), ContainsDict({
+ "self_link": EndsWith(api_url(subscription_db)),
+ "paths": Equals(["refs/heads/master", "refs/heads/next"]),
+ }))
def _makeSubscription(self, repository, subscriber):
with person_logged_in(subscriber):
@@ -3620,7 +3646,8 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
repository_url, "subscribe", person=subscriber_url,
notification_level="No email",
max_diff_lines="Send entire diff",
- code_review_level="Status changes only")
+ code_review_level="Status changes only",
+ paths=["refs/heads/next"])
self.assertEqual(200, response.status)
with person_logged_in(subscriber_db):
self.assertThat(
@@ -3631,6 +3658,7 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
BranchSubscriptionNotificationLevel.NOEMAIL),
max_diff_lines=BranchSubscriptionDiffSize.WHOLEDIFF,
review_level=CodeReviewNotificationLevel.STATUS,
+ paths=["refs/heads/next"],
))
repository = webservice.get(repository_url).jsonBody()
subscribers = webservice.get(
diff --git a/lib/lp/code/model/tests/test_gitsubscription.py b/lib/lp/code/model/tests/test_gitsubscription.py
index 37f7550..6910335 100644
--- a/lib/lp/code/model/tests/test_gitsubscription.py
+++ b/lib/lp/code/model/tests/test_gitsubscription.py
@@ -111,6 +111,41 @@ class TestGitSubscriptions(TestCaseWithFactory):
repository.unsubscribe(subscribee, owner)
self.assertFalse(repository.visibleByUser(subscribee))
+ def test_matchesPath_none_matches_anything(self):
+ # A paths=None subscription matches any path.
+ subscription = self.factory.makeGitSubscription(paths=None)
+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
+ self.assertTrue(subscription.matchesPath("refs/heads/next"))
+ self.assertTrue(subscription.matchesPath("nonsense"))
+
+ def test_matchesPath_exact(self):
+ # A subscription to a single path matches only that path.
+ subscription = self.factory.makeGitSubscription(
+ paths=["refs/heads/master"])
+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
+ self.assertFalse(subscription.matchesPath("refs/heads/master2"))
+ self.assertFalse(subscription.matchesPath("refs/heads/next"))
+
+ def test_matchesPath_fnmatch(self):
+ # A subscription to a path pattern matches anything that fnmatch
+ # accepts.
+ subscription = self.factory.makeGitSubscription(
+ paths=["refs/heads/*"])
+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
+ self.assertTrue(subscription.matchesPath("refs/heads/next"))
+ self.assertTrue(subscription.matchesPath("refs/heads/foo/bar"))
+ self.assertFalse(subscription.matchesPath("refs/tags/1.0"))
+
+ def test_matchesPath_multiple(self):
+ # A subscription to multiple path patterns matches any of them.
+ subscription = self.factory.makeGitSubscription(
+ paths=["refs/heads/*", "refs/tags/1.0"])
+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
+ self.assertTrue(subscription.matchesPath("refs/heads/next"))
+ self.assertTrue(subscription.matchesPath("refs/heads/foo/bar"))
+ self.assertTrue(subscription.matchesPath("refs/tags/1.0"))
+ self.assertFalse(subscription.matchesPath("refs/tags/1.0a"))
+
class TestGitSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
"""Tests for GitSubscription.canBeUnsubscribedByUser."""
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 568ddab..ed1d0d6 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -1792,7 +1792,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
return repository
def makeGitSubscription(self, repository=None, person=None,
- subscribed_by=None):
+ subscribed_by=None, paths=None):
"""Create a GitSubscription."""
if repository is None:
repository = self.makeGitRepository()
@@ -1802,7 +1802,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
subscribed_by = person
return repository.subscribe(removeSecurityProxy(person),
BranchSubscriptionNotificationLevel.NOEMAIL, None,
- CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
+ CodeReviewNotificationLevel.NOEMAIL, subscribed_by, paths=paths)
def makeGitRefs(self, repository=None, paths=None, **repository_kwargs):
"""Create and return a list of new, arbitrary GitRefs."""
