[Patchew-devel] [PATCH] rest: add support for series DELETE in the REST API, and a corresponding unit test

Shubham Jain posted 1 patch 6 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/patchew-ci tags/patchew/20180329054621.27225-1-shubhamjain7495@gmail.com
There is a newer version of this series
api/rest.py        |  2 +-
tests/test_rest.py | 13 +++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
[Patchew-devel] [PATCH] rest: add support for series DELETE in the REST API, and a corresponding unit test
Posted by Shubham Jain 6 years, 3 months ago
---
 api/rest.py        |  2 +-
 tests/test_rest.py | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/api/rest.py b/api/rest.py
index 7c131a4..e966559 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -231,7 +231,7 @@ class SeriesViewSet(BaseMessageViewSet):
     search_fields = (SEARCH_PARAM,)
 
 class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
-                           SeriesViewSet):
+                           SeriesViewSet,mixins.DestroyModelMixin):
     def collect_patches(self, series):
         if series.is_patch:
             patches = [series]
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 28ca10b..458f54e 100755
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -218,6 +218,19 @@ class RestTest(PatchewTestCase):
         resp = self.api_client.get(self.REST_BASE + 'projects/12345/series/?q=project:QEMU')
         self.assertEqual(resp.data['count'], 0)
 
+    def test_series_delete(self):
+        resp1 = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
+                                       self.p.id, '20160628014747.20971-1-famz@redhat.com')
+        
+        resp_before = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
+        self.assertEqual(resp_before.status_code, 200)
+
+        resp = self.api_client.delete(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
+        self.assertEqual(resp.status_code, 204)
+
+        resp_after = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
+        self.assertEqual(resp_after.status_code, 404)
+
     def test_message(self):
         series = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
                                          self.p.id, '20160628014747.20971-1-famz@redhat.com')
-- 
2.14.3 (Apple Git-98)

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] rest: add support for series DELETE in the REST API, and a corresponding unit test
Posted by Fam Zheng 6 years, 3 months ago
On Thu, 03/29 11:16, Shubham Jain wrote:
> ---
>  api/rest.py        |  2 +-
>  tests/test_rest.py | 13 +++++++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/api/rest.py b/api/rest.py
> index 7c131a4..e966559 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -231,7 +231,7 @@ class SeriesViewSet(BaseMessageViewSet):
>      search_fields = (SEARCH_PARAM,)
>  
>  class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
> -                           SeriesViewSet):
> +                           SeriesViewSet,mixins.DestroyModelMixin):

Please add a whitespace after comma:
 
   SeriesViewSet, mixins.DestroyModelMixin

>      def collect_patches(self, series):
>          if series.is_patch:
>              patches = [series]
> diff --git a/tests/test_rest.py b/tests/test_rest.py
> index 28ca10b..458f54e 100755
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -218,6 +218,19 @@ class RestTest(PatchewTestCase):
>          resp = self.api_client.get(self.REST_BASE + 'projects/12345/series/?q=project:QEMU')
>          self.assertEqual(resp.data['count'], 0)
>  
> +    def test_series_delete(self):
> +        resp1 = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
> +                                       self.p.id, '20160628014747.20971-1-famz@redhat.com')

The alignment is one column off            ^

> +        
> +        resp_before = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')

It would be more readable to wrap long lines (limit to 80 or 90 columns). Or
even put the message_id in a variable to avoid typing it 4 times.

Fam

> +        self.assertEqual(resp_before.status_code, 200)
> +
> +        resp = self.api_client.delete(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
> +        self.assertEqual(resp.status_code, 204)
> +
> +        resp_after = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
> +        self.assertEqual(resp_after.status_code, 404)
> +
>      def test_message(self):
>          series = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
>                                           self.p.id, '20160628014747.20971-1-famz@redhat.com')
> -- 
> 2.14.3 (Apple Git-98)
> 
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] rest: add support for series DELETE in the REST API, and a corresponding unit test
Posted by Paolo Bonzini 6 years, 3 months ago
On 29/03/2018 13:08, Fam Zheng wrote:
> On Thu, 03/29 11:16, Shubham Jain wrote:
>> ---
>>  api/rest.py        |  2 +-
>>  tests/test_rest.py | 13 +++++++++++++
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/api/rest.py b/api/rest.py
>> index 7c131a4..e966559 100644
>> --- a/api/rest.py
>> +++ b/api/rest.py
>> @@ -231,7 +231,7 @@ class SeriesViewSet(BaseMessageViewSet):
>>      search_fields = (SEARCH_PARAM,)
>>  
>>  class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
>> -                           SeriesViewSet):
>> +                           SeriesViewSet,mixins.DestroyModelMixin):
> 
> Please add a whitespace after comma:
>  
>    SeriesViewSet, mixins.DestroyModelMixin

