Launchpad itself

Merge ~ines-almeida/launchpad:social-accounts-edit-view into launchpad:master

  1. Git
  2. lp:~ines-almeida/launchpad
  3. social-accounts-edit-view
  4. Merge into master
Proposed by Ines Almeida
Status: Merged
Approved by: Ines Almeida
Approved revision: 76e4ff6d3470c39ff3fdb3b7e04968a75a3c9e62
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ines-almeida/launchpad:social-accounts-edit-view
Merge into: launchpad:master
Prerequisite: ~pelpsi/launchpad:social-account-interface-and-implementation
Diff against target: 363 lines (+284/-5)
4 files modified
lib/lp/registry/browser/configure.zcml (+7/-0)
lib/lp/registry/browser/person.py (+95/-5)
lib/lp/registry/stories/person/xx-person-edit-matrix-accounts.rst (+108/-0)
lib/lp/registry/templates/person-editmatrixaccounts.pt (+74/-0)
Reviewer Review Type Date Requested Status
Guruprasad Approve
Simone Pelosi Approve
Review via email: mp+458537@code.launchpad.net

Commit message

ui: Add edit matrix accounts view

Description of the change

This adds a new view: https://launchpad.net/~<username>/+editmatrixaccounts

This view allows users to add, edit or remove their matrix accounts.

(Update to view the matrix accounts in the profile will go in a separate MP)

To post a comment you must log in.
Revision history for this message
Ines Almeida (ines-almeida) :
Revision history for this message
Simone Pelosi (pelpsi) :
Revision history for this message
Ines Almeida (ines-almeida) wrote :

Conflicts are due to the fact that this branch was rebased with master and the Social Accounts API branch (https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/457527).

Will wait for API branch to be merged in before fixing conflicts. Setting this to "WIP" in the meantime

Revision history for this message
Ines Almeida (ines-almeida) wrote :

Rebased with the latest changes from the API branch (mentioned in the comment above), and fixed all conflicts.

Added a small TODO for me to handle tomorrow, but should be ready for review.

I made the `PersonEditMatrixAccountsView` in such a way that it should be quite simple to make it generic, but also there is no need for that now, specially since the template isn't generic as it is.

~ines-almeida/launchpad:social-accounts-edit-view updated
1b95067... by Ines Almeida

ui: rename URL for matrix form

be01687... by Ines Almeida

ui: Improve matrix form template and view; minor fix to IRC view

Revision history for this message
Guruprasad (lgp171188) wrote :

> This adds a new view: https://launchpad.net/~<username>/+editsocialaccounts-matrix

Don't forget to update the URL in this MP's commit message before merging. :)

Revision history for this message
Guruprasad (lgp171188) wrote :

Left a few minor comments/questions/suggestions. Will approve once they have been addressed. Nice work!

Revision history for this message
Guruprasad (lgp171188) wrote :

Don't forget to add appropriate tests!

~ines-almeida/launchpad:social-accounts-edit-view updated
3ea706b... by Simone Pelosi

Addressed comment: improved input validation

Revision history for this message
Ines Almeida (ines-almeida) :
Revision history for this message
Ines Almeida (ines-almeida) wrote (last edit ):

Given we don't want to add new `rst` tests, I can't base the tests on other social accounts, which means it will take a little longer to create them. I can focus on that tomorrow...

I did test the UI changes manually

~ines-almeida/launchpad:social-accounts-edit-view updated
dbae218... by Ines Almeida

ui: added notifications and validation to matrix edit view (plus minor style fix)

17f2089... by Simone Pelosi

Using instead of

755b261... by Simone Pelosi

Addressed comment: added unittest for `socialaccount`

Revision history for this message
Guruprasad (lgp171188) wrote :

> Given we don't want to add new `rst` tests, I can't base the tests on other social accounts, which means it will take a little longer to create them. I can focus on that tomorrow...

For browser and UI changes, I'd say that it is an okay compromise to write doctests unless the alternative can be implemented as fast.

Revision history for this message
Guruprasad (lgp171188) :
~ines-almeida/launchpad:social-accounts-edit-view updated
91b997c... by Simone Pelosi

Comment addressed: improved regex for validation

Added tests for new validation, added matrix account attributes
to the identity documentation.

df22e99... by Simone Pelosi

Added username validator

198d61a... by Simone Pelosi

Improving username validation

189d5b4... by Ines Almeida

ui: small wording update in social accounts ui

e8cf65a... by Ines Almeida

tests: add tests for matrix edit page

76e4ff6... by Ines Almeida

Minor conflict fix after conflict accounts API merge

Revision history for this message
Ines Almeida (ines-almeida) wrote :

