Merge lp:~cjwatson/launchpad/git-export-mp-collections into lp:launchpad

[Colin Watson]

Export merge proposal collections on GitRepository and GitRef.

Export merge proposal collections on GitRepository and GitRef.

10:54 <cjwatson> wgrant: Have you thought of any better names for landing_candidates et al? Otherwise I'm inclined to export the current names on the grounds that Tarmac will need them and it's probably least confusing there for them to match Bazaar.
10:55 <wgrant> shuf -n 2 /usr/share/dict/words says extrusions_begun, which is probably less confusing than landing_candidates.
10:56 <cjwatson> :-P
10:56 <wgrant> The current names aren't obviously worse than the Bazaar equivalents, are they?
10:57 <wgrant> Apart from the repo vs branch stuff, nothing has changed terribly.
10:57 <cjwatson> The current names are identical to the Bazaar equivalents.
10:57 <wgrant> Right, but there's no real reason we have to change them.
10:57 <wgrant> That I can recall.
10:57 <cjwatson> That was my thought.
10:57 <cjwatson> I mean, they suck but they can suck equally.
10:57 <wgrant> It was just a "this bit of Bazaar codehosting is stupid, let's see if we can do it better before stabilising the API"
10:57 <wgrant> And at this point I think the answer is no.
10:58 <cjwatson> Like most things that talk about sources and targets etc., it's very easy to get into directional confusion.
10:58 <wgrant> Yeah
10:58 <cjwatson> Especially for this kind of thing where it's really "merge proposals for which this branch etc. is the source"
10:58 <wgrant> The least confusing thing might almost be to have lp.merge_proposals...
10:59 <wgrant> But exporting the existing properties works.
11:03 <cjwatson> lp.merge_proposals has the nice property that we could introduce it later without too much clutter, indeed.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py	2015-05-15 12:20:05 +0000
+++ lib/lp/_schema_circular_imports.py	2015-06-05 11:17:24 +0000
@@ -562,6 +562,9 @@
     IGitRef, 'createMergeProposal', 'merge_target', IGitRef)
 patch_plain_parameter_type(
     IGitRef, 'createMergeProposal', 'merge_prerequisite', IGitRef)
+patch_collection_property(IGitRef, 'landing_targets', IBranchMergeProposal)
+patch_collection_property(IGitRef, 'landing_candidates', IBranchMergeProposal)
+patch_collection_property(IGitRef, 'dependent_landings', IBranchMergeProposal)
 patch_entry_return_type(IGitRef, 'createMergeProposal', IBranchMergeProposal)
 patch_collection_return_type(
     IGitRef, 'getMergeProposals', IBranchMergeProposal)
@@ -572,6 +575,12 @@
 patch_collection_property(IGitRepository, 'subscriptions', IGitSubscription)
 patch_entry_return_type(IGitRepository, 'subscribe', IGitSubscription)
 patch_entry_return_type(IGitRepository, 'getSubscription', IGitSubscription)
+patch_collection_property(
+    IGitRepository, 'landing_targets', IBranchMergeProposal)
+patch_collection_property(
+    IGitRepository, 'landing_candidates', IBranchMergeProposal)
+patch_collection_property(
+    IGitRepository, 'dependent_landings', IBranchMergeProposal)
 
 # ILiveFSFile
 patch_reference_property(ILiveFSFile, 'livefsbuild', ILiveFSBuild)

=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py	2015-05-26 12:18:12 +0000
+++ lib/lp/code/interfaces/gitref.py	2015-06-05 11:17:24 +0000
@@ -22,6 +22,7 @@
     REQUEST_USER,
     )
 from lazr.restful.fields import (
+    CollectionField,
     Reference,
     ReferenceChoice,
     )
