[v3,3/5] views: Stop using Bundle.public_url

Submitted by Stephen Finucane on March 16, 2017, 10:07 p.m.

Details

Message ID 20170316220728.10817-4-stephen@that.guru
State Accepted
Headers show

Commit Message

Stephen Finucane March 16, 2017, 10:07 p.m.
We now have 'get_mbox_url' which is consistent with patches. Let's drop
the older one.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Reviewed-by: Andy Doan <andy.doan@linaro.org>
---
 patchwork/models.py                        | 13 -------------
 patchwork/templates/patchwork/bundles.html |  5 ++---
 patchwork/views/xmlrpc.py                  |  2 +-
 3 files changed, 3 insertions(+), 17 deletions(-)

Comments

Daniel Axtens March 23, 2017, 1:59 a.m.
Hi Stephen,

I see you've merged this series, but I don't think the public URL is the
same as the mbox url.

I created a public bundle in one of my old test instances and navigated to
http://localhost:8000/project/patchwork/bundles/

This gave me a public url of    http://example.com/bundle/dja/aaa/
and an mbox url of              http://localhost:8000/bundle/dja/aaa/mbox/

The public URL, once corrected for the hostname, takes me to a bundle
detail page, which is distinct from the mbox view.

Am I missing something here?

Regards,
Daniel

