[Patchew-devel] [PATCH] rest: Add endpoint from update-project-head

Shubham posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/patchew-ci tags/patchew/20180529120532.4246-1-shubhamjain7495@gmail.com
There is a newer version of this series
api/rest.py | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
[Patchew-devel] [PATCH] rest: Add endpoint from update-project-head
Posted by Shubham 5 years, 10 months ago
From: Wingify <wingify@Wingifys-MacBook-Air-3.local>

Added extra action in the ProjectViewSet which will update the project head at endpoint /projects/../update_project_head. This is legacy conversion(UpdateProjectHeadView) into rest
---
 api/rest.py | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/api/rest.py b/api/rest.py
index fa6ca3f..e9282c3 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -18,7 +18,7 @@ from .models import Project, Message
 from .search import SearchEngine
 from rest_framework import (permissions, serializers, viewsets, filters,
     mixins, generics, renderers, status)
-from rest_framework.decorators import detail_route
+from rest_framework.decorators import detail_route, action
 from rest_framework.fields import SerializerMethodField, CharField, JSONField, EmailField
 from rest_framework.relations import HyperlinkedIdentityField
 from rest_framework.response import Response
@@ -141,6 +141,29 @@ class ProjectsViewSet(viewsets.ModelViewSet):
     serializer_class = ProjectSerializer
     permission_classes = (PatchewPermission,)
 
+    @action(methods=['post','get'], detail=True, permission_classes=[ImportPermission])
+    def update_project_head(self, request, pk=None):
+        """
+        updates the project head and message_id which are matched are merged. 
+        Data input format:
+        {
+            "old_head": "..",
+            "new_head": "..",
+            "message_ids": []
+        }
+        """
+        project = self.get_object()
+        head = project.project_head
+        if request.method == 'POST':
+            old_head = request.data['old_head']
+            message_ids = request.data['message_ids']
+            if old_head_0 and old_head_0 != old_head:
+                raise Exception("wrong old head")
+            ret = project.series_update(message_ids)
+            project.project_head = request.data['new_head']
+            Response({"new_head": project.project_head, "count": ret})
+        else:
+            return Response({'project_head': head})
 # Common classes for series and messages
 
 class HyperlinkedMessageField(HyperlinkedIdentityField):
-- 
2.15.1 (Apple Git-101)

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] rest: Add endpoint from update-project-head
Posted by Paolo Bonzini 5 years, 10 months ago
On 29/05/2018 14:05, Shubham wrote:
> From: Wingify <wingify@Wingifys-MacBook-Air-3.local>
> 
> Added extra action in the ProjectViewSet which will update the project head at endpoint /projects/../update_project_head. This is legacy conversion(UpdateProjectHeadView) into rest
> ---
>  api/rest.py | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/api/rest.py b/api/rest.py
> index fa6ca3f..e9282c3 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -18,7 +18,7 @@ from .models import Project, Message
>  from .search import SearchEngine
>  from rest_framework import (permissions, serializers, viewsets, filters,
>      mixins, generics, renderers, status)
> -from rest_framework.decorators import detail_route
> +from rest_framework.decorators import detail_route, action
>  from rest_framework.fields import SerializerMethodField, CharField, JSONField, EmailField
>  from rest_framework.relations import HyperlinkedIdentityField
>  from rest_framework.response import Response
> @@ -141,6 +141,29 @@ class ProjectsViewSet(viewsets.ModelViewSet):
>      serializer_class = ProjectSerializer
>      permission_classes = (PatchewPermission,)
>  
> +    @action(methods=['post','get'], detail=True, permission_classes=[ImportPermission])

I think this should be POST only.  Retrieving the current head should be
doable with the /projects/{pk}/ endpoint.

Also please make the URL use dashes, like update-project-head

> +    def update_project_head(self, request, pk=None):
> +        """
> +        updates the project head and message_id which are matched are merged. 
> +        Data input format:
> +        {
> +            "old_head": "..",
> +            "new_head": "..",
> +            "message_ids": []
> +        }
> +        """
> +        project = self.get_object()
> +        head = project.project_head
> +        if request.method == 'POST':
> +            old_head = request.data['old_head']
> +            message_ids = request.data['message_ids']
> +            if old_head_0 and old_head_0 != old_head:

