Merge ~twom/launchpad:git-branch-picker-unpicked into launchpad:master
- Git
- lp:~twom/launchpad
- git-branch-picker-unpicked
- Merge into master
Proposed by
Tom Wardill
Status: | Merged |
---|---|
Approved by: | Tom Wardill |
Approved revision: | b8f49cfde988017d2bb71c6bb52485d39072df21 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~twom/launchpad:git-branch-picker-unpicked |
Merge into: | launchpad:master |
Diff against target: |
1041 lines (+653/-78) 18 files modified
lib/lp/app/javascript/autocomplete.js (+102/-0) lib/lp/app/javascript/picker/picker.js (+4/-1) lib/lp/app/javascript/picker/picker_patcher.js (+23/-11) lib/lp/app/widgets/templates/form-picker-macros.pt (+1/-3) lib/lp/code/adapters/gitrepository.py (+1/-1) lib/lp/code/browser/widgets/configure.zcml (+9/-0) lib/lp/code/browser/widgets/gitref.py (+47/-27) lib/lp/code/browser/widgets/templates/gitref-picker.pt (+37/-0) lib/lp/code/browser/widgets/templates/gitref.pt (+3/-18) lib/lp/code/browser/widgets/tests/test_gitrefwidget.py (+1/-1) lib/lp/code/interfaces/gitrepository.py (+8/-0) lib/lp/code/model/gitref.py (+1/-0) lib/lp/code/vocabularies/configure.zcml (+24/-0) lib/lp/code/vocabularies/gitref.py (+175/-0) lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py (+189/-0) lib/lp/oci/browser/tests/test_ocirecipe.py (+8/-8) lib/lp/snappy/browser/tests/test_snap.py (+8/-6) lib/lp/snappy/model/snap.py (+12/-2) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+393928@code.launchpad.net |
Commit message
Add an autocompleting branch picker box to edit snap and oci recipes.
Description of the change
Add vocabularies and widgets based on them.
Add autocomplete JS.
To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) : | # |
- 4b87be0... by Tom Wardill
-
Remove unnecessary adapter
- 61f9bae... by Tom Wardill
-
Swap negative variable name
- f5b8d15... by Tom Wardill
-
Format imports
- 54b71ff... by Tom Wardill
-
Disallow head in pattern searches
- 2133a71... by Tom Wardill
-
Add tiebreak to order
- 4273b74... by Tom Wardill
-
Better display names
- bc82a23... by Tom Wardill
-
Handle remote refs
- 46de666... by Tom Wardill
-
Just close the picker
- 196f150... by Tom Wardill
-
Use an interface for the vocabulary
Revision history for this message
Tom Wardill (twom) : | # |
Revision history for this message
Colin Watson (cjwatson) wrote : | # |
I'd still like to see screenshots (let me know if you posted them somewhere and I missed it), but this otherwise mostly looks fine.
Revision history for this message
Tom Wardill (twom) wrote : | # |
- e97d357... by Tom Wardill
-
Better url detection
- 9ae4173... by Tom Wardill
-
Close the picker on git@ too
- 6743238... by Tom Wardill
-
Remove comment
- 576c5ea... by Tom Wardill
-
Remove blocking of HEAD, fix tie break in ordering
- 693a089... by Tom Wardill
-
Add labels
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
- b8f49cf... by Tom Wardill
-
Labels with colons
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/app/javascript/autocomplete.js b/lib/lp/app/javascript/autocomplete.js |
2 | new file mode 100644 |
3 | index 0000000..44b7c00 |
4 | --- /dev/null |
5 | +++ b/lib/lp/app/javascript/autocomplete.js |
6 | @@ -0,0 +1,102 @@ |
7 | +/* Copyright 2017 Canonical Ltd. This software is licensed under the |
8 | + * GNU Affero General Public License version 3 (see the file LICENSE). |
9 | + */ |
10 | + |
11 | +YUI.add('lp.app.autocomplete', function (Y) { |
12 | + |
13 | + var namespace = Y.namespace('lp.app.autocomplete'); |
14 | + |
15 | + namespace.getRepositoryCompletionURI = function (repo_node) { |
16 | + var entered_uri = repo_node.get('value'); |
17 | + if (entered_uri.startsWith("lp:")) { |
18 | + var split = "+code/" + entered_uri.split("lp:")[1] |
19 | + entered_uri = encodeURI(split); |
20 | + } |
21 | + else if (entered_uri.startsWith("~")) { |
22 | + entered_uri = encodeURI(entered_uri); |
23 | + } |
24 | + else if (entered_uri.includes("://")) { |
25 | + return null; |
26 | + } |
27 | + else if (entered_uri.startsWith("git@")) { |
28 | + return null; |
29 | + } |
30 | + else { |
31 | + entered_uri = encodeURI("+code/" + entered_uri); |
32 | + } |
33 | + |
34 | + var uri = '/'; |
35 | + uri += entered_uri; |
36 | + uri += '/@@+huge-vocabulary'; |
37 | + return uri; |
38 | + } |
39 | + |
40 | + namespace.getRepoNode = function (path_node) { |
41 | + var split = path_node._node['id'].split('.'); |
42 | + split[2] = 'repository'; |
43 | + var repository_target = split.join('.'); |
44 | + var target_repo = Y.one('[id="' + repository_target + '"]'); |
45 | + return target_repo; |
46 | + } |
47 | + |
48 | + namespace.getPathNode = function (path_node) { |
49 | + var split = path_node._node['id'].split('.'); |
50 | + split[2] = 'path'; |
51 | + var path_target = split.join('.'); |
52 | + var target_path = Y.one('[id="' + path_target + '"]'); |
53 | + return target_path; |
54 | + } |
55 | + |
56 | + namespace.setupVocabAutocomplete = function (config, node) { |
57 | + var qs = 'name=' + encodeURIComponent(config.vocabulary_name); |
58 | + |
59 | + /* autocomplete will substitute these with appropriate encoding. */ |
60 | + /* XXX cjwatson 2017-07-24: Perhaps we should pass batch={maxResults} |
61 | + * too, but we need to make sure that it doesn't exceed max_batch_size. |
62 | + */ |
63 | + qs += '&search_text={query}'; |
64 | + |
65 | + var repo_node = namespace.getRepoNode(node); |
66 | + var uri = namespace.getRepositoryCompletionURI(repo_node); |
67 | + |
68 | + node.plug(Y.Plugin.AutoComplete, { |
69 | + queryDelay: 500, // milliseconds |
70 | + requestTemplate: '?' + qs, |
71 | + resultHighlighter: 'wordMatch', |
72 | + resultListLocator: 'entries', |
73 | + resultTextLocator: 'value', |
74 | + source: uri |
75 | + }); |
76 | + |
77 | + repo_node.updated = function () { |
78 | + var uri = namespace.getRepositoryCompletionURI(this); |
79 | + path_node = namespace.getPathNode(this); |
80 | + path_node.ac.set("source", uri); |
81 | + } |
82 | + // ideally this should take node to rebind `this` in the function |
83 | + // but we're also calling it from the popup picker, which has a direct |
84 | + // reference to the repo_node, so maintain the local `this` binding. |
85 | + repo_node.on('valuechange', repo_node.updated); |
86 | + }; |
87 | + |
88 | + /** |
89 | + * Add autocompletion to a text field. |
90 | + * @param {Object} config Object literal of config name/value pairs. |
91 | + * config.vocabulary_name: the named vocabulary to select from. |
92 | + * config.input_element: the id of the text field to update with the |
93 | + * selected value. |
94 | + */ |
95 | + namespace.addAutocomplete = function (config) { |
96 | + var input_element = Y.one('[id="' + config.input_element + '"]'); |
97 | + // The node may already have been processed. |
98 | + if (input_element.ac) { |
99 | + return; |
100 | + } |
101 | + namespace.setupVocabAutocomplete(config, input_element); |
102 | + }; |
103 | + |
104 | +}, '0.1', { |
105 | + 'requires': [ |
106 | + 'autocomplete', 'autocomplete-sources', 'datasource', 'lp' |
107 | + ] |
108 | +}); |
109 | diff --git a/lib/lp/app/javascript/picker/picker.js b/lib/lp/app/javascript/picker/picker.js |
110 | index e97b517..ad94344 100644 |
111 | --- a/lib/lp/app/javascript/picker/picker.js |
112 | +++ b/lib/lp/app/javascript/picker/picker.js |
113 | @@ -24,7 +24,7 @@ ns.Picker = Y.Base.create('picker', Y.lp.ui.PrettyOverlay, [], { |
114 | /** |
115 | * The search input node. |
116 | * |
117 | - * @property _search_button |
118 | + * @property _search_input |
119 | * @type Node |
120 | * @private |
121 | */ |
122 | @@ -1301,6 +1301,9 @@ Y.extend(TextFieldPickerPlugin, Y.Plugin.Base, { |
123 | // then the I-beam disappears. |
124 | input.blur(); |
125 | input.focus(); |
126 | + if (typeof(input.updated) === "function") { |
127 | + input.updated(); |
128 | + } |
129 | }); |
130 | this.doAfter('show', function() { |
131 | var selected_value = null; |
132 | diff --git a/lib/lp/app/javascript/picker/picker_patcher.js b/lib/lp/app/javascript/picker/picker_patcher.js |
133 | index e220aa8..895b5b7 100644 |
134 | --- a/lib/lp/app/javascript/picker/picker_patcher.js |
135 | +++ b/lib/lp/app/javascript/picker/picker_patcher.js |
136 | @@ -50,7 +50,7 @@ var _addPicker = function(config, show_widget_id) { |
137 | * @param {Object} config Object literal of config name/value pairs. The |
138 | * values listed below are common for all picker types. |
139 | * config.vocabulary_name: the named vocabulary to select from. |
140 | - * config.vocabulary_filters: any vocaulary filters to use. |
141 | + * config.vocabulary_filters: any vocabulary filters to use. |
142 | * config.input_element: the id of the text field to update with the |
143 | * selected value. |
144 | * config.picker_type: the type of picker to create (default or person). |
145 | @@ -619,23 +619,35 @@ namespace.setup_vocab_picker = function (picker, vocabulary, config) { |
146 | // use the context to limit the results to the same project. |
147 | |
148 | var uri = ''; |
149 | + var context_ok = true; |
150 | if (Y.Lang.isFunction(config.getContextPath)) { |
151 | - uri += config.getContextPath() + '/'; |
152 | + var context_path = config.getContextPath(); |
153 | + if (context_path !== null) { |
154 | + uri += config.getContextPath() + '/'; |
155 | + } else { |
156 | + // No context, so proceed straight to validation. |
157 | + picker.set('error', e); |
158 | + picker.set('search_mode', false); |
159 | + picker.fire('validate', search_text); |
160 | + context_ok = false; |
161 | + } |
162 | } else if (Y.Lang.isValue(config.context)) { |
163 | uri += Y.lp.get_url_path( |
164 | config.context.get('web_link')) + '/'; |
165 | } |
166 | uri += '@@+huge-vocabulary?' + qs; |
167 | |
168 | - var yio = (config.yio !== undefined) ? config.yio : Y; |
169 | - yio.io(uri, { |
170 | - headers: {'Accept': 'application/json'}, |
171 | - timeout: 20000, |
172 | - on: { |
173 | - success: success_handler, |
174 | - failure: failure_handler |
175 | - } |
176 | - }); |
177 | + if (context_ok) { |
178 | + var yio = (config.yio !== undefined) ? config.yio : Y; |
179 | + yio.io(uri, { |
180 | + headers: {'Accept': 'application/json'}, |
181 | + timeout: 20000, |
182 | + on: { |
183 | + success: success_handler, |
184 | + failure: failure_handler |
185 | + } |
186 | + }); |
187 | + } |
188 | // Or we can pass in a vocabulary directly. |
189 | } else { |
190 | display_vocabulary(vocabulary, Y.Object.size(vocabulary), 1); |
191 | diff --git a/lib/lp/app/widgets/templates/form-picker-macros.pt b/lib/lp/app/widgets/templates/form-picker-macros.pt |
192 | index 80f5be1..9710e52 100644 |
193 | --- a/lib/lp/app/widgets/templates/form-picker-macros.pt |
194 | +++ b/lib/lp/app/widgets/templates/form-picker-macros.pt |
195 | @@ -33,9 +33,7 @@ |
196 | LPJS.use('node', 'lp.app.picker', function(Y) { |
197 | var config = ${view/json_config}; |
198 | var show_widget_id = '${view/show_widget_id}'; |
199 | - Y.on('domready', function(e) { |
200 | - Y.lp.app.picker.addPicker(config, show_widget_id); |
201 | - }); |
202 | + Y.lp.app.picker.addPicker(config, show_widget_id); |
203 | }); |
204 | "/> |
205 | </metal:form-picker> |
206 | diff --git a/lib/lp/code/adapters/gitrepository.py b/lib/lp/code/adapters/gitrepository.py |
207 | index e55d5bb..d265ca9 100644 |
208 | --- a/lib/lp/code/adapters/gitrepository.py |
209 | +++ b/lib/lp/code/adapters/gitrepository.py |
210 | @@ -1,7 +1,7 @@ |
211 | # Copyright 2015-2018 Canonical Ltd. This software is licensed under the |
212 | # GNU Affero General Public License version 3 (see the file LICENSE). |
213 | |
214 | -"""Components related to Git repositories.""" |
215 | +"""Components and adapters related to Git repositories.""" |
216 | |
217 | __metaclass__ = type |
218 | __all__ = [ |
219 | diff --git a/lib/lp/code/browser/widgets/configure.zcml b/lib/lp/code/browser/widgets/configure.zcml |
220 | index 913d8c6..e056e42 100644 |
221 | --- a/lib/lp/code/browser/widgets/configure.zcml |
222 | +++ b/lib/lp/code/browser/widgets/configure.zcml |
223 | @@ -16,4 +16,13 @@ |
224 | permission="zope.Public" |
225 | /> |
226 | |
227 | + <view |
228 | + type="zope.publisher.interfaces.browser.IBrowserRequest" |
229 | + for="zope.schema.interfaces.IChoice |
230 | + lp.code.vocabularies.gitref.GitRefVocabulary" |
231 | + provides="zope.formlib.interfaces.IInputWidget" |
232 | + factory="lp.code.browser.widgets.gitref.GitRefPickerWidget" |
233 | + permission="zope.Public" |
234 | + /> |
235 | + |
236 | </configure> |
237 | diff --git a/lib/lp/code/browser/widgets/gitref.py b/lib/lp/code/browser/widgets/gitref.py |
238 | index cfe8e40..b4acd01 100644 |
239 | --- a/lib/lp/code/browser/widgets/gitref.py |
240 | +++ b/lib/lp/code/browser/widgets/gitref.py |
241 | @@ -9,7 +9,6 @@ __all__ = [ |
242 | |
243 | import six |
244 | from zope.browserpage import ViewPageTemplateFile |
245 | -from zope.component import getUtility |
246 | from zope.formlib.interfaces import ( |
247 | ConversionError, |
248 | IInputWidget, |
249 | @@ -23,16 +22,12 @@ from zope.formlib.widget import ( |
250 | InputWidget, |
251 | ) |
252 | from zope.interface import implementer |
253 | -from zope.schema import ( |
254 | - Choice, |
255 | - TextLine, |
256 | - ) |
257 | +from zope.schema import Choice |
258 | from zope.schema.interfaces import IChoice |
259 | |
260 | from lp.app.errors import UnexpectedFormData |
261 | from lp.app.validators import LaunchpadValidationError |
262 | from lp.app.widgets.popup import VocabularyPickerWidget |
263 | -from lp.code.interfaces.gitref import IGitRefRemoteSet |
264 | from lp.code.interfaces.gitrepository import IGitRepository |
265 | from lp.services.fields import URIField |
266 | from lp.services.webapp.interfaces import ( |
267 | @@ -104,28 +99,34 @@ class GitRepositoryPickerWidget(VocabularyPickerWidget): |
268 | class GitRefWidget(BrowserWidget, InputWidget): |
269 | |
270 | template = ViewPageTemplateFile("templates/gitref.pt") |
271 | - display_label = False |
272 | _widgets_set_up = False |
273 | |
274 | # If True, allow entering external repository URLs. |
275 | allow_external = False |
276 | |
277 | + # If True, only allow reference paths to be branches (refs/heads/*). |
278 | + require_branch = False |
279 | + |
280 | def setUpSubWidgets(self): |
281 | if self._widgets_set_up: |
282 | return |
283 | + path_vocabulary = "GitBranch" if self.require_branch else "GitRef" |
284 | fields = [ |
285 | GitRepositoryField( |
286 | - __name__="repository", title=u"Git repository", |
287 | - required=False, vocabulary="GitRepository", |
288 | + __name__="repository", title=u"Repository", |
289 | + required=self.context.required, vocabulary="GitRepository", |
290 | allow_external=self.allow_external), |
291 | - TextLine(__name__="path", title=u"Git branch", required=False), |
292 | + Choice( |
293 | + __name__="path", title=u"Branch", |
294 | + required=self.context.required, |
295 | + vocabulary=path_vocabulary), |
296 | ] |
297 | for field in fields: |
298 | setUpWidget( |
299 | self, field.__name__, field, IInputWidget, prefix=self.name) |
300 | self._widgets_set_up = True |
301 | |
302 | - def setRenderedValue(self, value): |
303 | + def setRenderedValue(self, value, with_path=True): |
304 | """See `IWidget`.""" |
305 | self.setUpSubWidgets() |
306 | if value is not None: |
307 | @@ -133,7 +134,13 @@ class GitRefWidget(BrowserWidget, InputWidget): |
308 | self.repository_widget.setRenderedValue(value.repository_url) |
309 | else: |
310 | self.repository_widget.setRenderedValue(value.repository) |
311 | - self.path_widget.setRenderedValue(value.path) |
312 | + # if we're only talking about branches, we can deal in the |
313 | + # name, rather than the full ref/heads/* path |
314 | + if with_path: |
315 | + if self.require_branch: |
316 | + self.path_widget.setRenderedValue(value.name) |
317 | + else: |
318 | + self.path_widget.setRenderedValue(value.path) |
319 | else: |
320 | self.repository_widget.setRenderedValue(None) |
321 | self.path_widget.setRenderedValue(None) |
322 | @@ -174,27 +181,31 @@ class GitRefWidget(BrowserWidget, InputWidget): |
323 | "There is no Git repository named '%s' registered in " |
324 | "Launchpad." % entered_name)) |
325 | if self.path_widget.hasInput(): |
326 | - path = self.path_widget.getInputValue() |
327 | - else: |
328 | - path = None |
329 | - if not path: |
330 | - if self.context.required: |
331 | + # We've potentially just tried to change the repository that is |
332 | + # involved, or changing from a bzr branch to a git repo, so there |
333 | + # is no existing repository set up. We need to set this so we |
334 | + # can compare the ref against the 'new' repo. |
335 | + if IGitRepository.providedBy(repository): |
336 | + self.path_widget.vocabulary.setRepository(repository) |
337 | + else: |
338 | + self.path_widget.vocabulary.setRepositoryURL(repository) |
339 | + try: |
340 | + ref = self.path_widget.getInputValue() |
341 | + except ConversionError: |
342 | raise WidgetInputError( |
343 | self.name, self.label, |
344 | LaunchpadValidationError( |
345 | - "Please enter a Git branch path.")) |
346 | - else: |
347 | - return |
348 | - if self.allow_external and not IGitRepository.providedBy(repository): |
349 | - ref = getUtility(IGitRefRemoteSet).new(repository, path) |
350 | + "The repository at %s does not contain a branch named " |
351 | + "'%s'." % ( |
352 | + repository.display_name, |
353 | + self.path_widget._getFormInput()))) |
354 | else: |
355 | - ref = repository.getRefByPath(path) |
356 | - if ref is None: |
357 | - raise WidgetInputError( |
358 | + ref = None |
359 | + if not ref and (repository or self.context.required): |
360 | + raise WidgetInputError( |
361 | self.name, self.label, |
362 | LaunchpadValidationError( |
363 | - "The repository at %s does not contain a branch named " |
364 | - "'%s'." % (repository.display_name, path))) |
365 | + "Please enter a Git branch path.")) |
366 | return ref |
367 | |
368 | def error(self): |
369 | @@ -210,3 +221,12 @@ class GitRefWidget(BrowserWidget, InputWidget): |
370 | """See `IBrowserWidget`.""" |
371 | self.setUpSubWidgets() |
372 | return self.template() |
373 | + |
374 | + |
375 | +class GitRefPickerWidget(VocabularyPickerWidget): |
376 | + |
377 | + __call__ = ViewPageTemplateFile("templates/gitref-picker.pt") |
378 | + |
379 | + @property |
380 | + def repository_id(self): |
381 | + return self._prefix + "repository" |
382 | diff --git a/lib/lp/code/browser/widgets/templates/gitref-picker.pt b/lib/lp/code/browser/widgets/templates/gitref-picker.pt |
383 | new file mode 100644 |
384 | index 0000000..fbc32dd |
385 | --- /dev/null |
386 | +++ b/lib/lp/code/browser/widgets/templates/gitref-picker.pt |
387 | @@ -0,0 +1,37 @@ |
388 | +<tal:root |
389 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
390 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
391 | + omit-tag=""> |
392 | + |
393 | +<metal:form-picker use-macro="context/@@form-picker-macros/form-picker"> |
394 | + <script metal:fill-slot="add-picker" tal:content="structure string: |
395 | + LPJS.use('node', 'lp.app.autocomplete', 'lp.app.picker', function(Y) { |
396 | + var config = ${view/json_config}; |
397 | + var repository_id = '${view/repository_id}'; |
398 | + if (repository_id !== '') { |
399 | + config.getContextPath = function() { |
400 | + var repository_value = Y.DOM.byId(repository_id).value; |
401 | + // XXX cjwatson 2017-06-24: We don't have a straightforward |
402 | + // URL parser available to us at the moment. This will do for |
403 | + // now, since we just want to tell the difference between |
404 | + // internal and external repositories. |
405 | + // XXX twom 2020-11-18 |
406 | + // This just closes the picker, not ideal. |
407 | + // but would need some refactoring of picker_patcher.js |
408 | + if (repository_value.indexOf('://') !== -1) { |
409 | + return null; |
410 | + } |
411 | + else if (repository_value.indexOf('git@') !== -1) { |
412 | + return null; |
413 | + } |
414 | + return '/' + repository_value; |
415 | + }; |
416 | + } |
417 | + var show_widget_id = '${view/show_widget_id}'; |
418 | + Y.lp.app.picker.addPicker(config, show_widget_id); |
419 | + Y.lp.app.autocomplete.addAutocomplete(config); |
420 | + }); |
421 | + "/> |
422 | +</metal:form-picker> |
423 | + |
424 | +</tal:root> |
425 | diff --git a/lib/lp/code/browser/widgets/templates/gitref.pt b/lib/lp/code/browser/widgets/templates/gitref.pt |
426 | index f88a652..f49dfb7 100644 |
427 | --- a/lib/lp/code/browser/widgets/templates/gitref.pt |
428 | +++ b/lib/lp/code/browser/widgets/templates/gitref.pt |
429 | @@ -1,18 +1,3 @@ |
430 | -<table> |
431 | - <tr> |
432 | - <td> |
433 | - <tal:widget define="widget nocall:view/repository_widget"> |
434 | - <metal:block |
435 | - use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" /> |
436 | - </tal:widget> |
437 | - </td> |
438 | - </tr> |
439 | - <tr> |
440 | - <td> |
441 | - <tal:widget define="widget nocall:view/path_widget"> |
442 | - <metal:block |
443 | - use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" /> |
444 | - </tal:widget> |
445 | - </td> |
446 | - </tr> |
447 | -</table> |
448 | +<label tal:attributes="for view/repository_widget/name" tal:content="string:${view/repository_widget/label}:">Label</label><span title="Repository" tal:content="structure view/repository_widget" /> |
449 | + |
450 | +<label tal:attributes="for view/path_widget/name" tal:content="string:${view/path_widget/label}:">Label</label><span title="Branch" tal:content="structure view/path_widget" /> |
451 | diff --git a/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py b/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py |
452 | index b411651..2e2526c 100644 |
453 | --- a/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py |
454 | +++ b/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py |
455 | @@ -194,7 +194,7 @@ class TestGitRefWidget(WithScenarios, TestCaseWithFactory): |
456 | [ref] = self.factory.makeGitRefs() |
457 | form = { |
458 | "field.git_ref.repository": ref.repository.unique_name, |
459 | - "field.git_ref.path": "non-existent", |
460 | + "field.git_ref.path": u"non-existent", |
461 | } |
462 | self.assertGetInputValueError( |
463 | form, |
464 | diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py |
465 | index 192133a..ed17183 100644 |
466 | --- a/lib/lp/code/interfaces/gitrepository.py |
467 | +++ b/lib/lp/code/interfaces/gitrepository.py |
468 | @@ -13,6 +13,7 @@ __all__ = [ |
469 | 'IGitRepository', |
470 | 'IGitRepositoryDelta', |
471 | 'IGitRepositorySet', |
472 | + 'IHasGitRepositoryURL', |
473 | 'user_has_special_git_repository_access', |
474 | ] |
475 | |
476 | @@ -1227,6 +1228,13 @@ class GitIdentityMixin: |
477 | return identities |
478 | |
479 | |
480 | +class IHasGitRepositoryURL(Interface): |
481 | + """Marker interface for objects that have a Git repository URL.""" |
482 | + |
483 | + git_repository_url = Attribute( |
484 | + "The Git repository URL (possibly external)") |
485 | + |
486 | + |
487 | def user_has_special_git_repository_access(user, repository=None): |
488 | """Admins have special access. |
489 | |
490 | diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py |
491 | index f7dc142..406c8de 100644 |
492 | --- a/lib/lp/code/model/gitref.py |
493 | +++ b/lib/lp/code/model/gitref.py |
494 | @@ -933,6 +933,7 @@ class GitRefRemote(GitRefMixin): |
495 | |
496 | def __eq__(self, other): |
497 | return ( |
498 | + other is not None and |
499 | self.repository_url == other.repository_url and |
500 | self.path == other.path) |
501 | |
502 | diff --git a/lib/lp/code/vocabularies/configure.zcml b/lib/lp/code/vocabularies/configure.zcml |
503 | index 5782992..ef0933d 100644 |
504 | --- a/lib/lp/code/vocabularies/configure.zcml |
505 | +++ b/lib/lp/code/vocabularies/configure.zcml |
506 | @@ -88,4 +88,28 @@ |
507 | <allow interface="zope.schema.interfaces.IVocabularyTokenized"/> |
508 | </class> |
509 | |
510 | + <securedutility |
511 | + name="GitRef" |
512 | + component=".gitref.GitRefVocabulary" |
513 | + provides="zope.schema.interfaces.IVocabularyFactory"> |
514 | + <allow interface="zope.schema.interfaces.IVocabularyFactory"/> |
515 | + </securedutility> |
516 | + |
517 | + <class class=".gitref.GitRefVocabulary"> |
518 | + <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/> |
519 | + <allow interface=".gitref.IRepositoryManagerGitRefVocabulary"/> |
520 | + </class> |
521 | + |
522 | + <securedutility |
523 | + name="GitBranch" |
524 | + component=".gitref.GitBranchVocabulary" |
525 | + provides="zope.schema.interfaces.IVocabularyFactory"> |
526 | + <allow interface="zope.schema.interfaces.IVocabularyFactory"/> |
527 | + </securedutility> |
528 | + |
529 | + <class class=".gitref.GitBranchVocabulary"> |
530 | + <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/> |
531 | + <allow interface=".gitref.IRepositoryManagerGitRefVocabulary"/> |
532 | + </class> |
533 | + |
534 | </configure> |
535 | diff --git a/lib/lp/code/vocabularies/gitref.py b/lib/lp/code/vocabularies/gitref.py |
536 | new file mode 100644 |
537 | index 0000000..7d98208 |
538 | --- /dev/null |
539 | +++ b/lib/lp/code/vocabularies/gitref.py |
540 | @@ -0,0 +1,175 @@ |
541 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
542 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
543 | + |
544 | +"""Vocabularies that contain Git references.""" |
545 | + |
546 | +from __future__ import absolute_import, print_function, unicode_literals |
547 | + |
548 | + |
549 | +__metaclass__ = type |
550 | +__all__ = [ |
551 | + "GitBranchVocabulary", |
552 | + "GitRefVocabulary", |
553 | + ] |
554 | + |
555 | +from lazr.restful.interfaces import IReference |
556 | +from storm.databases.postgres import Case |
557 | +from storm.expr import ( |
558 | + Desc, |
559 | + Like, |
560 | + like_escape, |
561 | + ) |
562 | +from zope.component import getUtility |
563 | +from zope.interface import ( |
564 | + implementer, |
565 | + Interface, |
566 | + ) |
567 | +from zope.schema.vocabulary import SimpleTerm |
568 | +from zope.security.proxy import isinstance as zope_isinstance |
569 | + |
570 | +from lp.code.interfaces.gitref import IGitRefRemoteSet |
571 | +from lp.code.interfaces.gitrepository import ( |
572 | + IGitRepository, |
573 | + IHasGitRepositoryURL, |
574 | + ) |
575 | +from lp.code.model.gitref import ( |
576 | + GitRef, |
577 | + GitRefRemote, |
578 | + ) |
579 | +from lp.services.database.interfaces import IStore |
580 | +from lp.services.webapp.vocabulary import ( |
581 | + CountableIterator, |
582 | + IHugeVocabulary, |
583 | + StormVocabularyBase, |
584 | + ) |
585 | + |
586 | + |
587 | +class IRepositoryManagerGitRefVocabulary(Interface): |
588 | + |
589 | + def setRepository(self, repository): |
590 | + """Set the repository after the vocabulary was instantiated.""" |
591 | + |
592 | + def setRepositoryURL(self, repository_url): |
593 | + """Set the repository URL after the vocabulary was instantiated.""" |
594 | + |
595 | + |
596 | +@implementer(IHugeVocabulary) |
597 | +@implementer(IRepositoryManagerGitRefVocabulary) |
598 | +class GitRefVocabulary(StormVocabularyBase): |
599 | + """A vocabulary for references in a given Git repository.""" |
600 | + |
601 | + _table = GitRef |
602 | + displayname = "Select a branch or tag" |
603 | + step_title = "Search" |
604 | + |
605 | + def __init__(self, context): |
606 | + super(GitRefVocabulary, self).__init__(context=context) |
607 | + if IReference.providedBy(context): |
608 | + context = context.context |
609 | + try: |
610 | + self.repository = IGitRepository(context) |
611 | + except TypeError: |
612 | + self.repository = None |
613 | + try: |
614 | + self.repository_url = ( |
615 | + IHasGitRepositoryURL(context).git_repository_url) |
616 | + except TypeError: |
617 | + self.repository_url = None |
618 | + |
619 | + def setRepository(self, repository): |
620 | + """See `IRepositoryManagerGitRefVocabulary`.""" |
621 | + self.repository = repository |
622 | + self.repository_url = None |
623 | + |
624 | + def setRepositoryURL(self, repository_url): |
625 | + """See `IRepositoryManagerGitRefVocabulary`.""" |
626 | + self.repository = None |
627 | + self.repository_url = repository_url |
628 | + |
629 | + def _assertHasRepository(self): |
630 | + if self.repository is None and self.repository_url is None: |
631 | + raise AssertionError( |
632 | + "GitRefVocabulary cannot be used without setting a " |
633 | + "repository or a repository URL.") |
634 | + |
635 | + @property |
636 | + def _order_by(self): |
637 | + rank = Case( |
638 | + cases=[(self._table.path == self.repository.default_branch, 2)], |
639 | + default=1) |
640 | + return [ |
641 | + Desc(rank), |
642 | + Desc(self._table.committer_date), |
643 | + self._table.path] |
644 | + |
645 | + def toTerm(self, ref): |
646 | + """See `StormVocabularyBase`.""" |
647 | + return SimpleTerm(ref, ref.path, ref.name) |
648 | + |
649 | + def getTermByToken(self, token): |
650 | + """See `IVocabularyTokenized`.""" |
651 | + self._assertHasRepository() |
652 | + if self.repository is not None: |
653 | + ref = self.repository.getRefByPath(token) |
654 | + if ref is None: |
655 | + raise LookupError(token) |
656 | + else: |
657 | + ref = getUtility(IGitRefRemoteSet).new(self.repository_url, token) |
658 | + return self.toTerm(ref) |
659 | + |
660 | + def _makePattern(self, query=None): |
661 | + parts = ["%"] |
662 | + if query is not None: |
663 | + parts.extend([query.lower().translate(like_escape), "%"]) |
664 | + return "".join(parts) |
665 | + |
666 | + def searchForTerms(self, query=None, vocab_filter=None): |
667 | + """See `IHugeVocabulary.""" |
668 | + self._assertHasRepository() |
669 | + if self.repository is not None: |
670 | + pattern = self._makePattern(query=query) |
671 | + results = IStore(self._table).find( |
672 | + self._table, |
673 | + self._table.repository_id == self.repository.id, |
674 | + Like(self._table.path, pattern, "!")).order_by(self._order_by) |
675 | + else: |
676 | + results = self.emptySelectResults() |
677 | + return CountableIterator(results.count(), results, self.toTerm) |
678 | + |
679 | + def getTerm(self, value): |
680 | + # remote refs aren't database backed |
681 | + if zope_isinstance(value, GitRefRemote): |
682 | + return self.toTerm(value) |
683 | + return super(GitRefVocabulary, self).getTerm(value) |
684 | + |
685 | + def __len__(self): |
686 | + """See `IVocabulary`.""" |
687 | + return self.searchForTerms().count() |
688 | + |
689 | + def __contains__(self, obj): |
690 | + # We know nothing about GitRefRemote, so we just have to assume |
691 | + # that they exist in the remote repository |
692 | + if zope_isinstance(obj, GitRefRemote): |
693 | + return True |
694 | + if obj in self.repository.refs: |
695 | + return True |
696 | + return False |
697 | + |
698 | + |
699 | +class GitBranchVocabulary(GitRefVocabulary): |
700 | + """A vocabulary for branches in a given Git repository.""" |
701 | + |
702 | + displayname = "Select a branch" |
703 | + |
704 | + def _makePattern(self, query=None): |
705 | + parts = [] |
706 | + if query is None or not query.startswith("refs/heads/"): |
707 | + parts.append("refs/heads/".translate(like_escape)) |
708 | + parts.append("%") |
709 | + if query is not None: |
710 | + parts.extend([query.lower().translate(like_escape), "%"]) |
711 | + return "".join(parts) |
712 | + |
713 | + def toTerm(self, ref): |
714 | + """See `StormVocabularyBase`.""" |
715 | + return SimpleTerm(ref, ref.name, ref.name) |
716 | diff --git a/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py b/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py |
717 | new file mode 100644 |
718 | index 0000000..41f5f83 |
719 | --- /dev/null |
720 | +++ b/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py |
721 | @@ -0,0 +1,189 @@ |
722 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
723 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
724 | + |
725 | +"""Test the Git reference vocabularies.""" |
726 | + |
727 | +from __future__ import absolute_import, print_function, unicode_literals |
728 | + |
729 | +__metaclass__ = type |
730 | + |
731 | +from datetime import ( |
732 | + datetime, |
733 | + timedelta, |
734 | + ) |
735 | + |
736 | +import pytz |
737 | +from testtools.matchers import MatchesStructure |
738 | +from zope.schema.vocabulary import SimpleTerm |
739 | +from zope.security.proxy import removeSecurityProxy |
740 | + |
741 | +from lp.code.vocabularies.gitref import ( |
742 | + GitBranchVocabulary, |
743 | + GitRefVocabulary, |
744 | + ) |
745 | +from lp.services.webapp.vocabulary import IHugeVocabulary |
746 | +from lp.testing import TestCaseWithFactory |
747 | +from lp.testing.layers import ZopelessDatabaseLayer |
748 | + |
749 | + |
750 | +class TestGitRefVocabulary(TestCaseWithFactory): |
751 | + |
752 | + layer = ZopelessDatabaseLayer |
753 | + |
754 | + vocabulary_class = GitRefVocabulary |
755 | + |
756 | + def test_getTermByToken(self): |
757 | + [ref] = self.factory.makeGitRefs() |
758 | + vocab = self.vocabulary_class(ref.repository) |
759 | + term = SimpleTerm(ref, ref.path, ref.name) |
760 | + self.assertEqual(term.token, vocab.getTermByToken(ref.name).token) |
761 | + self.assertEqual(term.token, vocab.getTermByToken(ref.path).token) |
762 | + self.assertRaises(LookupError, vocab.getTermByToken, "nonexistent") |
763 | + |
764 | + def test_provides_IHugeVocabulary(self): |
765 | + vocab = self.vocabulary_class(self.factory.makeGitRepository()) |
766 | + self.assertProvides(vocab, IHugeVocabulary) |
767 | + |
768 | + def test_init_no_repository(self): |
769 | + # The repository is None if the context cannot be adapted to a |
770 | + # repository. |
771 | + vocab = self.vocabulary_class( |
772 | + self.factory.makeSnap(branch=self.factory.makeAnyBranch())) |
773 | + self.assertIsNone(vocab.repository) |
774 | + |
775 | + def test_setRepository(self): |
776 | + # Callers can set the repository after instantiation. |
777 | + vocab = self.vocabulary_class( |
778 | + self.factory.makeSnap(branch=self.factory.makeAnyBranch())) |
779 | + repository = self.factory.makeGitRepository() |
780 | + vocab.setRepository(repository) |
781 | + self.assertEqual(repository, vocab.repository) |
782 | + |
783 | + def test_toTerm(self): |
784 | + [ref] = self.factory.makeGitRefs() |
785 | + self.assertThat( |
786 | + self.vocabulary_class(ref.repository).toTerm(ref), |
787 | + MatchesStructure.byEquality( |
788 | + value=ref, token=ref.path, title=ref.name)) |
789 | + |
790 | + def test_searchForTerms(self): |
791 | + ref_master, ref_next, ref_next_squared, _ = ( |
792 | + self.factory.makeGitRefs(paths=[ |
793 | + "refs/heads/master", "refs/heads/next", |
794 | + "refs/heads/next-squared", "refs/tags/next-1.0"])) |
795 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
796 | + ref_master.path) |
797 | + vocab = self.vocabulary_class(ref_master.repository) |
798 | + self.assertContentEqual( |
799 | + [term.value.path for term in vocab.searchForTerms("master")], |
800 | + ["refs/heads/master"]) |
801 | + self.assertContentEqual( |
802 | + [term.value.path for term in vocab.searchForTerms("next")], |
803 | + ["refs/heads/next", "refs/heads/next-squared", |
804 | + "refs/tags/next-1.0"]) |
805 | + self.assertContentEqual( |
806 | + [term.value.path for term in vocab.searchForTerms( |
807 | + "refs/heads/next")], |
808 | + ["refs/heads/next", "refs/heads/next-squared"]) |
809 | + self.assertContentEqual( |
810 | + [term.value.path for term in vocab.searchForTerms("")], |
811 | + ["refs/heads/master", "refs/heads/next", |
812 | + "refs/heads/next-squared", "refs/tags/next-1.0"]) |
813 | + self.assertContentEqual( |
814 | + [term.token for term in vocab.searchForTerms("nonexistent")], []) |
815 | + |
816 | + def test_searchForTerms_ordering(self): |
817 | + # The default branch (if it matches) is shown first, followed by |
818 | + # other matches in decreasing order of last commit date. |
819 | + ref_master, ref_master_old, ref_master_older = ( |
820 | + self.factory.makeGitRefs(paths=[ |
821 | + "refs/heads/master", "refs/heads/master-old", |
822 | + "refs/heads/master-older"])) |
823 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
824 | + ref_master.path) |
825 | + now = datetime.now(pytz.UTC) |
826 | + removeSecurityProxy(ref_master_old).committer_date = ( |
827 | + now - timedelta(days=1)) |
828 | + removeSecurityProxy(ref_master_older).committer_date = ( |
829 | + now - timedelta(days=2)) |
830 | + vocab = self.vocabulary_class(ref_master.repository) |
831 | + self.assertEqual( |
832 | + [term.value.path for term in vocab.searchForTerms("master")], |
833 | + ["refs/heads/master", "refs/heads/master-old", |
834 | + "refs/heads/master-older"]) |
835 | + |
836 | + def test_len(self): |
837 | + ref_master, _, _, _ = self.factory.makeGitRefs(paths=[ |
838 | + "refs/heads/master", "refs/heads/next", |
839 | + "refs/heads/next-squared", "refs/tags/next-1.0"]) |
840 | + self.assertEqual(4, len(self.vocabulary_class(ref_master.repository))) |
841 | + |
842 | + |
843 | +class TestGitBranchVocabulary(TestCaseWithFactory): |
844 | + |
845 | + layer = ZopelessDatabaseLayer |
846 | + |
847 | + vocabulary_class = GitBranchVocabulary |
848 | + |
849 | + def test_getTermByToken(self): |
850 | + [ref] = self.factory.makeGitRefs() |
851 | + vocab = self.vocabulary_class(ref.repository) |
852 | + term = SimpleTerm(ref, ref.name, ref.name) |
853 | + self.assertEqual(term.token, vocab.getTermByToken(ref.name).token) |
854 | + self.assertEqual(term.token, vocab.getTermByToken(ref.path).token) |
855 | + self.assertRaises(LookupError, vocab.getTermByToken, "nonexistent") |
856 | + |
857 | + def test_toTerm(self): |
858 | + [ref] = self.factory.makeGitRefs() |
859 | + self.assertThat( |
860 | + self.vocabulary_class(ref.repository).toTerm(ref), |
861 | + MatchesStructure.byEquality( |
862 | + value=ref, token=ref.name, title=ref.name)) |
863 | + |
864 | + def test_searchForTerms(self): |
865 | + ref_master, ref_next, ref_next_squared, _ = ( |
866 | + self.factory.makeGitRefs(paths=[ |
867 | + "refs/heads/master", "refs/heads/next", |
868 | + "refs/heads/next-squared", "refs/tags/next-1.0"])) |
869 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
870 | + ref_master.path) |
871 | + vocab = self.vocabulary_class(ref_master.repository) |
872 | + self.assertContentEqual( |
873 | + [term.title for term in vocab.searchForTerms("master")], |
874 | + ["master"]) |
875 | + self.assertContentEqual( |
876 | + [term.title for term in vocab.searchForTerms("next")], |
877 | + ["next", "next-squared"]) |
878 | + self.assertContentEqual( |
879 | + [term.title for term in vocab.searchForTerms("refs/heads/next")], |
880 | + ["next", "next-squared"]) |
881 | + self.assertContentEqual( |
882 | + [term.title for term in vocab.searchForTerms("")], |
883 | + ["master", "next", "next-squared"]) |
884 | + self.assertContentEqual( |
885 | + [term.token for term in vocab.searchForTerms("nonexistent")], []) |
886 | + |
887 | + def test_searchForTerms_ordering(self): |
888 | + # The default branch (if it matches) is shown first, followed by |
889 | + # other matches in decreasing order of last commit date. |
890 | + ref_master, ref_master_old, ref_master_older = ( |
891 | + self.factory.makeGitRefs(paths=[ |
892 | + "refs/heads/master", "refs/heads/master-old", |
893 | + "refs/heads/master-older"])) |
894 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
895 | + ref_master.path) |
896 | + now = datetime.now(pytz.UTC) |
897 | + removeSecurityProxy(ref_master_old).committer_date = ( |
898 | + now - timedelta(days=1)) |
899 | + removeSecurityProxy(ref_master_older).committer_date = ( |
900 | + now - timedelta(days=2)) |
901 | + vocab = self.vocabulary_class(ref_master.repository) |
902 | + self.assertEqual( |
903 | + [term.title for term in vocab.searchForTerms("master")], |
904 | + ["master", "master-old", "master-older"]) |
905 | + |
906 | + def test_len(self): |
907 | + ref_master, _, _, _ = self.factory.makeGitRefs(paths=[ |
908 | + "refs/heads/master", "refs/heads/next", |
909 | + "refs/heads/next-squared", "refs/tags/next-1.0"]) |
910 | + self.assertEqual(3, len(self.vocabulary_class(ref_master.repository))) |
911 | diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py |
912 | index 82422f1..038e602 100644 |
913 | --- a/lib/lp/oci/browser/tests/test_ocirecipe.py |
914 | +++ b/lib/lp/oci/browser/tests/test_ocirecipe.py |
915 | @@ -195,9 +195,9 @@ class TestOCIRecipeAddView(BaseTestOCIRecipeView): |
916 | oci_project, view_name="+new-recipe", user=self.person) |
917 | browser.getControl(name="field.name").value = "recipe-name" |
918 | browser.getControl("Description").value = "Recipe description" |
919 | - browser.getControl("Git repository").value = ( |
920 | + browser.getControl(name="field.git_ref.repository").value = ( |
921 | git_ref.repository.identity) |
922 | - browser.getControl("Git branch").value = git_ref.path |
923 | + browser.getControl(name="field.git_ref.path").value = git_ref.path |
924 | browser.getControl("Create OCI recipe").click() |
925 | |
926 | content = find_main_content(browser.contents) |
927 | @@ -230,9 +230,9 @@ class TestOCIRecipeAddView(BaseTestOCIRecipeView): |
928 | oci_project, view_name="+new-recipe", user=self.person) |
929 | browser.getControl(name="field.name").value = "recipe-name" |
930 | browser.getControl("Description").value = "Recipe description" |
931 | - browser.getControl("Git repository").value = ( |
932 | + browser.getControl(name="field.git_ref.repository").value = ( |
933 | git_ref.repository.identity) |
934 | - browser.getControl("Git branch").value = git_ref.path |
935 | + browser.getControl(name="field.git_ref.path").value = git_ref.path |
936 | browser.getControl("Build-time ARG variables").value = ( |
937 | "VAR1=10\nVAR2=20") |
938 | browser.getControl("Create OCI recipe").click() |
939 | @@ -291,9 +291,9 @@ class TestOCIRecipeAddView(BaseTestOCIRecipeView): |
940 | processors = browser.getControl(name="field.processors") |
941 | processors.value = ["386", "amd64"] |
942 | browser.getControl(name="field.name").value = "recipe-name" |
943 | - browser.getControl("Git repository").value = ( |
944 | + browser.getControl(name="field.git_ref.repository").value = ( |
945 | git_ref.repository.identity) |
946 | - browser.getControl("Git branch").value = git_ref.path |
947 | + browser.getControl(name="field.git_ref.path").value = git_ref.path |
948 | browser.getControl("Create OCI recipe").click() |
949 | login_person(self.person) |
950 | recipe = getUtility(IOCIRecipeSet).getByName( |
951 | @@ -409,9 +409,9 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView): |
952 | browser.getControl("Owner").value = ["new-team"] |
953 | browser.getControl(name="field.name").value = "new-name" |
954 | browser.getControl("Description").value = "New description" |
955 | - browser.getControl("Git repository").value = ( |
956 | + browser.getControl(name="field.git_ref.repository").value = ( |
957 | new_git_ref.repository.identity) |
958 | - browser.getControl("Git branch").value = new_git_ref.path |
959 | + browser.getControl(name="field.git_ref.path").value = new_git_ref.path |
960 | browser.getControl("Build file path").value = "Dockerfile-2" |
961 | browser.getControl("Build directory context").value = "apath" |
962 | browser.getControl("Build daily").selected = True |
963 | diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py |
964 | index 10e82da..bbda603 100644 |
965 | --- a/lib/lp/snappy/browser/tests/test_snap.py |
966 | +++ b/lib/lp/snappy/browser/tests/test_snap.py |
967 | @@ -733,9 +733,9 @@ class TestSnapEditView(BaseTestSnapView): |
968 | browser.getControl(name="field.store_distro_series").value = [ |
969 | "ubuntu/%s/%s" % (new_series.name, new_snappy_series.name)] |
970 | browser.getControl("Git", index=0).click() |
971 | - browser.getControl("Git repository").value = ( |
972 | + browser.getControl(name="field.git_ref.repository").value = ( |
973 | new_git_ref.repository.identity) |
974 | - browser.getControl("Git branch").value = new_git_ref.path |
975 | + browser.getControl(name="field.git_ref.path").value = new_git_ref.path |
976 | browser.getControl("Build source tarball").selected = True |
977 | browser.getControl( |
978 | "Automatically build when branch changes").selected = True |
979 | @@ -952,8 +952,9 @@ class TestSnapEditView(BaseTestSnapView): |
980 | private_ref_path = private_ref.path |
981 | browser = self.getViewBrowser(snap, user=self.person) |
982 | browser.getLink("Edit snap package").click() |
983 | - browser.getControl("Git repository").value = private_ref_identity |
984 | - browser.getControl("Git branch").value = private_ref_path |
985 | + browser.getControl(name="field.git_ref.repository").value = ( |
986 | + private_ref_identity) |
987 | + browser.getControl(name="field.git_ref.path").value = private_ref_path |
988 | browser.getControl("Update snap package").click() |
989 | self.assertEqual( |
990 | "A public snap cannot have a private repository.", |
991 | @@ -973,8 +974,9 @@ class TestSnapEditView(BaseTestSnapView): |
992 | git_ref=old_ref, store_series=snappy_series) |
993 | browser = self.getViewBrowser(snap, user=self.person) |
994 | browser.getLink("Edit snap package").click() |
995 | - browser.getControl("Git repository").value = new_repository_url |
996 | - browser.getControl("Git branch").value = new_path |
997 | + browser.getControl( |
998 | + name="field.git_ref.repository").value = new_repository_url |
999 | + browser.getControl(name="field.git_ref.path").value = new_path |
1000 | browser.getControl("Update snap package").click() |
1001 | login_person(self.person) |
1002 | content = find_main_content(browser.contents) |
1003 | diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py |
1004 | index 1ef8f45..7e3d14c 100644 |
1005 | --- a/lib/lp/snappy/model/snap.py |
1006 | +++ b/lib/lp/snappy/model/snap.py |
1007 | @@ -46,7 +46,10 @@ from zope.component import ( |
1008 | getUtility, |
1009 | ) |
1010 | from zope.event import notify |
1011 | -from zope.interface import implementer |
1012 | +from zope.interface import ( |
1013 | + directlyProvides, |
1014 | + implementer, |
1015 | + ) |
1016 | from zope.security.interfaces import Unauthorized |
1017 | from zope.security.proxy import removeSecurityProxy |
1018 | |
1019 | @@ -82,7 +85,10 @@ from lp.code.interfaces.gitref import ( |
1020 | IGitRef, |
1021 | IGitRefRemoteSet, |
1022 | ) |
1023 | -from lp.code.interfaces.gitrepository import IGitRepository |
1024 | +from lp.code.interfaces.gitrepository import ( |
1025 | + IGitRepository, |
1026 | + IHasGitRepositoryURL, |
1027 | + ) |
1028 | from lp.code.model.branch import Branch |
1029 | from lp.code.model.branchcollection import GenericBranchCollection |
1030 | from lp.code.model.gitcollection import GenericGitCollection |
1031 | @@ -440,6 +446,10 @@ class Snap(Storm, WebhookTargetMixin): |
1032 | self.git_repository = None |
1033 | self.git_repository_url = None |
1034 | self.git_path = None |
1035 | + if self.git_repository_url is not None: |
1036 | + directlyProvides(self, IHasGitRepositoryURL) |
1037 | + else: |
1038 | + directlyProvides(self) |
1039 | |
1040 | @property |
1041 | def source(self): |
Mostly looks good - just a few small comments and questions. I'd like to see some screenshots before approving this, though, since it's a UI change.