Commit 5d0140ef authored by Jeremy Kerr's avatar Jeremy Kerr
Browse files

bundles: Remove separate public bundle views



Having two views for bundles (public and non-public) can cause confusion
when bundle owners want to share a URL; it's not obvious that the
private URL isn't shareable.

This change removes the private URLs, and puts all bundles under the
/bundle/<username>/<bundlename>/ URL space. For non-public bundles, this
will just 404 for non-owners.
Signed-off-by: default avatarJeremy Kerr <jk@ozlabs.org>
parent 627f5aca
......@@ -363,12 +363,19 @@ class Bundle(models.Model):
return None
site = Site.objects.get_current()
return 'http://%s%s' % (site.domain,
reverse('patchwork.views.bundle.public',
reverse('patchwork.views.bundle.bundle',
kwargs = {
'username': self.owner.username,
'bundlename': self.name
}))
@models.permalink
def get_absolute_url(self):
return ('patchwork.views.bundle.bundle', (), {
'username': self.owner.username,
'bundlename': self.name,
})
def mbox(self):
return '\n'.join([p.mbox().as_string(True)
for p in self.ordered_patches()])
......
......@@ -22,9 +22,13 @@ import datetime
from django.test import TestCase
from django.test.client import Client
from django.utils.http import urlencode
from django.conf import settings
from patchwork.models import Patch, Bundle, BundlePatch, Person
from patchwork.tests.utils import defaults, create_user, find_in_context
def bundle_url(bundle):
return '/bundle/%s/%s/' % (bundle.owner.username, bundle.name)
class BundleListTest(TestCase):
def setUp(self):
self.user = create_user()
......@@ -78,7 +82,7 @@ class BundleTestBase(TestCase):
class BundleViewTest(BundleTestBase):
def testEmptyBundle(self):
response = self.client.get('/user/bundle/%d/' % self.bundle.id)
response = self.client.get(bundle_url(self.bundle))
self.failUnlessEqual(response.status_code, 200)
page = find_in_context(response.context, 'page')
self.failUnlessEqual(len(page.object_list), 0)
......@@ -86,7 +90,7 @@ class BundleViewTest(BundleTestBase):
def testNonEmptyBundle(self):
self.bundle.append_patch(self.patches[0])
response = self.client.get('/user/bundle/%d/' % self.bundle.id)
response = self.client.get(bundle_url(self.bundle))
self.failUnlessEqual(response.status_code, 200)
page = find_in_context(response.context, 'page')
self.failUnlessEqual(len(page.object_list), 1)
......@@ -95,7 +99,7 @@ class BundleViewTest(BundleTestBase):
for patch in self.patches:
self.bundle.append_patch(patch)
response = self.client.get('/user/bundle/%d/' % self.bundle.id)
response = self.client.get(bundle_url(self.bundle))
pos = 0
for patch in self.patches:
......@@ -113,7 +117,7 @@ class BundleViewTest(BundleTestBase):
bundlepatch.save()
i += 1
response = self.client.get('/user/bundle/%d/' % self.bundle.id)
response = self.client.get(bundle_url(self.bundle))
pos = len(response.content)
for patch in self.patches:
next_pos = response.content.find(patch.name)
......@@ -147,7 +151,7 @@ class BundleUpdateTest(BundleTestBase):
'name': self.newname,
'public': self.publicString(not self.bundle.public)
}
response = self.client.post('/user/bundle/%d/' % self.bundle.id, data)
response = self.client.post(bundle_url(self.bundle), data)
self.assertEqual(response.status_code, 200)
bundle = Bundle.objects.get(pk = self.bundle.pk)
......@@ -162,15 +166,12 @@ class BundleUpdateTest(BundleTestBase):
'name': newname,
'public': self.publicString(self.bundle.public)
}
response = self.client.post('/user/bundle/%d/' % self.bundle.id, data)
self.assertEqual(response.status_code, 200)
response = self.client.post(bundle_url(self.bundle), data)
bundle = Bundle.objects.get(pk = self.bundle.pk)
self.assertRedirects(response, bundle_url(bundle))
self.assertEqual(bundle.name, newname)
self.assertEqual(bundle.public, self.bundle.public)
# check other forms for errors
self.checkPatchformErrors(response)
def testUpdatePublic(self):
newname = 'newbundlename'
data = {
......@@ -179,7 +180,7 @@ class BundleUpdateTest(BundleTestBase):
'name': self.bundle.name,
'public': self.publicString(not self.bundle.public)
}
response = self.client.post('/user/bundle/%d/' % self.bundle.id, data)
response = self.client.post(bundle_url(self.bundle), data)
self.assertEqual(response.status_code, 200)
bundle = Bundle.objects.get(pk = self.bundle.pk)
self.assertEqual(bundle.name, self.bundle.name)
......@@ -202,7 +203,7 @@ class BundlePublicViewTest(BundleTestBase):
super(BundlePublicViewTest, self).setUp()
self.client.logout()
self.bundle.append_patch(self.patches[0])
self.url = '/bundle/%s/%s/' % (self.user.username, self.bundle.name)
self.url = bundle_url(self.bundle)
def testPublicBundle(self):
self.bundle.public = True
......@@ -220,8 +221,52 @@ class BundlePublicViewTest(BundleTestBase):
class BundlePublicViewMboxTest(BundlePublicViewTest):
def setUp(self):
super(BundlePublicViewMboxTest, self).setUp()
self.url = '/bundle/%s/%s/mbox/' % (self.user.username,
self.bundle.name)
self.url = bundle_url(self.bundle) + "mbox/"
class BundlePublicModifyTest(BundleTestBase):
"""Ensure that non-owners can't modify bundles"""
def setUp(self):
super(BundlePublicModifyTest, self).setUp()
self.bundle.public = True
self.bundle.save()
self.other_user = create_user()
def testBundleFormPresence(self):
"""Check for presence of the modify form on the bundle"""
self.client.login(username = self.other_user.username,
password = self.other_user.username)
response = self.client.get(bundle_url(self.bundle))
self.assertNotContains(response, 'name="form" value="bundle"')
def testBundleFormSubmission(self):
oldname = 'oldbundlename'
newname = 'newbundlename'
data = {
'form': 'bundle',
'action': 'update',
'name': newname,
}
self.bundle.name = oldname
self.bundle.save()
# first, check that we can modify with the owner
self.client.login(username = self.user.username,
password = self.user.username)
response = self.client.post(bundle_url(self.bundle), data)
self.bundle = Bundle.objects.get(pk = self.bundle.pk)
self.assertEqual(self.bundle.name, newname)
# reset bundle name
self.bundle.name = oldname
self.bundle.save()
# log in with a different user, and check that we can no longer modify
self.client.login(username = self.other_user.username,
password = self.other_user.username)
response = self.client.post(bundle_url(self.bundle), data)
self.bundle = Bundle.objects.get(pk = self.bundle.pk)
self.assertNotEqual(self.bundle.name, newname)
class BundleCreateFromListTest(BundleTestBase):
def testCreateEmptyBundle(self):
......@@ -547,8 +592,7 @@ class BundleReorderTest(BundleTestBase):
'order_start': firstpatch.id,
'neworder': slice_ids}
response = self.client.post('/user/bundle/%d/' % self.bundle.id,
params)
response = self.client.post(bundle_url(self.bundle), params)
self.failUnlessEqual(response.status_code, 200)
......@@ -579,3 +623,23 @@ class BundleReorderTest(BundleTestBase):
def testBundleReorderMiddle(self):
# reorder only 2nd, 3rd, and 4th patches
self.checkReordering([0,2,3,1,4], 1, 4)
class BundleRedirTest(BundleTestBase):
# old URL: private bundles used to be under /user/bundle/<id>
def setUp(self):
super(BundleRedirTest, self).setUp()
@unittest.skipIf(not settings.COMPAT_REDIR, "compat redirections disabled")
def testBundleRedir(self):
url = '/user/bundle/%d/' % self.bundle.id
response = self.client.get(url)
self.assertRedirects(response, bundle_url(self.bundle))
@unittest.skipIf(not settings.COMPAT_REDIR, "compat redirections disabled")
def testMboxRedir(self):
url = '/user/bundle/%d/mbox/' % self.bundle.id
response = self.client.get(url)
self.assertRedirects(response,'/bundle/%s/%s/mbox/' %
(self.bundle.owner.username,
self.bundle.name))
......@@ -24,7 +24,8 @@ from django.core import mail
from django.core.urlresolvers import reverse
from django.conf import settings
from django.contrib.auth.models import User
from patchwork.models import EmailConfirmation, Person
from patchwork.models import EmailConfirmation, Person, Bundle
from patchwork.tests.utils import defaults
def _confirmation_url(conf):
return reverse('patchwork.views.confirm', kwargs = {'key': conf.key})
......@@ -126,3 +127,31 @@ class UserLoginRedirectTest(TestCase):
response = self.client.get(url)
self.assertRedirects(response, settings.LOGIN_URL + '?next=' + url)
class UserProfileTest(TestCase):
def setUp(self):
self.user = TestUser()
self.client.login(username = self.user.username,
password = self.user.password)
def testUserProfile(self):
response = self.client.get('/user/')
self.assertContains(response, 'User Profile: %s' % self.user.username)
self.assertContains(response, 'User Profile: %s' % self.user.username)
def testUserProfileNoBundles(self):
response = self.client.get('/user/')
self.assertContains(response, 'You have no bundles')
def testUserProfileBundles(self):
project = defaults.project
project.save()
bundle = Bundle(project = project, name = 'test-1',
owner = self.user.user)
bundle.save()
response = self.client.get('/user/')
self.assertContains(response, 'You have the following bundle')
self.assertContains(response, bundle.get_absolute_url())
......@@ -39,10 +39,6 @@ urlpatterns = patterns('',
(r'^user/bundles/$',
'patchwork.views.bundle.bundles'),
(r'^user/bundle/(?P<bundle_id>[^/]+)/$',
'patchwork.views.bundle.bundle'),
(r'^user/bundle/(?P<bundle_id>[^/]+)/mbox/$',
'patchwork.views.bundle.mbox'),
(r'^user/link/$', 'patchwork.views.user.link'),
(r'^user/unlink/(?P<person_id>[^/]+)/$', 'patchwork.views.user.unlink'),
......@@ -66,9 +62,9 @@ urlpatterns = patterns('',
# public view for bundles
(r'^bundle/(?P<username>[^/]*)/(?P<bundlename>[^/]*)/$',
'patchwork.views.bundle.public'),
'patchwork.views.bundle.bundle'),
(r'^bundle/(?P<username>[^/]*)/(?P<bundlename>[^/]*)/mbox/$',
'patchwork.views.bundle.public_mbox'),
'patchwork.views.bundle.mbox'),
(r'^confirm/(?P<key>[0-9a-f]+)/$', 'patchwork.views.confirm'),
......@@ -91,3 +87,13 @@ if settings.ENABLE_XMLRPC:
(r'^project/(?P<project_id>[^/]+)/pwclientrc/$',
'patchwork.views.pwclientrc'),
)
# redirect from old urls
if settings.COMPAT_REDIR:
urlpatterns += patterns('',
(r'^user/bundle/(?P<bundle_id>[^/]+)/$',
'patchwork.views.bundle.bundle_redir'),
(r'^user/bundle/(?P<bundle_id>[^/]+)/mbox/$',
'patchwork.views.bundle.mbox_redir'),
)
......@@ -21,7 +21,7 @@ from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import User
from django.shortcuts import render_to_response, get_object_or_404
from patchwork.requestcontext import PatchworkRequestContext
from django.http import HttpResponse, HttpResponseRedirect
from django.http import HttpResponse, HttpResponseRedirect, HttpResponseNotFound
import django.core.urlresolvers
from patchwork.models import Patch, Bundle, BundlePatch, Project
from patchwork.utils import get_patch_ids
......@@ -125,43 +125,58 @@ def bundles(request):
return render_to_response('patchwork/bundles.html', context)
@login_required
def bundle(request, bundle_id):
bundle = get_object_or_404(Bundle, id = bundle_id)
def bundle(request, username, bundlename):
bundle = get_object_or_404(Bundle, owner__username = username,
name = bundlename)
filter_settings = [(DelegateFilter, DelegateFilter.AnyDelegate)]
if request.method == 'POST' and request.POST.get('form') == 'bundle':
action = request.POST.get('action', '').lower()
if action == 'delete':
bundle.delete()
return HttpResponseRedirect(
django.core.urlresolvers.reverse(
'patchwork.views.user.profile')
)
elif action == 'update':
form = BundleForm(request.POST, instance = bundle)
if form.is_valid():
form.save()
is_owner = request.user == bundle.owner
if not (is_owner or bundle.public):
return HttpResponseNotFound()
if is_owner:
if request.method == 'POST' and request.POST.get('form') == 'bundle':
action = request.POST.get('action', '').lower()
if action == 'delete':
bundle.delete()
return HttpResponseRedirect(
django.core.urlresolvers.reverse(
'patchwork.views.user.profile')
)
elif action == 'update':
form = BundleForm(request.POST, instance = bundle)
if form.is_valid():
form.save()
# if we've changed the bundle name, redirect to new URL
bundle = Bundle.objects.get(pk = bundle.pk)
if bundle.name != bundlename:
return HttpResponseRedirect(bundle.get_absolute_url())
else:
form = BundleForm(instance = bundle)
else:
form = BundleForm(instance = bundle)
else:
form = BundleForm(instance = bundle)
if request.method == 'POST' and request.POST.get('form') == 'reorderform':
order = get_object_or_404(BundlePatch, bundle = bundle,
patch__id = request.POST.get('order_start')).order
for patch_id in request.POST.getlist('neworder'):
bundlepatch = get_object_or_404(BundlePatch,
bundle = bundle, patch__id = patch_id)
bundlepatch.order = order
bundlepatch.save()
order += 1
if request.method == 'POST' and \
request.POST.get('form') == 'reorderform':
order = get_object_or_404(BundlePatch, bundle = bundle,
patch__id = request.POST.get('order_start')).order
for patch_id in request.POST.getlist('neworder'):
bundlepatch = get_object_or_404(BundlePatch,
bundle = bundle, patch__id = patch_id)
bundlepatch.order = order
bundlepatch.save()
order += 1
else:
form = None
context = generic_list(request, bundle.project,
'patchwork.views.bundle.bundle',
view_args = {'bundle_id': bundle_id},
view_args = {'username': bundle.owner.username,
'bundlename': bundle.name},
filter_settings = filter_settings,
patches = bundle.ordered_patches(),
editable_order = True)
......@@ -171,7 +186,13 @@ def bundle(request, bundle_id):
return render_to_response('patchwork/bundle.html', context)
def mbox_response(bundle):
def mbox(request, username, bundlename):
bundle = get_object_or_404(Bundle, owner__username = username,
name = bundlename)
if not (request.user == bundle.owner or bundle.public):
return HttpResponseNotFound()
response = HttpResponse(mimetype='text/plain')
response['Content-Disposition'] = \
'attachment; filename=bundle-%d-%s.mbox' % (bundle.id, bundle.name)
......@@ -179,26 +200,18 @@ def mbox_response(bundle):
return response
@login_required
def mbox(request, bundle_id):
bundle = get_object_or_404(Bundle, id = bundle_id)
return mbox_response(bundle)
def public(request, username, bundlename):
user = get_object_or_404(User, username = username)
bundle = get_object_or_404(Bundle, name = bundlename, owner = user,
public = True)
filter_settings = [(DelegateFilter, DelegateFilter.AnyDelegate)]
context = generic_list(request, bundle.project,
'patchwork.views.bundle.public',
view_args = {'username': username, 'bundlename': bundlename},
filter_settings = filter_settings,
patches = bundle.patches.all())
def bundle_redir(request, bundle_id):
bundle = get_object_or_404(Bundle, id = bundle_id, owner = request.user)
return HttpResponseRedirect(bundle.get_absolute_url())
@login_required
def mbox_redir(request, bundle_id):
bundle = get_object_or_404(Bundle, id = bundle_id, owner = request.user)
return HttpResponseRedirect(django.core.urlresolvers.reverse(
'patchwork.views.bundle.mbox', kwargs = {
'username': request.user.username,
'bundlename': bundle.name,
}))
context['bundle'] = bundle
return render_to_response('patchwork/bundle-public.html', context)
def public_mbox(request, username, bundlename):
bundle = get_object_or_404(Bundle, name = bundlename, public = True)
return mbox_response(bundle)
return response
......@@ -110,6 +110,10 @@ NOTIFICATION_FROM_EMAIL = DEFAULT_FROM_EMAIL
# Set to True to enable the Patchwork XML-RPC interface
ENABLE_XMLRPC = False
# set to True to enable redirections or URLs from previous versions
# of patchwork
COMPAT_REDIR = True
try:
from local_settings import *
except ImportError, ex:
......
{% extends "base.html" %}
{% load person %}
{% block title %}{{project.name}}{% endblock %}
{% block heading %}bundle: {{bundle.owner.username}} /
{{bundle.name}}{% endblock %}
{% block body %}
<p>This bundle contains patches for the {{ bundle.project.linkname }}
project.</p>
<p><a
href="{% url patchwork.views.bundle.public_mbox username=bundle.owner.username bundlename=bundle.name %}"
>Download bundle as mbox</a></p>
{% include "patchwork/patch-list.html" %}
{% endblock %}
......@@ -14,17 +14,17 @@
</script>
{% endblock %}
{% block title %}{{project.name}}{% endblock %}
{% block heading %}bundle: {{bundle.name}}{% endblock %}
{% block heading %}bundle: {{bundle.owner.username}} /
{{bundle.name}}{% endblock %}
{% block body %}
<p>This bundle contains patches for the {{ bundle.project.linkname }}
project.</p>
<p><a href="{% url patchwork.views.bundle.mbox bundle_id=bundle.id %}"
>Download bundle as mbox</a></p>
<p><a href="{% url patchwork.views.bundle.mbox username=bundle.owner.username bundlename=bundle.name %}">Download bundle as mbox</a></p>
{% if bundleform %}
<form method="post">
{% csrf_token %}
<input type="hidden" name="form" value="bundle"/>
......@@ -45,6 +45,7 @@ project.</p>
</form>
<div style="clear: both; padding: 1em;"></div>
{% endif %}
{% include "patchwork/patch-list.html" %}
......
......@@ -17,8 +17,7 @@
</tr>
{% for bundle in bundles %}
<tr>
<td><a href="{% url patchwork.views.bundle.bundle bundle_id=bundle.id %}"
>{{ bundle.name }}</a></td>
<td><a href="{{ bundle.get_absolute_url }}">{{ bundle.name }}</a></td>
<td>{{ bundle.project.linkname }}</td>
<td>
{% if bundle.public %}
......@@ -27,7 +26,7 @@
</td>
<td style="text-align: right">{{ bundle.n_patches }}</td>
<td style="text-align: center;"><a
href="{% url patchwork.views.bundle.mbox bundle_id=bundle.id %}"
href="{% url patchwork.views.bundle.mbox username=bundle.owner.username bundlename=bundle.name %}"
><img src="/images/16-em-down.png" width="16" height="16" alt="download"
title="download"/></a></td>
<td style="text-align: center;">
......
......@@ -104,8 +104,7 @@ address.</p>
<p>You have the following bundle{{ bundle|length|pluralize }}:</p>
<ul>
{% for bundle in bundles %}
<li><a href="{% url patchwork.views.bundle.bundle bundle_id=bundle.id %}"
>{{ bundle.name }}</a></li>
<li><a href="{{ bundle.get_absolute_url }}">{{ bundle.name }}</a></li>
{% endfor %}
</ul>
<p>Visit the <a href="{%url patchwork.views.bundle.bundles %}">bundles
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment