Merge ~pappacena/launchpad:mp-edit-url into launchpad:master

[Thiago F. Pappacena]

Sending back on XML-RPC git the URL to currently open MP, if any

Unmerged commits

0612249... by Thiago F. Pappacena on 2020-08-28

Sending back on XML-RPC git the URL to currently open MP, if any

diff --git a/lib/lp/code/interfaces/gitref.py b/lib/lp/code/interfaces/gitref.py
index ef51d99..12fae27 100644
--- a/lib/lp/code/interfaces/gitref.py
+++ b/lib/lp/code/interfaces/gitref.py
@@ -273,6 +273,13 @@ class IGitRefView(IHasMergeProposals, IHasRecipes, IPrivacy, IInformationType):
         Target and prerequisite repositories are preloaded.
         """
 
+    def getOpenPrecachedLandingTargets(user):
+        """Return precached landing targets, excluding those ones in final
+        states.
+
+        Target and prerequisite repositories are preloaded.
+        """
+
     def getPrecachedLandingCandidates(user):
         """Return precached landing candidates.
 
diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
index f7dc142..07a6d09 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -245,6 +245,12 @@ class GitRefMixin:
         loader = partial(BranchMergeProposal.preloadDataForBMPs, user=user)
         return DecoratedResultSet(self.landing_targets, pre_iter_hook=loader)
 
+    def getOpenPrecachedLandingTargets(self, user):
+        """See `IGitRef`."""
+        final_states = BRANCH_MERGE_PROPOSAL_FINAL_STATES
+        return self.getPrecachedLandingTargets(user).find(
+            Not(BranchMergeProposal.queue_status.is_in(final_states)))
+
     @property
     def _api_landing_targets(self):
         return self.getPrecachedLandingTargets(getUtility(ILaunchBag).user)
diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
index 0c3a2f1..6991f17 100644
--- a/lib/lp/code/xmlrpc/git.py
+++ b/lib/lp/code/xmlrpc/git.py
@@ -480,8 +480,18 @@ class GitAPI(LaunchpadXMLRPCView):
             # but it doesn't make sense in the context of an internal service.
             return None
 
-        # We assemble the URL this way here because the ref may not exist yet.
         base_url = canonical_url(repository, rootsite='code')
+
+        # If we already have exactly one open MP for this branch,
+        # we should suggest the user to continue editing this MP instead of
+        # opening a new one.
+        ref = repository.getRefByPath(six.text_type(branch))
+        if ref is not None:
+            mps = ref.getOpenPrecachedLandingTargets(requester)
+            if mps.count() == 1:
+                return "%s/+merge/%s/" % (base_url, mps[0].id)
+
+        # We assemble the URL this way here because the ref may not exist yet.
         mp_url = "%s/+ref/%s/+register-merge" % (
             base_url, quote(branch))
         return mp_url
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index c3b8126..be81181 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -29,6 +29,7 @@ from zope.security.proxy import removeSecurityProxy
 from lp.app.enums import InformationType
 from lp.buildmaster.enums import BuildStatus
 from lp.code.enums import (
+    BranchMergeProposalStatus,
     GitGranteeType,
     GitRepositoryStatus,
     GitRepositoryType,
@@ -1810,6 +1811,37 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
         self.assertHasMergeProposalURL(repository, pushed_branch,
                                        {"uid": requester_non_owner.id})
 
+    def test_getMergeProposalURL_already_existing_open_mp(self):
+        mp = self.factory.makeBranchMergeProposalForGit(
+            set_state=BranchMergeProposalStatus.WORK_IN_PROGRESS)
+        repository = mp.source_git_ref.repository
+        requester_owner = mp.source_git_ref.repository.owner
+        pushed_branch_name = mp.source_git_ref.name
+
+        base_url = canonical_url(repository, rootsite='code')
+        result = self.git_api.getMergeProposalURL(
+            repository.getInternalPath(), pushed_branch_name,
+            {"uid": requester_owner.id})
+
+        expected_mp_url = "%s/+merge/%s/" % (base_url, mp.id)
+        self.assertEqual(expected_mp_url, result)
+
+    def test_getMergeProposalURL_already_closed_mp(self):
+        mp = self.factory.makeBranchMergeProposalForGit(
+            set_state=BranchMergeProposalStatus.MERGED)
+        repository = mp.source_git_ref.repository
+        requester_owner = mp.source_git_ref.repository.owner
+        pushed_branch_name = mp.source_git_ref.name
+
+        base_url = canonical_url(repository, rootsite='code')
+        result = self.git_api.getMergeProposalURL(
+            repository.getInternalPath(), pushed_branch_name,
+            {"uid": requester_owner.id})
+
+        expected_mp_url = "%s/+ref/%s/+register-merge" % (
+            base_url, quote(pushed_branch_name))
+        self.assertEqual(expected_mp_url, result)
+
     def test_getMergeProposalURL_user_macaroon(self):
         # The merge proposal URL is returned by LP for a non-default branch
         # pushed by a user with a suitable macaroon that