launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:codeimport-url-setters into launchpad:master.

Commit message:
Use a separate setter for URL that calls into updateFromData

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/386241

We want to allow URL on a codeimport to be set by a user with lower permissions than the other attributes, but still keep the same code flow of notifications, format checks etc.

Add an extra setter for URL (and only URL) with the new permissions, then call updateFromData to ensure everything else is correct.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:codeimport-url-setters into launchpad:master.

diff --git a/lib/lp/code/browser/codeimport.py b/lib/lp/code/browser/codeimport.py
index 8516afe..dfee026 100644
--- a/lib/lp/code/browser/codeimport.py
+++ b/lib/lp/code/browser/codeimport.py
@@ -570,9 +570,8 @@ def _makeEditAction(label, status, text):
                     'The code import has been ' + text + '.')
         elif self._is_edit_user and "url" in data:
             # Edit users can only change URL
-            new_url = data["url"]
-            if self.code_import.url != new_url:
-                self.code_import.url = new_url
+            event = self.code_import.updateURL(data["url"], self.user)
+            if event is not None:
                 self.request.response.addNotification(
                     'The code import URL has been updated.')
         else:
diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
index 09d5011..898e645 100644
--- a/lib/lp/code/configure.zcml
+++ b/lib/lp/code/configure.zcml
@@ -658,7 +658,8 @@
                        getImportDetailsForDisplay"/>
     <require
        permission="launchpad.Edit"
-       set_attributes="url"/>
+       attributes="updateURL"/>
+
     <require
        permission="launchpad.AnyPerson"
        attributes="tryFailingImportAgain
diff --git a/lib/lp/code/doc/codeimport.txt b/lib/lp/code/doc/codeimport.txt
index 6d6dc3e..feea404 100644
--- a/lib/lp/code/doc/codeimport.txt
+++ b/lib/lp/code/doc/codeimport.txt
@@ -497,7 +497,7 @@ The launchpad.Edit privilege is required to use CodeImport.updateFromData.
     >>> svn_import.updateFromData({}, nopriv)
     Traceback (most recent call last):
     ...
-    Unauthorized: (<CodeImport ...>, 'updateFromData', 'launchpad.Edit')
+    Unauthorized: (<CodeImport ...>, 'updateFromData', 'launchpad.Moderate')
 
 We saw above how changes to SVN details are displayed in emails above.
 CVS details are displayed in a similar way.
diff --git a/lib/lp/code/interfaces/codeimport.py b/lib/lp/code/interfaces/codeimport.py
index 7386e6b..3a54c3a 100644
--- a/lib/lp/code/interfaces/codeimport.py
+++ b/lib/lp/code/interfaces/codeimport.py
@@ -192,6 +192,17 @@ class ICodeImport(Interface):
             None if no changes were made.
         """
 
+    def updateURL(new_url, user):
+        """Update the URL for this `CodeImport`.
+
+        A separate setter as it has lower permissions than updateFromData.
+        :param new_url: string of the proposed new URL.
+        :param user: user who made the change, to record in the
+            `CodeImportEvent`.  May be ``None``.
+        :return: The MODIFY `CodeImportEvent`, if any changes were made, or
+            None if no changes were made.
+        """
+
     def tryFailingImportAgain(user):
         """Try a failing import again.
 
diff --git a/lib/lp/code/model/codeimport.py b/lib/lp/code/model/codeimport.py
index eab7d42..95d0b99 100644
--- a/lib/lp/code/model/codeimport.py
+++ b/lib/lp/code/model/codeimport.py
@@ -250,6 +250,12 @@ class CodeImport(StormBase):
             code_import_updated(self, event, new_whiteboard, user)
         return event
 
+    def updateURL(self, new_url, user):
+        if self.url != new_url:
+            data = {"url": new_url}
+            event = self.updateFromData(data, user)
+            return event
+
     def __repr__(self):
         return "<CodeImport for %s>" % self.target.unique_name