Merge lp:~inspirated/launchpad/export-Person-getBugSubscriberPackages into lp:launchpad

[Kamran Riaz Khan]

[ui=none][r=gmb][bug=331039,281443] Person.getBugSubscriberPackages() has been exported via the API.

Summary:
As part of my GSoC 2010 project, functionality is required in Arsenal to search all the bugs a team is subscribed to. This required an API call which would export all the source packages subscriptions of a team. Bug #281443 and Bug #331039 document other needs for this API call.

Proposed fix:
The method for searching a team's subscriptions is already present in lib/lp/registry/model/person.py as Person.getBugSubscriberPackages(). This branch just exports this method in the API.

[Graham Binns (gmb)]

Hi Kamran,

Thanks for making this change. Unfortunately before changes can be landed in Launchpad they must have tests to cover them. With your changes, the tests suite will at the moment almost certainly fail. You need to alter the existing tests to cover this change.

I haven't had time to run the whole test suite to check which tests fail, but you'll at least need to fix the tests in lib/lp/registry/stories/webservice/xx-person.txt (you can find out where the failures will be by running bin/test -cvvt xx-person from the root of your LP branch).

I'd be happy to help you with fixing the tests, just ping me in #launchpad-dev on Freenode and if I'm around I'll help you out.

review: Needs Fixing (code)

[Kamran Riaz Khan (inspirated)]

Hi Graham,

Thanks for taking time out to review. Here are the results for person test suite:

$ bin/test -cvvt xx-person.txtRunning tests at level 1
Running canonical.testing.layers.PageTestLayer tests:
  Set up canonical.testing.layers.BaseLayer in 0.082 seconds.
  Set up canonical.testing.layers.DatabaseLayer in 1 minutes 3.481 seconds.
  Set up canonical.testing.layers.LibrarianLayer in 22.484 seconds.
  Set up canonical.testing.layers.MemcachedLayer in 0.308 seconds.
  Set up canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Set up canonical.testing.layers.FunctionalLayer in 11.257 seconds.
  Set up canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Set up canonical.testing.layers.GoogleServiceLayer in 5.759 seconds.
  Set up canonical.testing.layers.PageTestLayer in 0.012 seconds.
  Running:
 lib/lp/registry/tests/../stories/webservice/xx-person.txt
  Ran 52 tests with 0 failures and 0 errors in 1 minutes 12.426 seconds.
Tearing down left over layers:
  Tear down canonical.testing.layers.PageTestLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LibrarianLayer in 0.015 seconds.
  Tear down canonical.testing.layers.MemcachedLayer in 0.113 seconds.
  Tear down canonical.testing.layers.GoogleServiceLayer in 0.455 seconds.
  Tear down canonical.testing.layers.FunctionalLayer ... not supported
  Tear down canonical.testing.layers.DatabaseLayer in 0.190 seconds.
  Tear down canonical.testing.layers.BaseLayer in 0.000 seconds.

