← Back to team overview

launchpad-reviewers team mailing list archive

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

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-export-mp-collections into lp:launchpad.

Commit message:
Export merge proposal collections on GitRepository and GitRef.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1462289 in Launchpad itself: "Launchpad API should expose  available merge requests on GitRepository and GitRef"
  https://bugs.launchpad.net/launchpad/+bug/1462289

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-export-mp-collections/+merge/261206

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.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-export-mp-collections into lp:launchpad.
=== 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))


Follow ups