launchpad-reviewers team mailing list archive

[Ines Almeida <mp+459182@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:social-accounts-generalise-UI into launchpad:master.

Commit message:
ui: generalize edit matrix accounts page
    
This will make it much more straight-forward to add new social platforms as we won't need different templates for different platforms

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/459182

UI tests re-run and don't seem to need any change, as the template remains the same.

Also adding a quick `SOCIAL_PLATFORM_TYPES_MAP` to the `__all__` due to a warning message when running tests.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:social-accounts-generalise-UI into launchpad:master.

diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index 35fe1d3..970fd02 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -1223,7 +1223,7 @@
         for="lp.registry.interfaces.person.IPerson"
         class="lp.registry.browser.person.PersonEditMatrixAccountsView"
         permission="launchpad.Edit"
-        template="../templates/person-editmatrixaccounts.pt"
+        template="../templates/person-editsocialaccounts.pt"
         />
     <browser:page
         name="+editjabberids"
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index de0003d..ae8b415 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -2458,12 +2458,9 @@ class PersonEditIRCNicknamesView(LaunchpadFormView):
         self.next_url = canonical_url(self.context)
 
 
-class PersonEditMatrixAccountsView(LaunchpadFormView):
-    # TODO: have a look into generalising this view and the relevant template
-    # (`person-editmatrixaccounts.pt`) for any social platform
-
+class PersonEditSocialAccountsView(LaunchpadFormView):
     schema = Interface
-    platform = MatrixPlatform
+    platform = None
 
     @property
     def page_title(self):
@@ -2473,6 +2470,7 @@ class PersonEditMatrixAccountsView(LaunchpadFormView):
 
     label = page_title
     next_url = None
+    new_identity = dict()
 
     @property
     def cancel_url(self):
@@ -2484,6 +2482,17 @@ class PersonEditMatrixAccountsView(LaunchpadFormView):
             platform=self.platform.platform_type
         )
 
+    @property
+    def identity_headers(self):
+        return [item.capitalize() for item in self.platform.identity_fields]
+
+    @property
+    def identity_examples(self):
+        return [
+            self.platform.identity_fields_example[field]
+            for field in self.platform.identity_fields
+        ]
+
     @action(_("Save Changes"), name="save")
     def save(self, action, data):
         """Process the social accounts form."""
@@ -2517,8 +2526,7 @@ class PersonEditMatrixAccountsView(LaunchpadFormView):
             try:
                 self.platform.validate_identity(new_account_identity)
             except SocialAccountIdentityError as e:
-                for field_key, field_value in new_account_identity.items():
-                    self.__setattr__(f"new_{field_key}", field_value)
+                self.__setattr__("new_identity", new_account_identity)
                 self.request.response.addErrorNotification(e)
                 return
 
@@ -2535,6 +2543,10 @@ class PersonEditMatrixAccountsView(LaunchpadFormView):
         )
 
 
+class PersonEditMatrixAccountsView(PersonEditSocialAccountsView):
+    platform = MatrixPlatform
+
+
 class PersonEditJabberIDsView(LaunchpadFormView):
     schema = IJabberID
     field_names = ["jabberid"]
diff --git a/lib/lp/registry/interfaces/socialaccount.py b/lib/lp/registry/interfaces/socialaccount.py
index 94e8d78..2c351f2 100644
--- a/lib/lp/registry/interfaces/socialaccount.py
+++ b/lib/lp/registry/interfaces/socialaccount.py
@@ -10,6 +10,7 @@ __all__ = [
     "SocialPlatformType",
     "SocialAccountIdentityError",
     "validate_social_account_identity",
+    "SOCIAL_PLATFORM_TYPES_MAP",
 ]
 
 import http.client
@@ -105,6 +106,7 @@ class ISocialAccountSet(Interface):
 class SocialPlatform:
     title = ""
     identity_fields = []
+    identity_fields_example = {}
     platform_type = None
     icon = ""
     display_format = ""
@@ -119,6 +121,10 @@ class SocialPlatform:
 class MatrixPlatform(SocialPlatform):
     title = "Matrix"
     identity_fields = ["username", "homeserver"]
+    identity_fields_example = {
+        "username": "mark",
+        "homeserver": "ubuntu.com",
+    }
     platform_type = SocialPlatformType.MATRIX
     icon = "social-matrix"
     display_format = "<strong>@{username}:{homeserver}</strong>"
diff --git a/lib/lp/registry/templates/person-editmatrixaccounts.pt b/lib/lp/registry/templates/person-editsocialaccounts.pt
similarity index 70%
rename from lib/lp/registry/templates/person-editmatrixaccounts.pt
rename to lib/lp/registry/templates/person-editsocialaccounts.pt
index 66ec724..486ab2d 100644
--- a/lib/lp/registry/templates/person-editmatrixaccounts.pt
+++ b/lib/lp/registry/templates/person-editsocialaccounts.pt
@@ -18,24 +18,18 @@
     <table>
 
       <tr>
-        <td><label>Homeserver</label></td>
-        <td><label>Username</label></td>
+        <td tal:repeat="header view/identity_headers">
+          <label tal:content="header"></label>
+        </td>
       </tr>
 
       <tr tal:repeat="social_account view/existing_accounts">
-        <td>
-          <input type="text"
-                 class="field_homeserver"
-                 tal:attributes="name string:homeserver_${social_account/id};
-                                 value social_account/identity/homeserver" />
-        </td>
-        <td>
+        <td tal:repeat="field view/platform/identity_fields">
           <input type="text"
-                 class="field_username"
-                 tal:attributes="name string:username_${social_account/id};
-                                 value social_account/identity/username" />
+                 tal:attributes="name string:${field}_${social_account/id};
+                                 value python:social_account.identity[field];
+                                 class string:field_${field}" />
         </td>
-
         <td>
           <label>
             <input type="checkbox"
@@ -47,23 +41,16 @@
       </tr>
 
       <tr>
-        <td>
-          <input name="new_homeserver"
-                 type="text"
-                 placeholder="Enter new homeserver"
-                 tal:attributes="value view/new_homeserver|nothing" />
-        </td>
-        <td>
-          <input name="new_username"
-                 type="text"
-                 placeholder="Enter new username"
-                 tal:attributes="value view/new_username|nothing" />
+        <td tal:repeat="field view/platform/identity_fields">
+          <input type="text"
+                 tal:attributes="value python:view.new_identity.get(field);
+                                 name string:new_${field};
+                                 placeholder string:Enter new ${field}" />
         </td>
       </tr>
 
       <tr>
-        <td class="formHelp">Example: ubuntu.com</td>
-        <td class="formHelp">Example: mark</td>
+        <td tal:repeat="example view/identity_examples" tal:content="string:Example: ${example}"></td>
       </tr>
     </table>
   </div>