Launchpad itself

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

  1. git-subscriptions-by-path
  2. Merge into devel
Proposed by Colin Watson
Status: Rejected
Rejected by: Colin Watson
Proposed branch: lp:~cjwatson/launchpad/git-subscriptions-by-path
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/git-subscription-tests
Diff against target: 743 lines (+287/-49)
13 files modified
lib/lp/code/browser/branchsubscription.py (+2/-2)
lib/lp/code/browser/gitsubscription.py (+107/-24)
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 (+6/-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 (+33/-4)
lib/lp/code/model/tests/test_gitsubscription.py (+36/-1)
lib/lp/testing/factory.py (+2/-2)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-subscriptions-by-path
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+310471@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.

To post a comment you must log in.
lp:~cjwatson/launchpad/git-subscriptions-by-path updated
18259. By Colin Watson

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

Revision history for this message
Colin Watson (cjwatson) wrote :

Superseded by https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/373730.

Unmerged revisions

18259. By Colin Watson

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

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