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