launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/fix-git-merge-editstatus into lp:launchpad.

Commit message:
Fix +edit-status for Git merge proposals.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1538355 in Launchpad itself: "oops when changing git-based MP status before ajax has loaded"
  https://bugs.launchpad.net/launchpad/+bug/1538355

For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/fix-git-merge-editstatus/+merge/298958

Fix +edit-status for Git merge proposals (fixes lp:1538355).

(Also includes a small extra cleanup of SNAP_FEATURE_FLAG that sneaked in recent commits.)
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/fix-git-merge-editstatus into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2016-05-14 09:54:32 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2016-07-01 20:50:16 +0000
@@ -477,6 +477,14 @@
             terms.append(SimpleTerm(status, status.name, title))
         return SimpleVocabulary(terms)
 
+    @property
+    def source_revid(self):
+        if IBranch.providedBy(self.context.merge_source):
+            source_revid = self.context.merge_source.last_scanned_id
+        else:
+            source_revid = self.context.merge_source.commit_sha1
+        return source_revid
+
 
 class DiffRenderingMixin:
     """A mixin class for handling diff text."""
@@ -758,16 +766,12 @@
     @property
     def status_config(self):
         """The config to configure the ChoiceSource JS widget."""
-        if IBranch.providedBy(self.context.merge_source):
-            source_revid = self.context.merge_source.last_scanned_id
-        else:
-            source_revid = self.context.merge_source.commit_sha1
         return simplejson.dumps({
             'status_widget_items': vocabulary_to_choice_edit_items(
                 self._createStatusVocabulary(),
                 css_class_prefix='mergestatus'),
             'status_value': self.context.queue_status.title,
-            'source_revid': source_revid,
+            'source_revid': self.source_revid,
             'user_can_edit_status': check_permission(
                 'launchpad.Edit', self.context),
             })

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2016-06-03 14:08:52 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2016-07-01 20:50:16 +0000
@@ -1758,6 +1758,18 @@
                 object=MatchesStructure.byEquality(
                     queue_status=BranchMergeProposalStatus.NEEDS_REVIEW))]))
 
+    def test_source_revid_bzr(self):
+        view = self._createView()
+        self.assertEqual(
+            self.proposal.merge_source.last_scanned_id, view.source_revid)
+
+    def test_source_revid_git(self):
+        git_proposal = self.factory.makeBranchMergeProposalForGit()
+        view = BranchMergeProposalChangeStatusView(
+            git_proposal, LaunchpadTestRequest())
+        self.assertEqual(
+            git_proposal.merge_source.commit_sha1, view.source_revid)
+
 
 class TestCommentAttachmentRendering(TestCaseWithFactory):
     """Test diff attachments are rendered correctly."""

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2016-07-01 11:53:31 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2016-07-01 20:50:16 +0000
@@ -1947,10 +1947,6 @@
 
     layer = ZopelessDatabaseLayer
 
-    def setUp(self):
-        super(TestGitRepositoryMarkSnapsStale, self).setUp()
-        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
-
     def test_same_repository(self):
         # On ref changes, snap packages using this ref become stale.
         [ref] = self.factory.makeGitRefs()

=== modified file 'lib/lp/code/templates/branchmergeproposal-editstatus.pt'
--- lib/lp/code/templates/branchmergeproposal-editstatus.pt	2009-08-19 08:45:32 +0000
+++ lib/lp/code/templates/branchmergeproposal-editstatus.pt	2016-07-01 20:50:16 +0000
@@ -13,7 +13,7 @@
 
     <tal:hidden-input metal:fill-slot="extra_info">
       <input type="hidden" name="revno"
-             tal:attributes="value context/source_branch/last_scanned_id"/>
+             tal:attributes="value view/source_revid"/>
     </tal:hidden-input>
 
   </div>

=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py	2016-06-20 20:32:36 +0000
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py	2016-07-01 20:50:16 +0000
@@ -51,7 +51,6 @@
 from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.osutils import override_environ
-from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import (
     dbuser,
@@ -753,10 +752,6 @@
 class TestMarkSnapsStale(BzrSyncTestCase):
     """Test that snap packages associated with the branch are marked stale."""
 
-    def setUp(self):
-        super(TestMarkSnapsStale, self).setUp()
-        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
-
     @run_as_db_user(config.launchpad.dbuser)
     def test_same_branch(self):
         # On tip change, snap packages using this branch become stale.