mozreview: people besides review requesters should be able autoland (bug 1205018) r?mdoglio draft
authorDan Minor <dminor@mozilla.com>
Fri, 18 Dec 2015 15:12:07 -0500
changeset 6657 1251be47b063e489437a775fb44824fd73f30ea7
parent 6656 0b17364db7ecdaa9628441ad898964073e7b122d
push id508
push userdminor@mozilla.com
push dateMon, 11 Jan 2016 16:02:11 +0000
reviewersmdoglio
bugs1205018
mozreview: people besides review requesters should be able autoland (bug 1205018) r?mdoglio This removes the check for whether or not the review is mutable by the current user, which allows people other than the review requester to autoland provided they have sufficient privileges.
pylib/mozreview/mozreview/autoland/resources.py
pylib/mozreview/mozreview/static/mozreview/js/autoland.js
pylib/mozreview/mozreview/tests/test-autoland-try.py
--- a/pylib/mozreview/mozreview/autoland/resources.py
+++ b/pylib/mozreview/mozreview/autoland/resources.py
@@ -98,17 +98,17 @@ class BaseAutolandTriggerResource(WebAPI
 
 class AutolandTriggerResource(BaseAutolandTriggerResource):
     """Resource to kick off Autoland to inbound for a particular review request."""
 
     name = 'autoland_trigger'
 
     @webapi_login_required
     @webapi_response_errors(DOES_NOT_EXIST, INVALID_FORM_DATA,
-                            NOT_LOGGED_IN, PERMISSION_DENIED)
+                            NOT_LOGGED_IN)
     @webapi_scm_groups_required('scm_level_3')
     @webapi_request_fields(
         required={
             'review_request_id': {
                 'type': int,
                 'description': 'The review request for which to trigger a Try '
                                'build',
             },
@@ -128,19 +128,16 @@ class AutolandTriggerResource(BaseAutola
             return DOES_NOT_EXIST
 
         if not is_pushed(rr) or not is_parent(rr):
             logging.error('Failed triggering Autoland because the review '
                           'request is not pushed, or not the parent review '
                           'request.')
             return NOT_PUSHED_PARENT_REVIEW_REQUEST
 
-        if not rr.is_mutable_by(request.user):
-            return PERMISSION_DENIED
-
         target_repository = rr.repository.extra_data.get(
             'landing_repository_url')
         push_bookmark = rr.repository.extra_data.get('landing_bookmark')
 
         if not target_repository:
             return AUTOLAND_CONFIGURATION_ERROR.with_message(
                 'Autoland has not been configured with a proper landing URL.')
 
@@ -240,17 +237,17 @@ autoland_trigger_resource = AutolandTrig
 
 class TryAutolandTriggerResource(BaseAutolandTriggerResource):
     """Resource to kick off Try Builds for a particular review request."""
 
     name = 'try_autoland_trigger'
 
     @webapi_login_required
     @webapi_response_errors(DOES_NOT_EXIST, INVALID_FORM_DATA,
-                            NOT_LOGGED_IN, PERMISSION_DENIED)
+                            NOT_LOGGED_IN)
     @webapi_scm_groups_required('scm_level_1')
     @webapi_request_fields(
         required={
             'review_request_id': {
                 'type': int,
                 'description': 'The review request for which to trigger a Try '
                                'build',
             },
@@ -276,19 +273,16 @@ class TryAutolandTriggerResource(BaseAut
             }
 
         if not is_pushed(rr) or not is_parent(rr):
             logging.error('Failed triggering Autoland because the review '
                           'request is not pushed, or not the parent review '
                           'request.')
             return NOT_PUSHED_PARENT_REVIEW_REQUEST
 
-        if not rr.is_mutable_by(request.user):
-            return PERMISSION_DENIED
-
         target_repository = rr.repository.extra_data.get(
             'try_repository_url')
 
         if not target_repository:
             return AUTOLAND_CONFIGURATION_ERROR.with_message(
                 'Autoland has not been configured with a proper try URL.')
 
         last_revision = json.loads(rr.extra_data.get('p2rb.commits'))[-1][0]
@@ -557,17 +551,17 @@ class ImportPullRequestTriggerResource(W
     def has_access_permissions(self, request, *args, **kwargs):
         return False
 
     def has_list_access_permissions(self, request, *args, **kwargs):
         return True
 
     @webapi_login_required
     @webapi_response_errors(DOES_NOT_EXIST, INVALID_FORM_DATA,
-                            NOT_LOGGED_IN, PERMISSION_DENIED)
+                            NOT_LOGGED_IN)
     @webapi_request_fields(
         required={
             'github_user': {
                 'type': six.text_type,
                 'description': 'The github user for the pullrequest',
             },
             'github_repo': {
                 'type': six.text_type,
--- a/pylib/mozreview/mozreview/static/mozreview/js/autoland.js
+++ b/pylib/mozreview/mozreview/static/mozreview/js/autoland.js
@@ -31,18 +31,16 @@
       )
       .show();
   }
 
   if (!MozReview.hasTryRepository) {
     try_trigger.attr('title', 'Try builds cannot be triggered for this repository');
   } else if ($("#draft-banner").is(":visible")) {
     try_trigger.attr('title', 'Try builds cannot be triggered on draft review requests');
-  } else if (!MozReview.currentIsMutableByUser) {
-    try_trigger.attr('title', 'Only the author may trigger a try build at this time');
   } else if (!MozReview.hasScmLevel1) {
     try_trigger.attr('title', 'You do not have the required scm level to trigger a try build');
   } else if (!MozReview.reviewRequestPending) {
     try_trigger.attr('title', 'You can not trigger a try build on a closed review request');
   } else {
     try_trigger.css('opacity', '1');
 
     $("#autoland-try-trigger").click(function() {
@@ -251,18 +249,16 @@
   }
 
   if (MozReview.landingRepository === '') {
     autoland_trigger.attr('title', 'Landing is not supported for this repository');
   } else if ($("#draft-banner").is(":visible")) {
     autoland_trigger.attr('title', 'Draft review requests cannot be landed');
   } else if (!MozReview.hasScmLevel3) {
     autoland_trigger.attr('title', 'You must have scm_level_3 access to land');
-  } else if (!MozReview.currentIsMutableByUser) {
-    autoland_trigger.attr('title', 'Only the author may land commits at this time');
   } else if (!MozReview.reviewRequestPending) {
     try_trigger.attr('title', 'You can not autoland from a closed review request');
   } else {
     MozReview.parentReviewRequest.ready({
       error: function () {
         autoland_trigger.attr('title', 'Error determining approval');
       },
       ready: function () {
--- a/pylib/mozreview/mozreview/tests/test-autoland-try.py
+++ b/pylib/mozreview/mozreview/tests/test-autoland-try.py
@@ -32,27 +32,34 @@ class AutolandTryTest(MozReviewWebDriver
 
             bb = self.user_bugzilla('mjane@example.com')
             bb.create_bug('TestProduct', 'TestComponent', 'First Bug')
 
             lr = self.create_basic_repo('mjane@example.com', 'mjane')
             lr.write('foo', 'first change')
             lr.run(['commit', '-m', 'Bug 1 - Test try'])
             lr.run(['push'])
+
+            # jsmith needs to push for the ldap association magic to happen
+            self.create_ldap(b'jsmith@example.com', b'jsmith', 2002, b'Jeremy')
+            lr = self.create_basic_repo('jsmith@example.com', 'jsmith',
+                                        'test_repo2')
+            lr.write('foo', 'first change')
+            lr.run(['commit', '-m', 'Bug 1 - Test try'])
+            lr.run(['push'])
         except Exception:
             MozReviewWebDriverTest.tearDownClass()
             raise
 
     def test_autoland_try(self):
         # We currently have four conditions for enabling the 'automation' menu
         # and try button (see static/mozreview/js/autoland.js):
         # 1. The review must be published
-        # 2. The review must be mutable by the current user
-        # 3. The user must have scm_level_1 or higher
-        # 4. The repository must have an associated try repository
+        # 2. The user must have scm_level_1 or higher
+        # 3. The repository must have an associated try repository
         # TODO: Ideally we'd test these conditions independently to ensure that
         #       the 'try' button will only show up when all four are met
         #       and not otherwise.
 
         # We should not be able to trigger a Try run without a HostingService
         # with an associated try repository.
         self.reviewboard_login('mjane@example.com', 'password2')
         self.load_rburl('r/1')
@@ -74,19 +81,62 @@ class AutolandTryTest(MozReviewWebDriver
         publish_btn = WebDriverWait(self.browser, 3).until(
             EC.visibility_of_element_located((By.ID,
             'btn-draft-publish')))
         publish_btn.click()
 
         WebDriverWait(self.browser, 10).until(
             EC.invisibility_of_element_located((By.ID, 'draft-banner')))
 
-        # Attempt to make intermittent failure with opacity of 'try' button
-        # less common. See Bug 1220733.
-        self.load_rburl('r/2')
+        automation_menu = self.browser.find_element_by_id('automation-menu')
+        automation_menu.click()
+        try_btn = self.browser.find_element_by_id('autoland-try-trigger')
+        self.assertEqual(
+            try_btn.value_of_css_property('opacity'), '1',
+            'Autoland to try should be enabled')
+
+        # Clicking the button should display a trychooser dialog
+        try_btn.click()
+        try_text = WebDriverWait(self.browser, 3).until(
+            EC.visibility_of_element_located((By.ID,
+            'mozreview-autoland-try-syntax')))
+        try_text.send_keys('try: stuff')
+        try_submit = self.browser.find_element_by_xpath('//input[@value="Submit"]')
+
+        # clicking the Submit button should display an activity indicator
+        try_submit.click()
+
+        element = WebDriverWait(self.browser, 10).until(
+            EC.visibility_of_element_located((By.ID, 'activity-indicator'))
+        )
+        try:
+            self.assertTrue('A server error occurred' not in element.text)
+        except StaleElementReferenceException:
+            # The activity indicator may already have disappeared by the time
+            # we check the text, but we want to be able to catch the server
+            # error if it shows up.
+            pass
+
+        # the try job should eventually create a new change description
+        WebDriverWait(self.browser, 10).until(
+            EC.visibility_of_element_located((By.CLASS_NAME, 'changedesc'))
+        )
+
+        time.sleep(10)
+        self.browser.refresh()
+        changedesc = self.browser.find_elements_by_class_name('changedesc')[1]
+        self.assertTrue('https://treeherder.mozilla.org/'
+            in changedesc.get_attribute('innerHTML'))
+
+        # We should not have closed the review automatically
+        with self.assertRaises(NoSuchElementException):
+            self.browser.find_element_by_id('submitted-banner')
+
+        # We should be able to trigger a Try run for another user.
+        self.reviewboard_login('jsmith@example.com', 'password1')
         self.load_rburl('r/1')
 
         automation_menu = self.browser.find_element_by_id('automation-menu')
         automation_menu.click()
         try_btn = self.browser.find_element_by_id('autoland-try-trigger')
         self.assertEqual(
             try_btn.value_of_css_property('opacity'), '1')
 
@@ -121,14 +171,8 @@ class AutolandTryTest(MozReviewWebDriver
         self.browser.refresh()
         changedesc = self.browser.find_elements_by_class_name('changedesc')[1]
         self.assertTrue('https://treeherder.mozilla.org/'
             in changedesc.get_attribute('innerHTML'))
 
         # We should not have closed the review automatically
         with self.assertRaises(NoSuchElementException):
             self.browser.find_element_by_id('submitted-banner')
-
-        # We should not be able to trigger a Try run for another user.
-        self.reviewboard_login('jsmith@example.com', 'password1')
-        self.load_rburl('r/1')
-        with self.assertRaises(NoSuchElementException):
-            self.browser.find_element_by_id('mozreview-autoland-try-trigger')