[PATCH v2 3/3] /api/events: Add 'actor' field to generated JSON

Johan Herland johan at herland.net
Thu Oct 10 03:44:15 AEDT 2019


On Wed, Oct 9, 2019 at 7:59 AM Daniel Axtens <dja at axtens.net> wrote:
> Johan Herland <johan at herland.net> writes:
> > Cc: Mauro Carvalho Chehab <mchehab at osg.samsung.com>
> > Signed-off-by: Johan Herland <johan at herland.net>
> > ---
> >  docs/api/schemas/latest/patchwork.yaml |  6 ++++++
> >  docs/api/schemas/patchwork.j2          |  8 ++++++++
> >  docs/api/schemas/v1.2/patchwork.yaml   |  6 ++++++
>
> I think you might also need to edit docs/usage/overview.rst, sorry.

Ah yes, an obvious oversight on my part. Will fix in v3.

Before I post v3, I'll wait for some more responses to the discussion
on patch 2/3 about the best way (or rather: least horrible way) to
determine the active user and forward it into the event creation.

> Apart from that, lgtm:
> Acked-by: Daniel Axtens <dja at axtens.net>

Thanks for reviewing!

...Johan

> Regards,
> Daniel
>
> >  patchwork/api/event.py                 | 10 +++++++---
> >  patchwork/tests/api/test_event.py      | 24 ++++++++++++++++++++++++
> >  5 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
> > index 45a6118..58d5d44 100644
> > --- a/docs/api/schemas/latest/patchwork.yaml
> > +++ b/docs/api/schemas/latest/patchwork.yaml
> > @@ -1495,6 +1495,12 @@ components:
> >            type: string
> >            format: iso8601
> >            readOnly: true
> > +        actor:
> > +          type: object
> > +          title: The user that caused/created this event
> > +          readOnly: true
> > +          allOf:
> > +            - $ref: '#/components/schemas/UserEmbedded'
> >          payload:
> >            type: object
> >      EventCoverCreated:
> > diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
> > index 16d85a3..1f1a7bd 100644
> > --- a/docs/api/schemas/patchwork.j2
> > +++ b/docs/api/schemas/patchwork.j2
> > @@ -1510,6 +1510,14 @@ components:
> >            type: string
> >            format: iso8601
> >            readOnly: true
> > +{% if version >= (1, 2) %}
> > +        actor:
> > +          type: object
> > +          title: The user that caused/created this event
> > +          readOnly: true
> > +          allOf:
> > +            - $ref: '#/components/schemas/UserEmbedded'
> > +{% endif %}
> >          payload:
> >            type: object
> >      EventCoverCreated:
> > diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml
> > index 3a96aa3..2aaf393 100644
> > --- a/docs/api/schemas/v1.2/patchwork.yaml
> > +++ b/docs/api/schemas/v1.2/patchwork.yaml
> > @@ -1495,6 +1495,12 @@ components:
> >            type: string
> >            format: iso8601
> >            readOnly: true
> > +        actor:
> > +          type: object
> > +          title: The user that caused/created this event
> > +          readOnly: true
> > +          allOf:
> > +            - $ref: '#/components/schemas/UserEmbedded'
> >          payload:
> >            type: object
> >      EventCoverCreated:
> > diff --git a/patchwork/api/event.py b/patchwork/api/event.py
> > index c0d973d..33ea104 100644
> > --- a/patchwork/api/event.py
> > +++ b/patchwork/api/event.py
> > @@ -23,6 +23,7 @@ from patchwork.models import Event
> >  class EventSerializer(ModelSerializer):
> >
> >      project = ProjectSerializer(read_only=True)
> > +    actor = UserSerializer()
> >      patch = PatchSerializer(read_only=True)
> >      series = SeriesSerializer(read_only=True)
> >      cover = CoverLetterSerializer(read_only=True)
> > @@ -50,7 +51,7 @@ class EventSerializer(ModelSerializer):
> >          data = super(EventSerializer, self).to_representation(instance)
> >          payload = OrderedDict()
> >          kept_fields = self._category_map[instance.category] + [
> > -            'id', 'category', 'project', 'date']
> > +            'id', 'category', 'project', 'date', 'actor']
> >
> >          for field in [x for x in data]:
> >              if field not in kept_fields:
> > @@ -65,10 +66,13 @@ class EventSerializer(ModelSerializer):
> >
> >      class Meta:
> >          model = Event
> > -        fields = ('id', 'category', 'project', 'date', 'patch', 'series',
> > -                  'cover', 'previous_state', 'current_state',
> > +        fields = ('id', 'category', 'project', 'date', 'actor', 'patch',
> > +                  'series', 'cover', 'previous_state', 'current_state',
> >                    'previous_delegate', 'current_delegate', 'created_check')
> >          read_only_fields = fields
> > +        versioned_fields = {
> > +            '1.2': ('actor', ),
> > +        }
> >
> >
> >  class EventList(ListAPIView):
> > diff --git a/patchwork/tests/api/test_event.py b/patchwork/tests/api/test_event.py
> > index 8816538..5e47ff3 100644
> > --- a/patchwork/tests/api/test_event.py
> > +++ b/patchwork/tests/api/test_event.py
> > @@ -35,11 +35,16 @@ class TestEventAPI(utils.APITestCase):
> >      def assertSerialized(self, event_obj, event_json):
> >          self.assertEqual(event_obj.id, event_json['id'])
> >          self.assertEqual(event_obj.category, event_json['category'])
> > +        if event_obj.actor is None:
> > +            self.assertIsNone(event_json['actor'])
> >
> >          # nested fields
> >
> >          self.assertEqual(event_obj.project.id,
> >                           event_json['project']['id'])
> > +        if event_obj.actor is not None:
> > +            self.assertEqual(event_obj.actor.id,
> > +                             event_json['actor']['id'])
> >
> >          # TODO(stephenfin): Check other fields
> >
> > @@ -66,10 +71,12 @@ class TestEventAPI(utils.APITestCase):
> >          # check-created
> >          create_check(patch=patch)
> >          # patch-delegated, patch-state-changed
> > +        actor = create_maintainer(project=patch.project)
> >          user = create_maintainer(project=patch.project)
> >          state = create_state()
> >          patch.delegate = user
> >          patch.state = state
> > +        self.assertTrue(patch.is_editable(actor))
> >          patch.save()
> >
> >          return Event.objects.all()
> > @@ -158,3 +165,20 @@ class TestEventAPI(utils.APITestCase):
> >          self.client.force_authenticate(user=user)
> >          resp = self.client.post(self.api_url(), {'category': 'patch-created'})
> >          self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
> > +
> > +    def test_change_delegate(self):
> > +        """Ensure changing patch delegate via API produces expected event"""
> > +        patch = create_patch()
> > +        delegate = create_maintainer(project=patch.project)
> > +        actor = create_maintainer(project=patch.project)
> > +
> > +        self.client.force_authenticate(user=actor)
> > +        patch_url = reverse('api-patch-detail', kwargs={'pk': patch.id})
> > +        resp = self.client.patch(patch_url, {'delegate': delegate.id})
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code, resp)
> > +
> > +        events = Event.objects.all()
> > +        delegation_event = events.get(category='patch-delegated')
> > +        self.assertEqual(actor.id, delegation_event.actor.id)
> > +        self.assertEqual(None, delegation_event.previous_delegate)
> > +        self.assertEqual(delegate.id, delegation_event.current_delegate.id)
> > --
> > 2.19.2
> >
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork



-- 
Johan Herland, <johan at herland.net>
www.herland.net


More information about the Patchwork mailing list