What is old_head_0?

> +                raise Exception("wrong old head")

This should return an HTTP status code of "409 Conflict" I think.

Finally, you need to add unit tests too.  You can copy-and-paste the
tests for the legacy API.

Thanks,

Paolo

> +            ret = project.series_update(message_ids)
> +            project.project_head = request.data['new_head']
> +            Response({"new_head": project.project_head, "count": ret})
> +        else:
> +            return Response({'project_head': head})
>  # Common classes for series and messages
>  
>  class HyperlinkedMessageField(HyperlinkedIdentityField):


_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] rest: Add endpoint from update-project-head
Posted by Shubham Jain 5 years, 10 months ago
Two questions regarding the patch. I couldn't find the test case in the
legacy update-head and hence couldn't think of how to test this.
Second, Is the data required in the POST correct? I mean, is there any
other way we can feed the message ids?

On Tue, May 29, 2018 at 5:35 PM Shubham <shubhamjain7495@gmail.com> wrote:

> From: Wingify <wingify@Wingifys-MacBook-Air-3.local>
>
> Added extra action in the ProjectViewSet which will update the project
> head at endpoint /projects/../update_project_head. This is legacy
> conversion(UpdateProjectHeadView) into rest
> ---
>  api/rest.py | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/api/rest.py b/api/rest.py
> index fa6ca3f..e9282c3 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -18,7 +18,7 @@ from .models import Project, Message
>  from .search import SearchEngine
>  from rest_framework import (permissions, serializers, viewsets, filters,
>      mixins, generics, renderers, status)
> -from rest_framework.decorators import detail_route
> +from rest_framework.decorators import detail_route, action
>  from rest_framework.fields import SerializerMethodField, CharField,
> JSONField, EmailField
>  from rest_framework.relations import HyperlinkedIdentityField
>  from rest_framework.response import Response
> @@ -141,6 +141,29 @@ class ProjectsViewSet(viewsets.ModelViewSet):
>      serializer_class = ProjectSerializer
>      permission_classes = (PatchewPermission,)
>
> +    @action(methods=['post','get'], detail=True,
> permission_classes=[ImportPermission])
> +    def update_project_head(self, request, pk=None):
> +        """
> +        updates the project head and message_id which are matched are
> merged.
> +        Data input format:
> +        {
> +            "old_head": "..",
> +            "new_head": "..",
> +            "message_ids": []
> +        }
> +        """
> +        project = self.get_object()
> +        head = project.project_head
> +        if request.method == 'POST':
> +            old_head = request.data['old_head']
> +            message_ids = request.data['message_ids']
> +            if old_head_0 and old_head_0 != old_head:
> +                raise Exception("wrong old head")
> +            ret = project.series_update(message_ids)
> +            project.project_head = request.data['new_head']
> +            Response({"new_head": project.project_head, "count": ret})
> +        else:
> +            return Response({'project_head': head})
>  # Common classes for series and messages
>
>  class HyperlinkedMessageField(HyperlinkedIdentityField):
> --
> 2.15.1 (Apple Git-101)
>
>
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] rest: Add endpoint from update-project-head
Posted by Paolo Bonzini 5 years, 10 months ago
On 29/05/2018 14:07, Shubham Jain wrote:
> Two questions regarding the patch. I couldn't find the test case in the
> legacy update-head and hence couldn't think of how to test this.

You're right, the only test case for update-head uses patchew-cli to
invoke it.  It's in commit ea3c69962a38c77c43672190dda314c56aaa983a.

tests/test_testing.py has some example of manipulating a git repository
from the tests.  See for example add_file_and_commit and
test_tester_project.

Thanks,

Paolo

