Merge ~cjwatson/launchpad:git-subscriptions-by-path into launchpad:master

Proposed by Colin Watson
Status: Needs review
Proposed branch: ~cjwatson/launchpad:git-subscriptions-by-path
Merge into: launchpad:master
Diff against target: 727 lines (+286/-46)
13 files modified
lib/lp/code/browser/branchsubscription.py (+1/-1)
lib/lp/code/browser/gitsubscription.py (+105/-23)
lib/lp/code/browser/tests/test_gitsubscription.py (+4/-1)
lib/lp/code/interfaces/gitref.py (+3/-1)
lib/lp/code/interfaces/gitrepository.py (+17/-3)
lib/lp/code/interfaces/gitsubscription.py (+18/-1)
lib/lp/code/model/gitref.py (+3/-3)
lib/lp/code/model/gitrepository.py (+10/-3)
lib/lp/code/model/gitsubscription.py (+28/-4)
lib/lp/code/model/tests/test_branchmergeproposal.py (+28/-0)
lib/lp/code/model/tests/test_gitrepository.py (+32/-4)
lib/lp/code/model/tests/test_gitsubscription.py (+35/-0)
lib/lp/testing/factory.py (+2/-2)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+373730@code.launchpad.net

Commit message

Allow subscribing to only particular reference paths within a Git repository

Description of the change

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.

To post a comment you must log in.

Unmerged commits

4da5b12... by Colin Watson

Make GitSubscription.paths be a JSON-encoded list instead.

a558b01... by Colin Watson