@@ -196,17 +197,30 @@
         and their subscriptions.
         """
 
-    # XXX cjwatson 2015-04-16: These names are too awful to set in stone by
-    # exporting them on the webservice; find better names before exporting.
-    landing_targets = Attribute(
-        "A collection of the merge proposals where this reference is the "
-        "source.")
-    landing_candidates = Attribute(
-        "A collection of the merge proposals where this reference is the "
-        "target.")
-    dependent_landings = Attribute(
-        "A collection of the merge proposals that are dependent on this "
-        "reference.")
+    landing_targets = exported(CollectionField(
+        title=_("Landing targets"),
+        description=_(
+            "A collection of the merge proposals where this reference is the "
+            "source."),
+        readonly=True,
+        # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
+        value_type=Reference(Interface)))
+    landing_candidates = exported(CollectionField(
+        title=_("Landing candidates"),
+        description=_(
+            "A collection of the merge proposals where this reference is the "
+            "target."),
+        readonly=True,
+        # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
+        value_type=Reference(Interface)))
+    dependent_landings = exported(CollectionField(
+        title=_("Dependent landings"),
+        description=_(
+            "A collection of the merge proposals that are dependent on this "
+            "reference."),
+        readonly=True,
+        # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
+        value_type=Reference(Interface)))
 
     # XXX cjwatson 2015-04-16: Rename in line with landing_targets above
     # once we have a better name.

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2015-05-26 12:18:12 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2015-06-05 11:17:24 +0000
@@ -449,17 +449,30 @@
         and their subscriptions.
         """
 