> Second, Is the data required in the POST correct? I mean, is there any
> other way we can feed the message ids?

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] rest: Add endpoint from update-project-head
Posted by Shubham Jain 5 years, 10 months ago
So I tried doing something like this. But I'm still not sure how to add the
the series to the git and which series should be added.
def test_update_project_head(self):
        resp = self.apply_and_retrieve('some.mbox.gz',
                                       self.p.id, 'some_message_id')
        self.api_client.login(username=self.user, password=self.password)

        resp_before = self.api_client.get(self.PROJECT_BASE + "series/"+
"some_message_id/")
        data = {
                "message_ids": [...],
                "old_head": "..",
                "new_head": "..."
                }
        resp = self.api_client.post(self.PROJECT_BASE +
"update_project_head/", data=data, content_type='application/json')
        resp_after = self.api_client.get(self.PROJECT_BASE + "series/"+
"some_message_id")
        self.assertEquals(resp_before.data['is_merged'], False)
        self.assertEquals(resp.status_code, 200)
        self.assertEquals(resp.data['count'],1)
        self.assertEquals(resp.data['new_head'],"..")
        self.assertEquals(resp_after.data['is_merged'], True)

On Tue, May 29, 2018 at 5:44 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 29/05/2018 14:07, Shubham Jain wrote:
> > Two questions regarding the patch. I couldn't find the test case in the
> > legacy update-head and hence couldn't think of how to test this.
>
> You're right, the only test case for update-head uses patchew-cli to
> invoke it.  It's in commit ea3c69962a38c77c43672190dda314c56aaa983a.
>
> tests/test_testing.py has some example of manipulating a git repository
> from the tests.  See for example add_file_and_commit and
> test_tester_project.
>
> Thanks,
>
> Paolo
>
> > Second, Is the data required in the POST correct? I mean, is there any
> > other way we can feed the message ids?
>
>
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] rest: Add endpoint from update-project-head
Posted by Paolo Bonzini 5 years, 10 months ago
On 30/05/2018 09:21, Shubham Jain wrote:
> So I tried doing something like this. But I'm still not sure how to add
> the the series to the git

Maybe test_need_apply is the right one:

    def test_need_apply(self):
        self.cli_import("0001-simple-patch.mbox.gz")
        s = Message.objects.series_heads()[0]
        self.assertEqual(s.is_complete, True)
        self.assertEqual(s.git_result.status, Result.PENDING)
        self.do_apply()

In fact, applying the series to git is not necessary now that I think of
it.  You can use a dummy old_head and new_head, just make sure you pass
the right message-id to message_ids.

Thanks,

Paolo

> and which series should be added. 
> def test_update_project_head(self):
>         resp = self.apply_and_retrieve('some.mbox.gz',
>                                        self.p.id <http://self.p.id>,
> 'some_message_id')
>         self.api_client.login(username=self.user,
> password=self.password)        
>         resp_before = self.api_client.get(self.PROJECT_BASE + "series/"+
> "some_message_id/")
>         data = {
>                 "message_ids": [...],
>                 "old_head": "..",
>                 "new_head": "..."
>                 }
>         resp = self.api_client.post(self.PROJECT_BASE +
> "update_project_head/", data=data, content_type='application/json')
>         resp_after = self.api_client.get(self.PROJECT_BASE + "series/"+
> "some_message_id")
>         self.assertEquals(resp_before.data['is_merged'], False)
>         self.assertEquals(resp.status_code, 200)
>         self.assertEquals(resp.data['count'],1)
>         self.assertEquals(resp.data['new_head'],"..")
>         self.assertEquals(resp_after.data['is_merged'], True)
> 
> On Tue, May 29, 2018 at 5:44 PM Paolo Bonzini <pbonzini@redhat.com
> <mailto:pbonzini@redhat.com>> wrote:
> 
>     On 29/05/2018 14:07, Shubham Jain wrote:
>     > Two questions regarding the patch. I couldn't find the test case
>     in the
>     > legacy update-head and hence couldn't think of how to test this.
> 
>     You're right, the only test case for update-head uses patchew-cli to
>     invoke it.  It's in commit ea3c69962a38c77c43672190dda314c56aaa983a.
> 
>     tests/test_testing.py has some example of manipulating a git repository
>     from the tests.  See for example add_file_and_commit and
>     test_tester_project.
> 
>     Thanks,
> 
>     Paolo
> 
>     > Second, Is the data required in the POST correct? I mean, is there any
>     > other way we can feed the message ids?
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel