Merge lp:~matiasb/launchpad/upload-snap-to-track-based-channels into lp:launchpad
- upload-snap-to-track-based-channels
- Merge into devel
Proposed by
Matias Bordese
Status: | Merged |
---|---|
Merged at revision: | 18338 |
Proposed branch: | lp:~matiasb/launchpad/upload-snap-to-track-based-channels |
Merge into: | lp:launchpad |
Diff against target: |
674 lines (+484/-37) 10 files modified
lib/lp/snappy/browser/snap.py (+5/-11) lib/lp/snappy/browser/tests/test_snap.py (+11/-21) lib/lp/snappy/browser/widgets/storechannels.py (+152/-0) lib/lp/snappy/browser/widgets/templates/storechannels.pt (+22/-0) lib/lp/snappy/browser/widgets/tests/test_storechannelswidget.py (+194/-0) lib/lp/snappy/interfaces/snap.py (+5/-3) lib/lp/snappy/templates/snap-edit.pt (+1/-1) lib/lp/snappy/templates/snap-new.pt (+1/-1) lib/lp/snappy/validators/channels.py (+53/-0) lib/lp/snappy/validators/tests/test_channels.py (+40/-0) |
To merge this branch: | bzr merge lp:~matiasb/launchpad/upload-snap-to-track-based-channels |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+320855@code.launchpad.net |
Commit message
Added initial channel track support when uploading snaps to the store.
Description of the change
To post a comment you must log in.
Revision history for this message
Matias Bordese (matiasb) wrote : | # |
Replied comments, pushing updates in a minute.
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/snappy/browser/snap.py' |
2 | --- lib/lp/snappy/browser/snap.py 2017-03-08 22:45:20 +0000 |
3 | +++ lib/lp/snappy/browser/snap.py 2017-03-30 14:10:04 +0000 |
4 | @@ -32,7 +32,6 @@ |
5 | List, |
6 | TextLine, |
7 | ) |
8 | -from zope.schema.interfaces import IVocabularyFactory |
9 | |
10 | from lp import _ |
11 | from lp.app.browser.launchpadform import ( |
12 | @@ -62,7 +61,6 @@ |
13 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
14 | from lp.services.features import getFeatureFlag |
15 | from lp.services.helpers import english_list |
16 | -from lp.services.propertycache import cachedproperty |
17 | from lp.services.scripts import log |
18 | from lp.services.webapp import ( |
19 | canonical_url, |
20 | @@ -82,6 +80,7 @@ |
21 | from lp.services.webapp.url import urlappend |
22 | from lp.services.webhooks.browser import WebhookTargetNavigationMixin |
23 | from lp.snappy.browser.widgets.snaparchive import SnapArchiveWidget |
24 | +from lp.snappy.browser.widgets.storechannels import StoreChannelsWidget |
25 | from lp.snappy.interfaces.snap import ( |
26 | CannotAuthorizeStoreUploads, |
27 | ISnap, |
28 | @@ -190,14 +189,9 @@ |
29 | else: |
30 | return 'Built on request' |
31 | |
32 | - @cachedproperty |
33 | + @property |
34 | def store_channels(self): |
35 | - vocabulary = getUtility( |
36 | - IVocabularyFactory, name='SnapStoreChannel')(self.context) |
37 | - channel_titles = [] |
38 | - for channel in self.context.store_channels: |
39 | - channel_titles.append(vocabulary.getTermByToken(channel).title) |
40 | - return ', '.join(channel_titles) |
41 | + return ', '.join(self.context.store_channels) |
42 | |
43 | |
44 | def builds_for_snap(snap): |
45 | @@ -388,7 +382,7 @@ |
46 | ] |
47 | custom_widget('store_distro_series', LaunchpadRadioWidget) |
48 | custom_widget('auto_build_archive', SnapArchiveWidget) |
49 | - custom_widget('store_channels', LabeledMultiCheckBoxWidget) |
50 | + custom_widget('store_channels', StoreChannelsWidget) |
51 | custom_widget('auto_build_pocket', LaunchpadDropdownWidget) |
52 | |
53 | help_links = { |
54 | @@ -694,7 +688,7 @@ |
55 | 'store_channels', |
56 | ] |
57 | custom_widget('store_distro_series', LaunchpadRadioWidget) |
58 | - custom_widget('store_channels', LabeledMultiCheckBoxWidget) |
59 | + custom_widget('store_channels', StoreChannelsWidget) |
60 | custom_widget('vcs', LaunchpadRadioWidget) |
61 | custom_widget('git_ref', GitRefWidget, allow_external=True) |
62 | custom_widget('auto_build_archive', SnapArchiveWidget) |
63 | |
64 | === modified file 'lib/lp/snappy/browser/tests/test_snap.py' |
65 | --- lib/lp/snappy/browser/tests/test_snap.py 2017-02-03 11:30:05 +0000 |
66 | +++ lib/lp/snappy/browser/tests/test_snap.py 2017-03-30 14:10:04 +0000 |
67 | @@ -393,6 +393,7 @@ |
68 | browser.getControl("Registered store package name").value = ( |
69 | "store-name") |
70 | self.assertFalse(browser.getControl("Stable").selected) |
71 | + browser.getControl(name="field.store_channels.track").value = "track" |
72 | browser.getControl("Edge").selected = True |
73 | root_macaroon = Macaroon() |
74 | root_macaroon.add_third_party_caveat( |
75 | @@ -419,7 +420,7 @@ |
76 | name=u"snap-name", source=branch, store_upload=True, |
77 | store_series=self.snappyseries, store_name=u"store-name", |
78 | store_secrets={"root": root_macaroon_raw}, |
79 | - store_channels=["edge"])) |
80 | + store_channels=["track/edge"])) |
81 | self.assertThat(self.request, MatchesStructure.byEquality( |
82 | url="http://sca.example/dev/api/acl/", method="POST")) |
83 | expected_body = { |
84 | @@ -946,12 +947,14 @@ |
85 | registrant=self.person, owner=self.person, |
86 | distroseries=self.distroseries, store_upload=True, |
87 | store_series=self.snappyseries, store_name=u"one", |
88 | - store_channels=["edge"]) |
89 | + store_channels=["track/edge"]) |
90 | view_url = canonical_url(snap, view_name="+edit") |
91 | browser = self.getNonRedirectingBrowser(url=view_url, user=self.person) |
92 | browser.getControl("Registered store package name").value = "two" |
93 | + self.assertEqual("track", browser.getControl("Track").value) |
94 | + self.assertTrue(browser.getControl("Edge").selected) |
95 | + browser.getControl("Track").value = "" |
96 | browser.getControl("Stable").selected = True |
97 | - self.assertTrue(browser.getControl("Edge").selected) |
98 | root_macaroon = Macaroon() |
99 | root_macaroon.add_third_party_caveat( |
100 | urlsplit(config.launchpad.openid_provider_root).netloc, "", |
101 | @@ -1373,24 +1376,11 @@ |
102 | view = create_initialized_view(snap, "+index") |
103 | self.assertEqual("", view.store_channels) |
104 | |
105 | - def test_store_channels_uses_titles(self): |
106 | - snap_store_client = FakeMethod() |
107 | - snap_store_client.listChannels = FakeMethod(result=[ |
108 | - {"name": "stable", "display_name": "Stable"}, |
109 | - {"name": "edge", "display_name": "Edge"}, |
110 | - {"name": "old", "display_name": "Old channel"}, |
111 | - ]) |
112 | - self.useFixture( |
113 | - ZopeUtilityFixture(snap_store_client, ISnapStoreClient)) |
114 | - snap = self.factory.makeSnap(store_channels=["stable", "old"]) |
115 | - view = create_initialized_view(snap, "+index") |
116 | - self.assertEqual("Stable, Old channel", view.store_channels) |
117 | - snap_store_client.listChannels.result = [ |
118 | - {"name": "stable", "display_name": "Stable"}, |
119 | - {"name": "edge", "display_name": "Edge"}, |
120 | - ] |
121 | - view = create_initialized_view(snap, "+index") |
122 | - self.assertEqual("Stable, old", view.store_channels) |
123 | + def test_store_channels_display(self): |
124 | + snap = self.factory.makeSnap( |
125 | + store_channels=["track/stable", "track/edge"]) |
126 | + view = create_initialized_view(snap, "+index") |
127 | + self.assertEqual("track/stable, track/edge", view.store_channels) |
128 | |
129 | |
130 | class TestSnapRequestBuildsView(BaseTestSnapView): |
131 | |
132 | === added file 'lib/lp/snappy/browser/widgets/storechannels.py' |
133 | --- lib/lp/snappy/browser/widgets/storechannels.py 1970-01-01 00:00:00 +0000 |
134 | +++ lib/lp/snappy/browser/widgets/storechannels.py 2017-03-30 14:10:04 +0000 |
135 | @@ -0,0 +1,152 @@ |
136 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
137 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
138 | + |
139 | +__metaclass__ = type |
140 | + |
141 | +__all__ = [ |
142 | + 'StoreChannelsWidget', |
143 | + ] |
144 | + |
145 | +from z3c.ptcompat import ViewPageTemplateFile |
146 | +from zope.formlib.interfaces import IInputWidget, WidgetInputError |
147 | +from zope.formlib.utility import setUpWidget |
148 | +from zope.formlib.widget import ( |
149 | + BrowserWidget, |
150 | + CustomWidgetFactory, |
151 | + InputErrors, |
152 | + InputWidget, |
153 | + ) |
154 | +from zope.interface import implementer |
155 | +from zope.schema import ( |
156 | + Choice, |
157 | + List, |
158 | + TextLine, |
159 | + ) |
160 | + |
161 | +from lp import _ |
162 | +from lp.app.errors import UnexpectedFormData |
163 | +from lp.app.validators import LaunchpadValidationError |
164 | +from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget |
165 | +from lp.services.webapp.interfaces import ( |
166 | + IAlwaysSubmittedWidget, |
167 | + ISingleLineWidgetLayout, |
168 | + ) |
169 | +from lp.snappy.validators.channels import ( |
170 | + channel_components_delimiter, |
171 | + split_channel_name, |
172 | + ) |
173 | + |
174 | + |
175 | +@implementer(ISingleLineWidgetLayout, IAlwaysSubmittedWidget, IInputWidget) |
176 | +class StoreChannelsWidget(BrowserWidget, InputWidget): |
177 | + |
178 | + template = ViewPageTemplateFile("templates/storechannels.pt") |
179 | + display_label = False |
180 | + _separator = channel_components_delimiter |
181 | + _default_track = 'latest' |
182 | + _widgets_set_up = False |
183 | + |
184 | + def __init__(self, field, value_type, request): |
185 | + # We don't use value_type. |
186 | + super(StoreChannelsWidget, self).__init__(field, request) |
187 | + |
188 | + def setUpSubWidgets(self): |
189 | + if self._widgets_set_up: |
190 | + return |
191 | + fields = [ |
192 | + TextLine(__name__="track", title=u"Track", required=False, |
193 | + description=_( |
194 | + "Track defines a series for your software. " |
195 | + "If not specified, the default track ('latest') is " |
196 | + "assumed.") |
197 | + ), |
198 | + List(__name__="risks", title=u"Risk", required=False, |
199 | + value_type=Choice(vocabulary="SnapStoreChannel"), |
200 | + description=_( |
201 | + "Risks denote the stability of your software.")), |
202 | + ] |
203 | + |
204 | + self.risks_widget = CustomWidgetFactory(LabeledMultiCheckBoxWidget) |
205 | + for field in fields: |
206 | + setUpWidget( |
207 | + self, field.__name__, field, IInputWidget, prefix=self.name) |
208 | + self.risks_widget.orientation = 'horizontal' |
209 | + self._widgets_set_up = True |
210 | + |
211 | + @property |
212 | + def has_risks_vocabulary(self): |
213 | + risks_widget = getattr(self, 'risks_widget', None) |
214 | + return risks_widget and bool(risks_widget.vocabulary) |
215 | + |
216 | + def buildChannelName(self, track, risk): |
217 | + """Return channel name composed from given track and risk.""" |
218 | + channel = risk |
219 | + if track and track != self._default_track: |
220 | + channel = track + self._separator + risk |
221 | + return channel |
222 | + |
223 | + def splitChannelName(self, channel): |
224 | + """Return extracted track and risk from given channel name.""" |
225 | + try: |
226 | + track, risk = split_channel_name(channel) |
227 | + except ValueError: |
228 | + raise AssertionError("Not a valid value: %r" % channel) |
229 | + return track, risk |
230 | + |
231 | + def setRenderedValue(self, value): |
232 | + """See `IWidget`.""" |
233 | + self.setUpSubWidgets() |
234 | + if value: |
235 | + # NOTE: atm target channels must belong to the same track |
236 | + tracks = set() |
237 | + risks = [] |
238 | + for channel in value: |
239 | + track, risk = self.splitChannelName(channel) |
240 | + tracks.add(track) |
241 | + risks.append(risk) |
242 | + if len(tracks) != 1: |
243 | + raise AssertionError("Not a valid value: %r" % value) |
244 | + track = tracks.pop() |
245 | + self.track_widget.setRenderedValue(track) |
246 | + self.risks_widget.setRenderedValue(risks) |
247 | + else: |
248 | + self.track_widget.setRenderedValue(None) |
249 | + self.risks_widget.setRenderedValue(None) |
250 | + |
251 | + def hasInput(self): |
252 | + """See `IInputWidget`.""" |
253 | + return ("%s.risks" % self.name) in self.request.form |
254 | + |
255 | + def hasValidInput(self): |
256 | + """See `IInputWidget`.""" |
257 | + try: |
258 | + self.getInputValue() |
259 | + return True |
260 | + except (InputErrors, UnexpectedFormData): |
261 | + return False |
262 | + |
263 | + def getInputValue(self): |
264 | + """See `IInputWidget`.""" |
265 | + self.setUpSubWidgets() |
266 | + risks = self.risks_widget.getInputValue() |
267 | + track = self.track_widget.getInputValue() |
268 | + if track and self._separator in track: |
269 | + error_msg = "Track name cannot include '%s'." % self._separator |
270 | + raise WidgetInputError( |
271 | + self.name, self.label, LaunchpadValidationError(error_msg)) |
272 | + channels = [self.buildChannelName(track, risk) for risk in risks] |
273 | + return channels |
274 | + |
275 | + def error(self): |
276 | + """See `IBrowserWidget`.""" |
277 | + try: |
278 | + if self.hasInput(): |
279 | + self.getInputValue() |
280 | + except InputErrors as error: |
281 | + self._error = error |
282 | + return super(StoreChannelsWidget, self).error() |
283 | + |
284 | + def __call__(self): |
285 | + """See `IBrowserWidget`.""" |
286 | + self.setUpSubWidgets() |
287 | + return self.template() |
288 | |
289 | === added file 'lib/lp/snappy/browser/widgets/templates/storechannels.pt' |
290 | --- lib/lp/snappy/browser/widgets/templates/storechannels.pt 1970-01-01 00:00:00 +0000 |
291 | +++ lib/lp/snappy/browser/widgets/templates/storechannels.pt 2017-03-30 14:10:04 +0000 |
292 | @@ -0,0 +1,22 @@ |
293 | +<table> |
294 | + <tr> |
295 | + <td> |
296 | + <tal:widget define="widget nocall:view/track_widget"> |
297 | + <metal:block |
298 | + use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" /> |
299 | + </tal:widget> |
300 | + <p class="formHelp"> |
301 | + To open a new track, <a href="https://snapcraft.io/community">ask the store admins</a>. |
302 | + </p> |
303 | + </td> |
304 | + </tr> |
305 | + <tr> |
306 | + <td> |
307 | + <tal:widget define="widget nocall:view/risks_widget" |
308 | + condition="widget/context/value_type/vocabulary"> |
309 | + <metal:block |
310 | + use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" /> |
311 | + </tal:widget> |
312 | + </td> |
313 | + </tr> |
314 | +</table> |
315 | |
316 | === added file 'lib/lp/snappy/browser/widgets/tests/test_storechannelswidget.py' |
317 | --- lib/lp/snappy/browser/widgets/tests/test_storechannelswidget.py 1970-01-01 00:00:00 +0000 |
318 | +++ lib/lp/snappy/browser/widgets/tests/test_storechannelswidget.py 2017-03-30 14:10:04 +0000 |
319 | @@ -0,0 +1,194 @@ |
320 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
321 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
322 | + |
323 | +__metaclass__ = type |
324 | + |
325 | +import re |
326 | + |
327 | +from BeautifulSoup import BeautifulSoup |
328 | +from zope.formlib.interfaces import ( |
329 | + IBrowserWidget, |
330 | + IInputWidget, |
331 | + WidgetInputError, |
332 | + ) |
333 | +from zope.schema import List |
334 | + |
335 | +from lp.app.validators import LaunchpadValidationError |
336 | +from lp.services.webapp.escaping import html_escape |
337 | +from lp.services.webapp.servers import LaunchpadTestRequest |
338 | +from lp.snappy.browser.widgets.storechannels import StoreChannelsWidget |
339 | +from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient |
340 | +from lp.snappy.vocabularies import SnapStoreChannelVocabulary |
341 | +from lp.testing import ( |
342 | + TestCaseWithFactory, |
343 | + verifyObject, |
344 | + ) |
345 | +from lp.testing.fakemethod import FakeMethod |
346 | +from lp.testing.fixture import ZopeUtilityFixture |
347 | +from lp.testing.layers import DatabaseFunctionalLayer |
348 | + |
349 | + |
350 | +class TestStoreChannelsWidget(TestCaseWithFactory): |
351 | + |
352 | + layer = DatabaseFunctionalLayer |
353 | + |
354 | + def setUp(self): |
355 | + super(TestStoreChannelsWidget, self).setUp() |
356 | + field = List(__name__="channels", title=u"Store channels") |
357 | + self.context = self.factory.makeSnap() |
358 | + field = field.bind(self.context) |
359 | + request = LaunchpadTestRequest() |
360 | + self.widget = StoreChannelsWidget(field, None, request) |
361 | + |
362 | + # setup fake store client response for available channels/risks |
363 | + self.risks = [ |
364 | + {"name": "stable", "display_name": "Stable"}, |
365 | + {"name": "candidate", "display_name": "Candidate"}, |
366 | + {"name": "beta", "display_name": "Beta"}, |
367 | + {"name": "edge", "display_name": "Edge"}, |
368 | + ] |
369 | + snap_store_client = FakeMethod() |
370 | + snap_store_client.listChannels = FakeMethod(result=self.risks) |
371 | + self.useFixture( |
372 | + ZopeUtilityFixture(snap_store_client, ISnapStoreClient)) |
373 | + |
374 | + def test_implements(self): |
375 | + self.assertTrue(verifyObject(IBrowserWidget, self.widget)) |
376 | + self.assertTrue(verifyObject(IInputWidget, self.widget)) |
377 | + |
378 | + def test_template(self): |
379 | + self.assertTrue( |
380 | + self.widget.template.filename.endswith("storechannels.pt"), |
381 | + "Template was not set up.") |
382 | + |
383 | + def test_setUpSubWidgets_first_call(self): |
384 | + # The subwidgets are set up and a flag is set. |
385 | + self.widget.setUpSubWidgets() |
386 | + self.assertTrue(self.widget._widgets_set_up) |
387 | + self.assertIsNotNone(getattr(self.widget, "track_widget", None)) |
388 | + self.assertIsInstance( |
389 | + self.widget.risks_widget.vocabulary, SnapStoreChannelVocabulary) |
390 | + self.assertTrue(self.widget.has_risks_vocabulary) |
391 | + |
392 | + def test_setUpSubWidgets_second_call(self): |
393 | + # The setUpSubWidgets method exits early if a flag is set to |
394 | + # indicate that the widgets were set up. |
395 | + self.widget._widgets_set_up = True |
396 | + self.widget.setUpSubWidgets() |
397 | + self.assertIsNone(getattr(self.widget, "track_widget", None)) |
398 | + self.assertIsNone(getattr(self.widget, "risks_widget", None)) |
399 | + self.assertIsNone(self.widget.has_risks_vocabulary) |
400 | + |
401 | + def test_buildChannelName_no_track(self): |
402 | + self.assertEqual("edge", self.widget.buildChannelName(None, "edge")) |
403 | + |
404 | + def test_buildChannelName_with_track(self): |
405 | + self.assertEqual( |
406 | + "track/edge", self.widget.buildChannelName("track", "edge")) |
407 | + |
408 | + def test_splitChannelName_no_track(self): |
409 | + self.assertEqual((None, "edge"), self.widget.splitChannelName("edge")) |
410 | + |
411 | + def test_splitChannelName_with_track(self): |
412 | + self.assertEqual( |
413 | + ("track", "edge"), self.widget.splitChannelName("track/edge")) |
414 | + |
415 | + def test_splitChannelName_invalid(self): |
416 | + self.assertRaises( |
417 | + AssertionError, self.widget.splitChannelName, "track/edge/invalid") |
418 | + |
419 | + def test_setRenderedValue_empty(self): |
420 | + self.widget.setRenderedValue([]) |
421 | + self.assertIsNone(self.widget.track_widget._getCurrentValue()) |
422 | + self.assertIsNone(self.widget.risks_widget._getCurrentValue()) |
423 | + |
424 | + def test_setRenderedValue_no_track(self): |
425 | + # Channels do not include a track |
426 | + risks = ['candidate', 'edge'] |
427 | + self.widget.setRenderedValue(risks) |
428 | + self.assertIsNone(self.widget.track_widget._getCurrentValue()) |
429 | + self.assertEqual(risks, self.widget.risks_widget._getCurrentValue()) |
430 | + |
431 | + def test_setRenderedValue_with_track(self): |
432 | + # Channels including a track |
433 | + channels = ['2.2/candidate', '2.2/edge'] |
434 | + self.widget.setRenderedValue(channels) |
435 | + self.assertEqual('2.2', self.widget.track_widget._getCurrentValue()) |
436 | + self.assertEqual( |
437 | + ['candidate', 'edge'], self.widget.risks_widget._getCurrentValue()) |
438 | + |
439 | + def test_setRenderedValue_invalid_value(self): |
440 | + # Multiple channels, different tracks, unsupported |
441 | + channels = ['2.2/candidate', '2.1/edge'] |
442 | + self.assertRaises( |
443 | + AssertionError, self.widget.setRenderedValue, channels) |
444 | + |
445 | + def test_hasInput_false(self): |
446 | + # hasInput is false when there is no risk set in the form data. |
447 | + self.widget.request = LaunchpadTestRequest( |
448 | + form={"field.channels.track": "track"}) |
449 | + self.assertFalse(self.widget.hasInput()) |
450 | + |
451 | + def test_hasInput_true(self): |
452 | + # hasInput is true if there are risks set in the form data. |
453 | + self.widget.request = LaunchpadTestRequest( |
454 | + form={"field.channels.risks": ["beta"]}) |
455 | + self.assertTrue(self.widget.hasInput()) |
456 | + |
457 | + def test_hasValidInput_false(self): |
458 | + # The field input is invalid if any of the submitted parts are |
459 | + # invalid. |
460 | + form = { |
461 | + "field.channels.track": "", |
462 | + "field.channels.risks": ["invalid"], |
463 | + } |
464 | + self.widget.request = LaunchpadTestRequest(form=form) |
465 | + self.assertFalse(self.widget.hasValidInput()) |
466 | + |
467 | + def test_hasValidInput_true(self): |
468 | + # The field input is valid when all submitted parts are valid. |
469 | + form = { |
470 | + "field.channels.track": "track", |
471 | + "field.channels.risks": ["stable", "beta"], |
472 | + } |
473 | + self.widget.request = LaunchpadTestRequest(form=form) |
474 | + self.assertTrue(self.widget.hasValidInput()) |
475 | + |
476 | + def assertGetInputValueError(self, form, message): |
477 | + self.widget.request = LaunchpadTestRequest(form=form) |
478 | + e = self.assertRaises(WidgetInputError, self.widget.getInputValue) |
479 | + self.assertEqual(LaunchpadValidationError(message), e.errors) |
480 | + self.assertEqual(html_escape(message), self.widget.error()) |
481 | + |
482 | + def test_getInputValue_invalid_track(self): |
483 | + # An error is raised when the track includes a '/'. |
484 | + form = {"field.channels.track": "tra/ck", |
485 | + "field.channels.risks": ["beta"]} |
486 | + self.assertGetInputValueError(form, "Track name cannot include '/'.") |
487 | + |
488 | + def test_getInputValue_no_track(self): |
489 | + self.widget.request = LaunchpadTestRequest( |
490 | + form={"field.channels.track": "", |
491 | + "field.channels.risks": ["beta", "edge"]}) |
492 | + expected = ["beta", "edge"] |
493 | + self.assertEqual(expected, self.widget.getInputValue()) |
494 | + |
495 | + def test_getInputValue_with_track(self): |
496 | + self.widget.request = LaunchpadTestRequest( |
497 | + form={"field.channels.track": "track", |
498 | + "field.channels.risks": ["beta", "edge"]}) |
499 | + expected = ["track/beta", "track/edge"] |
500 | + self.assertEqual(expected, self.widget.getInputValue()) |
501 | + |
502 | + def test_call(self): |
503 | + # The __call__ method sets up the widgets. |
504 | + markup = self.widget() |
505 | + self.assertIsNotNone(self.widget.track_widget) |
506 | + self.assertIsNotNone(self.widget.risks_widget) |
507 | + soup = BeautifulSoup(markup) |
508 | + fields = soup.findAll(["input"], {"id": re.compile(".*")}) |
509 | + expected_ids = [ |
510 | + "field.channels.risks.%d" % i for i in range(len(self.risks))] |
511 | + expected_ids.append("field.channels.track") |
512 | + ids = [field["id"] for field in fields] |
513 | + self.assertContentEqual(expected_ids, ids) |
514 | |
515 | === modified file 'lib/lp/snappy/interfaces/snap.py' |
516 | --- lib/lp/snappy/interfaces/snap.py 2017-02-06 14:34:35 +0000 |
517 | +++ lib/lp/snappy/interfaces/snap.py 2017-03-30 14:10:04 +0000 |
518 | @@ -94,6 +94,7 @@ |
519 | ISnappyDistroSeries, |
520 | ISnappySeries, |
521 | ) |
522 | +from lp.snappy.validators.channels import channels_validator |
523 | from lp.soyuz.interfaces.archive import IArchive |
524 | from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries |
525 | |
526 | @@ -537,11 +538,12 @@ |
527 | "authorize uploads of this snap package.")) |
528 | |
529 | store_channels = exported(List( |
530 | - value_type=Choice(vocabulary="SnapStoreChannel"), |
531 | - title=_("Store channels"), required=False, readonly=False, |
532 | + title=_("Store channels"), |
533 | + required=False, readonly=False, constraint=channels_validator, |
534 | description=_( |
535 | "Channels to release this snap package to after uploading it to " |
536 | - "the store."))) |
537 | + "the store. A channel is defined by a combination of an optional " |
538 | + " track and a risk, e.g. '2.1/stable', or 'stable'."))) |
539 | |
540 | |
541 | class ISnapAdminAttributes(Interface): |
542 | |
543 | === modified file 'lib/lp/snappy/templates/snap-edit.pt' |
544 | --- lib/lp/snappy/templates/snap-edit.pt 2016-07-28 12:42:21 +0000 |
545 | +++ lib/lp/snappy/templates/snap-edit.pt 2017-03-30 14:10:04 +0000 |
546 | @@ -87,7 +87,7 @@ |
547 | <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
548 | </tal:widget> |
549 | <tal:widget define="widget nocall:view/widgets/store_channels" |
550 | - condition="widget/context/value_type/vocabulary"> |
551 | + condition="widget/has_risks_vocabulary"> |
552 | <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
553 | </tal:widget> |
554 | </table> |
555 | |
556 | === modified file 'lib/lp/snappy/templates/snap-new.pt' |
557 | --- lib/lp/snappy/templates/snap-new.pt 2017-02-01 06:31:30 +0000 |
558 | +++ lib/lp/snappy/templates/snap-new.pt 2017-03-30 14:10:04 +0000 |
559 | @@ -63,7 +63,7 @@ |
560 | <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
561 | </tal:widget> |
562 | <tal:widget define="widget nocall:view/widgets/store_channels" |
563 | - condition="widget/context/value_type/vocabulary"> |
564 | + condition="widget/has_risks_vocabulary"> |
565 | <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
566 | </tal:widget> |
567 | </table> |
568 | |
569 | === added directory 'lib/lp/snappy/validators' |
570 | === added file 'lib/lp/snappy/validators/__init__.py' |
571 | === added file 'lib/lp/snappy/validators/channels.py' |
572 | --- lib/lp/snappy/validators/channels.py 1970-01-01 00:00:00 +0000 |
573 | +++ lib/lp/snappy/validators/channels.py 2017-03-30 14:10:04 +0000 |
574 | @@ -0,0 +1,53 @@ |
575 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
576 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
577 | + |
578 | +"""Validators for the .store_channels attribute.""" |
579 | + |
580 | +__metaclass__ = type |
581 | + |
582 | +from lp import _ |
583 | +from lp.app.validators import LaunchpadValidationError |
584 | +from lp.services.webapp.escaping import ( |
585 | + html_escape, |
586 | + structured, |
587 | + ) |
588 | + |
589 | + |
590 | +# delimiter separating channel components |
591 | +channel_components_delimiter = '/' |
592 | + |
593 | + |
594 | +def split_channel_name(channel): |
595 | + """Return extracted track and risk from given channel name.""" |
596 | + components = channel.split(channel_components_delimiter) |
597 | + if len(components) == 2: |
598 | + track, risk = components |
599 | + elif len(components) == 1: |
600 | + track = None |
601 | + risk = components[0] |
602 | + else: |
603 | + raise ValueError("Invalid channel name: %r" % channel) |
604 | + return track, risk |
605 | + |
606 | + |
607 | +def channels_validator(channels): |
608 | + """Return True if the channels in a list are valid, or raise a |
609 | + LaunchpadValidationError. |
610 | + """ |
611 | + tracks = set() |
612 | + for name in channels: |
613 | + try: |
614 | + track, risk = split_channel_name(name) |
615 | + except ValueError: |
616 | + message = _( |
617 | + "Invalid channel name '${name}'. Channel names must be of the " |
618 | + "form 'track/risk' or 'risk'.", |
619 | + mapping={'name': html_escape(name)}) |
620 | + raise LaunchpadValidationError(structured(message)) |
621 | + tracks.add(track) |
622 | + |
623 | + if len(tracks) != 1: |
624 | + message = _("Channels must belong to the same track.") |
625 | + raise LaunchpadValidationError(structured(message)) |
626 | + |
627 | + return True |
628 | |
629 | === added directory 'lib/lp/snappy/validators/tests' |
630 | === added file 'lib/lp/snappy/validators/tests/__init__.py' |
631 | === added file 'lib/lp/snappy/validators/tests/test_channels.py' |
632 | --- lib/lp/snappy/validators/tests/test_channels.py 1970-01-01 00:00:00 +0000 |
633 | +++ lib/lp/snappy/validators/tests/test_channels.py 2017-03-30 14:10:04 +0000 |
634 | @@ -0,0 +1,40 @@ |
635 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
636 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
637 | + |
638 | +__metaclass__ = type |
639 | + |
640 | +from lp.app.validators import LaunchpadValidationError |
641 | +from lp.snappy.validators.channels import ( |
642 | + channels_validator, |
643 | + split_channel_name, |
644 | + ) |
645 | +from lp.testing import TestCaseWithFactory |
646 | +from lp.testing.layers import LaunchpadFunctionalLayer |
647 | + |
648 | + |
649 | +class TestChannelsValidator(TestCaseWithFactory): |
650 | + |
651 | + layer = LaunchpadFunctionalLayer |
652 | + |
653 | + def test_split_channel_name_no_track(self): |
654 | + self.assertEqual((None, "edge"), split_channel_name("edge")) |
655 | + |
656 | + def test_split_channel_name_with_track(self): |
657 | + self.assertEqual(("track", "edge"), split_channel_name("track/edge")) |
658 | + |
659 | + def test_split_channel_name_invalid(self): |
660 | + self.assertRaises(ValueError, split_channel_name, "track/edge/invalid") |
661 | + |
662 | + def test_channels_validator_valid(self): |
663 | + self.assertTrue(channels_validator(['1.1/beta', '1.1/edge'])) |
664 | + self.assertTrue(channels_validator(['beta', 'edge'])) |
665 | + |
666 | + def test_channels_validator_multiple_tracks(self): |
667 | + self.assertRaises( |
668 | + LaunchpadValidationError, channels_validator, |
669 | + ['1.1/stable', '2.1/edge']) |
670 | + |
671 | + def test_channels_validator_invalid_channel(self): |
672 | + self.assertRaises( |
673 | + LaunchpadValidationError, channels_validator, |
674 | + ['1.1/stable/invalid']) |
Overall very good, thanks! I think this needs a few tweaks especially around making sure that invalid values can't be set over the API, but it's pretty close.