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
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):
232 '%s was already subscribed to this branch with: '232 '%s was already subscribed to this branch with: '
233 % person.displayname,233 % person.displayname,
234 subscription.notification_level, subscription.max_diff_lines,234 subscription.notification_level, subscription.max_diff_lines,
235 review_level)235 subscription.review_level)
236236
237237
238class BranchSubscriptionEditView(LaunchpadEditFormView):238class 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__ = [
11 'GitSubscriptionEditView',11 'GitSubscriptionEditView',
12 ]12 ]
1313
14from lazr.restful.interface import (
15 copy_field,
16 use_template,
17 )
14from zope.component import getUtility18from zope.component import getUtility
19from zope.formlib.textwidgets import TextWidget
20from zope.interface import Interface
1521
16from lp.app.browser.launchpadform import (22from lp.app.browser.launchpadform import (
17 action,23 action,
@@ -60,11 +66,62 @@ class GitRepositoryPortletSubscribersContent(LaunchpadView):
60 key=lambda subscription: subscription.person.displayname)66 key=lambda subscription: subscription.person.displayname)
6167
6268
69# XXX cjwatson 2016-11-09: We should just use IGitSubscription directly, but
70# we have to modify the description of `paths`: WADL generation demands that
71# the "*" characters be quoted as inline literals to avoid being treated as
72# mismatched emphasis characters, but "``" looks weird in HTML. Perhaps we
73# should arrange for forms to run descriptions through pydoc so that we
74# don't display unprocessed markup?
75# But in any case we need to adjust the description to say that the list is
76# space-separated, given our presentation of it.
77class IGitSubscriptionSchema(Interface):
78
79 use_template(IGitSubscription, include=[
80 "person",
81 "notification_level",
82 "max_diff_lines",
83 "review_level",
84 ])
85 paths = copy_field(IGitSubscription["paths"], description=(
86 u"A space-separated list of patterns matching subscribed reference "
87 u"paths. For example, 'refs/heads/master refs/heads/next' matches "
88 u"just those two branches, while 'refs/heads/releases/*' matches "
89 u"all branches under 'refs/heads/releases/'. Leave this empty to "
90 u"subscribe to the whole repository."))
91
92
93class SpaceDelimitedListWidget(TextWidget):
94 """A widget that represents a list as space-delimited text."""
95
96 def __init__(self, field, value_type, request):
97 # We don't use value_type.
98 super(SpaceDelimitedListWidget, self).__init__(field, request)
99
100 def _toFormValue(self, value):
101 """Convert a list to a space-separated string."""
102 if value == self.context.missing_value or value is None:
103 value = self._missing
104 else:
105 value = u" ".join(value)
106 return super(SpaceDelimitedListWidget, self)._toFormValue(value)
107
108 def _toFieldValue(self, value):
109 """Convert the input string into a list."""
110 value = super(SpaceDelimitedListWidget, self)._toFieldValue(value)
111 if value == self.context.missing_value:
112 return value
113 else:
114 return value.split()
115
116
63class _GitSubscriptionView(LaunchpadFormView):117class _GitSubscriptionView(LaunchpadFormView):
64 """Contains the common functionality of the Add and Edit views."""118 """Contains the common functionality of the Add and Edit views."""
65119
66 schema = IGitSubscription120 schema = IGitSubscriptionSchema
67 field_names = ['notification_level', 'max_diff_lines', 'review_level']121 field_names = [
122 'paths', 'notification_level', 'max_diff_lines', 'review_level',
123 ]
124 custom_widget_paths = SpaceDelimitedListWidget
68125
69 LEVELS_REQUIRING_LINES_SPECIFICATION = (126 LEVELS_REQUIRING_LINES_SPECIFICATION = (
70 BranchSubscriptionNotificationLevel.DIFFSONLY,127 BranchSubscriptionNotificationLevel.DIFFSONLY,
@@ -83,17 +140,26 @@ class _GitSubscriptionView(LaunchpadFormView):
83140
84 cancel_url = next_url141 cancel_url = next_url
85142
86 def add_notification_message(self, initial, notification_level,143 def add_notification_message(self, initial, paths, notification_level,
87 max_diff_lines, review_level):144 max_diff_lines, review_level):
145 items = []
146 if paths is not None:
147 items.append("Only paths matching %(paths)s.")
148 items.append("%(notification_level)s")
88 if notification_level in self.LEVELS_REQUIRING_LINES_SPECIFICATION:149 if notification_level in self.LEVELS_REQUIRING_LINES_SPECIFICATION:
89 lines_message = "<li>%s</li>" % max_diff_lines.description150 items.append("%(max_diff_lines)s")
90 else:151 items.append("%(review_level)s")
91 lines_message = ""152 format_str = (
92153 "%(initial)s<ul>" +
93 format_str = "%%s<ul><li>%%s</li>%s<li>%%s</li></ul>" % lines_message154 "".join("<li>%s</li>" % item for item in items) + "</ul>")
94 message = structured(155 message = structured(
95 format_str, initial, notification_level.description,156 format_str, initial=initial,
96 review_level.description)157 paths=(" ".join(paths) if paths is not None else None),
158 notification_level=notification_level.description,
159 max_diff_lines=(
160 max_diff_lines.description
161 if max_diff_lines is not None else None),
162 review_level=review_level.description)
97 self.request.response.addNotification(message)163 self.request.response.addNotification(message)
98164
99 def optional_max_diff_lines(self, notification_level, max_diff_lines):165 def optional_max_diff_lines(self, notification_level, max_diff_lines):
@@ -115,6 +181,7 @@ class GitSubscriptionAddView(_GitSubscriptionView):
115 self.request.response.addNotification(181 self.request.response.addNotification(
116 "You are already subscribed to this repository.")182 "You are already subscribed to this repository.")
117 else:183 else:
184 paths = data.get("paths")
118 notification_level = data["notification_level"]185 notification_level = data["notification_level"]
119 max_diff_lines = self.optional_max_diff_lines(186 max_diff_lines = self.optional_max_diff_lines(
120 notification_level, data["max_diff_lines"])187 notification_level, data["max_diff_lines"])
@@ -122,11 +189,11 @@ class GitSubscriptionAddView(_GitSubscriptionView):
122189
123 self.context.subscribe(190 self.context.subscribe(
124 self.user, notification_level, max_diff_lines, review_level,191 self.user, notification_level, max_diff_lines, review_level,
125 self.user)192 self.user, paths=paths)
126193
127 self.add_notification_message(194 self.add_notification_message(
128 "You have subscribed to this repository with: ",195 "You have subscribed to this repository with: ",
129 notification_level, max_diff_lines, review_level)196 paths, notification_level, max_diff_lines, review_level)
130197
131198
132class GitSubscriptionEditOwnView(_GitSubscriptionView):199class GitSubscriptionEditOwnView(_GitSubscriptionView):
@@ -146,15 +213,19 @@ class GitSubscriptionEditOwnView(_GitSubscriptionView):
146 # This is the case of URL hacking or stale page.213 # This is the case of URL hacking or stale page.
147 return {}214 return {}
148 else:215 else:
149 return {"notification_level": subscription.notification_level,216 return {
150 "max_diff_lines": subscription.max_diff_lines,217 "paths": subscription.paths,
151 "review_level": subscription.review_level}218 "notification_level": subscription.notification_level,
219 "max_diff_lines": subscription.max_diff_lines,
220 "review_level": subscription.review_level,
221 }
152222
153 @action("Change")223 @action("Change")
154 def change_details(self, action, data):224 def change_details(self, action, data):
155 # Be proactive in the checking to catch the stale post problem.225 # Be proactive in the checking to catch the stale post problem.
156 if self.context.hasSubscription(self.user):226 if self.context.hasSubscription(self.user):
157 subscription = self.context.getSubscription(self.user)227 subscription = self.context.getSubscription(self.user)
228 subscription.paths = data.get("paths")
158 subscription.notification_level = data["notification_level"]229 subscription.notification_level = data["notification_level"]
159 subscription.max_diff_lines = self.optional_max_diff_lines(230 subscription.max_diff_lines = self.optional_max_diff_lines(
160 subscription.notification_level,231 subscription.notification_level,
@@ -163,6 +234,7 @@ class GitSubscriptionEditOwnView(_GitSubscriptionView):
163234
164 self.add_notification_message(235 self.add_notification_message(
165 "Subscription updated to: ",236 "Subscription updated to: ",
237 subscription.paths,
166 subscription.notification_level,238 subscription.notification_level,
167 subscription.max_diff_lines,239 subscription.max_diff_lines,
168 subscription.review_level)240 subscription.review_level)
@@ -186,7 +258,9 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
186 """View used to subscribe someone other than the current user."""258 """View used to subscribe someone other than the current user."""
187259
188 field_names = [260 field_names = [
189 "person", "notification_level", "max_diff_lines", "review_level"]261 "person", "paths", "notification_level", "max_diff_lines",
262 "review_level",
263 ]
190 for_input = True264 for_input = True
191265
192 # Since we are subscribing other people, the current user266 # Since we are subscribing other people, the current user
@@ -214,6 +288,7 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
214 to the repository. Launchpad Admins are special and they can288 to the repository. Launchpad Admins are special and they can
215 subscribe any team.289 subscribe any team.
216 """290 """
291 paths = data.get("paths")
217 notification_level = data["notification_level"]292 notification_level = data["notification_level"]
218 max_diff_lines = self.optional_max_diff_lines(293 max_diff_lines = self.optional_max_diff_lines(
219 notification_level, data["max_diff_lines"])294 notification_level, data["max_diff_lines"])
@@ -223,17 +298,17 @@ class GitSubscriptionAddOtherView(_GitSubscriptionView):
223 if subscription is None:298 if subscription is None:
224 self.context.subscribe(299 self.context.subscribe(
225 person, notification_level, max_diff_lines, review_level,300 person, notification_level, max_diff_lines, review_level,
226 self.user)301 self.user, paths=paths)
227 self.add_notification_message(302 self.add_notification_message(
228 "%s has been subscribed to this repository with: "303 "%s has been subscribed to this repository with: "
229 % person.displayname, notification_level, max_diff_lines,304 % person.displayname,
230 review_level)305 paths, notification_level, max_diff_lines, review_level)
231 else:306 else:
232 self.add_notification_message(307 self.add_notification_message(
233 "%s was already subscribed to this repository with: "308 "%s was already subscribed to this repository with: "
234 % person.displayname,309 % person.displayname,
235 subscription.notification_level, subscription.max_diff_lines,310 subscription.paths, subscription.notification_level,
236 review_level)311 subscription.max_diff_lines, subscription.review_level)
237312
238313
239class GitSubscriptionEditView(LaunchpadEditFormView):314class GitSubscriptionEditView(LaunchpadEditFormView):
@@ -243,8 +318,15 @@ class GitSubscriptionEditView(LaunchpadEditFormView):
243 through the repository action item to edit the user's own subscription.318 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.319 This is the only current way to edit a team repository subscription.
245 """320 """
246 schema = IGitSubscription321 schema = IGitSubscriptionSchema
247 field_names = ["notification_level", "max_diff_lines", "review_level"]322 field_names = [
323 "paths", "notification_level", "max_diff_lines", "review_level",
324 ]
325
326 @property
327 def adapters(self):
328 """See `LaunchpadFormView`."""
329 return {IGitSubscriptionSchema: self.context}
248330
249 @property331 @property
250 def page_title(self):332 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):
116 with person_logged_in(subscriber):116 with person_logged_in(subscriber):
117 subscription = repository.getSubscription(subscriber)117 subscription = repository.getSubscription(subscriber)
118 self.assertThat(subscription, MatchesStructure.byEquality(118 self.assertThat(subscription, MatchesStructure.byEquality(
119 person=subscriber, repository=repository,119 person=subscriber, repository=repository, paths=None,
120 notification_level=(120 notification_level=(
121 BranchSubscriptionNotificationLevel.ATTRIBUTEONLY),121 BranchSubscriptionNotificationLevel.ATTRIBUTEONLY),
122 max_diff_lines=None,122 max_diff_lines=None,
@@ -147,6 +147,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
147 None, CodeReviewNotificationLevel.FULL, subscriber)147 None, CodeReviewNotificationLevel.FULL, subscriber)
148 browser = self.getViewBrowser(repository, user=subscriber)148 browser = self.getViewBrowser(repository, user=subscriber)
149 browser.getLink('Edit your subscription').click()149 browser.getLink('Edit your subscription').click()
150 browser.getControl('Paths').value = 'refs/heads/master refs/heads/next'
150 browser.getControl('Notification Level').displayValue = [151 browser.getControl('Notification Level').displayValue = [
151 'Branch attribute and revision notifications']152 'Branch attribute and revision notifications']
152 browser.getControl('Generated Diff Size Limit').displayValue = [153 browser.getControl('Generated Diff Size Limit').displayValue = [
@@ -154,6 +155,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
154 browser.getControl('Change').click()155 browser.getControl('Change').click()
155 self.assertTextMatchesExpressionIgnoreWhitespace(156 self.assertTextMatchesExpressionIgnoreWhitespace(
156 'Subscription updated to: '157 'Subscription updated to: '
158 'Only paths matching refs/heads/master refs/heads/next. '
157 'Send notifications for both branch attribute updates and new '159 'Send notifications for both branch attribute updates and new '
158 'revisions added to the branch. '160 'revisions added to the branch. '
159 'Limit the generated diff to 5000 lines. '161 'Limit the generated diff to 5000 lines. '
@@ -163,6 +165,7 @@ class TestGitSubscriptionAddView(BrowserTestCase):
163 subscription = repository.getSubscription(subscriber)165 subscription = repository.getSubscription(subscriber)
164 self.assertThat(subscription, MatchesStructure.byEquality(166 self.assertThat(subscription, MatchesStructure.byEquality(
165 person=subscriber, repository=repository,167 person=subscriber, repository=repository,
168 paths=['refs/heads/master', 'refs/heads/next'],
166 notification_level=BranchSubscriptionNotificationLevel.FULL,169 notification_level=BranchSubscriptionNotificationLevel.FULL,
167 max_diff_lines=BranchSubscriptionDiffSize.FIVEKLINES,170 max_diff_lines=BranchSubscriptionDiffSize.FIVEKLINES,
168 review_level=CodeReviewNotificationLevel.FULL))171 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):
197 "Persons subscribed to the repository containing this reference.")197 "Persons subscribed to the repository containing this reference.")
198198
199 def subscribe(person, notification_level, max_diff_lines,199 def subscribe(person, notification_level, max_diff_lines,
200 code_review_level, subscribed_by):200 code_review_level, subscribed_by, paths=None):
201 """Subscribe this person to the repository containing this reference.201 """Subscribe this person to the repository containing this reference.
202202
203 :param person: The `Person` to subscribe.203 :param person: The `Person` to subscribe.
@@ -209,6 +209,8 @@ class IGitRefView(IHasMergeProposals, IHasRecipes, IPrivacy, IInformationType):
209 cause notification.209 cause notification.
210 :param subscribed_by: The person who is subscribing the subscriber.210 :param subscribed_by: The person who is subscribing the subscriber.
211 Most often the subscriber themselves.211 Most often the subscriber themselves.
212 :param paths: An optional list of patterns matching reference paths
213 to subscribe to.
212 :return: A new or existing `GitSubscription`.214 :return: A new or existing `GitSubscription`.
213 """215 """
214216
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):
432 vocabulary=BranchSubscriptionDiffSize),432 vocabulary=BranchSubscriptionDiffSize),
433 code_review_level=Choice(433 code_review_level=Choice(
434 title=_("The level of code review notification emails."),434 title=_("The level of code review notification emails."),
435 vocabulary=CodeReviewNotificationLevel))435 vocabulary=CodeReviewNotificationLevel),
436 paths=List(
437 title=_("Paths"), required=False,
438 description=_(
439 "A list of patterns matching reference paths to subscribe "
440 "to. For example, ``['refs/heads/master', "
441 "'refs/heads/next']`` matches just those two branches, while "
442 "``['refs/heads/releases/*']`` matches all branches under "
443 "``refs/heads/releases/``. Omit this parameter to subscribe "
444 "to the whole repository.")))
436 # Really IGitSubscription, patched in _schema_circular_imports.py.445 # Really IGitSubscription, patched in _schema_circular_imports.py.
437 @operation_returns_entry(Interface)446 @operation_returns_entry(Interface)
438 @call_with(subscribed_by=REQUEST_USER)447 @call_with(subscribed_by=REQUEST_USER)
439 @export_write_operation()448 @export_write_operation()
440 @operation_for_version("devel")449 @operation_for_version("devel")
441 def subscribe(person, notification_level, max_diff_lines,450 def subscribe(person, notification_level, max_diff_lines,
442 code_review_level, subscribed_by):451 code_review_level, subscribed_by, paths=None):
443 """Subscribe this person to the repository.452 """Subscribe this person to the repository.
444453
445 :param person: The `Person` to subscribe.454 :param person: The `Person` to subscribe.
@@ -451,6 +460,8 @@ class IGitRepositoryView(IHasRecipes):
451 cause notification.460 cause notification.
452 :param subscribed_by: The person who is subscribing the subscriber.461 :param subscribed_by: The person who is subscribing the subscriber.
453 Most often the subscriber themselves.462 Most often the subscriber themselves.
463 :param paths: An optional list of patterns matching reference paths
464 to subscribe to.
454 :return: A new or existing `GitSubscription`.465 :return: A new or existing `GitSubscription`.
455 """466 """
456467
@@ -486,11 +497,14 @@ class IGitRepositoryView(IHasRecipes):
486 :return: A `ResultSet`.497 :return: A `ResultSet`.
487 """498 """
488499
489 def getNotificationRecipients():500 def getNotificationRecipients(path=None):
490 """Return a complete INotificationRecipientSet instance.501 """Return a complete INotificationRecipientSet instance.
491502
492 The INotificationRecipientSet instance contains the subscribers503 The INotificationRecipientSet instance contains the subscribers
493 and their subscriptions.504 and their subscriptions.
505
506 :param path: If not None, only consider subscriptions that match
507 this reference path.
494 """508 """
495509
496 landing_targets = Attribute(510 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 @@
1# Copyright 2009-2015 Canonical Ltd. This software is licensed under the1# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Git repository subscription interfaces."""4"""Git repository subscription interfaces."""
@@ -22,6 +22,8 @@ from zope.interface import Interface
22from zope.schema import (22from zope.schema import (
23 Choice,23 Choice,
24 Int,24 Int,
25 List,
26 TextLine,
25 )27 )
2628
27from lp import _29from lp import _
@@ -58,6 +60,18 @@ class IGitSubscription(Interface):
58 Reference(60 Reference(
59 title=_("Repository ID"), required=True, readonly=True,61 title=_("Repository ID"), required=True, readonly=True,
60 schema=IGitRepository))62 schema=IGitRepository))
63 paths = List(title=_("Paths"), value_type=TextLine(), required=False)
64 api_paths = exported(
65 List(
66 title=_("Paths"), value_type=TextLine(), required=True,
67 description=_(
68 "A list of patterns matching subscribed reference paths. "
69 "For example, ``['refs/heads/master', 'refs/heads/next']`` "
70 "matches just those two branches, while "
71 "``['refs/heads/releases/*']`` matches all branches under "
72 "``refs/heads/releases/``. Leave this empty to subscribe "
73 "to the whole repository.")),
74 exported_as="paths")
61 notification_level = exported(75 notification_level = exported(
62 Choice(76 Choice(
63 title=_("Notification Level"), required=True,77 title=_("Notification Level"), required=True,
@@ -97,3 +111,6 @@ class IGitSubscription(Interface):
97 @operation_for_version("devel")111 @operation_for_version("devel")
98 def canBeUnsubscribedByUser(user):112 def canBeUnsubscribedByUser(user):
99 """Can the user unsubscribe the subscriber from the repository?"""113 """Can the user unsubscribe the subscriber from the repository?"""
114
115 def matchesPath(path):
116 """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:
212 return self.repository.subscribers212 return self.repository.subscribers
213213
214 def subscribe(self, person, notification_level, max_diff_lines,214 def subscribe(self, person, notification_level, max_diff_lines,
215 code_review_level, subscribed_by):215 code_review_level, subscribed_by, paths=None):
216 """See `IGitRef`."""216 """See `IGitRef`."""
217 return self.repository.subscribe(217 return self.repository.subscribe(
218 person, notification_level, max_diff_lines, code_review_level,218 person, notification_level, max_diff_lines, code_review_level,
219 subscribed_by)219 subscribed_by, paths=paths)
220220
221 def getSubscription(self, person):221 def getSubscription(self, person):
222 """See `IGitRef`."""222 """See `IGitRef`."""
@@ -228,7 +228,7 @@ class GitRefMixin:
228228
229 def getNotificationRecipients(self):229 def getNotificationRecipients(self):
230 """See `IGitRef`."""230 """See `IGitRef`."""
231 return self.repository.getNotificationRecipients()231 return self.repository.getNotificationRecipients(path=self.path)
232232
233 @property233 @property
234 def landing_targets(self):234 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):
854 person.anyone_can_join())854 person.anyone_can_join())
855855
856 def subscribe(self, person, notification_level, max_diff_lines,856 def subscribe(self, person, notification_level, max_diff_lines,
857 code_review_level, subscribed_by):857 code_review_level, subscribed_by, paths=None):
858 """See `IGitRepository`."""858 """See `IGitRepository`."""
859 if not self.userCanBeSubscribed(person):859 if not self.userCanBeSubscribed(person):
860 raise SubscriptionPrivacyViolation(860 raise SubscriptionPrivacyViolation(
861 "Open and delegated teams cannot be subscribed to private "861 "Open and delegated teams cannot be subscribed to private "
862 "repositories.")862 "repositories.")
863 if paths is not None and isinstance(paths, six.string_types):
864 raise TypeError(
865 "The paths argument must be a sequence of strings, not a "
866 "string.")
863 # If the person is already subscribed, update the subscription with867 # If the person is already subscribed, update the subscription with
864 # the specified notification details.868 # the specified notification details.
865 subscription = self.getSubscription(person)869 subscription = self.getSubscription(person)
866 if subscription is None:870 if subscription is None:
867 subscription = GitSubscription(871 subscription = GitSubscription(
868 person=person, repository=self,872 person=person, repository=self, paths=paths,
869 notification_level=notification_level,873 notification_level=notification_level,
870 max_diff_lines=max_diff_lines, review_level=code_review_level,874 max_diff_lines=max_diff_lines, review_level=code_review_level,
871 subscribed_by=subscribed_by)875 subscribed_by=subscribed_by)
872 Store.of(subscription).flush()876 Store.of(subscription).flush()
873 else:877 else:
878 subscription.paths = paths
874 subscription.notification_level = notification_level879 subscription.notification_level = notification_level
875 subscription.max_diff_lines = max_diff_lines880 subscription.max_diff_lines = max_diff_lines
876 subscription.review_level = code_review_level881 subscription.review_level = code_review_level
@@ -928,10 +933,12 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
928 artifact, [person])933 artifact, [person])
929 store.flush()934 store.flush()
930935
931 def getNotificationRecipients(self):936 def getNotificationRecipients(self, path=None):
932 """See `IGitRepository`."""937 """See `IGitRepository`."""
933 recipients = NotificationRecipientSet()938 recipients = NotificationRecipientSet()
934 for subscription in self.subscriptions:939 for subscription in self.subscriptions:
940 if path is not None and not subscription.matchesPath(path):
941 continue
935 if subscription.person.is_team:942 if subscription.person.is_team:
936 rationale = 'Subscriber @%s' % subscription.person.name943 rationale = 'Subscriber @%s' % subscription.person.name
937 else:944 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 @@
1# Copyright 2015 Canonical Ltd. This software is licensed under the1# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type4__metaclass__ = type
@@ -6,8 +6,11 @@ __all__ = [
6 'GitSubscription',6 'GitSubscription',
7 ]7 ]
88
9import fnmatch
10
9from storm.locals import (11from storm.locals import (
10 Int,12 Int,
13 JSON,
11 Reference,14 Reference,
12 )15 )
13from zope.interface import implementer16from zope.interface import implementer
@@ -40,6 +43,8 @@ class GitSubscription(StormBase):
40 repository_id = Int(name='repository', allow_none=False)43 repository_id = Int(name='repository', allow_none=False)
41 repository = Reference(repository_id, 'GitRepository.id')44 repository = Reference(repository_id, 'GitRepository.id')
4245
46 paths = JSON(name='paths', allow_none=True)
47
43 notification_level = EnumCol(48 notification_level = EnumCol(
44 enum=BranchSubscriptionNotificationLevel, notNull=True,49 enum=BranchSubscriptionNotificationLevel, notNull=True,
45 default=DEFAULT)50 default=DEFAULT)
@@ -52,19 +57,38 @@ class GitSubscription(StormBase):
52 name='subscribed_by', allow_none=False, validator=validate_person)57 name='subscribed_by', allow_none=False, validator=validate_person)
53 subscribed_by = Reference(subscribed_by_id, 'Person.id')58 subscribed_by = Reference(subscribed_by_id, 'Person.id')
5459
55 def __init__(self, person, repository, notification_level, max_diff_lines,60 def __init__(self, person, repository, paths, notification_level,
56 review_level, subscribed_by):61 max_diff_lines, review_level, subscribed_by):
57 super(GitSubscription, self).__init__()62 super(GitSubscription, self).__init__()
58 self.person = person63 self.person = person
59 self.repository = repository64 self.repository = repository
65 self.paths = paths
60 self.notification_level = notification_level66 self.notification_level = notification_level
61 self.max_diff_lines = max_diff_lines67 self.max_diff_lines = max_diff_lines
62 self.review_level = review_level68 self.review_level = review_level
63 self.subscribed_by = subscribed_by69 self.subscribed_by = subscribed_by
6470
65 def canBeUnsubscribedByUser(self, user):71 def canBeUnsubscribedByUser(self, user):
66 """See `IBranchSubscription`."""72 """See `IGitSubscription`."""
67 if user is None:73 if user is None:
68 return False74 return False
69 permission_check = GitSubscriptionEdit(self)75 permission_check = GitSubscriptionEdit(self)
70 return permission_check.checkAuthenticated(IPersonRoles(user))76 return permission_check.checkAuthenticated(IPersonRoles(user))
77
78 @property
79 def api_paths(self):
80 """See `IGitSubscription`.
81
82 lazr.restful can't represent the difference between a NULL
83 collection and an empty collection, so we simulate the former.
84 """
85 return ["*"] if self.paths is None else self.paths
86
87 def matchesPath(self, path):
88 """See `IGitSubscription`."""
89 if self.paths is None:
90 return True
91 for pattern in self.paths:
92 if fnmatch.fnmatch(path, pattern):
93 return True
94 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(
1020 source_ref=source, target_ref=target,1020 source_ref=source, target_ref=target,
1021 prerequisite_ref=prerequisite, **kwargs)1021 prerequisite_ref=prerequisite, **kwargs)
10221022
1023 def test_getNotificationRecipients_path(self):
1024 # If the subscription specifies path patterns, then one of them must
1025 # match the reference.
1026 bmp = self.makeBranchMergeProposal()
1027 source_owner = bmp.merge_source.owner
1028 target_owner = bmp.merge_target.owner
1029 recipients = bmp.getNotificationRecipients(
1030 CodeReviewNotificationLevel.STATUS)
1031 subscriber_set = set([source_owner, target_owner])
1032 self.assertEqual(subscriber_set, set(recipients.keys()))
1033 # Subscribing somebody to a non-matching path has no effect.
1034 subscriber = self.factory.makePerson()
1035 bmp.merge_source.subscribe(
1036 subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1037 CodeReviewNotificationLevel.FULL, subscriber, paths=["no-match"])
1038 recipients = bmp.getNotificationRecipients(
1039 CodeReviewNotificationLevel.STATUS)
1040 self.assertEqual(subscriber_set, set(recipients.keys()))
1041 # Subscribing somebody to a matching path is effective.
1042 bmp.merge_source.subscribe(
1043 subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
1044 CodeReviewNotificationLevel.FULL, subscriber,
1045 paths=[bmp.merge_source.path])
1046 recipients = bmp.getNotificationRecipients(
1047 CodeReviewNotificationLevel.STATUS)
1048 subscriber_set.add(subscriber)
1049 self.assertEqual(subscriber_set, set(recipients.keys()))
1050
10231051
1024class TestMergeProposalWebhooksMixin:1052class TestMergeProposalWebhooksMixin:
10251053
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
26from storm.store import Store26from storm.store import Store
27from testtools.matchers import (27from testtools.matchers import (
28 AnyMatch,28 AnyMatch,
29 ContainsDict,
29 EndsWith,30 EndsWith,
30 Equals,31 Equals,
31 Is,32 Is,
@@ -3572,9 +3573,34 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
3572 with person_logged_in(ANONYMOUS):3573 with person_logged_in(ANONYMOUS):
3573 subscription_db = repository_db.getSubscription(subscriber_db)3574 subscription_db = repository_db.getSubscription(subscriber_db)
3574 self.assertIsNotNone(subscription_db)3575 self.assertIsNotNone(subscription_db)
3575 self.assertThat(3576 self.assertThat(response.jsonBody(), ContainsDict({
3576 response.jsonBody()["self_link"],3577 "self_link": EndsWith(api_url(subscription_db)),
3577 EndsWith(api_url(subscription_db)))3578 "paths": Equals(["*"]),
3579 }))
3580
3581 def test_subscribe_with_paths(self):
3582 # A user can subscribe to some reference paths in a repository.
3583 repository_db = self.factory.makeGitRepository()
3584 subscriber_db = self.factory.makePerson()
3585 webservice = webservice_for_person(
3586 subscriber_db, permission=OAuthPermission.WRITE_PUBLIC)
3587 webservice.default_api_version = "devel"
3588 with person_logged_in(ANONYMOUS):
3589 repository_url = api_url(repository_db)
3590 subscriber_url = api_url(subscriber_db)
3591 response = webservice.named_post(
3592 repository_url, "subscribe", person=subscriber_url,
3593 notification_level="Branch attribute notifications only",
3594 max_diff_lines="Don't send diffs", code_review_level="No email",
3595 paths=["refs/heads/master", "refs/heads/next"])
3596 self.assertEqual(200, response.status)
3597 with person_logged_in(ANONYMOUS):
3598 subscription_db = repository_db.getSubscription(subscriber_db)
3599 self.assertIsNotNone(subscription_db)
3600 self.assertThat(response.jsonBody(), ContainsDict({
3601 "self_link": EndsWith(api_url(subscription_db)),
3602 "paths": Equals(["refs/heads/master", "refs/heads/next"]),
3603 }))
35783604
3579 def _makeSubscription(self, repository, subscriber):3605 def _makeSubscription(self, repository, subscriber):
3580 with person_logged_in(subscriber):3606 with person_logged_in(subscriber):
@@ -3620,7 +3646,8 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
3620 repository_url, "subscribe", person=subscriber_url,3646 repository_url, "subscribe", person=subscriber_url,
3621 notification_level="No email",3647 notification_level="No email",
3622 max_diff_lines="Send entire diff",3648 max_diff_lines="Send entire diff",
3623 code_review_level="Status changes only")3649 code_review_level="Status changes only",
3650 paths=["refs/heads/next"])
3624 self.assertEqual(200, response.status)3651 self.assertEqual(200, response.status)
3625 with person_logged_in(subscriber_db):3652 with person_logged_in(subscriber_db):
3626 self.assertThat(3653 self.assertThat(
@@ -3631,6 +3658,7 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
3631 BranchSubscriptionNotificationLevel.NOEMAIL),3658 BranchSubscriptionNotificationLevel.NOEMAIL),
3632 max_diff_lines=BranchSubscriptionDiffSize.WHOLEDIFF,3659 max_diff_lines=BranchSubscriptionDiffSize.WHOLEDIFF,
3633 review_level=CodeReviewNotificationLevel.STATUS,3660 review_level=CodeReviewNotificationLevel.STATUS,
3661 paths=["refs/heads/next"],
3634 ))3662 ))
3635 repository = webservice.get(repository_url).jsonBody()3663 repository = webservice.get(repository_url).jsonBody()
3636 subscribers = webservice.get(3664 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):
111 repository.unsubscribe(subscribee, owner)111 repository.unsubscribe(subscribee, owner)
112 self.assertFalse(repository.visibleByUser(subscribee))112 self.assertFalse(repository.visibleByUser(subscribee))
113113
114 def test_matchesPath_none_matches_anything(self):
115 # A paths=None subscription matches any path.
116 subscription = self.factory.makeGitSubscription(paths=None)
117 self.assertTrue(subscription.matchesPath("refs/heads/master"))
118 self.assertTrue(subscription.matchesPath("refs/heads/next"))
119 self.assertTrue(subscription.matchesPath("nonsense"))
120
121 def test_matchesPath_exact(self):
122 # A subscription to a single path matches only that path.
123 subscription = self.factory.makeGitSubscription(
124 paths=["refs/heads/master"])
125 self.assertTrue(subscription.matchesPath("refs/heads/master"))
126 self.assertFalse(subscription.matchesPath("refs/heads/master2"))
127 self.assertFalse(subscription.matchesPath("refs/heads/next"))
128
129 def test_matchesPath_fnmatch(self):
130 # A subscription to a path pattern matches anything that fnmatch
131 # accepts.
132 subscription = self.factory.makeGitSubscription(
133 paths=["refs/heads/*"])
134 self.assertTrue(subscription.matchesPath("refs/heads/master"))
135 self.assertTrue(subscription.matchesPath("refs/heads/next"))
136 self.assertTrue(subscription.matchesPath("refs/heads/foo/bar"))
137 self.assertFalse(subscription.matchesPath("refs/tags/1.0"))
138
139 def test_matchesPath_multiple(self):
140 # A subscription to multiple path patterns matches any of them.
141 subscription = self.factory.makeGitSubscription(
142 paths=["refs/heads/*", "refs/tags/1.0"])
143 self.assertTrue(subscription.matchesPath("refs/heads/master"))
144 self.assertTrue(subscription.matchesPath("refs/heads/next"))
145 self.assertTrue(subscription.matchesPath("refs/heads/foo/bar"))
146 self.assertTrue(subscription.matchesPath("refs/tags/1.0"))
147 self.assertFalse(subscription.matchesPath("refs/tags/1.0a"))
148
114149
115class TestGitSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):150class TestGitSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
116 """Tests for GitSubscription.canBeUnsubscribedByUser."""151 """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):
1792 return repository1792 return repository
17931793
1794 def makeGitSubscription(self, repository=None, person=None,1794 def makeGitSubscription(self, repository=None, person=None,
1795 subscribed_by=None):1795 subscribed_by=None, paths=None):
1796 """Create a GitSubscription."""1796 """Create a GitSubscription."""
1797 if repository is None:1797 if repository is None:
1798 repository = self.makeGitRepository()1798 repository = self.makeGitRepository()
@@ -1802,7 +1802,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
1802 subscribed_by = person1802 subscribed_by = person
1803 return repository.subscribe(removeSecurityProxy(person),1803 return repository.subscribe(removeSecurityProxy(person),
1804 BranchSubscriptionNotificationLevel.NOEMAIL, None,1804 BranchSubscriptionNotificationLevel.NOEMAIL, None,
1805 CodeReviewNotificationLevel.NOEMAIL, subscribed_by)1805 CodeReviewNotificationLevel.NOEMAIL, subscribed_by, paths=paths)
18061806
1807 def makeGitRefs(self, repository=None, paths=None, **repository_kwargs):1807 def makeGitRefs(self, repository=None, paths=None, **repository_kwargs):
1808 """Create and return a list of new, arbitrary GitRefs."""1808 """Create and return a list of new, arbitrary GitRefs."""

Subscribers

People subscribed via source and target branches

to status/vote changes: