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

2019-11-30 Thread Stephen Finucane
On Thu, 2019-10-17 at 00:44 +0200, Johan Herland wrote:
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Johan Herland 
> Acked-by: Daniel Axtens 

This looks good to me. Since you're respinning though, have you
considered adding the ability to filter events by actor? I don't see it
here and I think it might be useless. If you do add this for v4, take a
look at commit e53d7985a15a90617d653645d1bb0c2693b73ff6 which will
provide a mechanism to exclude the field for API versions < 1.2.

Stephen

> ---
>  docs/api/schemas/latest/patchwork.yaml |  6 ++
>  docs/api/schemas/patchwork.j2  |  8 
>  docs/api/schemas/v1.2/patchwork.yaml   |  6 ++
>  docs/usage/overview.rst|  3 +++
>  patchwork/api/event.py | 10 +++---
>  patchwork/tests/api/test_event.py  | 24 
>  6 files changed, 54 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/docs/usage/overview.rst b/docs/usage/overview.rst
> index e84e13d..273c792 100644
> --- a/docs/usage/overview.rst
> +++ b/docs/usage/overview.rst
> @@ -228,6 +228,9 @@ properties:
>  ``date``
>When this event was created
>  
> +``actor``
> +  The user, if any, that caused/created this event
> +
>  ``payload``
>Additional information
>  
> 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 

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

2019-11-13 Thread Andrew Donnellan

On 17/10/19 9:44 am, Johan Herland wrote:

Cc: Mauro Carvalho Chehab 
Signed-off-by: Johan Herland 
Acked-by: Daniel Axtens 


Looks alright

Reviewed-by: Andrew Donnellan 

--
Andrew Donnellan  OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


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

2019-10-16 Thread Johan Herland
Cc: Mauro Carvalho Chehab 
Signed-off-by: Johan Herland 
Acked-by: Daniel Axtens 
---
 docs/api/schemas/latest/patchwork.yaml |  6 ++
 docs/api/schemas/patchwork.j2  |  8 
 docs/api/schemas/v1.2/patchwork.yaml   |  6 ++
 docs/usage/overview.rst|  3 +++
 patchwork/api/event.py | 10 +++---
 patchwork/tests/api/test_event.py  | 24 
 6 files changed, 54 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/docs/usage/overview.rst b/docs/usage/overview.rst
index e84e13d..273c792 100644
--- a/docs/usage/overview.rst
+++ b/docs/usage/overview.rst
@@ -228,6 +228,9 @@ properties:
 ``date``
   When this event was created
 
+``actor``
+  The user, if any, that caused/created this event
+
 ``payload``
   Additional information
 
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):