> We now have 'get_mbox_url' which is consistent with patches. Let's drop
> the older one.
>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Reviewed-by: Andy Doan <andy.doan@linaro.org>
> ---
>  patchwork/models.py                        | 13 -------------
>  patchwork/templates/patchwork/bundles.html |  5 ++---
>  patchwork/views/xmlrpc.py                  |  2 +-
>  3 files changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 199c118..ee929e3 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -29,8 +29,6 @@ import re
>  import django
>  from django.conf import settings
>  from django.contrib.auth.models import User
> -from django.contrib.sites.models import Site
> -from django.core.urlresolvers import reverse
>  from django.db import models
>  from django.utils.encoding import python_2_unicode_compatible
>  from django.utils.functional import cached_property
> @@ -723,17 +721,6 @@ class Bundle(models.Model):
>          return BundlePatch.objects.create(bundle=self, patch=patch,
>                                            order=max_order + 1)
>  
> -    def public_url(self):
> -        if not self.public:
> -            return None
> -        site = Site.objects.get_current()
> -        return 'http://%s%s' % (site.domain,
> -                                reverse('bundle-detail',
> -                                        kwargs={
> -                                            'username': self.owner.username,
> -                                            'bundlename': self.name
> -                                        }))
> -
>      @models.permalink
>      def get_absolute_url(self):
>          return ('bundle-detail', (), {
> diff --git a/patchwork/templates/patchwork/bundles.html b/patchwork/templates/patchwork/bundles.html
> index 83e1bd0..9e225ea 100644
> --- a/patchwork/templates/patchwork/bundles.html
> +++ b/patchwork/templates/patchwork/bundles.html
> @@ -24,12 +24,11 @@
>    <td>{{ bundle.project.linkname }}</td>
>    <td>
>     {% if bundle.public %}
> -    <a href="{{ bundle.public_url }}">{{ bundle.public_url }}</a>
> +    <a href="{{ bundle.get_mbox_url }}">{{ bundle.get_mbox_url }}</a>
>     {% endif %}
>    </td>
>    <td style="text-align: right">{{ bundle.patches.count }}</td>
> -  <td style="text-align: center;"><a
> -   href="{% url 'bundle-mbox' username=bundle.owner.username bundlename=bundle.name %}"
> +  <td style="text-align: center;"><ahref="{{ bundle.get_mbox_url }}"
>     ><span class="glyphicon glyphicon-download-alt"></span></a></td>
>    <td style="text-align: center;">
>     <form method="post"
> diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
> index 66e908e..bb8b729 100644
> --- a/patchwork/views/xmlrpc.py
> +++ b/patchwork/views/xmlrpc.py
> @@ -314,7 +314,7 @@ def bundle_to_dict(obj):
>          'id': obj.id,
>          'name': obj.name,
>          'n_patches': obj.patches.count(),
> -        'public_url': obj.public_url(),
> +        'public_url': obj.get_mbox_url(),
>      }
>  
>  
> -- 
> 2.9.3
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Stephen Finucane March 23, 2017, 12:40 p.m.
On Thu, 2017-03-23 at 12:59 +1100, Daniel Axtens wrote:
> Hi Stephen,
> 
> I see you've merged this series, but I don't think the public URL is
> the
> same as the mbox url.
> 
> I created a public bundle in one of my old test instances and
> navigated to
> http://localhost:8000/project/patchwork/bundles/
> 
> This gave me a public url of    http://example.com/bundle/dja/aaa/
> and an mbox url of              http://localhost:8000/bundle/dja/aaa/
> mbox/
> 
> The public URL, once corrected for the hostname, takes me to a bundle
> detail page, which is distinct from the mbox view.
> 
> Am I missing something here?
> 
> Regards,
> Daniel

Oh, damn - good spot. You're not mistaken - 'public_url' reversed
'bundle-detail' and not 'bundle-mbox'. 'public_url' should have been
updated to use 'reverse' rather than manually building the URL but it
is still a different resource.

I'll re-add that shortly, and revert the 'public_url' -> 'get_mbox_url'
changes.

Stephen

Patch hide | download patch | download mbox

diff --git a/patchwork/models.py b/patchwork/models.py
index 199c118..ee929e3 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -29,8 +29,6 @@  import re
 import django
 from django.conf import settings
 from django.contrib.auth.models import User
-from django.contrib.sites.models import Site
-from django.core.urlresolvers import reverse
 from django.db import models
 from django.utils.encoding import python_2_unicode_compatible
 from django.utils.functional import cached_property
@@ -723,17 +721,6 @@  class Bundle(models.Model):
         return BundlePatch.objects.create(bundle=self, patch=patch,
                                           order=max_order + 1)
 
-    def public_url(self):
-        if not self.public:
-            return None
-        site = Site.objects.get_current()
-        return 'http://%s%s' % (site.domain,
-                                reverse('bundle-detail',
-                                        kwargs={
-                                            'username': self.owner.username,
-                                            'bundlename': self.name
-                                        }))
-
     @models.permalink
     def get_absolute_url(self):
         return ('bundle-detail', (), {
diff --git a/patchwork/templates/patchwork/bundles.html b/patchwork/templates/patchwork/bundles.html
index 83e1bd0..9e225ea 100644
--- a/patchwork/templates/patchwork/bundles.html
+++ b/patchwork/templates/patchwork/bundles.html
@@ -24,12 +24,11 @@ 
   <td>{{ bundle.project.linkname }}</td>
   <td>
    {% if bundle.public %}
-    <a href="{{ bundle.public_url }}">{{ bundle.public_url }}</a>
+    <a href="{{ bundle.get_mbox_url }}">{{ bundle.get_mbox_url }}</a>
    {% endif %}
   </td>
   <td style="text-align: right">{{ bundle.patches.count }}</td>
-  <td style="text-align: center;"><a
-   href="{% url 'bundle-mbox' username=bundle.owner.username bundlename=bundle.name %}"
+  <td style="text-align: center;"><ahref="{{ bundle.get_mbox_url }}"
    ><span class="glyphicon glyphicon-download-alt"></span></a></td>
   <td style="text-align: center;">
    <form method="post"
diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
index 66e908e..bb8b729 100644
--- a/patchwork/views/xmlrpc.py
+++ b/patchwork/views/xmlrpc.py
@@ -314,7 +314,7 @@  def bundle_to_dict(obj):
         'id': obj.id,
         'name': obj.name,
         'n_patches': obj.patches.count(),
-        'public_url': obj.public_url(),
+        'public_url': obj.get_mbox_url(),
     }