| Message ID | 20191007221645.13186-4-johan@herland.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Store the 'actor' responsible for events | expand |
Johan Herland <johan@herland.net> writes: > Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > Signed-off-by: Johan Herland <johan@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. Apart from that, lgtm: Acked-by: Daniel Axtens <dja@axtens.net> 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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On Wed, Oct 9, 2019 at 7:59 AM Daniel Axtens <dja@axtens.net> wrote: > Johan Herland <johan@herland.net> writes: > > Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > > Signed-off-by: Johan Herland <johan@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@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@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/patchwork
On Wed, 2019-10-09 at 18:44 +0200, Johan Herland wrote: > 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. I'll try do just that tomorrow. If I don't, it'll be next week (I'm at PyCon IE this weekend). Just FYI. Stephen
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)
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com> Signed-off-by: Johan Herland <johan@herland.net> --- docs/api/schemas/latest/patchwork.yaml | 6 ++++++ docs/api/schemas/patchwork.j2 | 8 ++++++++ docs/api/schemas/v1.2/patchwork.yaml | 6 ++++++ patchwork/api/event.py | 10 +++++++--- patchwork/tests/api/test_event.py | 24 ++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 3 deletions(-)