Merge ~ilasc/launchpad:ui-oci-reg-creds into launchpad:master
- Git
- lp:~ilasc/launchpad
- ui-oci-reg-creds
- Merge into master
Status: | Rejected |
---|---|
Rejected by: | Ioana Lasc |
Proposed branch: | ~ilasc/launchpad:ui-oci-reg-creds |
Merge into: | launchpad:master |
Diff against target: |
2084 lines (+1715/-8) 17 files modified
lib/lp/oci/browser/configure.zcml (+12/-0) lib/lp/oci/browser/ocirecipe.py (+633/-4) lib/lp/oci/browser/tests/test_ocirecipe.py (+290/-0) lib/lp/oci/interfaces/ocipushrule.py (+6/-0) lib/lp/oci/model/ocipushrule.py (+13/-1) lib/lp/oci/model/ociregistrycredentials.py (+5/-3) lib/lp/oci/templates/ocirecipe-edit-credentials.pt (+59/-0) lib/lp/oci/templates/ocirecipe-edit-push-rules.pt (+73/-0) lib/lp/oci/templates/ocirecipe-index.pt (+32/-0) lib/lp/oci/vocabularies.py (+50/-0) lib/lp/oci/vocabularies.zcml (+10/-0) lib/lp/registry/browser/configure.zcml (+13/-0) lib/lp/registry/browser/person.py (+260/-0) lib/lp/registry/browser/tests/test_person.py (+176/-0) lib/lp/registry/templates/edit-oci-registry-credentials.pt (+43/-0) lib/lp/registry/templates/oci-registry-credentials.pt (+37/-0) lib/lp/registry/templates/person-index.pt (+3/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Needs Fixing | ||
Review via email: mp+383333@code.launchpad.net |
Commit message
Add CRUD views for OCI registry credentials and push rules
Description of the change
This branch adds several views in Launchpad for the management of OCI registry credentials and push rules:
View OCI push rules in a table in the context of an OCI recipe. From here allow the user to create, edit or delete entirely any push rule for the recipe they are viewing.
View, edit, delete OCI registry credentials from the context of existing push rules.
View and edit only (without ability to add) OCI registry credentials from the Person Page.
- c3c3343... by Ioana Lasc
-
Move to widgets for OCI credentials on person page
Ioana Lasc (ilasc) wrote : | # |
Thank you Colin! I started working from the bottom on this one (PersonPage view of credentials first). The OCIEditRegistry
The reason I used Form Fields manipulation instead of a fixed Schema Fields set for the widgets is: we don't have a fixed edit schema, we need to iterate over OCIRegistryCred
This posed the problem of the form fields possible having duplicate names:
File "/home/
raise ValueError(
ValueError: ('Duplicate name', 'owner')
which is why I went with this patterns in the form field names: __name__='owner_%d' % removeSecurityP
I'm pretty sure this isn't the right route to go take and probably what's causing the widgets not to display anymore.
Please let me know where/how I can adjust the approach.
- 00ec5f7... by Colin Watson
-
Fix template indentation
- aecfb18... by Colin Watson
-
Simplify template structure
- da8f3bf... by Colin Watson
-
Add missing nocall: to url widget
- 4b7cf6f... by Colin Watson
-
Fix some inconsistencies in TestOCIRegistry
CredentialsView - 5899320... by Colin Watson
-
Fix field naming in TestOCIRegistry
CredentialsView This view's fields are named using the registry credentials ID, not a
push rule ID. - 08bd3a7... by Colin Watson
-
Remove unnecessary removals of security proxies
This is a bad habit to be in, even during development, because it can
result in security vulnerabilities. It's best to only remove security
proxies with a specific justification. - bb1eb70... by Colin Watson
-
Fix incorrect field names in TestOCIRegistry
CredentialsView - faf9002... by Colin Watson
-
Simplify OCIEditRegistry
CredentialsView .setUpFields It's easier to do all the FormFields construction at the end.
I grouped the fields by function because that makes it easier to
configure widgets for all fields of a particular type in future. - 7d3bcc9... by Colin Watson
-
Explicitly mark fields as readonly=False
- 1a9f33f... by Colin Watson
-
Cache OCIEditRegistry
CredentialsView .oci_registry_ creds - 4f6539f... by Colin Watson
-
Fix OCIEditRegistry
CredentialsView field handling This reworks the field layout and parsing along the same lines as the
similar table in GitRepositoryPermissionsView. The update logic is now easier to follow, because the business of
parsing data from the form is separated from the business of applying
the changes to the database. Updates are done by iterating over the
parsed form rather than iterating over the existing credentials, because
there's no guarantee that the latter match what existed at the time the
form was generated, and we check for mismatches (of the form of an
attempt to update a credentials row with a nonexistent ID or one that
isn't owned by the context person).The template now has a way to get hold of the widgets for a given
OCIRegistryCredentials object, rather than trying to reuse the same
widgets on each row of the table. - 888a009... by Colin Watson
-
Add missing "field." prefix
zope.formlib widgets add a "field." prefix to the rendered name by
default. - 1f2221f... by Ioana Lasc
-
Move to widgets for edit credentials on OCIRecipe
- ba97937... by Ioana Lasc
-
Move to widgets for edit push rules on OCIRecipe
- 34d81fa... by Ioana Lasc
-
Remove unnecessary AddPushRule view
- cbd045c... by Ioana Lasc
-
Merge branch 'master' into ui-oci-reg-creds
- 1ae6dc8... by Ioana Lasc
-
Make OCI Username not required on Person Page
- 8206b60... by Ioana Lasc
-
Cleanup OCIRegistryCred
entialsVocabula ry
Ioana Lasc (ilasc) wrote : | # |
Thanks Colin, hopefully this is closer now to the final format and easier to read, ready for another review.
Colin Watson (cjwatson) : | # |
- cce167a... by Ioana Lasc
-
Use one set of fields for add and edit forms
For each of OCIRecipeEditCr
edentialsView and OCIRecipeEditPu shRulesView
we use only one set of form fields for both add form and edit form sections of the screen. - ca31c2f... by Ioana Lasc
-
Address code review comments
Cleanup variable naming and a wide replacement of title
case with sentence case in user visible messages as well as code. - ba90206... by Ioana Lasc
-
Fix indenting in new template files
- 75523b4... by Ioana Lasc
-
Address code review comments
A few name changes and removed the tal condition from the edit push rules table
as it caused the add new rule table row to not display when the push rule set was empty,
introducing the inability to add new push rules.
Ioana Lasc (ilasc) wrote : | # |
Thank you Colin, I have a few notes in response to previous review comments:
1: renamed all the things and used appropriate asserts everywhere - no issues. Indeed I had missed the removal of headers from 2 tables in your previous review, they are all covered now.
2: moved away from <metal:row> with widget_div with exception of the add new registry credentials section where form layout only works with <metal:row> to achieve the form structure you suggested in the previous review (which I agree with, it looks much better than what i had there initially): (x) Use existing credentials:
( ) credentials set 1
( ) credentials set 2
(x) credentials set 3
( ) New credentials:
Registry URL: [ input text field ]
Username: [ input text field ]
Password: [ input text field ]
Confirm password: [ input text field ]
3: I'm a bit confused about the removeSecurityProxy issue: I still had to apply it to both push rules and registry credentials in tests even though in this branch they both require launchpad-edit permissions and the user in the unit tests performing the actions should be able to edit.
4: On the OCIRegistryCred
I'm looking at how to write a more precise query for the OCIRegistryCred
Colin Watson (cjwatson) wrote : | # |
This is a very large merge proposal, and a few hours into reviewing it I've realised that I should have been advising you to split it up more so that reviewing it is more practical and so that we can iterate more quickly. Hopefully the specific comments below are helpful, but before you send it for another round of review, can you please try to break it up into several smaller MPs? For example, I think you could tackle the registry credentials UI and the push rules UI in separate merge proposals, maybe getting most of the read-only views in place first; and you could also split the model and vocabulary changes into their own merge proposals. This would make reviewing it more practical, and make it easier to iterate more quickly.
Let me (or somebody else on the team, depending on the timing of my upcoming leave) know if you need help working out how to split this up effectively.
- 3ff750d... by Ioana Lasc
-
Merge branch 'master' into ui-oci-reg-creds
Ioana Lasc (ilasc) wrote : | # |
Rejecting as this has now been split into several smaller branches / MPs.
Unmerged commits
- 3ff750d... by Ioana Lasc
-
Merge branch 'master' into ui-oci-reg-creds
- 75523b4... by Ioana Lasc
-
Address code review comments
A few name changes and removed the tal condition from the edit push rules table
as it caused the add new rule table row to not display when the push rule set was empty,
introducing the inability to add new push rules. - ba90206... by Ioana Lasc
-
Fix indenting in new template files
- ca31c2f... by Ioana Lasc
-
Address code review comments
Cleanup variable naming and a wide replacement of title
case with sentence case in user visible messages as well as code. - cce167a... by Ioana Lasc
-
Use one set of fields for add and edit forms
For each of OCIRecipeEditCr
edentialsView and OCIRecipeEditPu shRulesView
we use only one set of form fields for both add form and edit form sections of the screen. - 8206b60... by Ioana Lasc
-
Cleanup OCIRegistryCred
entialsVocabula ry - 1ae6dc8... by Ioana Lasc
-
Make OCI Username not required on Person Page
- cbd045c... by Ioana Lasc
-
Merge branch 'master' into ui-oci-reg-creds
- 34d81fa... by Ioana Lasc
-
Remove unnecessary AddPushRule view
- ba97937... by Ioana Lasc
-
Move to widgets for edit push rules on OCIRecipe
Preview Diff
1 | diff --git a/lib/lp/oci/browser/configure.zcml b/lib/lp/oci/browser/configure.zcml |
2 | index b66e509..c531f28 100644 |
3 | --- a/lib/lp/oci/browser/configure.zcml |
4 | +++ b/lib/lp/oci/browser/configure.zcml |
5 | @@ -61,6 +61,18 @@ |
6 | template="../../app/templates/generic-edit.pt" /> |
7 | <browser:page |
8 | for="lp.oci.interfaces.ocirecipe.IOCIRecipe" |
9 | + class="lp.oci.browser.ocirecipe.OCIRecipeEditPushRulesView" |
10 | + permission="launchpad.Edit" |
11 | + name="+edit-push-rules" |
12 | + template="../templates/ocirecipe-edit-push-rules.pt" /> |
13 | + <browser:page |
14 | + for="lp.oci.interfaces.ocirecipe.IOCIRecipe" |
15 | + class="lp.oci.browser.ocirecipe.OCIRecipeEditCredentialsView" |
16 | + permission="launchpad.Edit" |
17 | + name="+edit-credentials" |
18 | + template="../templates/ocirecipe-edit-credentials.pt" /> |
19 | + <browser:page |
20 | + for="lp.oci.interfaces.ocirecipe.IOCIRecipe" |
21 | class="lp.oci.browser.ocirecipe.OCIRecipeDeleteView" |
22 | permission="launchpad.Edit" |
23 | name="+delete" |
24 | diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py |
25 | index 5fcab06..8a024fc 100644 |
26 | --- a/lib/lp/oci/browser/ocirecipe.py |
27 | +++ b/lib/lp/oci/browser/ocirecipe.py |
28 | @@ -11,6 +11,8 @@ __all__ = [ |
29 | 'OCIRecipeAdminView', |
30 | 'OCIRecipeContextMenu', |
31 | 'OCIRecipeDeleteView', |
32 | + 'OCIRecipeEditCredentialsView', |
33 | + 'OCIRecipeEditPushRulesView', |
34 | 'OCIRecipeEditView', |
35 | 'OCIRecipeNavigation', |
36 | 'OCIRecipeNavigationMenu', |
37 | @@ -23,11 +25,19 @@ from lazr.restful.interface import ( |
38 | use_template, |
39 | ) |
40 | from zope.component import getUtility |
41 | +from zope.formlib.boolwidgets import CheckBoxWidget |
42 | +from zope.formlib.form import FormFields |
43 | +from zope.formlib.textwidgets import TextWidget |
44 | +from zope.formlib.widget import CustomWidgetFactory |
45 | from zope.interface import Interface |
46 | from zope.schema import ( |
47 | + Bool, |
48 | Choice, |
49 | List, |
50 | + Password, |
51 | + TextLine, |
52 | ) |
53 | +from zope.security.proxy import removeSecurityProxy |
54 | |
55 | from lp.app.browser.launchpadform import ( |
56 | action, |
57 | @@ -36,10 +46,17 @@ from lp.app.browser.launchpadform import ( |
58 | ) |
59 | from lp.app.browser.lazrjs import InlinePersonEditPickerWidget |
60 | from lp.app.browser.tales import format_link |
61 | -from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget |
62 | +from lp.app.errors import UnexpectedFormData |
63 | +from lp.app.widgets.itemswidgets import ( |
64 | + LabeledMultiCheckBoxWidget, |
65 | + LaunchpadRadioWidget, |
66 | + ) |
67 | from lp.buildmaster.interfaces.processor import IProcessorSet |
68 | from lp.code.browser.widgets.gitref import GitRefWidget |
69 | -from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet |
70 | +from lp.oci.interfaces.ocipushrule import ( |
71 | + IOCIPushRuleSet, |
72 | + OCIPushRuleAlreadyExists, |
73 | + ) |
74 | from lp.oci.interfaces.ocirecipe import ( |
75 | IOCIRecipe, |
76 | IOCIRecipeSet, |
77 | @@ -50,6 +67,10 @@ from lp.oci.interfaces.ocirecipe import ( |
78 | OCIRecipeFeatureDisabled, |
79 | ) |
80 | from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet |
81 | +from lp.oci.interfaces.ociregistrycredentials import ( |
82 | + IOCIRegistryCredentialsSet, |
83 | + OCIRegistryCredentialsAlreadyExist, |
84 | + ) |
85 | from lp.services.features import getFeatureFlag |
86 | from lp.services.helpers import english_list |
87 | from lp.services.propertycache import cachedproperty |
88 | @@ -72,7 +93,6 @@ from lp.soyuz.browser.build import get_build_by_id_str |
89 | |
90 | |
91 | class OCIRecipeNavigation(WebhookTargetNavigationMixin, Navigation): |
92 | - |
93 | usedfor = IOCIRecipe |
94 | |
95 | @stepthrough('+build-request') |
96 | @@ -138,12 +158,18 @@ class OCIRecipeContextMenu(ContextMenu): |
97 | |
98 | facet = 'overview' |
99 | |
100 | - links = ('request_builds',) |
101 | + links = ('request_builds', 'edit_push_rules') |
102 | |
103 | @enabled_with_permission('launchpad.Edit') |
104 | def request_builds(self): |
105 | return Link('+request-builds', 'Request builds', icon='add') |
106 | |
107 | + @enabled_with_permission('launchpad.Edit') |
108 | + def edit_push_rules(self): |
109 | + return Link('+edit-push-rules', |
110 | + 'Edit push rules', |
111 | + icon='edit') |
112 | + |
113 | |
114 | class OCIProjectRecipesView(LaunchpadView): |
115 | """Default view for the list of OCI recipes of an OCI project.""" |
116 | @@ -184,6 +210,15 @@ class OCIRecipeView(LaunchpadView): |
117 | def builds(self): |
118 | return builds_for_recipe(self.context) |
119 | |
120 | + @cachedproperty |
121 | + def push_rules(self): |
122 | + return list( |
123 | + getUtility(IOCIPushRuleSet).findByRecipe(self.context)) |
124 | + |
125 | + @property |
126 | + def has_push_rules(self): |
127 | + return len(self.push_rules) > 0 |
128 | + |
129 | @property |
130 | def person_picker(self): |
131 | field = copy_field( |
132 | @@ -234,6 +269,598 @@ def new_builds_notification_text(builds, already_pending=None): |
133 | return builds_text |
134 | |
135 | |
136 | +class OCIRecipeEditCredentialsView(LaunchpadFormView): |
137 | + """View for +ocirecipe-edit-credentials.pt.""" |
138 | + |
139 | + @cachedproperty |
140 | + def oci_registry_credentials(self): |
141 | + return list(getUtility( |
142 | + IOCIRegistryCredentialsSet).findByOwner(self.context.owner)) |
143 | + |
144 | + class schema(Interface): |
145 | + """Schema for editing registry credentials.""" |
146 | + |
147 | + def _getFieldName(self, name, credentials_id): |
148 | + """Get the combined field name for an `OCIRegistryCredentials` ID. |
149 | + |
150 | + In order to be able to render a table, we encode the credentials ID |
151 | + in the form field name. |
152 | + """ |
153 | + return "%s.%d" % (name, credentials_id) |
154 | + |
155 | + def _parseFieldName(self, field_name): |
156 | + """Parse a combined field name as described in `_getFieldName`. |
157 | + |
158 | + :raises UnexpectedFormData: if the field name cannot be parsed or |
159 | + the `OCIRegistryCredentials` cannot be found. |
160 | + """ |
161 | + field_bits = field_name.split(".") |
162 | + if len(field_bits) != 2: |
163 | + raise UnexpectedFormData( |
164 | + "Cannot parse field name: %s" % field_name) |
165 | + field_type = field_bits[0] |
166 | + try: |
167 | + credentials_id = int(field_bits[1]) |
168 | + except ValueError: |
169 | + raise UnexpectedFormData( |
170 | + "Cannot parse field name: %s" % field_name) |
171 | + return field_type, credentials_id |
172 | + |
173 | + def setUpWidgets(self): |
174 | + LaunchpadFormView.setUpWidgets(self) |
175 | + |
176 | + def setUpFields(self): |
177 | + """See `LaunchpadFormView`.""" |
178 | + LaunchpadFormView.setUpFields(self) |
179 | + |
180 | + owner_fields = [] |
181 | + username_fields = [] |
182 | + password_fields = [] |
183 | + url_fields = [] |
184 | + delete_fields = [] |
185 | + |
186 | + for elem in self.oci_registry_credentials: |
187 | + owner_fields.append( |
188 | + TextLine( |
189 | + title=u'Owner', |
190 | + __name__=self._getFieldName('owner', elem.id), |
191 | + default=elem.owner.name, |
192 | + required=True, readonly=False)) |
193 | + username_fields.append( |
194 | + TextLine( |
195 | + title=u'Username', |
196 | + __name__=self._getFieldName('username', elem.id), |
197 | + default=elem.username, |
198 | + required=False, readonly=False)) |
199 | + password_fields.append( |
200 | + Password( |
201 | + title=u'Password', |
202 | + __name__=self._getFieldName('password', elem.id), |
203 | + default=None, |
204 | + required=False, readonly=False)) |
205 | + password_fields.append( |
206 | + Password( |
207 | + title=u'Confirm password', |
208 | + __name__=self._getFieldName('confirm_password', elem.id), |
209 | + default=None, |
210 | + required=False, readonly=False)) |
211 | + url_fields.append( |
212 | + TextLine( |
213 | + title=u'Registry URL', |
214 | + __name__=self._getFieldName('url', elem.id), |
215 | + default=elem.url, |
216 | + required=True, readonly=False)) |
217 | + delete_fields.append( |
218 | + Bool( |
219 | + title=u'Delete', |
220 | + __name__=self._getFieldName('delete', elem.id), |
221 | + default=False, |
222 | + required=True, readonly=False)) |
223 | + # The fields from the Add New Credentials |
224 | + url_fields.append( |
225 | + TextLine( |
226 | + title=u'Registry URL', |
227 | + __name__=u'add_url', |
228 | + required=False, readonly=False)) |
229 | + username_fields.append( |
230 | + TextLine( |
231 | + title=u'Username', |
232 | + __name__=u'add_username', |
233 | + required=False, readonly=False)) |
234 | + password_fields.append( |
235 | + Password( |
236 | + title=u'Password', |
237 | + __name__=u'add_password', |
238 | + required=False, readonly=False)) |
239 | + password_fields.append( |
240 | + Password( |
241 | + title=u'Confirm password', |
242 | + __name__=u'add_confirm_password', |
243 | + required=False, readonly=False)) |
244 | + self.form_fields += FormFields(*owner_fields) |
245 | + self.form_fields += FormFields(*username_fields) |
246 | + self.form_fields += FormFields(*password_fields) |
247 | + self.form_fields += FormFields(*url_fields) |
248 | + self.form_fields += FormFields(*delete_fields) |
249 | + |
250 | + @property |
251 | + def label(self): |
252 | + return 'Edit OCI registry credentials' |
253 | + |
254 | + @property |
255 | + def cancel_url(self): |
256 | + return canonical_url(self.context) |
257 | + |
258 | + def getCredentialsWidgets(self, credentials): |
259 | + widgets_by_name = {widget.name: widget for widget in self.widgets} |
260 | + owner_field_name = ( |
261 | + "field." + self._getFieldName("owner", credentials.id)) |
262 | + username_field_name = ( |
263 | + "field." + self._getFieldName("username", credentials.id)) |
264 | + password_field_name = ( |
265 | + "field." + self._getFieldName("password", credentials.id)) |
266 | + confirm_password_field_name = ( |
267 | + "field." + self._getFieldName("confirm_password", |
268 | + credentials.id)) |
269 | + url_field_name = "field." + self._getFieldName("url", credentials.id) |
270 | + delete_field_name = ( |
271 | + "field." + self._getFieldName("delete", credentials.id)) |
272 | + return { |
273 | + "owner": widgets_by_name[owner_field_name], |
274 | + "username": widgets_by_name[username_field_name], |
275 | + "password": widgets_by_name[password_field_name], |
276 | + "confirm_password": widgets_by_name[confirm_password_field_name], |
277 | + "url": widgets_by_name[url_field_name], |
278 | + "delete": widgets_by_name[delete_field_name] |
279 | + } |
280 | + |
281 | + def parseData(self, data): |
282 | + """Rearrange form data to make it easier to process.""" |
283 | + parsed_data = {} |
284 | + add_url = data["add_url"] |
285 | + add_username = data["add_username"] |
286 | + add_password = data["add_password"] |
287 | + add_confirm_password = data["add_confirm_password"] |
288 | + if add_url or add_username or add_password or add_confirm_password: |
289 | + parsed_data.setdefault(None, { |
290 | + "username": add_username, |
291 | + "password": add_password, |
292 | + "confirm_password": add_confirm_password, |
293 | + "url": add_url, |
294 | + "action": "add", |
295 | + }) |
296 | + for field_name in sorted( |
297 | + name for name in data if name.split(".")[0] == "owner"): |
298 | + _, credentials_id = self._parseFieldName(field_name) |
299 | + username_field_name = self._getFieldName( |
300 | + "username", credentials_id) |
301 | + password_field_name = self._getFieldName( |
302 | + "password", credentials_id) |
303 | + confirm_password_field_name = self._getFieldName( |
304 | + "confirm_password", credentials_id) |
305 | + url_field_name = self._getFieldName("url", credentials_id) |
306 | + delete_field_name = self._getFieldName("delete", credentials_id) |
307 | + if data.get(delete_field_name): |
308 | + action = "delete" |
309 | + else: |
310 | + action = "change" |
311 | + parsed_data.setdefault(credentials_id, { |
312 | + "username": data.get(username_field_name), |
313 | + "password": data.get(password_field_name), |
314 | + "confirm_password": data.get(confirm_password_field_name), |
315 | + "url": data.get(url_field_name), |
316 | + "action": action, |
317 | + }) |
318 | + |
319 | + return parsed_data |
320 | + |
321 | + def updateCredentialsFromData(self, parsed_data): |
322 | + credentials_map = { |
323 | + credentials.id: credentials |
324 | + for credentials in self.oci_registry_credentials} |
325 | + |
326 | + for credentials_id, parsed_credentials in parsed_data.items(): |
327 | + credentials = credentials_map.get(credentials_id) |
328 | + action = parsed_credentials["action"] |
329 | + |
330 | + if action == "change": |
331 | + username = parsed_credentials["username"] |
332 | + password = parsed_credentials["password"] |
333 | + confirm_password = parsed_credentials["confirm_password"] |
334 | + if password or confirm_password: |
335 | + if password != confirm_password: |
336 | + self.setFieldError( |
337 | + self._getFieldName( |
338 | + "confirm_password", credentials_id), |
339 | + "Passwords do not match.") |
340 | + else: |
341 | + credentials.setCredentials({ |
342 | + "username": username, |
343 | + "password": password, |
344 | + }) |
345 | + elif username != credentials.username: |
346 | + removeSecurityProxy(credentials).username = username |
347 | + if parsed_credentials["url"] != credentials.url: |
348 | + credentials.url = parsed_credentials["url"] |
349 | + elif action == "delete": |
350 | + push_rule_set = getUtility(IOCIPushRuleSet) |
351 | + if not push_rule_set.findByRegistryCredentials( |
352 | + credentials).is_empty(): |
353 | + self.setFieldError( |
354 | + self._getFieldName("delete", credentials_id), |
355 | + "These credentials cannot be deleted as there are " |
356 | + "push rules defined that still use them.") |
357 | + else: |
358 | + credentials.destroySelf() |
359 | + elif action == "add": |
360 | + parsed_add_credentials = parsed_data[credentials] |
361 | + url = parsed_add_credentials["url"] |
362 | + password = parsed_add_credentials["password"] |
363 | + confirm_password = parsed_add_credentials["confirm_password"] |
364 | + username = parsed_add_credentials["username"] |
365 | + if url: |
366 | + if password or confirm_password: |
367 | + if password == confirm_password: |
368 | + credentials = { |
369 | + 'username': username, |
370 | + 'password': password} |
371 | + try: |
372 | + getUtility(IOCIRegistryCredentialsSet).new( |
373 | + owner=self.context.owner, |
374 | + url=url, |
375 | + credentials=credentials) |
376 | + except OCIRegistryCredentialsAlreadyExist: |
377 | + self.setFieldError("add_url", |
378 | + "Credentials already exist " |
379 | + "with the same URL and " |
380 | + "username.") |
381 | + else: |
382 | + self.setFieldError("add_password", |
383 | + "Please make sure the new " |
384 | + "password matches the " |
385 | + "confirm password field.") |
386 | + else: |
387 | + credentials = {'username': username} |
388 | + try: |
389 | + getUtility(IOCIRegistryCredentialsSet).new( |
390 | + owner=self.context.owner, |
391 | + url=url, |
392 | + credentials=credentials) |
393 | + except OCIRegistryCredentialsAlreadyExist: |
394 | + self.setFieldError("add_url", |
395 | + "Credentials already exist " |
396 | + "with the same URL and " |
397 | + "username.") |
398 | + else: |
399 | + self.setFieldError("add_url", |
400 | + "Registry URL cannot be empty.") |
401 | + else: |
402 | + raise AssertionError("unknown action: %s" % action) |
403 | + |
404 | + @action("Save") |
405 | + def save(self, action, data): |
406 | + parsed_data = self.parseData(data) |
407 | + self.updateCredentialsFromData(parsed_data) |
408 | + |
409 | + if not self.errors: |
410 | + self.request.response.addNotification("Saved credentials") |
411 | + self.next_url = canonical_url(self.context) |
412 | + |
413 | + |
414 | +class OCIRecipeEditPushRulesView(LaunchpadFormView): |
415 | + """View for +ocirecipe-edit-push-rules.pt.""" |
416 | + |
417 | + class schema(Interface): |
418 | + """Schema for editing push rules.""" |
419 | + |
420 | + @cachedproperty |
421 | + def push_rules(self): |
422 | + return list( |
423 | + getUtility(IOCIPushRuleSet).findByRecipe(self.context)) |
424 | + |
425 | + @property |
426 | + def has_push_rules(self): |
427 | + return len(self.push_rules) > 0 |
428 | + |
429 | + def _getFieldName(self, name, rule_id): |
430 | + """Get the combined field name for an `OCIPushRule` ID. |
431 | + |
432 | + In order to be able to render a table, we encode the rule ID |
433 | + in the form field name. |
434 | + """ |
435 | + return "%s.%d" % (name, rule_id) |
436 | + |
437 | + def _parseFieldName(self, field_name): |
438 | + """Parse a combined field name as described in `_getFieldName`. |
439 | + |
440 | + :raises UnexpectedFormData: if the field name cannot be parsed or |
441 | + the `OCIPushRule` cannot be found. |
442 | + """ |
443 | + field_bits = field_name.split(".") |
444 | + if len(field_bits) != 2: |
445 | + raise UnexpectedFormData( |
446 | + "Cannot parse field name: %s" % field_name) |
447 | + field_type = field_bits[0] |
448 | + try: |
449 | + rule_id = int(field_bits[1]) |
450 | + except ValueError: |
451 | + raise UnexpectedFormData( |
452 | + "Cannot parse field name: %s" % field_name) |
453 | + return field_type, rule_id |
454 | + |
455 | + def setUpWidgets(self): |
456 | + LaunchpadFormView.setUpWidgets(self) |
457 | + |
458 | + def setUpFields(self): |
459 | + """See `LaunchpadFormView`.""" |
460 | + LaunchpadFormView.setUpFields(self) |
461 | + image_fields = [] |
462 | + checkbox_fields = [] |
463 | + username_fields = [] |
464 | + password_fields = [] |
465 | + url_fields = [] |
466 | + delete_fields = [] |
467 | + existing_credentials = [] |
468 | + for elem in list(self.context.push_rules): |
469 | + image_fields.append( |
470 | + TextLine( |
471 | + title=u'Image name', |
472 | + __name__=self._getFieldName('image_name', elem.id), |
473 | + default=elem.image_name, |
474 | + required=True, readonly=False)) |
475 | + delete_fields.append( |
476 | + Bool( |
477 | + title=u'Delete', |
478 | + __name__=self._getFieldName('delete', elem.id), |
479 | + default=False, |
480 | + required=True, readonly=False)) |
481 | + url_fields.append( |
482 | + TextLine( |
483 | + title=u'Registry URL', |
484 | + __name__=u'add_url', |
485 | + required=False, readonly=False)) |
486 | + image_fields.append( |
487 | + TextLine( |
488 | + title=u'Image name', |
489 | + __name__=u'add_image_name', |
490 | + required=False, readonly=False)) |
491 | + username_fields.append( |
492 | + TextLine( |
493 | + title=u'Username', |
494 | + __name__=u'add_username', |
495 | + required=False, readonly=False)) |
496 | + password_fields.append( |
497 | + Password( |
498 | + title=u'Password', |
499 | + __name__=u'add_password', |
500 | + required=False, readonly=False)) |
501 | + password_fields.append( |
502 | + Password( |
503 | + title=u'Confirm password', |
504 | + __name__=u'add_confirm_password', |
505 | + required=False, readonly=False)) |
506 | + existing_credentials.append( |
507 | + Choice( |
508 | + vocabulary='OCIRegistryCredentials', |
509 | + title='Choose credentials', |
510 | + required=False, |
511 | + __name__=u'existing_credentials')) |
512 | + checkbox_fields.append( |
513 | + Bool( |
514 | + title=u'Add new credentials', |
515 | + __name__=u'add_new_credentials', |
516 | + default=False, |
517 | + readonly=False)) |
518 | + checkbox_fields.append( |
519 | + Bool( |
520 | + title=u'Use existing credentials', |
521 | + __name__=u'use_existing_credentials', |
522 | + default=False, |
523 | + readonly=False)) |
524 | + self.form_fields += FormFields(*image_fields) |
525 | + self.form_fields += FormFields(*delete_fields) |
526 | + self.form_fields += FormFields(*url_fields) |
527 | + self.form_fields += FormFields(*checkbox_fields) |
528 | + self.form_fields += FormFields(*username_fields) |
529 | + self.form_fields += FormFields(*password_fields) |
530 | + self.form_fields += FormFields(*existing_credentials) |
531 | + |
532 | + custom_widget_use_existing_credentials = CustomWidgetFactory( |
533 | + CheckBoxWidget) |
534 | + custom_widget_existing_credentials = CustomWidgetFactory( |
535 | + LaunchpadRadioWidget, |
536 | + widget_class='field subordinate') |
537 | + |
538 | + custom_widget_add_new_credentials = CustomWidgetFactory( |
539 | + CheckBoxWidget) |
540 | + custom_widget_add_url = CustomWidgetFactory( |
541 | + TextWidget, |
542 | + widget_class='field subordinate') |
543 | + custom_widget_add_username = CustomWidgetFactory( |
544 | + TextWidget, |
545 | + widget_class='field subordinate') |
546 | + custom_widget_add_password = CustomWidgetFactory( |
547 | + TextWidget, |
548 | + widget_class='field subordinate') |
549 | + custom_widget_add_confirm_password = CustomWidgetFactory( |
550 | + TextWidget, |
551 | + widget_class='field subordinate') |
552 | + |
553 | + @property |
554 | + def label(self): |
555 | + return 'Edit OCI push rules for %s' % self.context.name |
556 | + |
557 | + page_title = 'Edit OCI push rules' |
558 | + |
559 | + @property |
560 | + def cancel_url(self): |
561 | + return canonical_url(self.context) |
562 | + |
563 | + def getRulesWidgets(self, rule): |
564 | + widgets_by_name = {widget.name: widget for widget in self.widgets} |
565 | + image_field_name = ( |
566 | + "field." + self._getFieldName("image_name", rule.id)) |
567 | + delete_field_name = ( |
568 | + "field." + self._getFieldName("delete", rule.id)) |
569 | + return { |
570 | + "image_name": widgets_by_name[image_field_name], |
571 | + "delete": widgets_by_name[delete_field_name], |
572 | + } |
573 | + |
574 | + def parseData(self, data): |
575 | + """Rearrange form data to make it easier to process.""" |
576 | + parsed_data = {} |
577 | + add_image_name = data["add_image_name"] |
578 | + add_url = data["add_url"] |
579 | + add_username = data["add_username"] |
580 | + add_password = data["add_password"] |
581 | + add_confirm_password = data["add_confirm_password"] |
582 | + add_existing_credentials = data["existing_credentials"] |
583 | + |
584 | + # parse data from the Add new rule section of the form |
585 | + if (add_url or add_username or add_password or |
586 | + add_confirm_password or add_image_name or |
587 | + add_existing_credentials): |
588 | + parsed_data.setdefault(None, { |
589 | + "image_name": add_image_name, |
590 | + "url": add_url, |
591 | + "username": add_username, |
592 | + "password": add_password, |
593 | + "confirm_password": add_confirm_password, |
594 | + "existing_credentials": data["existing_credentials"], |
595 | + "add_new_credentials": data["add_new_credentials"], |
596 | + "use_existing_credentials": data["use_existing_credentials"], |
597 | + "action": "add", |
598 | + }) |
599 | + |
600 | + for field_name in sorted( |
601 | + name for name in data if name.split(".")[0] == "image_name"): |
602 | + _, rule_id = self._parseFieldName(field_name) |
603 | + image_field_name = self._getFieldName("image_name", rule_id) |
604 | + delete_field_name = self._getFieldName("delete", rule_id) |
605 | + if data.get(delete_field_name): |
606 | + action = "delete" |
607 | + else: |
608 | + action = "change" |
609 | + parsed_data.setdefault(rule_id, { |
610 | + "image_name": data.get(image_field_name), |
611 | + "action": action, |
612 | + }) |
613 | + |
614 | + return parsed_data |
615 | + |
616 | + def updatePushRulesFromData(self, parsed_data): |
617 | + rules_map = { |
618 | + rule.id: rule |
619 | + for rule in self.context.push_rules} |
620 | + for rule_id, parsed_rules in parsed_data.items(): |
621 | + rule = rules_map.get(rule_id) |
622 | + action = parsed_rules["action"] |
623 | + |
624 | + if action == "change": |
625 | + image_name = parsed_rules["image_name"] |
626 | + if not image_name: |
627 | + self.setFieldError( |
628 | + self._getFieldName( |
629 | + "image_name", rule_id), |
630 | + "Image name must be set.") |
631 | + else: |
632 | + removeSecurityProxy(rule).image_name = image_name |
633 | + elif action == "delete": |
634 | + removeSecurityProxy(rule).destroySelf() |
635 | + elif action == "add": |
636 | + add_data = parsed_data[None] |
637 | + image_name = add_data["image_name"] |
638 | + url = add_data["url"] |
639 | + password = add_data["password"] |
640 | + confirm_password = add_data["confirm_password"] |
641 | + username = add_data["username"] |
642 | + existing_credentials = add_data["existing_credentials"] |
643 | + checked_use_existing_credentials = add_data["use_existing_credentials"] |
644 | + checked_add_new_credentials = add_data["add_new_credentials"] |
645 | + |
646 | + if image_name: |
647 | + if not (checked_add_new_credentials |
648 | + or checked_use_existing_credentials): |
649 | + self.setFieldError("existing_credentials", |
650 | + "You must check either use existing" |
651 | + " or introduce new credentials for " |
652 | + "this rule.") |
653 | + return |
654 | + if checked_add_new_credentials and checked_use_existing_credentials: |
655 | + self.setFieldError("existing_credentials", |
656 | + "You can either use existing " |
657 | + "credentials or introduce new ones " |
658 | + "for this rule but not both.") |
659 | + return |
660 | + if checked_use_existing_credentials: |
661 | + if existing_credentials: |
662 | + try: |
663 | + getUtility(IOCIPushRuleSet).new( |
664 | + self.context, |
665 | + existing_credentials, |
666 | + image_name) |
667 | + except OCIPushRuleAlreadyExists: |
668 | + self.setFieldError("add_image_name", |
669 | + "A push rule already exists" |
670 | + " with the same URL, image " |
671 | + "name and credentials.") |
672 | + return |
673 | + if checked_add_new_credentials: |
674 | + if url: |
675 | + if password == confirm_password: |
676 | + credentials = { |
677 | + 'username': username, |
678 | + 'password': password} |
679 | + try: |
680 | + creds = getUtility( |
681 | + IOCIRegistryCredentialsSet |
682 | + ).getOrCreate( |
683 | + owner=self.context.owner, |
684 | + url=url, |
685 | + credentials=credentials) |
686 | + except OCIRegistryCredentialsAlreadyExist: |
687 | + self.setFieldError( |
688 | + "add_url", |
689 | + "Credentials already exist with" |
690 | + " the same URL and username.") |
691 | + return |
692 | + try: |
693 | + getUtility(IOCIPushRuleSet).new( |
694 | + self.context, creds, image_name) |
695 | + except OCIPushRuleAlreadyExists: |
696 | + self.setFieldError( |
697 | + "add_image_name", |
698 | + "A push rule already exists with the " |
699 | + "same URL, image name, and " |
700 | + "credentials.") |
701 | + return |
702 | + else: |
703 | + self.setFieldError( |
704 | + "password", |
705 | + "Please make sure the new password matches" |
706 | + " the confirm password field.") |
707 | + else: |
708 | + self.setFieldError( |
709 | + "add_url", |
710 | + "Registry URL must be set.") |
711 | + else: |
712 | + self.setFieldError( |
713 | + "add_image_name", |
714 | + "Image name must be set.") |
715 | + else: |
716 | + raise AssertionError("unknown action: %s" % action) |
717 | + |
718 | + @action("Save") |
719 | + def save(self, action, data): |
720 | + parsed_data = self.parseData(data) |
721 | + self.updatePushRulesFromData(parsed_data) |
722 | + |
723 | + if not self.errors: |
724 | + self.request.response.addNotification("Saved push rules") |
725 | + self.next_url = canonical_url(self.context) |
726 | + |
727 | + |
728 | class OCIRecipeRequestBuildsView(LaunchpadFormView): |
729 | """A view for requesting builds of an OCI recipe.""" |
730 | |
731 | @@ -313,6 +940,7 @@ class IOCIRecipeEditSchema(Interface): |
732 | "build_file", |
733 | "build_daily", |
734 | "require_virtualized", |
735 | + "push_rules", |
736 | ]) |
737 | |
738 | |
739 | @@ -444,6 +1072,7 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin): |
740 | "git_ref", |
741 | "build_file", |
742 | "build_daily", |
743 | + "push_rules", |
744 | ) |
745 | custom_widget_git_ref = GitRefWidget |
746 | |
747 | diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py |
748 | index 6633966..2a24edd 100644 |
749 | --- a/lib/lp/oci/browser/tests/test_ocirecipe.py |
750 | +++ b/lib/lp/oci/browser/tests/test_ocirecipe.py |
751 | @@ -19,12 +19,15 @@ from fixtures import FakeLogger |
752 | import pytz |
753 | import soupmatchers |
754 | from testtools.matchers import ( |
755 | + Equals, |
756 | + MatchesDict, |
757 | MatchesSetwise, |
758 | MatchesStructure, |
759 | ) |
760 | from zope.component import getUtility |
761 | from zope.publisher.interfaces import NotFound |
762 | from zope.security.interfaces import Unauthorized |
763 | +from zope.security.proxy import removeSecurityProxy |
764 | from zope.testbrowser.browser import LinkNotFoundError |
765 | |
766 | from lp.app.interfaces.launchpad import ILaunchpadCelebrities |
767 | @@ -35,11 +38,16 @@ from lp.oci.browser.ocirecipe import ( |
768 | OCIRecipeEditView, |
769 | OCIRecipeView, |
770 | ) |
771 | +from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet |
772 | from lp.oci.interfaces.ocirecipe import ( |
773 | CannotModifyOCIRecipeProcessor, |
774 | IOCIRecipeSet, |
775 | OCI_RECIPE_ALLOW_CREATE, |
776 | ) |
777 | +from lp.oci.interfaces.ociregistrycredentials import ( |
778 | + IOCIRegistryCredentialsSet, |
779 | + ) |
780 | +from lp.oci.tests.helpers import OCIConfigHelperMixin |
781 | from lp.services.database.constants import UTC_NOW |
782 | from lp.services.features.testing import FeatureFixture |
783 | from lp.services.propertycache import get_property_cache |
784 | @@ -817,6 +825,288 @@ class TestOCIRecipeRequestBuildsView(BaseTestOCIRecipeView): |
785 | extract_text(find_main_content(browser.contents))) |
786 | |
787 | |
788 | +class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin, |
789 | + BaseTestOCIRecipeView): |
790 | + def setUp(self): |
791 | + super(TestOCIRecipeEditPushRulesView, self).setUp() |
792 | + self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu |
793 | + self.distroseries = self.factory.makeDistroSeries( |
794 | + distribution=self.ubuntu, name="shiny", displayname="Shiny") |
795 | + self.architectures = [] |
796 | + for processor, architecture in ("386", "i386"), ("amd64", "amd64"): |
797 | + das = self.factory.makeDistroArchSeries( |
798 | + distroseries=self.distroseries, architecturetag=architecture, |
799 | + processor=getUtility(IProcessorSet).getByName(processor)) |
800 | + das.addOrUpdateChroot(self.factory.makeLibraryFileAlias()) |
801 | + self.architectures.append(das) |
802 | + self.useFixture(FeatureFixture({ |
803 | + OCI_RECIPE_ALLOW_CREATE: "on", |
804 | + "oci.build_series.%s" % self.distroseries.distribution.name: |
805 | + self.distroseries.name, |
806 | + })) |
807 | + oci_project = self.factory.makeOCIProject( |
808 | + pillar=self.distroseries.distribution, |
809 | + ociprojectname="oci-project-name") |
810 | + self.recipe = self.factory.makeOCIRecipe( |
811 | + name="recipe-name", registrant=self.person, owner=self.person, |
812 | + oci_project=oci_project) |
813 | + |
814 | + self.setConfig() |
815 | + |
816 | + def test_edit_oci_push_rules(self): |
817 | + url = unicode(self.factory.getUniqueURL()) |
818 | + credentials = {'username': 'foo', 'password': 'bar'} |
819 | + registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
820 | + owner=self.person, |
821 | + url=url, |
822 | + credentials=credentials) |
823 | + image_name = self.factory.getUniqueUnicode() |
824 | + push_rule = getUtility(IOCIPushRuleSet).new( |
825 | + recipe=self.recipe, |
826 | + registry_credentials=registry_credentials, |
827 | + image_name=image_name) |
828 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
829 | + browser.getLink("Edit push rules").click() |
830 | + # assert image name is displayed correctly |
831 | + with person_logged_in(self.person): |
832 | + self.assertEqual(image_name, browser.getControl( |
833 | + name="field.image_name.%d" % push_rule.id).value) |
834 | + |
835 | + # assert image name is required |
836 | + with person_logged_in(self.person): |
837 | + browser.getControl( |
838 | + name="field.image_name.%d" % push_rule.id).value = "" |
839 | + browser.getControl("Save").click() |
840 | + self.assertIn("Required input is missing", browser.contents) |
841 | + |
842 | + # set image name to valid string |
843 | + with person_logged_in(self.person): |
844 | + browser.getControl( |
845 | + name="field.image_name.%d" % push_rule.id).value = "testimage1" |
846 | + browser.getControl("Save").click() |
847 | + # and assert model changed |
848 | + self.assertEqual( |
849 | + removeSecurityProxy(push_rule).image_name, "testimage1") |
850 | + |
851 | + def test_delete_oci_push_rules(self): |
852 | + url = unicode(self.factory.getUniqueURL()) |
853 | + credentials = {'username': 'foo', 'password': 'bar'} |
854 | + registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
855 | + owner=self.person, |
856 | + url=url, |
857 | + credentials=credentials) |
858 | + image_name = self.factory.getUniqueUnicode() |
859 | + push_rule = getUtility(IOCIPushRuleSet).new( |
860 | + recipe=self.recipe, |
861 | + registry_credentials=registry_credentials, |
862 | + image_name=image_name) |
863 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
864 | + browser.getLink("Edit push rules").click() |
865 | + with person_logged_in(self.person): |
866 | + browser.getControl( |
867 | + name="field.delete.%d" % push_rule.id).value = True |
868 | + browser.getControl("Save").click() |
869 | + |
870 | + with person_logged_in(self.person): |
871 | + self.assertIsNone(removeSecurityProxy( |
872 | + getUtility(IOCIPushRuleSet).getByID(push_rule.id))) |
873 | + |
874 | + def test_add_oci_push_rules_validations(self): |
875 | + # Test add new rule works when there are |
876 | + # no rules in the DB |
877 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
878 | + browser.getLink("Edit push rules").click() |
879 | + |
880 | + # Save does not error if there are no changes |
881 | + # on the form |
882 | + browser.getControl("Save").click() |
883 | + self.assertIn("Saved push rules", browser.contents) |
884 | + |
885 | + # Only with image name we fail with Registry URL must be set |
886 | + browser.getLink("Edit push rules").click() |
887 | + browser.getControl( |
888 | + name="field.add_image_name").value = "imagename1" |
889 | + browser.getControl( |
890 | + name="field.add_new_credentials").value = True |
891 | + browser.getControl("Save").click() |
892 | + self.assertIn("Registry URL must be set", browser.contents) |
893 | + |
894 | + # Either Use existing credentials or Add new |
895 | + # must be checked |
896 | + browser.getControl( |
897 | + name="field.add_new_credentials").value = False |
898 | + browser.getControl( |
899 | + name="field.use_existing_credentials").value = False |
900 | + browser.getControl("Save").click() |
901 | + self.assertIn("You must check either use existing or introduce" |
902 | + " new credentials for this rule.", browser.contents) |
903 | + |
904 | + # Both Use existing credentials And Add new |
905 | + # are checked, we notify the user to check only one |
906 | + browser.getControl( |
907 | + name="field.add_new_credentials").value = True |
908 | + browser.getControl( |
909 | + name="field.use_existing_credentials").value = True |
910 | + browser.getControl("Save").click() |
911 | + self.assertIn("You can either use existing credentials or introduce" |
912 | + " new ones for this rule but not both.", |
913 | + browser.contents) |
914 | + |
915 | + # No image name entered on the form |
916 | + # we assume user is only editing and we allow |
917 | + # Save on the form |
918 | + browser.getControl( |
919 | + name="field.add_image_name").value = "" |
920 | + browser.getControl("Save").click() |
921 | + self.assertIn("Saved push rules", browser.contents) |
922 | + |
923 | + def test_add_oci_push_rules(self): |
924 | + # Image name and Registry URL will create an empty Credentials object |
925 | + # and a valid push rule based on the empty creds |
926 | + |
927 | + url = unicode(self.factory.getUniqueURL()) |
928 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
929 | + browser.getLink("Edit push rules").click() |
930 | + browser.getControl( |
931 | + name="field.add_image_name").value = "imagename1" |
932 | + browser.getControl( |
933 | + name="field.add_url").value = url |
934 | + browser.getControl( |
935 | + name="field.add_new_credentials").value = True |
936 | + browser.getControl("Save").click() |
937 | + with person_logged_in(self.person): |
938 | + rules = list(removeSecurityProxy( |
939 | + getUtility(IOCIPushRuleSet).findByRecipe(self.recipe))) |
940 | + self.assertEqual(len(rules), 1) |
941 | + rule = rules[0] |
942 | + self.assertIsNotNone(rule.registry_credentials) |
943 | + self.assertEqual(u'imagename1', rule.image_name) |
944 | + self.assertEqual(url, rule.registry_url) |
945 | + self.assertEqual(url, |
946 | + rule.registry_credentials.url) |
947 | + self.assertEqual( |
948 | + None, |
949 | + rule.registry_credentials.username) |
950 | + with person_logged_in(self.person): |
951 | + self.assertThat( |
952 | + rule.registry_credentials.getCredentials(), |
953 | + MatchesDict( |
954 | + {"password": Equals(None)})) |
955 | + |
956 | + # Previously added registry credentials can now be chosen |
957 | + # from the radio widget when adding a new rule |
958 | + browser.getLink("Edit push rules").click() |
959 | + |
960 | + browser.getControl( |
961 | + name="field.use_existing_credentials").value = True |
962 | + browser.getControl( |
963 | + name="field.add_image_name").value = "imagename1" |
964 | + browser.getControl( |
965 | + name="field.existing_credentials").value = ( |
966 | + browser.getControl( |
967 | + name="field.existing_credentials").options[1].strip()) |
968 | + browser.getControl("Save").click() |
969 | + self.assertIn( |
970 | + "A push rule already exists with the same URL, " |
971 | + "image name and credentials.", browser.contents) |
972 | + |
973 | + # We display correctly the radio buttons widget when |
974 | + # username is empty in registry credentials and |
975 | + # allow correctly adding new rule based on it |
976 | + browser.getControl( |
977 | + name="field.use_existing_credentials").value = True |
978 | + browser.getControl( |
979 | + name="field.add_image_name").value = "imagename2" |
980 | + browser.getControl("Save").click() |
981 | + with person_logged_in(self.person): |
982 | + rules = list(removeSecurityProxy( |
983 | + getUtility(IOCIPushRuleSet).findByRecipe(self.recipe))) |
984 | + self.assertEqual(len(rules), 2) |
985 | + rule = rules[1] |
986 | + self.assertIsNotNone(rule.registry_credentials) |
987 | + self.assertEqual(u'imagename2', rule.image_name) |
988 | + self.assertEqual(url, rule.registry_url) |
989 | + self.assertEqual( |
990 | + url, |
991 | + rule.registry_credentials.url) |
992 | + self.assertEqual( |
993 | + None, |
994 | + rule.registry_credentials.username) |
995 | + with person_logged_in(self.person): |
996 | + self.assertThat( |
997 | + rule.registry_credentials.getCredentials(), |
998 | + MatchesDict({ |
999 | + "password": Equals(None)})) |
1000 | + |
1001 | + browser.getLink("Edit push rules").click() |
1002 | + browser.getControl( |
1003 | + name="field.add_new_credentials").value = True |
1004 | + browser.getControl( |
1005 | + name="field.add_image_name").value = "imagename3" |
1006 | + browser.getControl( |
1007 | + name="field.add_url").value = url |
1008 | + browser.getControl( |
1009 | + name="field.add_username").value = "username" |
1010 | + browser.getControl( |
1011 | + name="field.add_password").value = "password" |
1012 | + browser.getControl( |
1013 | + name="field.add_confirm_password").value = "password" |
1014 | + browser.getControl("Save").click() |
1015 | + with person_logged_in(self.person): |
1016 | + rules = list(removeSecurityProxy( |
1017 | + getUtility(IOCIPushRuleSet).findByRecipe(self.recipe))) |
1018 | + self.assertEqual(len(rules), 3) |
1019 | + rule = rules[2] |
1020 | + self.assertIsNotNone(rule.registry_credentials) |
1021 | + self.assertEqual("imagename3", rule.image_name) |
1022 | + self.assertEqual(url, rule.registry_url) |
1023 | + self.assertEqual(url, |
1024 | + rule.registry_credentials.url) |
1025 | + self.assertEqual( |
1026 | + "username", |
1027 | + rule.registry_credentials.username) |
1028 | + with person_logged_in(self.person): |
1029 | + self.assertThat( |
1030 | + rule.registry_credentials.getCredentials(), |
1031 | + MatchesDict( |
1032 | + {"username": Equals("username"), |
1033 | + "password": Equals("password")})) |
1034 | + |
1035 | + def test_add_oci_registry_creds(self): |
1036 | + url = unicode(self.factory.getUniqueURL()) |
1037 | + credentials = {'username': 'foo', 'password': 'bar'} |
1038 | + image_name = self.factory.getUniqueUnicode() |
1039 | + registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
1040 | + owner=self.person, |
1041 | + url=url, |
1042 | + credentials=credentials) |
1043 | + getUtility(IOCIPushRuleSet).new( |
1044 | + recipe=self.recipe, |
1045 | + registry_credentials=registry_credentials, |
1046 | + image_name=image_name) |
1047 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
1048 | + browser.getLink("Edit push rules").click() |
1049 | + browser.getLink("Edit credentials").click() |
1050 | + |
1051 | + browser.getControl(name="field.add_url").value = url |
1052 | + browser.getControl(name="field.add_username").value = "new_username" |
1053 | + browser.getControl(name="field.add_password").value = "password" |
1054 | + browser.getControl(name="field.add_confirm_password" |
1055 | + ).value = "password" |
1056 | + |
1057 | + browser.getControl("Save").click() |
1058 | + with person_logged_in(self.person): |
1059 | + creds = list(getUtility( |
1060 | + IOCIRegistryCredentialsSet).findByOwner( |
1061 | + self.person)) |
1062 | + |
1063 | + self.assertEqual(url, creds[1].url) |
1064 | + self.assertThat( |
1065 | + removeSecurityProxy(creds[1]).getCredentials(), |
1066 | + MatchesDict({"username": Equals("new_username"), |
1067 | + "password": Equals("password")})) |
1068 | + |
1069 | + |
1070 | class TestOCIProjectRecipesView(BaseTestOCIRecipeView): |
1071 | def setUp(self): |
1072 | super(TestOCIProjectRecipesView, self).setUp() |
1073 | diff --git a/lib/lp/oci/interfaces/ocipushrule.py b/lib/lp/oci/interfaces/ocipushrule.py |
1074 | index 5b43f25..2596d8d 100644 |
1075 | --- a/lib/lp/oci/interfaces/ocipushrule.py |
1076 | +++ b/lib/lp/oci/interfaces/ocipushrule.py |
1077 | @@ -128,5 +128,11 @@ class IOCIPushRuleSet(Interface): |
1078 | def new(recipe, registry_credentials, image_name): |
1079 | """Create an `IOCIPushRule`.""" |
1080 | |
1081 | + def findByRecipe(self, recipe): |
1082 | + """Find matching `IOCIPushRule`s by recipe.""" |
1083 | + |
1084 | + def findByRegistryCredentials(self, credentials): |
1085 | + """Find matching `IOCIPushRule` by credentials.""" |
1086 | + |
1087 | def getByID(id): |
1088 | """Get a single `IOCIPushRule` by its ID.""" |
1089 | diff --git a/lib/lp/oci/model/ocipushrule.py b/lib/lp/oci/model/ocipushrule.py |
1090 | index dbdb22e..24d5e27 100644 |
1091 | --- a/lib/lp/oci/model/ocipushrule.py |
1092 | +++ b/lib/lp/oci/model/ocipushrule.py |
1093 | @@ -69,7 +69,7 @@ class OCIPushRule(Storm): |
1094 | |
1095 | def destroySelf(self): |
1096 | """See `IOCIPushRule`.""" |
1097 | - IStore(OCIPushRule).get(self.id).remove() |
1098 | + IStore(OCIPushRule).remove(self) |
1099 | |
1100 | |
1101 | @implementer(IOCIPushRuleSet) |
1102 | @@ -90,3 +90,15 @@ class OCIPushRuleSet: |
1103 | def getByID(self, id): |
1104 | """See `IOCIPushRuleSet`.""" |
1105 | return IStore(OCIPushRule).get(OCIPushRule, id) |
1106 | + |
1107 | + def findByRecipe(self, recipe): |
1108 | + store = IStore(OCIPushRule) |
1109 | + return store.find( |
1110 | + OCIPushRule, |
1111 | + OCIPushRule.recipe == recipe) |
1112 | + |
1113 | + def findByRegistryCredentials(self, credentials): |
1114 | + store = IStore(OCIPushRule) |
1115 | + return store.find( |
1116 | + OCIPushRule, |
1117 | + OCIPushRule.registry_credentials == credentials) |
1118 | diff --git a/lib/lp/oci/model/ociregistrycredentials.py b/lib/lp/oci/model/ociregistrycredentials.py |
1119 | index 49c740f..d86b140 100644 |
1120 | --- a/lib/lp/oci/model/ociregistrycredentials.py |
1121 | +++ b/lib/lp/oci/model/ociregistrycredentials.py |
1122 | @@ -122,11 +122,13 @@ class OCIRegistryCredentials(Storm): |
1123 | def username(self): |
1124 | return self._credentials.get('username') |
1125 | |
1126 | + @username.setter |
1127 | + def username(self, value): |
1128 | + self._credentials['username'] = value |
1129 | + |
1130 | def destroySelf(self): |
1131 | """See `IOCIRegistryCredentials`.""" |
1132 | - store = IStore(OCIRegistryCredentials) |
1133 | - store.find( |
1134 | - OCIRegistryCredentials, OCIRegistryCredentials.id == self).remove() |
1135 | + IStore(OCIRegistryCredentials).remove(self) |
1136 | |
1137 | |
1138 | @implementer(IOCIRegistryCredentialsSet) |
1139 | diff --git a/lib/lp/oci/templates/ocirecipe-edit-credentials.pt b/lib/lp/oci/templates/ocirecipe-edit-credentials.pt |
1140 | new file mode 100644 |
1141 | index 0000000..89483da |
1142 | --- /dev/null |
1143 | +++ b/lib/lp/oci/templates/ocirecipe-edit-credentials.pt |
1144 | @@ -0,0 +1,59 @@ |
1145 | +<html |
1146 | + xmlns="http://www.w3.org/1999/xhtml" |
1147 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
1148 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
1149 | + xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
1150 | + metal:use-macro="view/macro:page/main_only" |
1151 | + i18n:domain="launchpad"> |
1152 | +<body> |
1153 | + |
1154 | +<div metal:fill-slot="main"> |
1155 | + <div metal:use-macro="context/@@launchpad_form/form"> |
1156 | + <metal:formbody fill-slot="widgets"> |
1157 | + |
1158 | + <table class="form"> |
1159 | + <tr tal:repeat="credentials view/oci_registry_credentials"> |
1160 | + <tal:credentials_widgets |
1161 | + define="credentials_widgets python:view.getCredentialsWidgets(credentials); |
1162 | + parity python:'even' if repeat['credentials'].even() else 'odd'"> |
1163 | + <td tal:define="widget nocall:credentials_widgets/url"> |
1164 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1165 | + </td> |
1166 | + <td tal:define="widget nocall:credentials_widgets/owner"> |
1167 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1168 | + </td> |
1169 | + <td tal:define="widget nocall:credentials_widgets/username"> |
1170 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1171 | + </td> |
1172 | + <td tal:define="widget nocall:credentials_widgets/password"> |
1173 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1174 | + </td> |
1175 | + <td tal:define="widget nocall:credentials_widgets/confirm_password"> |
1176 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1177 | + </td> |
1178 | + <td tal:define="widget nocall:credentials_widgets/delete"> |
1179 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1180 | + </td> |
1181 | + </tal:credentials_widgets> |
1182 | + </tr> |
1183 | + <tr> |
1184 | + <td tal:define="widget nocall:view/widgets/add_url"> |
1185 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1186 | + </td> |
1187 | + <td tal:define="widget nocall:view/widgets/add_username"> |
1188 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1189 | + </td> |
1190 | + <td tal:define="widget nocall:view/widgets/add_password"> |
1191 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1192 | + </td> |
1193 | + <td tal:define="widget nocall:view/widgets/add_confirm_password"> |
1194 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1195 | + </td> |
1196 | + </tr> |
1197 | + </table> |
1198 | + |
1199 | + </metal:formbody> |
1200 | + </div> |
1201 | +</div> |
1202 | +</body> |
1203 | +</html> |
1204 | diff --git a/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt |
1205 | new file mode 100644 |
1206 | index 0000000..bcccdb9 |
1207 | --- /dev/null |
1208 | +++ b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt |
1209 | @@ -0,0 +1,73 @@ |
1210 | +<html |
1211 | + xmlns="http://www.w3.org/1999/xhtml" |
1212 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
1213 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
1214 | + xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
1215 | + metal:use-macro="view/macro:page/main_only" |
1216 | + i18n:domain="launchpad"> |
1217 | +<body> |
1218 | + |
1219 | +<div metal:fill-slot="main"> |
1220 | + <div metal:use-macro="context/@@launchpad_form/form"> |
1221 | + <metal:formbody fill-slot="widgets"> |
1222 | + <table class="form"> |
1223 | + <tr> |
1224 | + <td><label>OCI credentials</label></td> |
1225 | + <td><label>Image name</label></td> |
1226 | + <td><label>Delete rule</label></td> |
1227 | + </tr> |
1228 | + <tr tal:repeat="rule context/push_rules"> |
1229 | + <tal:rules_widgets |
1230 | + define="rules_widgets python:view.getRulesWidgets(rule); |
1231 | + parity python:'even' if repeat['rule'].even() else 'odd'"> |
1232 | + <td> |
1233 | + <tal:editable condition="context/required:launchpad.Edit"> |
1234 | + <a tal:attributes="href context/fmt:url/+edit-credentials">Edit credentials</a> |
1235 | + </tal:editable> |
1236 | + </td> |
1237 | + <td tal:define="widget nocall:rules_widgets/image_name"> |
1238 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1239 | + </td> |
1240 | + <td tal:define="widget nocall:rules_widgets/delete"> |
1241 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1242 | + </td> |
1243 | + </tal:rules_widgets> |
1244 | + </tr> |
1245 | + <tr> |
1246 | + <td tal:define="widget nocall:view/widgets/add_image_name"> |
1247 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1248 | + </td> |
1249 | + </tr> |
1250 | + <tr> |
1251 | + <td tal:define="widget nocall:view/widgets/use_existing_credentials"> |
1252 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
1253 | + </td> |
1254 | + <td> |
1255 | + <tal:widget define="widget nocall:view/widgets/existing_credentials"> |
1256 | + <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
1257 | + </tal:widget> |
1258 | + </td> |
1259 | + </tr> |
1260 | + <tr> |
1261 | + <td tal:define="widget nocall:view/widgets/add_new_credentials"> |
1262 | + <metal:row use-macro="context/@@launchpad_form/widget_div" /> |
1263 | + </td> |
1264 | + <td tal:define="widget nocall:view/widgets/add_url"> |
1265 | + <metal:row use-macro="context/@@launchpad_form/widget_row" /> |
1266 | + </td> |
1267 | + <td tal:define="widget nocall:view/widgets/add_username"> |
1268 | + <metal:row use-macro="context/@@launchpad_form/widget_row" /> |
1269 | + </td> |
1270 | + <td tal:define="widget nocall:view/widgets/add_password"> |
1271 | + <metal:row use-macro="context/@@launchpad_form/widget_row" /> |
1272 | + </td> |
1273 | + <td tal:define="widget nocall:view/widgets/add_confirm_password"> |
1274 | + <metal:row use-macro="context/@@launchpad_form/widget_row" /> |
1275 | + </td> |
1276 | + </tr> |
1277 | + </table> |
1278 | + </metal:formbody> |
1279 | + </div> |
1280 | +</div> |
1281 | +</body> |
1282 | +</html> |
1283 | diff --git a/lib/lp/oci/templates/ocirecipe-index.pt b/lib/lp/oci/templates/ocirecipe-index.pt |
1284 | index f7df628..88a95b3 100644 |
1285 | --- a/lib/lp/oci/templates/ocirecipe-index.pt |
1286 | +++ b/lib/lp/oci/templates/ocirecipe-index.pt |
1287 | @@ -116,6 +116,38 @@ |
1288 | tal:condition="link/enabled"> |
1289 | <tal:request-builds replace="structure link/fmt:link"/> |
1290 | </div> |
1291 | + |
1292 | + |
1293 | + <h2>Recipe push rules</h2> |
1294 | + <table id="push-rules-listing" tal:condition="view/has_push_rules" class="listing" |
1295 | + style="margin-bottom: 1em; "> |
1296 | + <thead> |
1297 | + <tr> |
1298 | + <th>Registry URL</th> |
1299 | + <th>Username</th> |
1300 | + <th>Image Name</th> |
1301 | + </tr> |
1302 | + </thead> |
1303 | + <tbody> |
1304 | + <tal:recipe-push-rules repeat="item view/push_rules"> |
1305 | + <tr tal:define="rule item" |
1306 | + tal:attributes="id string:rule-${rule/id}"> |
1307 | + <td tal:content="rule/registry_credentials/url"/> |
1308 | + <td tal:content="rule/registry_credentials/username"/> |
1309 | + <td tal:content="rule/image_name"/> |
1310 | + </tr> |
1311 | + </tal:recipe-push-rules> |
1312 | + </tbody> |
1313 | + </table> |
1314 | + <p tal:condition="not: view/has_push_rules"> |
1315 | + This OCI recipe has no push rules defined yet. |
1316 | + </p> |
1317 | + |
1318 | + <div tal:define="link context/menu:context/edit_push_rules" |
1319 | + tal:condition="link/enabled"> |
1320 | + <tal:edit-push-rules replace="structure link/fmt:link"/> |
1321 | + </div> |
1322 | + |
1323 | </div> |
1324 | |
1325 | </body> |
1326 | diff --git a/lib/lp/oci/vocabularies.py b/lib/lp/oci/vocabularies.py |
1327 | index aa1fae3..69cc049 100644 |
1328 | --- a/lib/lp/oci/vocabularies.py |
1329 | +++ b/lib/lp/oci/vocabularies.py |
1330 | @@ -8,10 +8,15 @@ from __future__ import absolute_import, print_function, unicode_literals |
1331 | __metaclass__ = type |
1332 | __all__ = [] |
1333 | |
1334 | +from zope.component import getUtility |
1335 | from zope.interface import implementer |
1336 | from zope.schema.vocabulary import SimpleTerm |
1337 | |
1338 | +from lp.oci.interfaces.ociregistrycredentials import ( |
1339 | + IOCIRegistryCredentialsSet, |
1340 | + ) |
1341 | from lp.oci.model.ocirecipe import OCIRecipe |
1342 | +from lp.oci.model.ociregistrycredentials import OCIRegistryCredentials |
1343 | from lp.services.webapp.vocabulary import ( |
1344 | IHugeVocabulary, |
1345 | StormVocabularyBase, |
1346 | @@ -35,6 +40,51 @@ class OCIRecipeDistroArchSeriesVocabulary(StormVocabularyBase): |
1347 | return len(self.context.getAllowedArchitectures()) |
1348 | |
1349 | |
1350 | +class OCIRegistryCredentialsVocabulary(StormVocabularyBase): |
1351 | + |
1352 | + _table = OCIRegistryCredentials |
1353 | + |
1354 | + def toTerm(self, obj): |
1355 | + if obj.username: |
1356 | + token = "%s %s" % ( |
1357 | + obj.url, |
1358 | + obj.username) |
1359 | + else: |
1360 | + token = obj.url |
1361 | + |
1362 | + return SimpleTerm(obj, token) |
1363 | + |
1364 | + @property |
1365 | + def _entries(self): |
1366 | + return list(getUtility( |
1367 | + IOCIRegistryCredentialsSet).findByOwner(self.context.owner)) |
1368 | + |
1369 | + def __contains__(self, value): |
1370 | + """See `IVocabulary`.""" |
1371 | + return value in self._entries |
1372 | + |
1373 | + def __iter__(self): |
1374 | + for obj in self._entries: |
1375 | + yield self.toTerm(obj) |
1376 | + |
1377 | + def __len__(self): |
1378 | + return len(self._entries) |
1379 | + |
1380 | + def getTermByToken(self, token): |
1381 | + """See `IVocabularyTokenized`.""" |
1382 | + try: |
1383 | + if ' ' in token: |
1384 | + url, username = token.split(' ') |
1385 | + else: |
1386 | + username = None |
1387 | + url = token |
1388 | + for obj in self._entries: |
1389 | + if obj.url == url and obj.username == username: |
1390 | + return self.toTerm(obj) |
1391 | + except ValueError: |
1392 | + raise LookupError(token) |
1393 | + |
1394 | + |
1395 | @implementer(IHugeVocabulary) |
1396 | class OCIRecipeVocabulary(StormVocabularyBase): |
1397 | """All OCI Recipes of a given OCI project.""" |
1398 | diff --git a/lib/lp/oci/vocabularies.zcml b/lib/lp/oci/vocabularies.zcml |
1399 | index 1a6b75c..24a57d8 100644 |
1400 | --- a/lib/lp/oci/vocabularies.zcml |
1401 | +++ b/lib/lp/oci/vocabularies.zcml |
1402 | @@ -16,6 +16,16 @@ |
1403 | </class> |
1404 | |
1405 | <securedutility |
1406 | + name="OCIRegistryCredentials" |
1407 | + component="lp.oci.vocabularies.OCIRegistryCredentialsVocabulary" |
1408 | + provides="zope.schema.interfaces.IVocabularyFactory"> |
1409 | + <allow interface="zope.schema.interfaces.IVocabularyFactory" /> |
1410 | + </securedutility> |
1411 | + |
1412 | + <class class="lp.oci.vocabularies.OCIRegistryCredentialsVocabulary"> |
1413 | + <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary" /> |
1414 | + </class> |
1415 | + <securedutility |
1416 | name="OCIRecipe" |
1417 | component="lp.oci.vocabularies.OCIRecipeVocabulary" |
1418 | provides="zope.schema.interfaces.IVocabularyFactory"> |
1419 | diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml |
1420 | index 3991dd5..d50517e 100644 |
1421 | --- a/lib/lp/registry/browser/configure.zcml |
1422 | +++ b/lib/lp/registry/browser/configure.zcml |
1423 | @@ -1231,6 +1231,19 @@ |
1424 | template="../templates/person-oauth-tokens.pt" |
1425 | /> |
1426 | <browser:page |
1427 | + name="+oci-registry-credentials" |
1428 | + for="lp.registry.interfaces.person.IPerson" |
1429 | + class="lp.registry.browser.person.PersonOCIRegistryCredentialsView" |
1430 | + permission="launchpad.Edit" |
1431 | + template="../templates/oci-registry-credentials.pt" |
1432 | + /> |
1433 | + <browser:page |
1434 | + for="lp.registry.interfaces.person.IPerson" |
1435 | + class="lp.registry.browser.person.OCIEditRegistryCredentialsView" |
1436 | + permission="launchpad.Edit" |
1437 | + name="+edit-oci-registry-credentials" |
1438 | + template="../templates/edit-oci-registry-credentials.pt" /> |
1439 | + <browser:page |
1440 | name="+livefs" |
1441 | for="lp.registry.interfaces.person.IPerson" |
1442 | class="lp.registry.browser.person.PersonLiveFSView" |
1443 | diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py |
1444 | index d512b7b..57fe9ca 100644 |
1445 | --- a/lib/lp/registry/browser/person.py |
1446 | +++ b/lib/lp/registry/browser/person.py |
1447 | @@ -8,6 +8,7 @@ __all__ = [ |
1448 | 'BeginTeamClaimView', |
1449 | 'CommonMenuLinks', |
1450 | 'EmailToPersonView', |
1451 | + 'OCIEditRegistryCredentialsView', |
1452 | 'PeopleSearchView', |
1453 | 'PersonAccountAdministerView', |
1454 | 'PersonAdministerView', |
1455 | @@ -30,6 +31,7 @@ __all__ = [ |
1456 | 'PersonLiveFSView', |
1457 | 'PersonNavigation', |
1458 | 'PersonOAuthTokensView', |
1459 | + 'PersonOCIRegistryCredentialsView', |
1460 | 'PersonOverviewMenu', |
1461 | 'PersonOwnedTeamsView', |
1462 | 'PersonRdfContentsView', |
1463 | @@ -89,7 +91,9 @@ from zope.interface import ( |
1464 | from zope.interface.exceptions import Invalid |
1465 | from zope.publisher.interfaces import NotFound |
1466 | from zope.schema import ( |
1467 | + Bool, |
1468 | Choice, |
1469 | + Password, |
1470 | Text, |
1471 | TextLine, |
1472 | ) |
1473 | @@ -133,6 +137,10 @@ from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin |
1474 | from lp.code.errors import InvalidNamespace |
1475 | from lp.code.interfaces.branchnamespace import IBranchNamespaceSet |
1476 | from lp.code.interfaces.gitlookup import IGitTraverser |
1477 | +from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet |
1478 | +from lp.oci.interfaces.ociregistrycredentials import ( |
1479 | + IOCIRegistryCredentialsSet, |
1480 | + ) |
1481 | from lp.registry.browser import BaseRdfView |
1482 | from lp.registry.browser.branding import BrandingChangeView |
1483 | from lp.registry.browser.menu import ( |
1484 | @@ -570,6 +578,15 @@ class PersonNavigation(BranchTraversalMixin, Navigation): |
1485 | return None |
1486 | return irc_nick |
1487 | |
1488 | + @stepthrough('+oci-registry-credential') |
1489 | + def traverse_oci_registry_credential(self, id): |
1490 | + """Traverse to this person's OCI registry credentials |
1491 | + on the webservice layer.""" |
1492 | + oci_credentials = getUtility(IOCIRegistryCredentialsSet).get(id) |
1493 | + if oci_credentials is None or oci_credentials.person != self.context: |
1494 | + return None |
1495 | + return oci_credentials |
1496 | + |
1497 | @stepto('+archivesubscriptions') |
1498 | def traverse_archive_subscription(self): |
1499 | """Traverse to the archive subscription for this person.""" |
1500 | @@ -802,6 +819,7 @@ class PersonOverviewMenu(ApplicationMenu, PersonMenuMixin, |
1501 | 'view_ppa_subscriptions', |
1502 | 'ppa', |
1503 | 'oauth_tokens', |
1504 | + 'oci_registry_credentials', |
1505 | 'related_software_summary', |
1506 | 'view_recipes', |
1507 | 'view_snaps', |
1508 | @@ -824,6 +842,12 @@ class PersonOverviewMenu(ApplicationMenu, PersonMenuMixin, |
1509 | return Link(target, text, enabled=enabled, icon='info') |
1510 | |
1511 | @enabled_with_permission('launchpad.Edit') |
1512 | + def oci_registry_credentials(self): |
1513 | + target = '+oci-registry-credentials' |
1514 | + text = 'OCI registry credentials' |
1515 | + return Link(target, text, icon='info') |
1516 | + |
1517 | + @enabled_with_permission('launchpad.Edit') |
1518 | def editlanguages(self): |
1519 | target = '+editlanguages' |
1520 | text = 'Set preferred languages' |
1521 | @@ -1547,6 +1571,16 @@ class PersonView(LaunchpadView, FeedsMixin, ContactViaWebLinksMixin): |
1522 | check_permission('launchpad.Edit', self.context)) |
1523 | |
1524 | @property |
1525 | + def should_show_ociregistrycreds_section(self): |
1526 | + """Should the 'OCI registry credentials' section be shown? |
1527 | + |
1528 | + It's shown when the person has OCI credentials registered or has rights |
1529 | + to register new ones. |
1530 | + """ |
1531 | + return bool(self.context.ociregistrycreds) or ( |
1532 | + check_permission('launchpad.Edit', self.context)) |
1533 | + |
1534 | + @property |
1535 | def should_show_jabberids_section(self): |
1536 | """Should the 'Jabber IDs' section be shown? |
1537 | |
1538 | @@ -3621,6 +3655,232 @@ class PersonOAuthTokensView(LaunchpadView): |
1539 | canonical_url(self.context, view_name='+oauth-tokens')) |
1540 | |
1541 | |
1542 | +class PersonOCIRegistryCredentialsView(LaunchpadView): |
1543 | + """View for Person:+oci-registry-credentials.""" |
1544 | + |
1545 | + @cachedproperty |
1546 | + def oci_registry_credentials(self): |
1547 | + return list(getUtility( |
1548 | + IOCIRegistryCredentialsSet).findByOwner(self.context)) |
1549 | + |
1550 | + @property |
1551 | + def page_title(self): |
1552 | + return "OCI registry credentials" |
1553 | + |
1554 | + @property |
1555 | + def has_credentials(self): |
1556 | + return len(self.oci_registry_credentials) > 0 |
1557 | + |
1558 | + |
1559 | +class OCIEditRegistryCredentialsView(LaunchpadFormView): |
1560 | + """View for Person:+edit-oci-registry-credentials.""" |
1561 | + |
1562 | + @cachedproperty |
1563 | + def oci_registry_credentials(self): |
1564 | + return list(getUtility( |
1565 | + IOCIRegistryCredentialsSet).findByOwner(self.context)) |
1566 | + |
1567 | + class schema(Interface): |
1568 | + """Schema for editing registry credentials.""" |
1569 | + |
1570 | + def _getFieldName(self, name, credentials_id): |
1571 | + """Get the combined field name for an `OCIRegistryCredentials` ID. |
1572 | + |
1573 | + In order to be able to render a table, we encode the credentials ID |
1574 | + in the form field name. |
1575 | + """ |
1576 | + return "%s.%d" % (name, credentials_id) |
1577 | + |
1578 | + def _parseFieldName(self, field_name): |
1579 | + """Parse a combined field name as described in `_getFieldName`. |
1580 | + |
1581 | + :raises UnexpectedFormData: if the field name cannot be parsed or |
1582 | + the `OCIRegistryCredentials` cannot be found. |
1583 | + """ |
1584 | + field_bits = field_name.split(".") |
1585 | + if len(field_bits) != 2: |
1586 | + raise UnexpectedFormData( |
1587 | + "Cannot parse field name: %s" % field_name) |
1588 | + field_type = field_bits[0] |
1589 | + try: |
1590 | + credentials_id = int(field_bits[1]) |
1591 | + except ValueError: |
1592 | + raise UnexpectedFormData( |
1593 | + "Cannot parse field name: %s" % field_name) |
1594 | + return field_type, credentials_id |
1595 | + |
1596 | + def setUpWidgets(self): |
1597 | + LaunchpadFormView.setUpWidgets(self) |
1598 | + |
1599 | + def setUpFields(self): |
1600 | + """See `LaunchpadFormView`.""" |
1601 | + LaunchpadFormView.setUpFields(self) |
1602 | + |
1603 | + owner_fields = [] |
1604 | + username_fields = [] |
1605 | + password_fields = [] |
1606 | + url_fields = [] |
1607 | + delete_fields = [] |
1608 | + |
1609 | + for elem in self.oci_registry_credentials: |
1610 | + owner_fields.append( |
1611 | + TextLine( |
1612 | + title=u'Owner', |
1613 | + __name__=self._getFieldName('owner', elem.id), |
1614 | + default=elem.owner.name, |
1615 | + required=True, readonly=False)) |
1616 | + username_fields.append( |
1617 | + TextLine( |
1618 | + title=u'Username', |
1619 | + __name__=self._getFieldName('username', elem.id), |
1620 | + default=elem.username, |
1621 | + required=False, readonly=False)) |
1622 | + password_fields.append( |
1623 | + Password( |
1624 | + title=u'Password', |
1625 | + __name__=self._getFieldName('password', elem.id), |
1626 | + default=None, |
1627 | + required=False, readonly=False)) |
1628 | + password_fields.append( |
1629 | + Password( |
1630 | + title=u'Confirm Password', |
1631 | + __name__=self._getFieldName('confirm_password', elem.id), |
1632 | + default=None, |
1633 | + required=False, readonly=False)) |
1634 | + url_fields.append( |
1635 | + TextLine( |
1636 | + title=u'Registry URL', |
1637 | + __name__=self._getFieldName('url', elem.id), |
1638 | + default=elem.url, |
1639 | + required=True, readonly=False)) |
1640 | + delete_fields.append( |
1641 | + Bool( |
1642 | + title=u'Delete', |
1643 | + __name__=self._getFieldName('delete', elem.id), |
1644 | + default=False, |
1645 | + required=True, readonly=False)) |
1646 | + |
1647 | + self.form_fields += FormFields(*owner_fields) |
1648 | + self.form_fields += FormFields(*username_fields) |
1649 | + self.form_fields += FormFields(*password_fields) |
1650 | + self.form_fields += FormFields(*url_fields) |
1651 | + self.form_fields += FormFields(*delete_fields) |
1652 | + |
1653 | + @property |
1654 | + def label(self): |
1655 | + return 'Edit OCI Registry Credentials' |
1656 | + |
1657 | + @property |
1658 | + def cancel_url(self): |
1659 | + return canonical_url(self.context) |
1660 | + |
1661 | + def getCredentialsWidgets(self, credentials): |
1662 | + widgets_by_name = {widget.name: widget for widget in self.widgets} |
1663 | + owner_field_name = ( |
1664 | + "field." + self._getFieldName("owner", credentials.id)) |
1665 | + username_field_name = ( |
1666 | + "field." + self._getFieldName("username", credentials.id)) |
1667 | + password_field_name = ( |
1668 | + "field." + self._getFieldName("password", credentials.id)) |
1669 | + confirm_password_field_name = ( |
1670 | + "field." + self._getFieldName("confirm_password", credentials.id)) |
1671 | + url_field_name = "field." + self._getFieldName("url", credentials.id) |
1672 | + delete_field_name = ( |
1673 | + "field." + self._getFieldName("delete", credentials.id)) |
1674 | + return { |
1675 | + "owner": widgets_by_name[owner_field_name], |
1676 | + "username": widgets_by_name[username_field_name], |
1677 | + "password": widgets_by_name[password_field_name], |
1678 | + "confirm_password": widgets_by_name[confirm_password_field_name], |
1679 | + "url": widgets_by_name[url_field_name], |
1680 | + "delete": widgets_by_name[delete_field_name], |
1681 | + } |
1682 | + |
1683 | + def parseData(self, data): |
1684 | + """Rearrange form data to make it easier to process.""" |
1685 | + parsed_data = {} |
1686 | + |
1687 | + for field_name in sorted( |
1688 | + name for name in data if name.split(".")[0] == "owner"): |
1689 | + _, credentials_id = self._parseFieldName(field_name) |
1690 | + username_field_name = self._getFieldName( |
1691 | + "username", credentials_id) |
1692 | + password_field_name = self._getFieldName( |
1693 | + "password", credentials_id) |
1694 | + confirm_password_field_name = self._getFieldName( |
1695 | + "confirm_password", credentials_id) |
1696 | + url_field_name = self._getFieldName("url", credentials_id) |
1697 | + delete_field_name = self._getFieldName("delete", credentials_id) |
1698 | + if data.get(delete_field_name): |
1699 | + action = "delete" |
1700 | + else: |
1701 | + action = "change" |
1702 | + parsed_data.setdefault(credentials_id, { |
1703 | + "username": data.get(username_field_name), |
1704 | + "password": data.get(password_field_name), |
1705 | + "confirm_password": data.get(confirm_password_field_name), |
1706 | + "url": data.get(url_field_name), |
1707 | + "action": action, |
1708 | + }) |
1709 | + |
1710 | + return parsed_data |
1711 | + |
1712 | + def updateCredentialsFromData(self, parsed_data): |
1713 | + credentials_map = { |
1714 | + credentials.id: credentials |
1715 | + for credentials in self.oci_registry_credentials} |
1716 | + |
1717 | + for credentials_id, parsed_credentials in parsed_data.items(): |
1718 | + credentials = credentials_map.get(credentials_id) |
1719 | + action = parsed_credentials["action"] |
1720 | + if credentials is None: |
1721 | + self.addError(structured( |
1722 | + "Cannot edit credentials with nonexistent ID %d", |
1723 | + credentials_id)) |
1724 | + continue |
1725 | + |
1726 | + if action == "change": |
1727 | + username = parsed_credentials["username"] |
1728 | + password = parsed_credentials["password"] |
1729 | + confirm_password = parsed_credentials["confirm_password"] |
1730 | + if password or confirm_password: |
1731 | + if password != confirm_password: |
1732 | + self.setFieldError( |
1733 | + self._getFieldName( |
1734 | + "confirm_password", credentials_id), |
1735 | + "Passwords do not match.") |
1736 | + else: |
1737 | + credentials.setCredentials({ |
1738 | + "username": username, |
1739 | + "password": password, |
1740 | + }) |
1741 | + elif username != credentials.username: |
1742 | + removeSecurityProxy(credentials).username = username |
1743 | + if parsed_credentials["url"] != credentials.url: |
1744 | + credentials.url = parsed_credentials["url"] |
1745 | + elif action == "delete": |
1746 | + push_rule_set = getUtility(IOCIPushRuleSet) |
1747 | + if not push_rule_set.findByRegistryCredentials( |
1748 | + credentials).is_empty(): |
1749 | + self.setFieldError( |
1750 | + self._getFieldName("delete", credentials_id), |
1751 | + "These credentials cannot be deleted as there are " |
1752 | + "push rules defined that still use them.") |
1753 | + else: |
1754 | + credentials.destroySelf() |
1755 | + else: |
1756 | + raise AssertionError("unknown action: %s" % action) |
1757 | + |
1758 | + @action("Save") |
1759 | + def save(self, action, data): |
1760 | + parsed_data = self.parseData(data) |
1761 | + self.updateCredentialsFromData(parsed_data) |
1762 | + |
1763 | + if not self.errors: |
1764 | + self.request.response.addNotification("Saved credentials") |
1765 | + self.next_url = canonical_url(self.context) |
1766 | + |
1767 | + |
1768 | class PersonLiveFSView(LaunchpadView): |
1769 | """Default view for the list of live filesystems owned by a person.""" |
1770 | page_title = 'LiveFS' |
1771 | diff --git a/lib/lp/registry/browser/tests/test_person.py b/lib/lp/registry/browser/tests/test_person.py |
1772 | index 10bbba7..df10c92 100644 |
1773 | --- a/lib/lp/registry/browser/tests/test_person.py |
1774 | +++ b/lib/lp/registry/browser/tests/test_person.py |
1775 | @@ -19,6 +19,7 @@ from testtools.matchers import ( |
1776 | DocTestMatches, |
1777 | Equals, |
1778 | LessThan, |
1779 | + MatchesDict, |
1780 | Not, |
1781 | ) |
1782 | from testtools.testcase import ExpectedException |
1783 | @@ -34,6 +35,12 @@ from lp.app.errors import NotFoundError |
1784 | from lp.app.interfaces.launchpad import ILaunchpadCelebrities |
1785 | from lp.blueprints.enums import SpecificationImplementationStatus |
1786 | from lp.buildmaster.enums import BuildStatus |
1787 | +from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet |
1788 | +from lp.oci.interfaces.ocirecipe import OCI_RECIPE_ALLOW_CREATE |
1789 | +from lp.oci.interfaces.ociregistrycredentials import ( |
1790 | + IOCIRegistryCredentialsSet, |
1791 | + ) |
1792 | +from lp.oci.tests.helpers import OCIConfigHelperMixin |
1793 | from lp.registry.browser.person import PersonView |
1794 | from lp.registry.browser.team import TeamInvitationView |
1795 | from lp.registry.enums import PersonVisibility |
1796 | @@ -48,6 +55,7 @@ from lp.registry.model.karma import KarmaCategory |
1797 | from lp.registry.model.milestone import milestone_sort_key |
1798 | from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache |
1799 | from lp.services.config import config |
1800 | +from lp.services.database.interfaces import IStore |
1801 | from lp.services.features.testing import FeatureFixture |
1802 | from lp.services.identity.interfaces.account import AccountStatus |
1803 | from lp.services.identity.interfaces.emailaddress import IEmailAddressSet |
1804 | @@ -1281,6 +1289,174 @@ class TestPersonRelatedProjectsView(TestCaseWithFactory): |
1805 | self.assertThat(view(), next_match) |
1806 | |
1807 | |
1808 | +class TestPersonOCIRegistryCredentialsView(BrowserTestCase, |
1809 | + OCIConfigHelperMixin, |
1810 | + TestCaseWithFactory): |
1811 | + |
1812 | + layer = DatabaseFunctionalLayer |
1813 | + |
1814 | + def setUp(self): |
1815 | + super(TestPersonOCIRegistryCredentialsView, self).setUp() |
1816 | + self.setConfig() |
1817 | + self.person = self.factory.makePerson( |
1818 | + name="test-person", displayname="Test Person") |
1819 | + self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu |
1820 | + self.distroseries = self.factory.makeDistroSeries( |
1821 | + distribution=self.ubuntu, name="shiny", displayname="Shiny") |
1822 | + self.useFixture(FeatureFixture({ |
1823 | + OCI_RECIPE_ALLOW_CREATE: "on", |
1824 | + "oci.build_series.%s" % self.distroseries.distribution.name: |
1825 | + self.distroseries.name, |
1826 | + })) |
1827 | + oci_project = self.factory.makeOCIProject( |
1828 | + pillar=self.distroseries.distribution) |
1829 | + self.recipe = self.factory.makeOCIRecipe( |
1830 | + registrant=self.person, owner=self.person, |
1831 | + oci_project=oci_project) |
1832 | + |
1833 | + def test_view_oci_registry_creds_on_person_page(self): |
1834 | + # Verify view helper attributes. |
1835 | + url = unicode(self.factory.getUniqueURL()) |
1836 | + credentials = {'username': 'foo', 'password': 'bar'} |
1837 | + getUtility(IOCIRegistryCredentialsSet).new( |
1838 | + owner=self.user, |
1839 | + url=url, |
1840 | + credentials=credentials) |
1841 | + view = create_initialized_view(self.user, '+oci-registry-credentials') |
1842 | + self.assertEqual('OCI registry credentials', view.page_title) |
1843 | + with person_logged_in(self.user): |
1844 | + self.assertEqual( |
1845 | + credentials.get('username'), |
1846 | + view.oci_registry_credentials[0].getCredentials()['username']) |
1847 | + self.assertEqual(url, view.oci_registry_credentials[0].url) |
1848 | + |
1849 | + def test_edit_oci_registry_creds_on_person_page(self): |
1850 | + url = unicode(self.factory.getUniqueURL()) |
1851 | + newurl = unicode(self.factory.getUniqueURL()) |
1852 | + third_url = unicode(self.factory.getUniqueURL()) |
1853 | + credentials = {'username': 'foo', 'password': 'bar'} |
1854 | + credentials2 = {'username': 'foo2', 'password': 'bar'} |
1855 | + registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
1856 | + owner=self.user, |
1857 | + url=url, |
1858 | + credentials=credentials) |
1859 | + IStore(registry_credentials).flush() |
1860 | + registry_credentials_id = removeSecurityProxy(registry_credentials).id |
1861 | + getUtility(IOCIRegistryCredentialsSet).new( |
1862 | + owner=self.user, |
1863 | + url=newurl, |
1864 | + credentials=credentials2) |
1865 | + browser = self.getViewBrowser( |
1866 | + self.user, view_name='+oci-registry-credentials', user=self.user) |
1867 | + browser.getLink("Edit credentials").click() |
1868 | + |
1869 | + # Change only the username |
1870 | + username_control = browser.getControl( |
1871 | + name="field.username.%d" % registry_credentials_id) |
1872 | + username_control.value = 'different_username' |
1873 | + browser.getControl("Save").click() |
1874 | + with person_logged_in(self.user): |
1875 | + self.assertThat( |
1876 | + registry_credentials.getCredentials(), |
1877 | + MatchesDict( |
1878 | + {"username": Equals("different_username"), |
1879 | + "password": Equals("bar")})) |
1880 | + |
1881 | + # change only the registry url |
1882 | + browser = self.getViewBrowser( |
1883 | + self.user, view_name='+oci-registry-credentials', user=self.user) |
1884 | + browser.getLink("Edit credentials").click() |
1885 | + url_control = browser.getControl( |
1886 | + name="field.url.%d" % registry_credentials_id) |
1887 | + url_control.value = newurl |
1888 | + browser.getControl("Save").click() |
1889 | + with person_logged_in(self.user): |
1890 | + self.assertEqual(newurl, registry_credentials.url) |
1891 | + |
1892 | + # change only the password |
1893 | + browser = self.getViewBrowser( |
1894 | + self.user, view_name='+oci-registry-credentials', user=self.user) |
1895 | + browser.getLink("Edit credentials").click() |
1896 | + password_control = browser.getControl( |
1897 | + name="field.password.%d" % registry_credentials_id) |
1898 | + password_control.value = 'newpassword' |
1899 | + |
1900 | + browser.getControl("Save").click() |
1901 | + self.assertIn("Passwords do not match.", browser.contents) |
1902 | + |
1903 | + # change all fields with one edit action |
1904 | + username_control = browser.getControl( |
1905 | + name="field.username.%d" % registry_credentials_id) |
1906 | + username_control.value = 'third_different_username' |
1907 | + url_control = browser.getControl( |
1908 | + name="field.url.%d" % registry_credentials_id) |
1909 | + url_control.value = third_url |
1910 | + password_control = browser.getControl( |
1911 | + name="field.password.%d" % registry_credentials_id) |
1912 | + password_control.value = 'third_newpassword' |
1913 | + confirm_password_control = browser.getControl( |
1914 | + name="field.confirm_password.%d" % registry_credentials_id) |
1915 | + confirm_password_control.value = 'third_newpassword' |
1916 | + browser.getControl("Save").click() |
1917 | + with person_logged_in(self.user): |
1918 | + self.assertThat( |
1919 | + registry_credentials.getCredentials(), |
1920 | + MatchesDict( |
1921 | + {"username": Equals("third_different_username"), |
1922 | + "password": Equals("third_newpassword")})) |
1923 | + self.assertEqual(third_url, registry_credentials.url) |
1924 | + |
1925 | + def test_delete_oci_registry_creds_on_person_page(self): |
1926 | + # Test that we do not delete creds when there are |
1927 | + # push rules defined to use them |
1928 | + url = unicode(self.factory.getUniqueURL()) |
1929 | + credentials = {'username': 'foo', 'password': 'bar'} |
1930 | + registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
1931 | + owner=self.person, |
1932 | + url=url, |
1933 | + credentials=credentials) |
1934 | + IStore(registry_credentials).flush() |
1935 | + registry_credentials_id = removeSecurityProxy(registry_credentials).id |
1936 | + image_name = self.factory.getUniqueUnicode() |
1937 | + push_rule = getUtility(IOCIPushRuleSet).new( |
1938 | + recipe=self.recipe, |
1939 | + registry_credentials=registry_credentials, |
1940 | + image_name=image_name) |
1941 | + |
1942 | + browser = self.getViewBrowser( |
1943 | + self.person, view_name='+oci-registry-credentials', |
1944 | + user=self.person) |
1945 | + browser.getLink("Edit credentials").click() |
1946 | + # assert full rule is displayed |
1947 | + self.assertEqual(url, browser.getControl( |
1948 | + name="field.url.%d" % registry_credentials_id).value) |
1949 | + self.assertEqual(credentials.get('username'), browser.getControl( |
1950 | + name="field.username.%d" % registry_credentials_id).value) |
1951 | + |
1952 | + # mark one line of credentials for delete |
1953 | + delete_control = browser.getControl( |
1954 | + name="field.delete.%d" % registry_credentials_id) |
1955 | + delete_control.getControl('Delete').selected = True |
1956 | + browser.getControl("Save").click() |
1957 | + self.assertIn("These credentials cannot be deleted as there are " |
1958 | + "push rules defined that still use them.", |
1959 | + browser.contents) |
1960 | + |
1961 | + # make sure we don't have any push rules defined to use |
1962 | + # the credentials we want to remove |
1963 | + with person_logged_in(self.person): |
1964 | + removeSecurityProxy(push_rule).destroySelf() |
1965 | + |
1966 | + delete_control = browser.getControl( |
1967 | + name="field.delete.%d" % registry_credentials_id) |
1968 | + delete_control.getControl('Delete').selected = True |
1969 | + browser.getControl("Save").click() |
1970 | + credentials_set = getUtility(IOCIRegistryCredentialsSet) |
1971 | + with person_logged_in(self.person): |
1972 | + self.assertEqual( |
1973 | + 0, credentials_set.findByOwner(self.person).count()) |
1974 | + |
1975 | + |
1976 | class TestPersonLiveFSView(BrowserTestCase): |
1977 | layer = DatabaseFunctionalLayer |
1978 | |
1979 | diff --git a/lib/lp/registry/templates/edit-oci-registry-credentials.pt b/lib/lp/registry/templates/edit-oci-registry-credentials.pt |
1980 | new file mode 100644 |
1981 | index 0000000..2a387c4 |
1982 | --- /dev/null |
1983 | +++ b/lib/lp/registry/templates/edit-oci-registry-credentials.pt |
1984 | @@ -0,0 +1,43 @@ |
1985 | +<html |
1986 | + xmlns="http://www.w3.org/1999/xhtml" |
1987 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
1988 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
1989 | + xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
1990 | + metal:use-macro="view/macro:page/main_only" |
1991 | + i18n:domain="launchpad"> |
1992 | +<body> |
1993 | + |
1994 | +<div metal:fill-slot="main"> |
1995 | + <div metal:use-macro="context/@@launchpad_form/form"> |
1996 | + <metal:formbody fill-slot="widgets"> |
1997 | + <table class="form"> |
1998 | + <tr tal:repeat="credentials view/oci_registry_credentials"> |
1999 | + <tal:credentials_widgets |
2000 | + define="credentials_widgets python:view.getCredentialsWidgets(credentials); |
2001 | + parity python:'even' if repeat['credentials'].even() else 'odd'"> |
2002 | + <td tal:define="widget nocall:credentials_widgets/url"> |
2003 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
2004 | + </td> |
2005 | + <td tal:define="widget nocall:credentials_widgets/owner"> |
2006 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
2007 | + </td> |
2008 | + <td tal:define="widget nocall:credentials_widgets/username"> |
2009 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
2010 | + </td> |
2011 | + <td tal:define="widget nocall:credentials_widgets/password"> |
2012 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
2013 | + </td> |
2014 | + <td tal:define="widget nocall:credentials_widgets/confirm_password"> |
2015 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
2016 | + </td> |
2017 | + <td tal:define="widget nocall:credentials_widgets/delete"> |
2018 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
2019 | + </td> |
2020 | + </tal:credentials_widgets> |
2021 | + </tr> |
2022 | + </table> |
2023 | + </metal:formbody> |
2024 | + </div> |
2025 | +</div> |
2026 | +</body> |
2027 | +</html> |
2028 | diff --git a/lib/lp/registry/templates/oci-registry-credentials.pt b/lib/lp/registry/templates/oci-registry-credentials.pt |
2029 | new file mode 100644 |
2030 | index 0000000..d4257a0 |
2031 | --- /dev/null |
2032 | +++ b/lib/lp/registry/templates/oci-registry-credentials.pt |
2033 | @@ -0,0 +1,37 @@ |
2034 | +<html |
2035 | + xmlns="http://www.w3.org/1999/xhtml" |
2036 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
2037 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
2038 | + xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
2039 | + metal:use-macro="view/macro:page/main_only" |
2040 | + i18n:domain="launchpad" |
2041 | +> |
2042 | +<body> |
2043 | +<div metal:fill-slot="main"> |
2044 | + <p id="no-participation" tal:condition="not: view/has_credentials"> |
2045 | + <span tal:replace="context/title"/> |
2046 | + has not set any credentials yet. |
2047 | + </p> |
2048 | + <table id="oci-credentials" class="listing" tal:condition="view/has_credentials"> |
2049 | + <thead> |
2050 | + <tr> |
2051 | + <th>Action</th> |
2052 | + <th>Registry URL</th> |
2053 | + <th>Username</th> |
2054 | + </tr> |
2055 | + </thead> |
2056 | + <tbody> |
2057 | + <tr tal:repeat="oci_credentials view/oci_registry_credentials"> |
2058 | + <td> |
2059 | + <tal:editable condition="context/required:launchpad.Edit"> |
2060 | + <a tal:attributes="href context/fmt:url/+edit-oci-registry-credentials">Edit credentials</a> |
2061 | + </tal:editable> |
2062 | + </td> |
2063 | + <td tal:content="oci_credentials/url"/> |
2064 | + <td tal:content="oci_credentials/username"/> |
2065 | + </tr> |
2066 | + </tbody> |
2067 | + </table> |
2068 | +</div> |
2069 | +</body> |
2070 | +</html> |
2071 | diff --git a/lib/lp/registry/templates/person-index.pt b/lib/lp/registry/templates/person-index.pt |
2072 | index 4cfee46..9005140 100644 |
2073 | --- a/lib/lp/registry/templates/person-index.pt |
2074 | +++ b/lib/lp/registry/templates/person-index.pt |
2075 | @@ -82,6 +82,9 @@ |
2076 | tal:define="link context/menu:overview/oauth_tokens" |
2077 | tal:condition="link/enabled" |
2078 | tal:content="structure link/fmt:link" /> |
2079 | + <li |
2080 | + tal:define="link context/menu:overview/oci_registry_credentials" |
2081 | + tal:content="structure link/fmt:link" /> |
2082 | </ul> |
2083 | |
2084 | <div class="yui-g"> |
Thanks for getting started on this. It looks like a workable starting point, but (as you noted) there's a fair bit to clean up.
There are several things that I found myself repeatedly writing throughout this review, so I'm collecting them here at the top to avoid repeating myself too much:
* Essentially everything in Launchpad should be sentence case, not Headline Case; the only exception is button text (see https:/ /dev.launchpad. net/UserInterfa ceWording# Capitalisation). Any time you find yourself writing something like "Edit Push Rules", please change it to "Edit push rules".
* Avoid abbreviations in user-visible text, or (normally) URLs. In particular, write "credentials" rather than "creds".
* In most cases here you should drop the "OCI" qualifier on user-visible text. I realise that in some reviews I've been telling you to add "OCI" and in others I've been telling you to drop it. My rationale here is that we should include it where overall context is necessary or where it would otherwise be ambiguous - especially "OCI project" vs. "project" - but otherwise it isn't needed. If you're editing a recipe for an OCI project, then it's clear that registry credentials are going to be for an OCI registry, and this doesn't need to be spelled out explicitly. The exceptions in this MP are where you're adding/editing OCI registry credentials for a person; in that case, the "OCI" bit doesn't go without saying and should be included.
* Use helpers such as self.assertIn, self.assertEqual, etc. rather than things like self.assertTrue(foo in bar).
I've left a number of other specific comments. I feel this is probably a bit of a first-pass review - the refactorings I've suggested of the views will likely end up needing another full pass, and I'd encourage you to talk those through if you're at all confused about them.