launchpad-reviewers team mailing list archive

[Guilherme Salgado <salgado@xxxxxxxxxxxxx>]

On Wed, 2010-12-01 at 22:24 +0000, James Westby wrote:
> You have been requested to review the proposed merge of lp:~james-w/launchpad/export-specification-bug-links into lp:launchpad.
> 
> Hi,
> 
> Here is a small branch to export specification/bug links, which is the
> last thing required to get the Ubuntu work items tracker using the API
> for blueprints, rather than screen-scraping.
> 
> I'm not entirely sure that List->CollectionField doesn't change other
> behavior, but lazr.restful doesn't do the right thing with List.

I don't think we use the field you changed for constructing any forms,
so I'd be surprised if this has any side effects.

> 
> Also, I needed to add the inheritance to have lazr.restful add the bugs
> property to the specifications. Previously the model just had
> implements(IBugLinkTarget). I don't know if there is another way to achieve
> that.

As we talked on IRC, I thought just exporting IBugLinkTarget as an entry
would make it work, but apparently it didn't?

> === modified file 'lib/lp/blueprints/interfaces/specification.py'
> --- lib/lp/blueprints/interfaces/specification.py	2010-12-01 12:54:42 +0000
> +++ lib/lp/blueprints/interfaces/specification.py	2010-12-01 22:24:03 +0000
> @@ -58,6 +58,7 @@
>      IHasSpecifications,
>      ISpecificationTarget,
>      )
> +from lp.bugs.interfaces.buglink import IBugLinkTarget
>  from lp.blueprints.interfaces.sprint import ISprint
>  from lp.code.interfaces.branchlink import IHasLinkedBranches
>  from lp.registry.interfaces.milestone import IMilestone
> @@ -529,7 +530,8 @@
>          """Return the SpecificationBranch link for the branch, or None."""
>  
> 
> -class ISpecification(ISpecificationPublic, ISpecificationEditRestricted):
> +class ISpecification(ISpecificationPublic, ISpecificationEditRestricted,
> +                     IBugLinkTarget):
>      """A Specification."""
>  
>      export_as_webservice_entry()
> 
> === modified file 'lib/lp/blueprints/tests/test_webservice.py'
> --- lib/lp/blueprints/tests/test_webservice.py	2010-12-01 12:54:42 +0000
> +++ lib/lp/blueprints/tests/test_webservice.py	2010-12-01 22:24:03 +0000
> @@ -14,7 +14,11 @@
>      )
>  from lp.testing import (
>      launchpadlib_for,
> +<<<<<<< TREE
>      launchpadlib_for_anonymous,
> +=======
> +    person_logged_in,
> +>>>>>>> MERGE-SOURCE
>      TestCaseWithFactory,

A trivial conflict here.

>      ws_object,
>      )
> @@ -154,6 +158,16 @@
>          self.assertEqual(1, spec_webservice.dependencies.total_size)
>          self.assertEqual(spec2.name, spec_webservice.dependencies[0].name)
>  
> +    def test_representation_contains_bug_links(self):
> +        spec = self.factory.makeSpecification()
> +        bug = self.factory.makeBug()
> +        person = self.factory.makePerson()
> +        with person_logged_in(person):
> +            spec.linkBug(bug)
> +        spec_webservice = self.getSpecOnWebservice(spec)
> +        self.assertEqual(1, spec_webservice.bugs.total_size)
> +        self.assertEqual(bug.id, spec_webservice.bugs[0].id)
> +
>  
>  class SpecificationTargetTests(SpecificationWebserviceTestCase):
>      """Tests for accessing specifications via their targets."""
> 
> === modified file 'lib/lp/bugs/interfaces/buglink.py'
> --- lib/lp/bugs/interfaces/buglink.py	2010-08-20 20:31:18 +0000
> +++ lib/lp/bugs/interfaces/buglink.py	2010-12-01 22:24:03 +0000
> @@ -14,6 +14,13 @@
>      'IUnlinkBugsForm',
>      ]
>  
> +from lazr.restful.declarations import (
> +    export_as_webservice_entry,
> +    exported,
> +    )
> +from lazr.restful.fields import (
> +    CollectionField,
> +    )
>  from zope.interface import (
>      implements,
>      Interface,
> @@ -52,9 +59,10 @@
>  
>      Examples include an IQuestion, and an ICve.
>      """
> +    export_as_webservice_entry()
>  
> -    bugs = List(title=_("Bugs related to this object."),
> -                value_type=Object(schema=IBug), readonly=True)
> +    bugs = exported(CollectionField(title=_("Bugs related to this object."),
> +                value_type=Object(schema=IBug), readonly=True))

I think you should use Reference instead of Object here.  I don't
remember why we have Reference (which is a wrapper around Object), but
that's what we seem to use in conjunction with CollectionFields.

Also, this interface is provided only by ISpecification and IQuestion
(which is not yet exposed on the API), so I think it makes sense to
expose it only on the 'devel' version of the webservice, like all the
Blueprints bits that are currently exposed. To do that you just need a
couple extra arguments to exported:

    ('devel', dict(exported=True)), exported=False)

>      bug_links = List(title=_("The links between bugs and this object."),
>                       value_type=Object(schema=IBugLink), readonly=True)
>  
> 

-- 
Guilherme Salgado <https://launchpad.net/~salgado>

https://code.launchpad.net/~james-w/launchpad/export-specification-bug-links/+merge/42426
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~james-w/launchpad/export-specification-bug-links into lp:launchpad.