You also need to override "perform_destroy" so that the entire subthread
is destroyed (see api/views.py for how to do it).  In the test you can
check whether all the messages are gone with the
/projects/{p.id}/messages/{msgid} endpoint.  Unlike
/projects/{p.id}/series/, that endpoint accepts any message id and not
just the series head.

Also, please send a new patch with all the changes, not just the
incremental difference.  You can pass "-v2" to "git send-email" and/or
"git format-patch" to generate a patch with "PATCH v2" in the subject.

Thanks,

Paolo

>>      def collect_patches(self, series):
>>          if series.is_patch:
>>              patches = [series]
>> diff --git a/tests/test_rest.py b/tests/test_rest.py
>> index 28ca10b..458f54e 100755
>> --- a/tests/test_rest.py
>> +++ b/tests/test_rest.py
>> @@ -218,6 +218,19 @@ class RestTest(PatchewTestCase):
>>          resp = self.api_client.get(self.REST_BASE + 'projects/12345/series/?q=project:QEMU')
>>          self.assertEqual(resp.data['count'], 0)
>>  
>> +    def test_series_delete(self):
>> +        resp1 = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
>> +                                       self.p.id, '20160628014747.20971-1-famz@redhat.com')
> 
> The alignment is one column off            ^
> 
>> +        
>> +        resp_before = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
> 
> It would be more readable to wrap long lines (limit to 80 or 90 columns). Or
> even put the message_id in a variable to avoid typing it 4 times.
> Fam
> 
>> +        self.assertEqual(resp_before.status_code, 200)
>> +
>> +        resp = self.api_client.delete(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
>> +        self.assertEqual(resp.status_code, 204)
>> +
>> +        resp_after = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
>> +        self.assertEqual(resp_after.status_code, 404)
>> +
>>      def test_message(self):
>>          series = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
>>                                           self.p.id, '20160628014747.20971-1-famz@redhat.com')
>> -- 
>> 2.14.3 (Apple Git-98)
>>
>> _______________________________________________
>> Patchew-devel mailing list
>> Patchew-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/patchew-devel
> 
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
[Patchew-devel] [PATCH] Fixed the max chars in line
Posted by Shubham Jain 6 years, 3 months ago
Wraped long lines (limit to 90 columns) to improve the readability of the code
---
 tests/test_rest.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tests/test_rest.py b/tests/test_rest.py
index 458f54e..79de3fd 100755
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -219,16 +219,17 @@ class RestTest(PatchewTestCase):
         self.assertEqual(resp.data['count'], 0)
 
     def test_series_delete(self):
-        resp1 = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
-                                       self.p.id, '20160628014747.20971-1-famz@redhat.com')
+        test_message_id = '20160628014747.20971-1-famz@redhat.com'
+        resp1 = self.apply_and_retrieve('0001-simple-patch.mbox.gz',self.p.id, test_message_id)
+        resp_before = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) 
+                                          + '/series/' + test_message_id + '/')
+        resp = self.api_client.delete(self.REST_BASE + 'projects/' + str(self.p.id) 
+                                      + '/series/' + test_message_id + '/')
+        resp_after = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) 
+                                         + '/series/' + test_message_id + '/')
         
-        resp_before = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
         self.assertEqual(resp_before.status_code, 200)
-
-        resp = self.api_client.delete(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
         self.assertEqual(resp.status_code, 204)
-
-        resp_after = self.api_client.get(self.REST_BASE + 'projects/' + str(self.p.id) + '/series/20160628014747.20971-1-famz@redhat.com/')
         self.assertEqual(resp_after.status_code, 404)
 
     def test_message(self):
-- 
2.14.3 (Apple Git-98)

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