launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing code



Diff comments:

> === modified file 'lib/lp/blueprints/interfaces/specification.py'
> --- lib/lp/blueprints/interfaces/specification.py	2013-06-28 00:49:47 +0000
> +++ lib/lp/blueprints/interfaces/specification.py	2014-06-17 04:31:31 +0000
> @@ -18,8 +18,11 @@
>  
>  from lazr.restful.declarations import (
>      call_with,
> +    collection_default_content,
>      error_status,
> +    export_as_webservice_collection,
>      export_as_webservice_entry,
> +    export_factory_operation,
>      export_operation_as,
>      export_write_operation,
>      exported,
> @@ -712,6 +715,29 @@
>  
>  class ISpecificationSet(IHasSpecifications):
>      """A container for specifications."""
> +    export_as_webservice_collection(ISpecification)
> +
> +    @collection_default_content()
> +    def empty_list():
> +        """Return an empty set - only exists to keep lazr.restful happy."""
> +
> +    @call_with(owner=REQUEST_USER)
> +    @operation_parameters(
> +        target=Reference(
> +            schema=ISpecificationTarget, required=True,
> +            title=(u"The product or distribution context of this "
> +                   u"specification.")))
> +    @export_factory_operation(
> +        ISpecification, ['name', 'title', 'specurl', 'summary',
> +                         'definition_status', 'assignee', 'drafter',
> +                         'whiteboard'])
> +    @operation_for_version('devel')
> +    def createSpecification(name, title, specurl, summary, definition_status,
> +                            owner, target, approver=None, assignee=None,
> +                            drafter=None, whiteboard=None,
> +                            information_type=None,
> +                            priority=SpecificationPriority.UNDEFINED):
> +        """Create a new Specification."""
>  
>      displayname = Attribute('Displayname')
>  
> @@ -739,10 +765,10 @@
>          """Return the specification with the given name for the given pillar.
>          """
>  
> -    def new(name, title, specurl, summary, definition_status,
> -        owner, approver=None, product=None, distribution=None, assignee=None,
> -        drafter=None, whiteboard=None,
> -        priority=SpecificationPriority.UNDEFINED):
> +    def new(name, title, specurl, summary, definition_status, owner,
> +            approver=None, product=None, distribution=None, assignee=None,
> +            drafter=None, whiteboard=None, information_type=None,
> +            priority=SpecificationPriority.UNDEFINED):
>          """Create a new specification."""
>  
>      def getDependencyDict(specifications):
> 
> === modified file 'lib/lp/blueprints/interfaces/webservice.py'
> --- lib/lp/blueprints/interfaces/webservice.py	2013-06-27 15:11:38 +0000
> +++ lib/lp/blueprints/interfaces/webservice.py	2014-06-17 04:31:31 +0000
> @@ -22,6 +22,7 @@
>  from lp.blueprints.interfaces.specification import (
>      GoalProposeError,
>      ISpecification,
> +    ISpecificationSet,
>      )
>  from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch
>  from lp.blueprints.interfaces.specificationsubscription import (
> 
> === modified file 'lib/lp/blueprints/model/specification.py'
> --- lib/lp/blueprints/model/specification.py	2013-12-13 12:51:41 +0000
> +++ lib/lp/blueprints/model/specification.py	2014-06-17 04:31:31 +0000
> @@ -1111,3 +1111,23 @@
>      def get(self, spec_id):
>          """See lp.blueprints.interfaces.specification.ISpecificationSet."""
>          return Specification.get(spec_id)
> +
> +    def empty_list(self):
> +        """See `ISpecificationSet`."""
> +        return []
> +
> +    def createSpecification(self, name, title, specurl, summary,
> +                            definition_status, owner, target, approver=None,
> +                            assignee=None, drafter=None, whiteboard=None,
> +                            information_type=None,
> +                            priority=SpecificationPriority.UNDEFINED):
> +        """See `ISpecificationSet`."""
> +        spec = self.new(
> +            name=name, title=title, specurl=specurl, summary=summary,
> +            definition_status=definition_status, owner=owner,
> +            approver=approver,assignee=assignee, drafter=drafter,
> +            whiteboard=whiteboard, priority=priority,
> +            information_type=information_type
> +        )

This probably doesn't perform permission checks on the priority, and neither non-public information_type defaults nor restrictions are applied.

Also, there's a missing space and a misplaced closing paren.

> +        spec.setTarget(target)
> +        return spec
> 
> === modified file 'lib/lp/blueprints/tests/test_webservice.py'
> --- lib/lp/blueprints/tests/test_webservice.py	2013-06-27 09:40:16 +0000
> +++ lib/lp/blueprints/tests/test_webservice.py	2014-06-17 04:31:31 +0000
> @@ -11,7 +11,9 @@
>  
>  from lp.blueprints.enums import SpecificationDefinitionStatus
>  from lp.services.webapp.interaction import ANONYMOUS
> +from lp.services.webapp.interfaces import OAuthPermission
>  from lp.testing import (
> +    api_url,
>      launchpadlib_for,
>      person_logged_in,
>      TestCaseWithFactory,
> @@ -48,6 +50,55 @@
>          return launchpadlib.load(pillar_name)
>  
>  
> +class SpecificationWebserviceTests(SpecificationWebserviceTestCase):
> +    """Test accessing specification top-level webservice."""
> +    layer = AppServerLayer
> +
> +    def test_collection(self):
> +        # `ISpecificationSet` is exposed as a webservice via /specs
> +        # and is represented by an empty collection.
> +        user = self.factory.makePerson()
> +        webservice = webservice_for_person(user)
> +        response = webservice.get('/specs')
> +        self.assertEqual(200, response.status)
> +        self.assertEqual(
> +            ['entries', 'resource_type_link', 'start', 'total_size'],
> +            sorted(response.jsonBody().keys()))
> +        self.assertEqual(0, response.jsonBody()['total_size'])
> +
> +    def test_creation_for_products(self):
> +        # `ISpecificationSet.createSpecification` is exposed and
> +        # allows specification creation for products.
> +        user = self.factory.makePerson()
> +        product = self.factory.makeProduct()
> +        product_url = api_url(product)
> +        webservice = webservice_for_person(
> +            user, permission=OAuthPermission.WRITE_PUBLIC)
> +        response = webservice.named_post(
> +            '/specs', 'createSpecification',
> +            name='test-prod', title='Product', specurl='http://test.com',
> +            definition_status='Approved', summary='A summary',
> +            target=product_url,
> +            api_version='devel')
> +        self.assertEqual(201, response.status)
> +
> +    def test_creation_for_distribution(self):
> +        # `ISpecificationSet.createSpecification` also allows
> +        # specification creation for distributions.
> +        user = self.factory.makePerson()
> +        distribution = self.factory.makeDistribution()
> +        distribution_url = api_url(distribution)
> +        webservice = webservice_for_person(
> +            user, permission=OAuthPermission.WRITE_PUBLIC)
> +        response = webservice.named_post(
> +            '/specs', 'createSpecification',
> +            name='test-distro', title='Distro', specurl='http://test.com',
> +            definition_status='Approved', summary='A summary',
> +            target=distribution_url,
> +            api_version='devel')
> +        self.assertEqual(201, response.status)
> +
> +
>  class SpecificationAttributeWebserviceTests(SpecificationWebserviceTestCase):
>      """Test accessing specification attributes over the webservice."""
>      layer = AppServerLayer
> 


-- 
https://code.launchpad.net/~cprov/launchpad/create-spec-webservice/+merge/223332
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.