Allow subscribing to only particular reference paths within a Git repository.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/browser/branchsubscription.py b/lib/lp/code/browser/branchsubscription.py
2index ca3833f..e30faaf 100644
3--- a/lib/lp/code/browser/branchsubscription.py
4+++ b/lib/lp/code/browser/branchsubscription.py
5@@ -232,7 +232,7 @@ class BranchSubscriptionAddOtherView(_BranchSubscriptionView):
6 '%s was already subscribed to this branch with: '
7 % person.displayname,
8 subscription.notification_level, subscription.max_diff_lines,
9- review_level)
10+ subscription.review_level)
11
12
13 class BranchSubscriptionEditView(LaunchpadEditFormView):
14diff --git a/lib/lp/code/browser/gitsubscription.py b/lib/lp/code/browser/gitsubscription.py
15index 3eda78c..d533d29 100644
16--- a/lib/lp/code/browser/gitsubscription.py
17+++ b/lib/lp/code/browser/gitsubscription.py
18@@ -11,7 +11,13 @@ __all__ = [
19 'GitSubscriptionEditView',
20 ]
21
22+from lazr.restful.interface import (
23+ copy_field,
24+ use_template,
25+ )
26 from zope.component import getUtility
27+from zope.formlib.textwidgets import TextWidget
28+from zope.interface import Interface
29
30 from lp.app.browser.launchpadform import (
31 action,
32@@ -60,11 +66,62 @@ class GitRepositoryPortletSubscribersContent(LaunchpadView):
33 key=lambda subscription: subscription.person.displayname)
34
35
36+# XXX cjwatson 2016-11-09: We should just use IGitSubscription directly, but
37+# we have to modify the description of `paths`: WADL generation demands that
38+# the "*" characters be quoted as inline literals to avoid being treated as
39+# mismatched emphasis characters, but "``" looks weird in HTML. Perhaps we
40+# should arrange for forms to run descriptions through pydoc so that we
41+# don't display unprocessed markup?
42+# But in any case we need to adjust the description to say that the list is
43+# space-separated, given our presentation of it.
44+class IGitSubscriptionSchema(Interface):
45+
46+ use_template(IGitSubscription, include=[
47+ "person",
48+ "notification_level",
49+ "max_diff_lines",
50+ "review_level",
51+ ])
52+ paths = copy_field(IGitSubscription["paths"], description=(
53+ u"A space-separated list of patterns matching subscribed reference "
54+ u"paths. For example, 'refs/heads/master refs/heads/next' matches "
55+ u"just those two branches, while 'refs/heads/releases/*' matches "
56+ u"all branches under 'refs/heads/releases/'. Leave this empty to "
57+ u"subscribe to the whole repository."))
58+
59+
60+class SpaceDelimitedListWidget(TextWidget):
61+ """A widget that represents a list as space-delimited text."""
62+
63+ def __init__(self, field, value_type, request):
64+ # We don't use value_type.
65+ super(SpaceDelimitedListWidget, self).__init__(field, request)
66+
67+ def _toFormValue(self, value):
68+ """Convert a list to a space-separated string."""
69+ if value == self.context.missing_value or value is None:
70+ value = self._missing
71+ else:
72+ value = u" ".join(value)
73+ return super(SpaceDelimitedListWidget, self)._toFormValue(value)
74+
75+ def _toFieldValue(self, value):
76+ """Convert the input string into a list."""
77+ value = super(SpaceDelimitedListWidget, self)._toFieldValue(value)
78+ if value == self.context.missing_value:
79+ return value
80+ else:
81+ return value.split()
82+
83+
84 class _GitSubscriptionView(LaunchpadFormView):
85 """Contains the common functionality of the Add and Edit views."""
86
87- schema = IGitSubscription
88- field_names = ['notification_level', 'max_diff_lines', 'review_level']
89+ schema = IGitSubscriptionSchema
90+ field_names = [
91+ 'paths', 'notification_level', 'max_diff_lines', 'review_level',
92+ ]
93+ custom_widget_paths = SpaceDelimitedListWidget
94
95 LEVELS_REQUIRING_LINES_SPECIFICATION = (
96 BranchSubscriptionNotificationLevel.DIFFSONLY,
97@@ -83,17 +140,26 @@ class _GitSubscriptionView(LaunchpadFormView):
98
99 cancel_url = next_url
100
101- def add_notification_message(self, initial, notification_level,
102+ def add_notification_message(self, initial, paths, notification_level,
103 max_diff_lines, review_level):
104+ items = []
105+ if paths is not None:
106+ items.append("Only paths matching %(paths)s.")
107+ items.append("%(notification_level)s")
108 if notification_level in self.LEVELS_REQUIRING_LINES_SPECIFICATION:
109- lines_message = "<li>%s</li>" % max_diff_lines.description
110- else:
111- lines_message = ""
112-
113- format_str = "%%s<ul><li>%%s</li>%s<li>%%s</li></ul>" % lines_message
114+ items.append("%(max_diff_lines)s")
115+ items.append("%(review_level)s")
116+ format_str = (
117+ "%(initial)s<ul>" +
118+ "".join("<li>%s</li>" % item for item in items) + "</ul>")
119 message = structured(
120- format_str, initial, notification_level.description,
121- review_level.description)
122+ format_str, initial=initial,
123+ paths=(" ".join(paths) if paths is not None else None),
124+ notification_level=notification_level.description,
125+ max_diff_lines=(
126+ max_diff_lines.description
127+ if max_diff_lines is not None else None),
128+ review_level=review_level.description)
129 self.request.response.addNotification(message)
130
131 def optional_max_diff_lines(self, notification_level, max_diff_lines):
132@@ -115,6 +181,7 @@ class GitSubscriptionAddView(_GitSubscriptionView):
133 self.request.response.addNotification(
134 "You are already subscribed to this repository.")
135 else:
136+ paths = data.get("paths")
137 notification_level = data["notification_level"]
138 max_diff_lines = self.optional_max_diff_lines(
139 notification_level, data["max_diff_lines"])
140@@ -122,11 +189,11 @@ class GitSubscriptionAddView(_GitSubscriptionView):
141
142 self.context.subscribe(
143 self.user, notification_level, max_diff_lines, review_level,
144- self.user)
145+ self.user, paths=paths)
146
147 self.add_notification_message(
148 "You have subscribed to this repository with: ",
149- notification_level, max_diff_lines, review_level)
150+ paths, notification_level, max_diff_lines, review_level)
151
152
153 class GitSubscriptionEditOwnView(_GitSubscriptionView):
154@@ -146,15 +213,19 @@ class GitSubscriptionEditOwnView(_GitSubscriptionView):
155 # This is the case of URL hacking or stale page.
156 return {}
157 else:
158- return {"notification_level": subscription.notification_level,
159- "max_diff_lines": subscription.max_diff_lines,
160- "review_level": subscription.review_level}
161+ return {
162+ "paths": subscription.paths,
163+ "notification_level": subscription.notification_level,
164+ "max_diff_lines": subscription.max_diff_lines,
165+ "review_level": subscription.review_level,
166+ }
167
168 @action("Change")
169 def change_details(self, action, data):
170 # Be proactive in the checking to catch the stale post problem.
171 if self.context.hasSubscription(self.user):
172 subscription = self.context.getSubscription(self.user)
173+ subscription.paths = data.get("paths")
174 subscription.notification_level = data["notification_level"]
175 subscription.max_diff_lines = self.optional_max_diff_lines(
176 subscription.notification_level,
177@@ -163,6 +234,7 @@ class GitSubscriptionEditOwnView(_GitSubscriptionView):
178
179 self.add_notification_message(
180 "Subscription updated to: ",
181+ subscription.paths,
182 subscription.notification_level,
183 subscription.max_diff_lines,
184 subscription.review_level)
185@@ -186,7 +258,9 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
186 """View used to subscribe someone other than the current user."""
187
188 field_names = [
189- "person", "notification_level", "max_diff_lines", "review_level"]
190+ "person", "paths", "notification_level", "max_diff_lines",
191+ "review_level",
192+ ]
193 for_input = True
194
195 # Since we are subscribing other people, the current user
196@@ -214,6 +288,7 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
197 to the repository. Launchpad Admins are special and they can
198 subscribe any team.
199 """
200+ paths = data.get("paths")
201 notification_level = data["notification_level"]
202 max_diff_lines = self.optional_max_diff_lines(
203 notification_level, data["max_diff_lines"])
204@@ -223,17 +298,17 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
205 if subscription is None:
206 self.context.subscribe(
207 person, notification_level, max_diff_lines, review_level,
208- self.user)
209+ self.user, paths=paths)
210 self.add_notification_message(
211 "%s has been subscribed to this repository with: "
212- % person.displayname, notification_level, max_diff_lines,
213- review_level)
214+ % person.displayname,
215+ paths, notification_level, max_diff_lines, review_level)
216 else:
217 self.add_notification_message(
218 "%s was already subscribed to this repository with: "
219 % person.displayname,
220- subscription.notification_level, subscription.max_diff_lines,
221- review_level)
222+ subscription.paths, subscription.notification_level,
223+ subscription.max_diff_lines, subscription.review_level)
224
225
226 class GitSubscriptionEditView(LaunchpadEditFormView):
227@@ -243,8 +318,15 @@ class GitSubscriptionEditView(LaunchpadEditFormView):
228 through the repository action item to edit the user's own subscription.
229 This is the only current way to edit a team repository subscription.
230 """
231- schema = IGitSubscription
232- field_names = ["notification_level", "max_diff_lines", "review_level"]
233+ schema = IGitSubscriptionSchema
234+ field_names = [
235+ "paths", "notification_level", "max_diff_lines", "review_level",
236+ ]
237+
238+ @property
239+ def adapters(self):
240+ """See `LaunchpadFormView`."""
241+ return {IGitSubscriptionSchema: self.context}
242
243 @property
244 def page_title(self):
245diff --git a/lib/lp/code/browser/tests/test_gitsubscription.py b/lib/lp/code/browser/tests/test_gitsubscription.py
246index cb49310..7065930 100644
247--- a/lib/lp/code/browser/tests/test_gitsubscription.py
248+++ b/lib/lp/code/browser/tests/test_gitsubscription.py
249@@ -116,7 +116,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
250 with person_logged_in(subscriber):
251 subscription = repository.getSubscription(subscriber)
252 self.assertThat(subscription, MatchesStructure.byEquality(
253- person=subscriber, repository=repository,
254+ person=subscriber, repository=repository, paths=None,
255 notification_level=(
256 BranchSubscriptionNotificationLevel.ATTRIBUTEONLY),
257 max_diff_lines=None,
258@@ -147,6 +147,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
259 None, CodeReviewNotificationLevel.FULL, subscriber)
260 browser = self.getViewBrowser(repository, user=subscriber)
261 browser.getLink('Edit your subscription').click()
262+ browser.getControl('Paths').value = 'refs/heads/master refs/heads/next'
263 browser.getControl('Notification Level').displayValue = [
264 'Branch attribute and revision notifications']
265 browser.getControl('Generated Diff Size Limit').displayValue = [
266@@ -154,6 +155,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
267 browser.getControl('Change').click()
268 self.assertTextMatchesExpressionIgnoreWhitespace(
269 'Subscription updated to: '
270+ 'Only paths matching refs/heads/master refs/heads/next. '
271 'Send notifications for both branch attribute updates and new '
272 'revisions added to the branch. '
273 'Limit the generated diff to 5000 lines. '
274@@ -163,6 +165,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
275 subscription = repository.getSubscription(subscriber)
276 self.assertThat(subscription, MatchesStructure.byEquality(
277 person=subscriber, repository=repository,
278+ paths=['refs/heads/master', 'refs/heads/next'],
279 notification_level=BranchSubscriptionNotificationLevel.FULL,
280 max_diff_lines=BranchSubscriptionDiffSize.FIVEKLINES,
281 review_level=CodeReviewNotificationLevel.FULL))
282diff --git a/lib/lp/code/interfaces/gitref.py b/lib/lp/code/interfaces/gitref.py
283index 16198eb..c1eca9b 100644
284--- a/lib/lp/code/interfaces/gitref.py
285+++ b/lib/lp/code/interfaces/gitref.py
286@@ -197,7 +197,7 @@ class IGitRefView(IHasMergeProposals, IHasRecipes, IPrivacy, IInformationType):
287 "Persons subscribed to the repository containing this reference.")
288
289 def subscribe(person, notification_level, max_diff_lines,
290- code_review_level, subscribed_by):
291+ code_review_level, subscribed_by, paths=None):
292 """Subscribe this person to the repository containing this reference.
293
294 :param person: The `Person` to subscribe.
295@@ -209,6 +209,8 @@ class IGitRefView(IHasMergeProposals, IHasRecipes, IPrivacy, IInformationType):
296 cause notification.
297 :param subscribed_by: The person who is subscribing the subscriber.
298 Most often the subscriber themselves.
299+ :param paths: An optional list of patterns matching reference paths
300+ to subscribe to.
301 :return: A new or existing `GitSubscription`.
302 """
303
304diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
305index d03d25d..9b03412 100644
306--- a/lib/lp/code/interfaces/gitrepository.py
307+++ b/lib/lp/code/interfaces/gitrepository.py
308@@ -432,14 +432,23 @@ class IGitRepositoryView(IHasRecipes):
309 vocabulary=BranchSubscriptionDiffSize),
310 code_review_level=Choice(
311 title=_("The level of code review notification emails."),
312- vocabulary=CodeReviewNotificationLevel))
313+ vocabulary=CodeReviewNotificationLevel),
314+ paths=List(
315+ title=_("Paths"), required=False,
316+ description=_(
317+ "A list of patterns matching reference paths to subscribe "
318+ "to. For example, ``['refs/heads/master', "
319+ "'refs/heads/next']`` matches just those two branches, while "
320+ "``['refs/heads/releases/*']`` matches all branches under "
321+ "``refs/heads/releases/``. Omit this parameter to subscribe "
322+ "to the whole repository.")))
323 # Really IGitSubscription, patched in _schema_circular_imports.py.
324 @operation_returns_entry(Interface)
325 @call_with(subscribed_by=REQUEST_USER)
326 @export_write_operation()
327 @operation_for_version("devel")
328 def subscribe(person, notification_level, max_diff_lines,
329- code_review_level, subscribed_by):
330+ code_review_level, subscribed_by, paths=None):
331 """Subscribe this person to the repository.
332
333 :param person: The `Person` to subscribe.
334@@ -451,6 +460,8 @@ class IGitRepositoryView(IHasRecipes):
335 cause notification.
336 :param subscribed_by: The person who is subscribing the subscriber.
337 Most often the subscriber themselves.
338+ :param paths: An optional list of patterns matching reference paths
339+ to subscribe to.
340 :return: A new or existing `GitSubscription`.
341 """
342
343@@ -486,11 +497,14 @@ class IGitRepositoryView(IHasRecipes):
344 :return: A `ResultSet`.
345 """
346
347- def getNotificationRecipients():
348+ def getNotificationRecipients(path=None):
349 """Return a complete INotificationRecipientSet instance.
350
351 The INotificationRecipientSet instance contains the subscribers
352 and their subscriptions.
353+
354+ :param path: If not None, only consider subscriptions that match
355+ this reference path.
356 """
357
358 landing_targets = Attribute(
359diff --git a/lib/lp/code/interfaces/gitsubscription.py b/lib/lp/code/interfaces/gitsubscription.py
360index 94929db..37c3916 100644
361--- a/lib/lp/code/interfaces/gitsubscription.py
362+++ b/lib/lp/code/interfaces/gitsubscription.py
363@@ -1,4 +1,4 @@
364-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
365+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
366 # GNU Affero General Public License version 3 (see the file LICENSE).
367
368 """Git repository subscription interfaces."""
369@@ -22,6 +22,8 @@ from zope.interface import Interface
370 from zope.schema import (
371 Choice,
372 Int,
373+ List,
374+ TextLine,
375 )
376
377 from lp import _
378@@ -58,6 +60,18 @@ class IGitSubscription(Interface):
379 Reference(
380 title=_("Repository ID"), required=True, readonly=True,
381 schema=IGitRepository))
382+ paths = List(title=_("Paths"), value_type=TextLine(), required=False)
383+ api_paths = exported(
384+ List(
385+ title=_("Paths"), value_type=TextLine(), required=True,
386+ description=_(
387+ "A list of patterns matching subscribed reference paths. "
388+ "For example, ``['refs/heads/master', 'refs/heads/next']`` "
389+ "matches just those two branches, while "
390+ "``['refs/heads/releases/*']`` matches all branches under "
391+ "``refs/heads/releases/``. Leave this empty to subscribe "
392+ "to the whole repository.")),
393+ exported_as="paths")
394 notification_level = exported(
395 Choice(
396 title=_("Notification Level"), required=True,
397@@ -97,3 +111,6 @@ class IGitSubscription(Interface):
398 @operation_for_version("devel")
399 def canBeUnsubscribedByUser(user):
400 """Can the user unsubscribe the subscriber from the repository?"""
401+
402+ def matchesPath(path):
403+ """Does this subscription match this reference path?"""
404diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
405index 526b815..84968ce 100644
406--- a/lib/lp/code/model/gitref.py
407+++ b/lib/lp/code/model/gitref.py
408@@ -212,11 +212,11 @@ class GitRefMixin:
409 return self.repository.subscribers
410
411 def subscribe(self, person, notification_level, max_diff_lines,
412- code_review_level, subscribed_by):
413+ code_review_level, subscribed_by, paths=None):
414 """See `IGitRef`."""
415 return self.repository.subscribe(
416 person, notification_level, max_diff_lines, code_review_level,
417- subscribed_by)
418+ subscribed_by, paths=paths)
419
420 def getSubscription(self, person):
421 """See `IGitRef`."""
422@@ -228,7 +228,7 @@ class GitRefMixin:
423
424 def getNotificationRecipients(self):
425 """See `IGitRef`."""
426- return self.repository.getNotificationRecipients()
427+ return self.repository.getNotificationRecipients(path=self.path)
428
429 @property
430 def landing_targets(self):
431diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
432index 5d4607a..aaf365c 100644
433--- a/lib/lp/code/model/gitrepository.py
434+++ b/lib/lp/code/model/gitrepository.py
435@@ -854,23 +854,28 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
436 person.anyone_can_join())
437
438 def subscribe(self, person, notification_level, max_diff_lines,
439- code_review_level, subscribed_by):
440+ code_review_level, subscribed_by, paths=None):
441 """See `IGitRepository`."""
442 if not self.userCanBeSubscribed(person):
443 raise SubscriptionPrivacyViolation(
444 "Open and delegated teams cannot be subscribed to private "
445 "repositories.")
446+ if paths is not None and isinstance(paths, six.string_types):
447+ raise TypeError(
448+ "The paths argument must be a sequence of strings, not a "
449+ "string.")
450 # If the person is already subscribed, update the subscription with
451 # the specified notification details.
452 subscription = self.getSubscription(person)
453 if subscription is None:
454 subscription = GitSubscription(
455- person=person, repository=self,
456+ person=person, repository=self, paths=paths,
457 notification_level=notification_level,
458 max_diff_lines=max_diff_lines, review_level=code_review_level,
459 subscribed_by=subscribed_by)
460 Store.of(subscription).flush()
461 else:
462+ subscription.paths = paths
463 subscription.notification_level = notification_level
464 subscription.max_diff_lines = max_diff_lines
465 subscription.review_level = code_review_level
466@@ -928,10 +933,12 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
467 artifact, [person])
468 store.flush()
469
470- def getNotificationRecipients(self):
471+ def getNotificationRecipients(self, path=None):
472 """See `IGitRepository`."""
473 recipients = NotificationRecipientSet()
474 for subscription in self.subscriptions:
475+ if path is not None and not subscription.matchesPath(path):
476+ continue
477 if subscription.person.is_team:
478 rationale = 'Subscriber @%s' % subscription.person.name
479 else:
480diff --git a/lib/lp/code/model/gitsubscription.py b/lib/lp/code/model/gitsubscription.py
481index fe3a254..2240884 100644
482--- a/lib/lp/code/model/gitsubscription.py
483+++ b/lib/lp/code/model/gitsubscription.py
484@@ -1,4 +1,4 @@
485-# Copyright 2015 Canonical Ltd. This software is licensed under the
486+# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
487 # GNU Affero General Public License version 3 (see the file LICENSE).
488
489 __metaclass__ = type
490@@ -6,8 +6,11 @@ __all__ = [
491 'GitSubscription',
492 ]
493
494+import fnmatch
495+
496 from storm.locals import (
497 Int,
498+ JSON,
499 Reference,
500 )
501 from zope.interface import implementer
502@@ -40,6 +43,8 @@ class GitSubscription(StormBase):
503 repository_id = Int(name='repository', allow_none=False)
504 repository = Reference(repository_id, 'GitRepository.id')
505
506+ paths = JSON(name='paths', allow_none=True)
507+
508 notification_level = EnumCol(
509 enum=BranchSubscriptionNotificationLevel, notNull=True,
510 default=DEFAULT)
511@@ -52,19 +57,38 @@ class GitSubscription(StormBase):
512 name='subscribed_by', allow_none=False, validator=validate_person)
513 subscribed_by = Reference(subscribed_by_id, 'Person.id')
514
515- def __init__(self, person, repository, notification_level, max_diff_lines,
516- review_level, subscribed_by):
517+ def __init__(self, person, repository, paths, notification_level,
518+ max_diff_lines, review_level, subscribed_by):
519 super(GitSubscription, self).__init__()
520 self.person = person
521 self.repository = repository
522+ self.paths = paths
523 self.notification_level = notification_level
524 self.max_diff_lines = max_diff_lines
525 self.review_level = review_level
526 self.subscribed_by = subscribed_by
527
528 def canBeUnsubscribedByUser(self, user):
529- """See `IBranchSubscription`."""
530+ """See `IGitSubscription`."""
531 if user is None:
532 return False
533 permission_check = GitSubscriptionEdit(self)
534 return permission_check.checkAuthenticated(IPersonRoles(user))
535+
536+ @property
537+ def api_paths(self):
538+ """See `IGitSubscription`.
539+
540+ lazr.restful can't represent the difference between a NULL
541+ collection and an empty collection, so we simulate the former.
542+ """
543+ return ["*"] if self.paths is None else self.paths
544+
545+ def matchesPath(self, path):
546+ """See `IGitSubscription`."""
547+ if self.paths is None:
548+ return True
549+ for pattern in self.paths:
550+ if fnmatch.fnmatch(path, pattern):
551+ return True
552+ return False
553diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
554index c899c10..8fa4e16 100644
555--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
556+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
557@@ -1020,6 +1020,34 @@ class TestMergeProposalNotificationGit(
558 source_ref=source, target_ref=target,
559 prerequisite_ref=prerequisite, **kwargs)
560
561+ def test_getNotificationRecipients_path(self):
562+ # If the subscription specifies path patterns, then one of them must
563+ # match the reference.
564+ bmp = self.makeBranchMergeProposal()
565+ source_owner = bmp.merge_source.owner
566+ target_owner = bmp.merge_target.owner
567+ recipients = bmp.getNotificationRecipients(
568+ CodeReviewNotificationLevel.STATUS)
569+ subscriber_set = set([source_owner, target_owner])
570+ self.assertEqual(subscriber_set, set(recipients.keys()))
571+ # Subscribing somebody to a non-matching path has no effect.
572+ subscriber = self.factory.makePerson()
573+ bmp.merge_source.subscribe(
574+ subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
575+ CodeReviewNotificationLevel.FULL, subscriber, paths=["no-match"])
576+ recipients = bmp.getNotificationRecipients(
577+ CodeReviewNotificationLevel.STATUS)
578+ self.assertEqual(subscriber_set, set(recipients.keys()))
579+ # Subscribing somebody to a matching path is effective.
580+ bmp.merge_source.subscribe(
581+ subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
582+ CodeReviewNotificationLevel.FULL, subscriber,
583+ paths=[bmp.merge_source.path])
584+ recipients = bmp.getNotificationRecipients(
585+ CodeReviewNotificationLevel.STATUS)
586+ subscriber_set.add(subscriber)
587+ self.assertEqual(subscriber_set, set(recipients.keys()))
588+
589
590 class TestMergeProposalWebhooksMixin:
591
592diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
593index e679142..e70d5f7 100644
594--- a/lib/lp/code/model/tests/test_gitrepository.py
595+++ b/lib/lp/code/model/tests/test_gitrepository.py
596@@ -26,6 +26,7 @@ from storm.exceptions import LostObjectError
597 from storm.store import Store
598 from testtools.matchers import (
599 AnyMatch,
600+ ContainsDict,
601 EndsWith,
602 Equals,
603 Is,
604@@ -3572,9 +3573,34 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
605 with person_logged_in(ANONYMOUS):
606 subscription_db = repository_db.getSubscription(subscriber_db)
607 self.assertIsNotNone(subscription_db)
608- self.assertThat(
609- response.jsonBody()["self_link"],
610- EndsWith(api_url(subscription_db)))
611+ self.assertThat(response.jsonBody(), ContainsDict({
612+ "self_link": EndsWith(api_url(subscription_db)),
613+ "paths": Equals(["*"]),
614+ }))
615+
616+ def test_subscribe_with_paths(self):
617+ # A user can subscribe to some reference paths in a repository.
618+ repository_db = self.factory.makeGitRepository()
619+ subscriber_db = self.factory.makePerson()
620+ webservice = webservice_for_person(
621+ subscriber_db, permission=OAuthPermission.WRITE_PUBLIC)
622+ webservice.default_api_version = "devel"
623+ with person_logged_in(ANONYMOUS):
624+ repository_url = api_url(repository_db)
625+ subscriber_url = api_url(subscriber_db)
626+ response = webservice.named_post(
627+ repository_url, "subscribe", person=subscriber_url,
628+ notification_level="Branch attribute notifications only",
629+ max_diff_lines="Don't send diffs", code_review_level="No email",
630+ paths=["refs/heads/master", "refs/heads/next"])
631+ self.assertEqual(200, response.status)
632+ with person_logged_in(ANONYMOUS):
633+ subscription_db = repository_db.getSubscription(subscriber_db)
634+ self.assertIsNotNone(subscription_db)
635+ self.assertThat(response.jsonBody(), ContainsDict({
636+ "self_link": EndsWith(api_url(subscription_db)),
637+ "paths": Equals(["refs/heads/master", "refs/heads/next"]),
638+ }))
639
640 def _makeSubscription(self, repository, subscriber):
641 with person_logged_in(subscriber):
642@@ -3620,7 +3646,8 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
643 repository_url, "subscribe", person=subscriber_url,
644 notification_level="No email",
645 max_diff_lines="Send entire diff",
646- code_review_level="Status changes only")
647+ code_review_level="Status changes only",
648+ paths=["refs/heads/next"])
649 self.assertEqual(200, response.status)
650 with person_logged_in(subscriber_db):
651 self.assertThat(
652@@ -3631,6 +3658,7 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
653 BranchSubscriptionNotificationLevel.NOEMAIL),
654 max_diff_lines=BranchSubscriptionDiffSize.WHOLEDIFF,
655 review_level=CodeReviewNotificationLevel.STATUS,
656+ paths=["refs/heads/next"],
657 ))
658 repository = webservice.get(repository_url).jsonBody()
659 subscribers = webservice.get(
660diff --git a/lib/lp/code/model/tests/test_gitsubscription.py b/lib/lp/code/model/tests/test_gitsubscription.py
661index 37f7550..6910335 100644
662--- a/lib/lp/code/model/tests/test_gitsubscription.py
663+++ b/lib/lp/code/model/tests/test_gitsubscription.py
664@@ -111,6 +111,41 @@ class TestGitSubscriptions(TestCaseWithFactory):
665 repository.unsubscribe(subscribee, owner)
666 self.assertFalse(repository.visibleByUser(subscribee))
667
668+ def test_matchesPath_none_matches_anything(self):
669+ # A paths=None subscription matches any path.
670+ subscription = self.factory.makeGitSubscription(paths=None)
671+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
672+ self.assertTrue(subscription.matchesPath("refs/heads/next"))
673+ self.assertTrue(subscription.matchesPath("nonsense"))
674+
675+ def test_matchesPath_exact(self):
676+ # A subscription to a single path matches only that path.
677+ subscription = self.factory.makeGitSubscription(
678+ paths=["refs/heads/master"])
679+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
680+ self.assertFalse(subscription.matchesPath("refs/heads/master2"))
681+ self.assertFalse(subscription.matchesPath("refs/heads/next"))
682+
683+ def test_matchesPath_fnmatch(self):
684+ # A subscription to a path pattern matches anything that fnmatch
685+ # accepts.
686+ subscription = self.factory.makeGitSubscription(
687+ paths=["refs/heads/*"])
688+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
689+ self.assertTrue(subscription.matchesPath("refs/heads/next"))
690+ self.assertTrue(subscription.matchesPath("refs/heads/foo/bar"))
691+ self.assertFalse(subscription.matchesPath("refs/tags/1.0"))
692+
693+ def test_matchesPath_multiple(self):
694+ # A subscription to multiple path patterns matches any of them.
695+ subscription = self.factory.makeGitSubscription(
696+ paths=["refs/heads/*", "refs/tags/1.0"])
697+ self.assertTrue(subscription.matchesPath("refs/heads/master"))
698+ self.assertTrue(subscription.matchesPath("refs/heads/next"))
699+ self.assertTrue(subscription.matchesPath("refs/heads/foo/bar"))
700+ self.assertTrue(subscription.matchesPath("refs/tags/1.0"))
701+ self.assertFalse(subscription.matchesPath("refs/tags/1.0a"))
702+
703
704 class TestGitSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
705 """Tests for GitSubscription.canBeUnsubscribedByUser."""
706diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
707index 568ddab..ed1d0d6 100644
708--- a/lib/lp/testing/factory.py
709+++ b/lib/lp/testing/factory.py
710@@ -1792,7 +1792,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
711 return repository
712
713 def makeGitSubscription(self, repository=None, person=None,
714- subscribed_by=None):
715+ subscribed_by=None, paths=None):
716 """Create a GitSubscription."""
717 if repository is None:
718 repository = self.makeGitRepository()
719@@ -1802,7 +1802,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
720 subscribed_by = person
721 return repository.subscribe(removeSecurityProxy(person),
722 BranchSubscriptionNotificationLevel.NOEMAIL, None,
723- CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
724+ CodeReviewNotificationLevel.NOEMAIL, subscribed_by, paths=paths)
725
726 def makeGitRefs(self, repository=None, paths=None, **repository_kwargs):
727 """Create and return a list of new, arbitrary GitRefs."""

Subscribers

People subscribed via source and target branches

to status/vote changes: