launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25176
[Merge] ~cjwatson/launchpad:oci-registry-credentials-owner into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:oci-registry-credentials-owner into launchpad:master.
Commit message:
Allow setting owner when adding OCI registry credentials
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/389418
This also properly defaults the owner to the team when editing credentials for a team, rather than to the logged-in user.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:oci-registry-credentials-owner into launchpad:master.
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index 5ee1b57..96ffbe9 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -3676,16 +3676,19 @@ class PersonOCIRegistryCredentialsView(LaunchpadView):
class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"""View for Person:+edit-oci-registry-credentials."""
- @cachedproperty
- def oci_registry_credentials(self):
+ @property
+ def default_owner(self):
if IPerson.providedBy(self.context):
- owner = self.context
+ return self.context
elif IOCIRecipe.providedBy(self.context):
- owner = self.context.owner
+ return self.context.owner
else:
raise ValueError("Invalid context for this view")
- return list(getUtility(IOCIRegistryCredentialsSet).findByOwner(owner))
+ @cachedproperty
+ def oci_registry_credentials(self):
+ return list(getUtility(IOCIRegistryCredentialsSet).findByOwner(
+ self.default_owner))
schema = Interface
@@ -3748,6 +3751,12 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
title=u'Registry URL',
__name__=u'add_url',
required=False, readonly=False)
+ add_owner = Choice(
+ __name__=u'add_owner',
+ vocabulary=(
+ 'AllUserTeamsParticipationPlusSelfSimpleDisplay'),
+ default=self.default_owner,
+ required=False, readonly=False)
add_username = TextLine(
title=u'Username',
__name__=u'add_username',
@@ -3761,7 +3770,9 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
__name__=u'add_confirm_password',
required=False, readonly=False)
- return add_url, add_username, add_password, add_confirm_password
+ return (
+ add_url, add_owner, add_username,
+ add_password, add_confirm_password)
def _parseFieldName(self, field_name):
"""Parse a combined field name as described in `_getFieldName`.
@@ -3827,6 +3838,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"""Rearrange form data to make it easier to process."""
parsed_data = {}
add_url = data["add_url"]
+ add_owner = data["add_owner"]
add_username = data["add_username"]
add_password = data["add_password"]
add_confirm_password = data["add_confirm_password"]
@@ -3836,6 +3848,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"password": add_password,
"confirm_password": add_confirm_password,
"url": add_url,
+ "owner": add_owner,
"action": "add",
})
for field_name in (
@@ -3903,13 +3916,8 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
credentials.destroySelf()
def addCredentials(self, parsed_add_credentials):
- if IPerson.providedBy(self.context):
- owner = self.context
- elif IOCIRecipe.providedBy(self.context):
- owner = self.context.owner
- else:
- raise ValueError("Invalid context for this view")
url = parsed_add_credentials["url"]
+ owner = parsed_add_credentials["owner"]
password = parsed_add_credentials["password"]
confirm_password = parsed_add_credentials["confirm_password"]
username = parsed_add_credentials["username"]
diff --git a/lib/lp/registry/browser/tests/test_person.py b/lib/lp/registry/browser/tests/test_person.py
index c7167bd..950f466 100644
--- a/lib/lp/registry/browser/tests/test_person.py
+++ b/lib/lp/registry/browser/tests/test_person.py
@@ -22,10 +22,13 @@ from testscenarios import (
WithScenarios,
)
from testtools.matchers import (
+ AfterPreprocessing,
DocTestMatches,
Equals,
LessThan,
+ MatchesAll,
MatchesDict,
+ MatchesSetwise,
MatchesStructure,
Not,
)
@@ -1319,6 +1322,21 @@ class TestPersonRelatedProjectsView(TestCaseWithFactory):
self.assertThat(view(), next_match)
+class MatchesOCIRegistryCredentials(MatchesAll):
+ """Matches an `OCIRegistryCredentials` object.
+
+ `main_matcher` matches the `OCIRegistryCredentials` object itself, while
+ `credentials_matcher` matches the result of its `getCredentials` method.
+ """
+
+ def __init__(self, main_matcher, credentials_matcher):
+ super(MatchesOCIRegistryCredentials, self).__init__(
+ main_matcher,
+ AfterPreprocessing(
+ lambda matchee: removeSecurityProxy(matchee).getCredentials(),
+ credentials_matcher))
+
+
class TestPersonOCIRegistryCredentialsView(
WithScenarios, BrowserTestCase, OCIConfigHelperMixin):
@@ -1359,10 +1377,10 @@ class TestPersonOCIRegistryCredentialsView(
view = create_initialized_view(
self.owner, '+oci-registry-credentials', principal=self.user)
self.assertEqual('OCI registry credentials', view.page_title)
- self.assertEqual(
- credentials.get('username'),
- view.oci_registry_credentials[0].getCredentials()['username'])
- self.assertEqual(url, view.oci_registry_credentials[0].url)
+ self.assertThat(view.oci_registry_credentials, MatchesSetwise(
+ MatchesOCIRegistryCredentials(
+ MatchesStructure.byEquality(owner=self.owner, url=url),
+ Equals(credentials))))
def test_edit_oci_registry_credentials(self):
url = unicode(self.factory.getUniqueURL())
@@ -1383,13 +1401,13 @@ class TestPersonOCIRegistryCredentialsView(
username_control.value = 'different_username'
browser.getControl("Save").click()
with person_logged_in(self.user):
- self.assertThat(registry_credentials, MatchesStructure.byEquality(
- owner=self.owner, url=url))
self.assertThat(
- registry_credentials.getCredentials(),
- MatchesDict(
- {"username": Equals("different_username"),
- "password": Equals("bar")}))
+ registry_credentials, MatchesOCIRegistryCredentials(
+ MatchesStructure.byEquality(owner=self.owner, url=url),
+ MatchesDict({
+ "username": Equals("different_username"),
+ "password": Equals("bar"),
+ })))
# change only the registry url
browser = self.getViewBrowser(
@@ -1400,13 +1418,13 @@ class TestPersonOCIRegistryCredentialsView(
url_control.value = newurl
browser.getControl("Save").click()
with person_logged_in(self.user):
- self.assertThat(registry_credentials, MatchesStructure.byEquality(
- owner=self.owner, url=newurl))
self.assertThat(
- registry_credentials.getCredentials(),
- MatchesDict(
- {"username": Equals("different_username"),
- "password": Equals("bar")}))
+ registry_credentials, MatchesOCIRegistryCredentials(
+ MatchesStructure.byEquality(owner=self.owner, url=newurl),
+ MatchesDict({
+ "username": Equals("different_username"),
+ "password": Equals("bar"),
+ })))
# change only the password
browser = self.getViewBrowser(
@@ -1435,13 +1453,14 @@ class TestPersonOCIRegistryCredentialsView(
confirm_password_control.value = 'third_newpassword'
browser.getControl("Save").click()
with person_logged_in(self.user):
- self.assertThat(registry_credentials, MatchesStructure.byEquality(
- owner=self.owner, url=third_url))
self.assertThat(
- registry_credentials.getCredentials(),
- MatchesDict(
- {"username": Equals("third_different_username"),
- "password": Equals("third_newpassword")}))
+ registry_credentials, MatchesOCIRegistryCredentials(
+ MatchesStructure.byEquality(
+ owner=self.owner, url=third_url),
+ MatchesDict({
+ "username": Equals("third_different_username"),
+ "password": Equals("third_newpassword"),
+ })))
def test_add_oci_registry_credentials(self):
url = unicode(self.factory.getUniqueURL())
@@ -1453,12 +1472,18 @@ class TestPersonOCIRegistryCredentialsView(
recipe=self.recipe,
registry_credentials=registry_credentials,
image_name=image_name)
+ owner_name = self.owner.name
+ new_owner = self.factory.makeTeam(members=[self.user])
+ new_owner_name = new_owner.name
browser = self.getViewBrowser(
self.owner, view_name='+oci-registry-credentials', user=self.user)
browser.getLink("Edit OCI registry credentials").click()
+ self.assertEqual(
+ [owner_name], browser.getControl(name="field.add_owner").value)
browser.getControl(name="field.add_url").value = url
+ browser.getControl(name="field.add_owner").value = [new_owner_name]
browser.getControl(name="field.add_username").value = "new_username"
browser.getControl(name="field.add_password").value = "password"
browser.getControl(
@@ -1466,13 +1491,24 @@ class TestPersonOCIRegistryCredentialsView(
browser.getControl("Save").click()
with person_logged_in(self.user):
- creds = list(
- getUtility(IOCIRegistryCredentialsSet).findByOwner(self.owner))
- self.assertEqual(url, creds[1].url)
self.assertThat(
- removeSecurityProxy(creds[1]).getCredentials(),
- MatchesDict({"username": Equals("new_username"),
- "password": Equals("password")}))
+ getUtility(IOCIRegistryCredentialsSet).findByOwner(self.owner),
+ MatchesSetwise(
+ MatchesOCIRegistryCredentials(
+ MatchesStructure.byEquality(owner=self.owner, url=url),
+ MatchesDict({
+ "username": Equals("foo"),
+ "password": Equals("bar"),
+ }))))
+ self.assertThat(
+ getUtility(IOCIRegistryCredentialsSet).findByOwner(new_owner),
+ MatchesSetwise(
+ MatchesOCIRegistryCredentials(
+ MatchesStructure.byEquality(owner=new_owner, url=url),
+ MatchesDict({
+ "username": Equals("new_username"),
+ "password": Equals("password"),
+ }))))
def test_delete_oci_registry_credentials(self):
# Test that we do not delete credentials when there are
diff --git a/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt b/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
index 89483da..554372f 100644
--- a/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
+++ b/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
@@ -40,6 +40,9 @@
<td tal:define="widget nocall:view/widgets/add_url">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
</td>
+ <td tal:define="widget nocall:view/widgets/add_owner">
+ <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+ </td>
<td tal:define="widget nocall:view/widgets/add_username">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
</td>