Added tests and rebased branch from master after API branch got merged

Revision history for this message
Simone Pelosi (pelpsi) wrote :

LGTM!

review: Approve
Revision history for this message
Guruprasad (lgp171188) wrote :

LGTM 👍 Left a suggestion on a couple of scenarios to write tests for.

review: Approve
Revision history for this message
Ines Almeida (ines-almeida) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
2index 046db06..35fe1d3 100644
3--- a/lib/lp/registry/browser/configure.zcml
4+++ b/lib/lp/registry/browser/configure.zcml
5@@ -1219,6 +1219,13 @@
6 template="../templates/person-editircnicknames.pt"
7 />
8 <browser:page
9+ name="+editmatrixaccounts"
10+ for="lp.registry.interfaces.person.IPerson"
11+ class="lp.registry.browser.person.PersonEditMatrixAccountsView"
12+ permission="launchpad.Edit"
13+ template="../templates/person-editmatrixaccounts.pt"
14+ />
15+ <browser:page
16 name="+editjabberids"
17 for="lp.registry.interfaces.person.IPerson"
18 class="lp.registry.browser.person.PersonEditJabberIDsView"
19diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
20index 8f044fd..be1ba98 100644
21--- a/lib/lp/registry/browser/person.py
22+++ b/lib/lp/registry/browser/person.py
23@@ -19,6 +19,7 @@ __all__ = [
24 "PersonEditEmailsView",
25 "PersonEditIRCNicknamesView",
26 "PersonEditJabberIDsView",
27+ "PersonEditMatrixAccountsView",
28 "PersonEditTimeZoneView",
29 "PersonEditSSHKeysView",
30 "PersonEditView",
31@@ -152,7 +153,11 @@ from lp.registry.interfaces.persontransferjob import (
32 from lp.registry.interfaces.pillar import IPillarNameSet
33 from lp.registry.interfaces.poll import IPollSubset
34 from lp.registry.interfaces.product import InvalidProductName, IProduct
35-from lp.registry.interfaces.socialaccount import ISocialAccountSet
36+from lp.registry.interfaces.socialaccount import (
37+ ISocialAccountSet,
38+ MatrixPlatform,
39+ SocialAccountIdentityError,
40+)
41 from lp.registry.interfaces.ssh import ISSHKeySet, SSHKeyAdditionError
42 from lp.registry.interfaces.teammembership import (
43 ITeamMembershipSet,
44@@ -824,6 +829,7 @@ class PersonOverviewMenu(
45 "editmailinglists",
46 "editircnicknames",
47 "editjabberids",
48+ "editmatrixaccounts",
49 "editsshkeys",
50 "editpgpkeys",
51 "editlocation",
52@@ -889,6 +895,12 @@ class PersonOverviewMenu(
53 return Link(target, text, icon="edit", summary=text)
54
55 @enabled_with_permission("launchpad.Edit")
56+ def editmatrixaccounts(self):
57+ target = "+editmatrixaccounts"
58+ text = "Edit Matrix accounts"
59+ return Link(target, text, icon="edit", summary=text)
60+
61+ @enabled_with_permission("launchpad.Edit")
62 def editjabberids(self):
63 target = "+editjabberids"
64 text = "Update Jabber IDs"
65@@ -2372,15 +2384,12 @@ class PersonEditIRCNicknamesView(LaunchpadFormView):
66 return smartquote("%s's IRC nicknames" % self.context.displayname)
67
68 label = page_title
69+ next_url = None
70
71 @property
72 def cancel_url(self):
73 return canonical_url(self.context)
74
75- @property
76- def next_url(self):
77- return canonical_url(self.context)
78-
79 @action(_("Save Changes"), name="save")
80 def save(self, action, data):
81 """Process the IRC nicknames form."""
82@@ -2417,6 +2426,87 @@ class PersonEditIRCNicknamesView(LaunchpadFormView):
83 self.request.response.addErrorNotification(
84 "Neither Nickname nor Network can be empty."
85 )
86+ return
87+
88+ # If we there were no errors, return user to profile page
89+ self.next_url = canonical_url(self.context)
90+
91+
92+class PersonEditMatrixAccountsView(LaunchpadFormView):
93+ # TODO: have a look into generalising this view and the relevant template
94+ # (`person-editmatrixaccounts.pt`) for any social platform
95+
96+ schema = Interface
97+ platform = MatrixPlatform
98+
99+ @property
100+ def page_title(self):
101+ return smartquote(
102+ f"{self.context.displayname}'s {self.platform.title} accounts"
103+ )
104+
105+ label = page_title
106+ next_url = None
107+
108+ @property
109+ def cancel_url(self):
110+ return canonical_url(self.context)
111+
112+ @property
113+ def existing_accounts(self):
114+ return self.context.getSocialAccountsByPlatform(
115+ platform=self.platform.platform_type
116+ )
117+
118+ @action(_("Save Changes"), name="save")
119+ def save(self, action, data):
120+ """Process the social accounts form."""
121+ form = self.request.form
122+
123+ # Update or remove existing accounts
124+ for social_account in self.existing_accounts:
125+ if form.get(f"remove_{social_account.id}"):
126+ social_account.destroySelf()
127+
128+ else:
129+ updated_identity = {
130+ field: form.get(f"{field}_{social_account.id}")
131+ for field in self.platform.identity_fields
132+ }
133+ try:
134+ self.platform.validate_identity(updated_identity)
135+ except SocialAccountIdentityError as e:
136+ self.request.response.addErrorNotification(e)
137+ return
138+
139+ social_account.identity = updated_identity
140+
141+ # Add new account
142+ new_account_identity = {
143+ field_name: form.get(f"new_{field_name}")
144+ for field_name in self.platform.identity_fields
145+ }
146+
147+ if any(new_account_identity.values()):
148+ try:
149+ self.platform.validate_identity(new_account_identity)
150+ except SocialAccountIdentityError as e:
151+ for field_key, field_value in new_account_identity.items():
152+ self.__setattr__(f"new_{field_key}", field_value)
153+ self.request.response.addErrorNotification(e)
154+ return
155+
156+ getUtility(ISocialAccountSet).new(
157+ person=self.context,
158+ platform=self.platform.platform_type,
159+ identity=new_account_identity,
160+ )
161+
162+ # If we there were no errors, return user to profile page
163+ self.next_url = canonical_url(self.context)
164+ self.request.response.addNotification(
165+ f"{self.platform.title} accounts saved successfully."
166+ )
167
168
169 class PersonEditJabberIDsView(LaunchpadFormView):
170diff --git a/lib/lp/registry/stories/person/xx-person-edit-matrix-accounts.rst b/lib/lp/registry/stories/person/xx-person-edit-matrix-accounts.rst
171new file mode 100644
172index 0000000..df4a893
173--- /dev/null
174+++ b/lib/lp/registry/stories/person/xx-person-edit-matrix-accounts.rst
175@@ -0,0 +1,108 @@
176+===============
177+Matrix Accounts
178+===============
179+
180+
181+In their Launchpad profile, users can register their Matrix accounts.
182+
183+Adding a new account
184+--------------------
185+
186+To register a Matrix account with their account, the user visits their
187+profile page and uses the 'Edit Matrix accounts' link.
188+
189+ >>> def go_to_edit_matrix_accounts_page(browser):
190+ ... browser.open("http://launchpad.test/~no-priv/+editmatrixaccounts")
191+ ...
192+
193+ >>> go_to_edit_matrix_accounts_page(user_browser)
194+ >>> print(user_browser.title)
195+ No Privileges Person's Matrix...
196+
197+The user enters the username and homeserver combination in the text inputs
198+and clicks on the 'Save Changes' button.
199+
200+ >>> def add_matrix_account(browser, username, homeserver):
201+ ... browser.getControl(name="new_homeserver").value = homeserver
202+ ... browser.getControl(name="new_username").value = username
203+ ... browser.getControl("Save Changes").click()
204+ ...
205+
206+ >>> add_matrix_account(user_browser, "mark", "ubuntu.com")
207+ >>> def show_notifications(browser, type):
208+ ... for notification in find_tags_by_class(browser.contents, type):
209+ ... print(extract_text(notification))
210+ ...
211+
212+ >>> show_notifications(user_browser, "informational")
213+ Matrix accounts saved successfully.
214+
215+In this case, the user tried registering a Matrix account with invalid
216+usernames or homeservers, an error is displayed and the user can enter
217+another one:
218+ >>> go_to_edit_matrix_accounts_page(user_browser)
219+ >>> add_matrix_account(user_browser, "<b>mark</b>", "ubuntu.com")
220+ >>> show_notifications(user_browser, "error")
221+ Username must be valid.
222+
223+User remains on the edit Matrix accounts page, and can reenter the username and
224+homeserver, but none should be empty:
225+ >>> print(user_browser.title)
226+ No Privileges Person's Matrix...
227+
228+ >>> add_matrix_account(user_browser, "", "ubuntu.com")
229+ >>> show_notifications(user_browser, "error")
230+ You must provide the following fields: username, homeserver.
231+
232+
233+Editing an account
234+-------------------
235+
236+To edit an existing Matrix account, the user can update the corresponding
237+input fields:
238+
239+ >>> def edit_existing_matrix_account(
240+ ... browser, index, username, homeserver
241+ ... ):
242+ ... main_content = find_tag_by_id(browser.contents, "maincontent")
243+ ... table_row = main_content.find_all("tr")[index + 1]
244+ ...
245+ ... homeserver_input = table_row.find(
246+ ... "input", {"class": "field_homeserver"}
247+ ... ).attrs["name"]
248+ ... username_input = table_row.find(
249+ ... "input", {"class": "field_username"}
250+ ... ).attrs["name"]
251+ ...
252+ ... browser.getControl(name=homeserver_input).value = homeserver
253+ ... browser.getControl(name=username_input).value = username
254+ ... browser.getControl("Save Changes").click()
255+
256+ >>> go_to_edit_matrix_accounts_page(user_browser)
257+ >>> edit_existing_matrix_account(user_browser, 0, "fred", "test.com")
258+ >>> show_notifications(user_browser, "informational")
259+ Matrix accounts saved successfully.
260+
261+Edited fields will also show an error if not valid:
262+
263+ >>> go_to_edit_matrix_accounts_page(user_browser)
264+ >>> edit_existing_matrix_account(user_browser, 0, "fred", "<server>")
265+ >>> show_notifications(user_browser, "error")
266+ Homeserver must be a valid domain.
267+
268+ >>> edit_existing_matrix_account(user_browser, 0, "&fred", "test.com")
269+ >>> show_notifications(user_browser, "error")
270+ Username must be valid.
271+
272+
273+Removing an account
274+-------------------
275+
276+To remove an existing Matrix account, the user simply checks the 'Remove'
277+checkbox besides the ID:
278+ >>> go_to_edit_matrix_accounts_page(user_browser)
279+ >>> user_browser.getControl("Remove", index=0).click()
280+ >>> user_browser.getControl("Save Changes").click()
281+
282+ >>> show_notifications(user_browser, "informational")
283+ Matrix accounts saved successfully.
284diff --git a/lib/lp/registry/templates/person-editmatrixaccounts.pt b/lib/lp/registry/templates/person-editmatrixaccounts.pt
285new file mode 100644
286index 0000000..66ec724
287--- /dev/null
288+++ b/lib/lp/registry/templates/person-editmatrixaccounts.pt
289@@ -0,0 +1,74 @@
290+<html
291+ xmlns="http://www.w3.org/1999/xhtml"
292+ xmlns:tal="http://xml.zope.org/namespaces/tal"
293+ xmlns:metal="http://xml.zope.org/namespaces/metal"
294+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
295+ metal:use-macro="view/macro:page/main_only"
296+ i18n:domain="launchpad"
297+>
298+<body>
299+
300+<div metal:fill-slot="main">
301+<div metal:use-macro="context/@@launchpad_form/form">
302+ <div metal:fill-slot="widgets">
303+
304+ <p tal:condition="view/error_message"
305+ tal:content="structure view/error_message/escapedtext" class="error message" />
306+
307+ <table>
308+
309+ <tr>
310+ <td><label>Homeserver</label></td>
311+ <td><label>Username</label></td>
312+ </tr>
313+
314+ <tr tal:repeat="social_account view/existing_accounts">
315+ <td>
316+ <input type="text"
317+ class="field_homeserver"
318+ tal:attributes="name string:homeserver_${social_account/id};
319+ value social_account/identity/homeserver" />
320+ </td>
321+ <td>
322+ <input type="text"
323+ class="field_username"
324+ tal:attributes="name string:username_${social_account/id};
325+ value social_account/identity/username" />
326+ </td>
327+
328+ <td>
329+ <label>
330+ <input type="checkbox"
331+ value="Remove"
332+ tal:attributes="name string:remove_${social_account/id}" />
333+ Remove
334+ </label>
335+ </td>
336+ </tr>
337+
338+ <tr>
339+ <td>
340+ <input name="new_homeserver"
341+ type="text"
342+ placeholder="Enter new homeserver"
343+ tal:attributes="value view/new_homeserver|nothing" />
344+ </td>
345+ <td>
346+ <input name="new_username"
347+ type="text"
348+ placeholder="Enter new username"
349+ tal:attributes="value view/new_username|nothing" />
350+ </td>
351+ </tr>
352+
353+ <tr>
354+ <td class="formHelp">Example: ubuntu.com</td>
355+ <td class="formHelp">Example: mark</td>
356+ </tr>
357+ </table>
358+ </div>
359+</div>
360+</div>
361+
362+</body>
363+</html>

Subscribers

People subscribed via source and target branches

to status/vote changes: