[PATCH 3/6] views: Add 'bundle_to_mbox' views

[Daniel Axtens]

I've fat-fingered the initial response to this message and accidentally
omitted the list - here's the info for posterity.


Stephen Finucane <stephen at that.guru> writes:

> On Thu, 2017-02-02 at 17:07 +1100, Daniel Axtens wrote:
>> Hi Stephen,
>> 
>> I'm not much of a fan of:
>> 
>>  - mixing cleanup with refactoring,
>
> I thought as much when sending this, but did it anyway. I'll break out
> the import refactoring into a separate patch for v2.
>
>>  - calling this patch a 'views:' patch, given that the view should be
>>    exactly the same before and after,
>
> +1
>  
>>  - adding code to __init__.py
>
> That entire file needs to be broken (I've already started on it), but
> for now that's where the other '_to_mbox' files live. I figure it's
> better to be consistent now and leave any larger refactoring to later
> changes.
>
>>  - testing a view that is a very thin wrapper around the refactored
>>    method, rather than testing the refactored method directly. I
>> would
>>    be more OK if you were testing anything the view adds (like the
>>    content disposition or something like that), but you're only
>> testing
>>    the content, which is just the result of the function.
>
> Also agree on this point, but alas this is how the existing tests in
> that file all function. I'd like to refactor that entire file, but I
> would much rather get 2.0 out before doing so :)
>
> Stephen
>
>> But having said that, these are all nits, and I'm overall quite OK
>> with
>> the idea and patch.
>> 
>> Regards,
>> Daniel
>> 
>> > Signed-off-by: Stephen Finucane <stephen at that.guru>
>> > ---
>> >  patchwork/tests/test_bundles.py | 20 ++++++++++++++++++++
>> >  patchwork/views/__init__.py     |  4 ++++
>> >  patchwork/views/bundle.py       | 26 ++++++++++++++++----------
>> >  3 files changed, 40 insertions(+), 10 deletions(-)
>> > 
>> > diff --git a/patchwork/tests/test_bundles.py
>> > b/patchwork/tests/test_bundles.py
>> > index c185110..2e34174 100644
>> > --- a/patchwork/tests/test_bundles.py
>> > +++ b/patchwork/tests/test_bundles.py
>> > @@ -42,6 +42,11 @@ def bundle_url(bundle):
>> >          'username': bundle.owner.username, 'bundlename':
>> > bundle.name})
>> >  
>> >  
>> > +def bundle_mbox_url(bundle):
>> > +    return reverse('bundle-mbox', kwargs={
>> > +        'username': bundle.owner.username, 'bundlename':
>> > bundle.name})
>> > +
>> > +
>> >  class BundleListTest(TestCase):
>> >  
>> >      def setUp(self):
>> > @@ -120,6 +125,21 @@ class BundleViewTest(BundleTestBase):
>> >              pos = next_pos
>> >  
>> >  
>> > +class BundleMboxTest(BundleTestBase):
>> > +
>> > +    def test_empty_bundle(self):
>> > +        response = self.client.get(bundle_mbox_url(self.bundle))
>> > +        self.assertEqual(response.status_code, 200)
>> > +        self.assertEqual(response.content, '')
>> > +
>> > +    def test_non_empty_bundle(self):
>> > +        self.bundle.append_patch(self.patches[0])
>> > +
>> > +        response = self.client.get(bundle_mbox_url(self.bundle))
>> > +        self.assertEqual(response.status_code, 200)
>> > +        self.assertNotEqual(response.content, '')
>> > +
>> > +
>> >  class BundleUpdateTest(BundleTestBase):
>> >  
>> >      def test_no_action(self):
>> > diff --git a/patchwork/views/__init__.py
>> > b/patchwork/views/__init__.py
>> > index db53cdf..298e0e2 100644
>> > --- a/patchwork/views/__init__.py
>> > +++ b/patchwork/views/__init__.py
>> > @@ -408,3 +408,7 @@ def patch_to_mbox(patch):
>> >          mail = mail.as_string(True)
>> >  
>> >      return mail
>> > +
>> > +
>> > +def bundle_to_mbox(bundle):
>> > +    return '\n'.join([patch_to_mbox(p) for p in
>> > bundle.ordered_patches()])
>> > diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
>> > index aea828d..ec8016b 100644
>> > --- a/patchwork/views/bundle.py
>> > +++ b/patchwork/views/bundle.py
>> > @@ -17,18 +17,25 @@
>> >  # along with Patchwork; if not, write to the Free Software
>> >  # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-
>> > 1307  USA
>> >  
>> > -from __future__ import absolute_import
>> > -
>> >  from django.contrib.auth.decorators import login_required
>> >  import django.core.urlresolvers
>> > -from django.http import (HttpResponse, HttpResponseRedirect,
>> > -                         HttpResponseNotFound)
>> > -from django.shortcuts import render, get_object_or_404
>> > +from django.http import HttpResponse
>> > +from django.http import HttpResponseNotFound
>> > +from django.http import HttpResponseRedirect
>> > +from django.shortcuts import get_object_or_404
>> > +from django.shortcuts import render
>> >  
>> >  from patchwork.filters import DelegateFilter
>> > -from patchwork.forms import BundleForm, DeleteBundleForm
>> > -from patchwork.models import Patch, Bundle, BundlePatch, Project
>> > -from patchwork.views import generic_list, patch_to_mbox,
>> > get_patch_ids
>> > +from patchwork.forms import BundleForm
>> > +from patchwork.forms import DeleteBundleForm
>> > +from patchwork.models import Bundle
>> > +from patchwork.models import BundlePatch
>> > +from patchwork.models import Patch
>> > +from patchwork.models import Project
>> > +from patchwork.views import bundle_to_mbox
>> > +from patchwork.views import generic_list
>> > +from patchwork.views import get_patch_ids
>> > +from patchwork.views import patch_to_mbox
>> >  
>> >  
>> >  @login_required
>> > @@ -136,8 +143,7 @@ def bundle_mbox(request, username, bundlename):
>> >      response = HttpResponse(content_type='text/plain')
>> >      response['Content-Disposition'] = \
>> >          'attachment; filename=bundle-%d-%s.mbox' % (bundle.id,
>> > bundle.name)
>> > -    response.write('\n'.join(
>> > -        [patch_to_mbox(p) for p in bundle.ordered_patches()]))
>> > +    response.write(bundle_to_mbox(bundle))
>> >  
>> >      return response
>> >  
>> > -- 
>> > 2.9.3
>> > 
>> > _______________________________________________
>> > Patchwork mailing list
>> > Patchwork at lists.ozlabs.org
>> > https://lists.ozlabs.org/listinfo/patchwork