-    # XXX cjwatson 2015-04-16: These names are too awful to set in stone by
-    # exporting them on the webservice; find better names before exporting.
-    landing_targets = Attribute(
-        "A collection of the merge proposals where this repository is the "
-        "source.")
-    landing_candidates = Attribute(
-        "A collection of the merge proposals where this repository is the "
-        "target.")
-    dependent_landings = Attribute(
-        "A collection of the merge proposals that are dependent on this "
-        "repository.")
+    landing_targets = exported(CollectionField(
+        title=_("Landing targets"),
+        description=_(
+            "A collection of the merge proposals where this repository is the "
+            "source."),
+        readonly=True,
+        # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
+        value_type=Reference(Interface)))
+    landing_candidates = exported(CollectionField(
+        title=_("Landing candidates"),
+        description=_(
+            "A collection of the merge proposals where this repository is the "
+            "target."),
+        readonly=True,
+        # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
+        value_type=Reference(Interface)))
+    dependent_landings = exported(CollectionField(
+        title=_("Dependent landings"),
+        description=_(
+            "A collection of the merge proposals that are dependent on this "
+            "repository."),
+        readonly=True,
+        # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
+        value_type=Reference(Interface)))
 
     def getMergeProposalByID(id):
         """Return this repository's merge proposal with this id, or None."""

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2015-06-01 16:05:12 +0000
+++ lib/lp/code/model/gitrepository.py	2015-06-05 11:17:24 +0000
@@ -789,7 +789,7 @@
         """See `IGitRepository`."""
         return Store.of(self).find(
             BranchMergeProposal,
-            BranchMergeProposal.prerequisite_git_repository,
+            BranchMergeProposal.prerequisite_git_repository == self,
             Not(BranchMergeProposal.queue_status.is_in(
                 BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
 

=== modified file 'lib/lp/code/model/tests/test_gitref.py'
--- lib/lp/code/model/tests/test_gitref.py	2015-05-21 16:36:14 +0000
+++ lib/lp/code/model/tests/test_gitref.py	2015-06-05 11:17:24 +0000
@@ -77,3 +77,50 @@
         self.assertEqual(
             unicode(hashlib.sha1(u"refs/heads/master").hexdigest()),
             result["commit_sha1"])
+
+    def test_landing_candidates(self):
+        bmp_db = self.factory.makeBranchMergeProposalForGit()
+        with person_logged_in(bmp_db.registrant):
+            bmp_url = api_url(bmp_db)
+            ref_url = api_url(bmp_db.target_git_ref)
+        webservice = webservice_for_person(
+            bmp_db.registrant, permission=OAuthPermission.READ_PUBLIC)
+        webservice.default_api_version = "devel"
+        ref = webservice.get(ref_url).jsonBody()
+        landing_candidates = webservice.get(
+            ref["landing_candidates_collection_link"]).jsonBody()
+        self.assertEqual(1, len(landing_candidates["entries"]))
+        self.assertThat(
+            landing_candidates["entries"][0]["self_link"], EndsWith(bmp_url))
+
+    def test_landing_targets(self):
+        bmp_db = self.factory.makeBranchMergeProposalForGit()
+        with person_logged_in(bmp_db.registrant):
+            bmp_url = api_url(bmp_db)
+            ref_url = api_url(bmp_db.source_git_ref)
+        webservice = webservice_for_person(
+            bmp_db.registrant, permission=OAuthPermission.READ_PUBLIC)
+        webservice.default_api_version = "devel"
+        ref = webservice.get(ref_url).jsonBody()
+        landing_targets = webservice.get(
+            ref["landing_targets_collection_link"]).jsonBody()
+        self.assertEqual(1, len(landing_targets["entries"]))
+        self.assertThat(
+            landing_targets["entries"][0]["self_link"], EndsWith(bmp_url))
+
+    def test_dependent_landings(self):
+        [ref] = self.factory.makeGitRefs()
+        bmp_db = self.factory.makeBranchMergeProposalForGit(
+            prerequisite_ref=ref)
+        with person_logged_in(bmp_db.registrant):
+            bmp_url = api_url(bmp_db)
+            ref_url = api_url(ref)
+        webservice = webservice_for_person(
+            bmp_db.registrant, permission=OAuthPermission.READ_PUBLIC)
+        webservice.default_api_version = "devel"
+        ref = webservice.get(ref_url).jsonBody()
+        dependent_landings = webservice.get(
+            ref["dependent_landings_collection_link"]).jsonBody()
+        self.assertEqual(1, len(dependent_landings["entries"]))
+        self.assertThat(
+            dependent_landings["entries"][0]["self_link"], EndsWith(bmp_url))

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2015-06-01 23:02:20 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2015-06-05 11:17:24 +0000
@@ -2082,3 +2082,50 @@
         self.assertEqual(200, response.status)
         with person_logged_in(subscriber_db):
             self.assertNotIn(subscriber_db, repository_db.subscribers)
+
+    def test_landing_candidates(self):
+        bmp_db = self.factory.makeBranchMergeProposalForGit()
+        with person_logged_in(bmp_db.registrant):
+            bmp_url = api_url(bmp_db)
+            repository_url = api_url(bmp_db.target_git_repository)
+        webservice = webservice_for_person(
+            bmp_db.registrant, permission=OAuthPermission.READ_PUBLIC)
+        webservice.default_api_version = "devel"
+        repository = webservice.get(repository_url).jsonBody()
+        landing_candidates = webservice.get(
+            repository["landing_candidates_collection_link"]).jsonBody()
+        self.assertEqual(1, len(landing_candidates["entries"]))
+        self.assertThat(
+            landing_candidates["entries"][0]["self_link"], EndsWith(bmp_url))
+
+    def test_landing_targets(self):
+        bmp_db = self.factory.makeBranchMergeProposalForGit()
+        with person_logged_in(bmp_db.registrant):
+            bmp_url = api_url(bmp_db)
+            repository_url = api_url(bmp_db.source_git_repository)
+        webservice = webservice_for_person(
+            bmp_db.registrant, permission=OAuthPermission.READ_PUBLIC)
+        webservice.default_api_version = "devel"
+        repository = webservice.get(repository_url).jsonBody()
+        landing_targets = webservice.get(
+            repository["landing_targets_collection_link"]).jsonBody()
+        self.assertEqual(1, len(landing_targets["entries"]))
+        self.assertThat(
+            landing_targets["entries"][0]["self_link"], EndsWith(bmp_url))
+
+    def test_dependent_landings(self):
+        [ref] = self.factory.makeGitRefs()
+        bmp_db = self.factory.makeBranchMergeProposalForGit(
+            prerequisite_ref=ref)
+        with person_logged_in(bmp_db.registrant):
+            bmp_url = api_url(bmp_db)
+            repository_url = api_url(ref.repository)
+        webservice = webservice_for_person(
+            bmp_db.registrant, permission=OAuthPermission.READ_PUBLIC)
+        webservice.default_api_version = "devel"
+        repository = webservice.get(repository_url).jsonBody()
+        dependent_landings = webservice.get(
+            repository["dependent_landings_collection_link"]).jsonBody()
+        self.assertEqual(1, len(dependent_landings["entries"]))
+        self.assertThat(
+            dependent_landings["entries"][0]["self_link"], EndsWith(bmp_url))