Message ID | 20191006225725.5388-4-johan@herland.net |
---|---|
State | Superseded |
Headers | show |
Series | Store the user responsible for patch-related events | expand |
On Mon, 2019-10-07 at 00:57 +0200, Johan Herland wrote: > Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > Signed-off-by: Johan Herland <johan@herland.net> This looks pretty good and should stay almost the same even if earlier patches in the series change. If you can fix up that version check I think we're good to go. Stephen > --- > docs/api/schemas/latest/patchwork.yaml | 6 ++++++ > docs/api/schemas/patchwork.j2 | 6 ++++++ > docs/api/schemas/v1.0/patchwork.yaml | 6 ++++++ > docs/api/schemas/v1.1/patchwork.yaml | 6 ++++++ > docs/api/schemas/v1.2/patchwork.yaml | 6 ++++++ > patchwork/api/event.py | 10 +++++++--- > patchwork/tests/api/test_event.py | 7 +++++++ > 7 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml > index 45a6118..0fb2171 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 > + user: > + type: object > + title: The user responsible for 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..0acb665 100644 > --- a/docs/api/schemas/patchwork.j2 > +++ b/docs/api/schemas/patchwork.j2 > @@ -1510,6 +1510,12 @@ components: > type: string > format: iso8601 > readOnly: true > + user: > + type: object > + title: The user responsible for this event > + readOnly: true > + allOf: > + - $ref: '#/components/schemas/UserEmbedded' You need to surround this with version checks to make sure this doesn't get emitted for the v1.0 and v1.1 APIs as they won't have this field. > payload: > type: object > EventCoverCreated: > diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml > index 02f3a15..a59c64c 100644 > --- a/docs/api/schemas/v1.0/patchwork.yaml > +++ b/docs/api/schemas/v1.0/patchwork.yaml > @@ -1460,6 +1460,12 @@ components: > type: string > format: iso8601 > readOnly: true > + user: > + type: object > + title: The user responsible for this event > + readOnly: true > + allOf: > + - $ref: '#/components/schemas/UserEmbedded' > payload: > type: object > EventCoverCreated: So the above hunk would go... > diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml > index 0c086ed..728b878 100644 > --- a/docs/api/schemas/v1.1/patchwork.yaml > +++ b/docs/api/schemas/v1.1/patchwork.yaml > @@ -1485,6 +1485,12 @@ components: > type: string > format: iso8601 > readOnly: true > + user: > + type: object > + title: The user responsible for this event > + readOnly: true > + allOf: > + - $ref: '#/components/schemas/UserEmbedded' > payload: > type: object > EventCoverCreated: As would this one. > diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml > index 3a96aa3..9093bba 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 > + user: > + type: object > + title: The user responsible for 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..f1b46e5 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) > + user = 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', 'user'] > > 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', 'user', 'patch', > + 'series', 'cover', 'previous_state', 'current_state', > 'previous_delegate', 'current_delegate', 'created_check') > read_only_fields = fields > + versioned_fields = { > + '1.2': ('user', ), > + } > > > class EventList(ListAPIView): > diff --git a/patchwork/tests/api/test_event.py b/patchwork/tests/api/test_event.py > index 8816538..fd42e3d 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.user is None: > + self.assertIsNone(event_json['user']) > > # nested fields > > self.assertEqual(event_obj.project.id, > event_json['project']['id']) > + if event_obj.user is not None: > + self.assertEqual(event_obj.user.id, > + event_json['user']['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 > + maintainer = 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(maintainer)) > patch.save() > > return Event.objects.all() It'd be good to have an actual check that the person that edited the thing is set on the Event.user field.
On Mon, Oct 7, 2019 at 5:55 PM Stephen Finucane <stephen@that.guru> wrote: > On Mon, 2019-10-07 at 00:57 +0200, Johan Herland wrote: > > Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > > Signed-off-by: Johan Herland <johan@herland.net> > > This looks pretty good and should stay almost the same even if earlier > patches in the series change. If you can fix up that version check I > think we're good to go. Thanks again for reviewing. > > diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 > > index 16d85a3..0acb665 100644 > > --- a/docs/api/schemas/patchwork.j2 > > +++ b/docs/api/schemas/patchwork.j2 > > @@ -1510,6 +1510,12 @@ components: > > type: string > > format: iso8601 > > readOnly: true > > + user: > > + type: object > > + title: The user responsible for this event > > + readOnly: true > > + allOf: > > + - $ref: '#/components/schemas/UserEmbedded' > > You need to surround this with version checks to make sure this doesn't > get emitted for the v1.0 and v1.1 APIs as they won't have this field. Will fix. > It'd be good to have an actual check that the person that edited the > thing is set on the Event.user field. Sure, I'll add a test that updates a patch via the REST API, and then asserts that the resulting event has the correct actor set. Have fun! ...Johan
diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml index 45a6118..0fb2171 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 + user: + type: object + title: The user responsible for 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..0acb665 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -1510,6 +1510,12 @@ components: type: string format: iso8601 readOnly: true + user: + type: object + title: The user responsible for this event + readOnly: true + allOf: + - $ref: '#/components/schemas/UserEmbedded' payload: type: object EventCoverCreated: diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml index 02f3a15..a59c64c 100644 --- a/docs/api/schemas/v1.0/patchwork.yaml +++ b/docs/api/schemas/v1.0/patchwork.yaml @@ -1460,6 +1460,12 @@ components: type: string format: iso8601 readOnly: true + user: + type: object + title: The user responsible for this event + readOnly: true + allOf: + - $ref: '#/components/schemas/UserEmbedded' payload: type: object EventCoverCreated: diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml index 0c086ed..728b878 100644 --- a/docs/api/schemas/v1.1/patchwork.yaml +++ b/docs/api/schemas/v1.1/patchwork.yaml @@ -1485,6 +1485,12 @@ components: type: string format: iso8601 readOnly: true + user: + type: object + title: The user responsible for this event + readOnly: true + allOf: + - $ref: '#/components/schemas/UserEmbedded' payload: type: object EventCoverCreated: diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml index 3a96aa3..9093bba 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 + user: + type: object + title: The user responsible for 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..f1b46e5 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) + user = 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', 'user'] 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', 'user', 'patch', + 'series', 'cover', 'previous_state', 'current_state', 'previous_delegate', 'current_delegate', 'created_check') read_only_fields = fields + versioned_fields = { + '1.2': ('user', ), + } class EventList(ListAPIView): diff --git a/patchwork/tests/api/test_event.py b/patchwork/tests/api/test_event.py index 8816538..fd42e3d 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.user is None: + self.assertIsNone(event_json['user']) # nested fields self.assertEqual(event_obj.project.id, event_json['project']['id']) + if event_obj.user is not None: + self.assertEqual(event_obj.user.id, + event_json['user']['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 + maintainer = 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(maintainer)) patch.save() return Event.objects.all()
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 | 6 ++++++ docs/api/schemas/v1.0/patchwork.yaml | 6 ++++++ docs/api/schemas/v1.1/patchwork.yaml | 6 ++++++ docs/api/schemas/v1.2/patchwork.yaml | 6 ++++++ patchwork/api/event.py | 10 +++++++--- patchwork/tests/api/test_event.py | 7 +++++++ 7 files changed, 44 insertions(+), 3 deletions(-)