The tests do not introduce any regression (since the branch doesn't implement anything new in LP, it just exports a method that is already present).

[Kamran Riaz Khan (inspirated)]

Added new doctest in revision #11012.

$ bin/test -cvvt xx-person.txt
Running tests at level 1
Running canonical.testing.layers.PageTestLayer tests:
  Set up canonical.testing.layers.BaseLayer in 0.033 seconds.
  Set up canonical.testing.layers.DatabaseLayer in 1.283 seconds.
  Set up canonical.testing.layers.LibrarianLayer in 14.543 seconds.
  Set up canonical.testing.layers.MemcachedLayer in 0.186 seconds.
  Set up canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Set up canonical.testing.layers.FunctionalLayer in 9.068 seconds.
  Set up canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Set up canonical.testing.layers.GoogleServiceLayer in 4.523 seconds.
  Set up canonical.testing.layers.PageTestLayer in 0.011 seconds.
  Running:
 lib/lp/registry/tests/../stories/webservice/xx-person.txt
  Ran 53 tests with 0 failures and 0 errors in 59.176 seconds.
Tearing down left over layers:
  Tear down canonical.testing.layers.PageTestLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LibrarianLayer in 0.006 seconds.
  Tear down canonical.testing.layers.MemcachedLayer in 0.103 seconds.
  Tear down canonical.testing.layers.GoogleServiceLayer in 0.312 seconds.
  Tear down canonical.testing.layers.FunctionalLayer ... not supported
  Tear down canonical.testing.layers.DatabaseLayer in 0.218 seconds.
  Tear down canonical.testing.layers.BaseLayer in 0.000 seconds.

[Graham Binns (gmb)]

The new test tests the method directly, not through the webservice; it should be tested using webservice.named_get() as used elsewhere in xx-person.txt

review: Needs Fixing (code)

[Kamran Riaz Khan (inspirated)]

The new diff uses webservice.named_get to test the method. Test results:

$ bin/test -cvvt xx-person.txt
Running tests at level 1
Running canonical.testing.layers.PageTestLayer tests:
  Set up canonical.testing.layers.BaseLayer in 0.024 seconds.
  Set up canonical.testing.layers.DatabaseLayer in 1.479 seconds.
  Set up canonical.testing.layers.LibrarianLayer in 14.752 seconds.
  Set up canonical.testing.layers.MemcachedLayer in 0.175 seconds.
  Set up canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Set up canonical.testing.layers.FunctionalLayer in 9.167 seconds.
  Set up canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Set up canonical.testing.layers.GoogleServiceLayer in 3.470 seconds.
  Set up canonical.testing.layers.PageTestLayer in 0.011 seconds.
  Running:
 lib/lp/registry/tests/../stories/webservice/xx-person.txt
  Ran 54 tests with 0 failures and 0 errors in 51.925 seconds.
Tearing down left over layers:
  Tear down canonical.testing.layers.PageTestLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Tear down canonical.testing.layers.LibrarianLayer in 0.007 seconds.
  Tear down canonical.testing.layers.MemcachedLayer in 0.110 seconds.
  Tear down canonical.testing.layers.GoogleServiceLayer in 0.282 seconds.
  Tear down canonical.testing.layers.FunctionalLayer ... not supported
  Tear down canonical.testing.layers.DatabaseLayer in 0.192 seconds.
  Tear down canonical.testing.layers.BaseLayer in 0.000 seconds.

[Graham Binns (gmb)]

I'm happy for this to land now. I see some test failures when I run the test locally, but they seem unconnected to your changes, so I'll run it through ec2 test for you and see what comes out the other side.

review: Approve (code)

[Bryce Harrington (bryce)]

kamran, small change needed to address a test failure:

=== modified file 'lib/lp/registry/stories/webservice/xx-person.txt'
--- a/lib/lp/registry/stories/webservice/xx-person.txt 2010-06-15 17:12:08 +0000
+++ b/lib/lp/registry/stories/webservice/xx-person.txt 2010-07-09 17:20:55 +0000
@@ -656,6 +656,7 @@
     start: 0
     total_size: 1
     ---
+ bug_reported_acknowledgement: None
     bug_reporting_guidelines: None
     display_name: u'...'
     distribution_link: u'...'

I attempted to land a branch of your branch with this fix added, but it wouldn't go; the branch has to be the one attached to the merge proposal. So if you could please apply this patch and update the branch, we can land it after that.

[Kamran Riaz Khan (inspirated)]

Thanks for the fix, Bryce. The bug_reported_acknowledgement field wasn't present at the time I last committed to this branch. Pushed in rev 11014.

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2010-06-29 10:17:06 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2010-07-10 07:46:00 +0000
@@ -425,5 +425,9 @@
 # IPOTemplateSharingSubset
 patch_reference_property(IPOTemplateSharingSubset, 'product', IProduct)
 
+# IPerson
+patch_collection_return_type(
+        IPerson, 'getBugSubscriberPackages', IDistributionSourcePackage)
+
 # IProductSeries
 patch_reference_property(IProductSeries, 'product', IProduct)

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2010-07-09 14:58:42 +0000
+++ lib/lp/registry/interfaces/person.py	2010-07-10 07:46:00 +0000
@@ -904,6 +904,8 @@
         The results are ordered using Person.sortingColumns.
         """
 
+    @operation_returns_collection_of(Interface)
+    @export_read_operation()
     def getBugSubscriberPackages():
         """Return the packages for which this person is a bug subscriber.
 

=== modified file 'lib/lp/registry/stories/webservice/xx-person.txt'
--- lib/lp/registry/stories/webservice/xx-person.txt	2010-05-12 03:31:34 +0000
+++ lib/lp/registry/stories/webservice/xx-person.txt	2010-07-10 07:46:00 +0000
@@ -638,6 +638,36 @@
 
 IPerson supports a bunch of operations.
 
+Teams can subscribe to source packages:
+
+    >>> login('[e-mail]')
+    >>> pythons_db = factory.makeTeam(name='pythons')
+    >>> package_db = factory.makeDistributionSourcePackage(
+    ...     sourcepackagename="fooix")
+    >>> ignored = package_db.addSubscription(None, pythons_db)
+    >>> logout()
+
+Subscribed packages can be listed with getBugSubscriberPackages:
+
+    >>> from lazr.restful.testing.webservice import pprint_collection
+    >>> subscriptions = webservice.named_get("/~pythons",
+    ...     "getBugSubscriberPackages").jsonBody()
+    >>> pprint_collection(subscriptions)
+    start: 0
+    total_size: 1
+    ---
+    bug_reported_acknowledgement: None
+    bug_reporting_guidelines: None
+    display_name: u'...'
+    distribution_link: u'...'
+    name: u'fooix'
+    official_bug_tags: []
+    resource_type_link: u'...'
+    self_link: u'...'
+    title: u'...'
+    upstream_product_link: None
+    ---
+
 
 === Team membership operations ===