Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect to collection listing after editing/deleting an item from it #7810

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -17,6 +17,7 @@
<p>{% trans "Are you sure you want to delete this document?" %}</p>
<form action="{% url 'wagtaildocs:delete' document.id %}" method="POST">
{% csrf_token %}
<input type="hidden" value="{{ next }}" name="next">
<input type="submit" value='{% trans "Yes, delete" %}' class="button serious" />
<a href="{% url 'wagtaildocs:index' %}" class="button button-secondary">{% trans "No, don't delete" %}</a>
</form>
Expand Down
3 changes: 2 additions & 1 deletion wagtail/documents/templates/wagtaildocs/documents/edit.html
Expand Up @@ -33,6 +33,7 @@
<div class="col10 divider-after">
<form action="{% url 'wagtaildocs:edit' document.id %}" method="POST" enctype="multipart/form-data" novalidate>
{% csrf_token %}
<input type="hidden" value="{{ next }}" name="next">
<ul class="fields">
{% for field in form %}
{% if field.name == 'file' %}
Expand All @@ -46,7 +47,7 @@
<li>
<input type="submit" value="{% trans 'Save' %}" class="button" />
{% if user_can_delete %}
<a href="{% url 'wagtaildocs:delete' document.id %}" class="button button-secondary no">{% trans "Delete document" %}</a>
<a href="{% url 'wagtaildocs:delete' document.id %}{% if next %}?next={{ next|urlencode }}{% endif %}" class="button button-secondary no">{% trans "Delete document" %}</a>
{% endif %}
</li>
</ul>
Expand Down
Expand Up @@ -40,7 +40,7 @@
<tr>
{% include "wagtailadmin/bulk_actions/listing_checkbox_cell.html" with obj_type="document" obj=doc aria_labelledby_prefix="document_" aria_labelledby=doc.pk|unlocalize aria_labelledby_suffix="_title" %}
<td id="document_{{ doc.pk|unlocalize }}_title" class="title">
<div class="title-wrapper"><a href="{% url 'wagtaildocs:edit' doc.id %}">{{ doc.title }}</a></div>
<div class="title-wrapper"><a href="{% url 'wagtaildocs:edit' doc.id %}{% if next %}?next={{ next|urlencode }}{% endif %}">{{ doc.title }}</a></div>
</td>
<td><a href="{{ doc.url }}" class="nolink" download>{{ doc.filename }}</a></td>
{% if collections %}
Expand Down
85 changes: 77 additions & 8 deletions wagtail/documents/tests/test_admin_views.py
@@ -1,12 +1,14 @@
import json

from unittest import mock
from urllib.parse import quote

from django.contrib.auth.models import Group, Permission
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase
from django.test.utils import override_settings
from django.urls import reverse
from django.utils.http import urlencode

from wagtail.admin.admin_url_finder import AdminURLFinder
from wagtail.core.models import Collection, GroupCollectionPermission, Page
Expand All @@ -21,14 +23,17 @@ class TestDocumentIndexView(TestCase, WagtailTestUtils):
def setUp(self):
self.login()

def get(self, params={}):
return self.client.get(reverse('wagtaildocs:index'), params)

def test_simple(self):
response = self.client.get(reverse('wagtaildocs:index'))
response = self.get()
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'wagtaildocs/documents/index.html')
self.assertContains(response, "Add a document")

def test_search(self):
response = self.client.get(reverse('wagtaildocs:index'), {'q': "Hello"})
response = self.get({'q': "Hello"})
self.assertEqual(response.status_code, 200)
self.assertEqual(response.context['query_string'], "Hello")

Expand All @@ -52,7 +57,7 @@ def test_pagination(self):
def test_pagination_invalid(self):
self.make_docs()

response = self.client.get(reverse('wagtaildocs:index'), {'p': 'Hello World!'})
response = self.get({'p': 'Hello World!'})

# Check response
self.assertEqual(response.status_code, 200)
Expand All @@ -64,7 +69,7 @@ def test_pagination_invalid(self):
def test_pagination_out_of_range(self):
self.make_docs()

response = self.client.get(reverse('wagtaildocs:index'), {'p': 99999})
response = self.get({'p': 99999})

# Check response
self.assertEqual(response.status_code, 200)
Expand All @@ -76,13 +81,13 @@ def test_pagination_out_of_range(self):
def test_ordering(self):
orderings = ['title', '-created_at']
for ordering in orderings:
response = self.client.get(reverse('wagtaildocs:index'), {'ordering': ordering})
response = self.get({'ordering': ordering})
self.assertEqual(response.status_code, 200)

def test_index_without_collections(self):
self.make_docs()

response = self.client.get(reverse('wagtaildocs:index'))
response = self.get()
self.assertNotContains(response, '<th>Collection</th>')
self.assertNotContains(response, '<td>Root</td>')

Expand All @@ -93,7 +98,7 @@ def test_index_with_collection(self):

self.make_docs()

response = self.client.get(reverse('wagtaildocs:index'))
response = self.get()
self.assertContains(response, '<th>Collection</th>')
self.assertContains(response, '<td>Root</td>')
self.assertEqual(
Expand All @@ -105,10 +110,26 @@ def test_collection_nesting(self):
evil_plans = root_collection.add_child(name="Evil plans")
evil_plans.add_child(name="Eviler plans")

response = self.client.get(reverse('wagtaildocs:index'))
response = self.get()
# "Eviler Plans" should be prefixed with &#x21b3 (↳) and 4 non-breaking spaces.
self.assertContains(response, '&nbsp;&nbsp;&nbsp;&nbsp;&#x21b3 Eviler plans')

def test_edit_document_link_contains_next_url(self):
root_collection = Collection.get_first_root_node()
evil_plans_collection = root_collection.add_child(name="Evil plans")

doc = models.Document.objects.create(
title="Test doc",
collection=evil_plans_collection
)

response = self.get({'collection_id': evil_plans_collection.id})
self.assertEqual(response.status_code, 200)

edit_url = reverse('wagtaildocs:edit', args=(doc.id,))
next_url = quote(response._request.get_full_path())
self.assertContains(response, '%s?next=%s' % (edit_url, next_url))


class TestDocumentAddView(TestCase, WagtailTestUtils):
def setUp(self):
Expand Down Expand Up @@ -380,6 +401,27 @@ def test_simple_with_collection_nesting(self):
# "Eviler Plans" should be prefixed with &#x21b3 (↳) and 4 non-breaking spaces.
self.assertContains(response, '&nbsp;&nbsp;&nbsp;&nbsp;&#x21b3 Eviler plans')

def test_next_url_is_present_in_edit_form(self):
root_collection = Collection.get_first_root_node()
evil_plans_collection = root_collection.add_child(name="Evil plans")
doc = models.Document.objects.create(
title="Test doc",
file=get_test_document_file(),
collection=evil_plans_collection
)
expected_next_url = (
reverse('wagtaildocs:index')
+ "?"
+ urlencode({"collection_id": evil_plans_collection.id})
)

response = self.client.get(
reverse('wagtaildocs:edit', args=(doc.id,)),
{"next": expected_next_url}
)
self.assertEqual(response.status_code, 200)
self.assertContains(response, f'<input type="hidden" value="{expected_next_url}" name="next">')

def test_post(self):
# Build a fake file
fake_file = get_test_document_file()
Expand All @@ -397,6 +439,33 @@ def test_post(self):
# Document title should be changed
self.assertEqual(models.Document.objects.get(id=self.document.id).title, "Test document changed!")

def test_edit_with_next_url(self):
root_collection = Collection.get_first_root_node()
evil_plans_collection = root_collection.add_child(name="Evil plans")
doc = models.Document.objects.create(
title="Test doc",
file=get_test_document_file(),
collection=evil_plans_collection
)
expected_next_url = (
reverse('wagtaildocs:index')
+ "?"
+ urlencode({"collection_id": evil_plans_collection.id})
)

response = self.client.post(
reverse('wagtaildocs:edit', args=(doc.id,)),
{
"title": "Edited",
"collection": evil_plans_collection.id,
"next": expected_next_url,
}
)
self.assertRedirects(response, expected_next_url)

doc.refresh_from_db()
self.assertEqual(doc.title, "Edited")

def test_with_missing_source_file(self):
# Build a fake file
fake_file = get_test_document_file()
Expand Down
21 changes: 18 additions & 3 deletions wagtail/documents/views/documents.py
Expand Up @@ -6,13 +6,15 @@
from django.template.response import TemplateResponse
from django.urls import reverse
from django.utils.decorators import method_decorator
from django.utils.http import urlencode
from django.utils.translation import gettext as _
from django.views.generic import TemplateView

from wagtail.admin import messages
from wagtail.admin.auth import PermissionPolicyChecker
from wagtail.admin.forms.search import SearchForm
from wagtail.admin.models import popular_tags_for_model
from wagtail.admin.views.pages.utils import get_valid_next_url_from_request
from wagtail.core.models import Collection
from wagtail.documents import get_document_model
from wagtail.documents.forms import get_document_form
Expand Down Expand Up @@ -72,6 +74,7 @@ def get_context_data(self, **kwargs):
'documents': documents,
'query_string': query_string,
'is_searching': bool(query_string),
'next': self.request.get_full_path(),
})
return context

Expand Down Expand Up @@ -151,6 +154,8 @@ def edit(request, document_id):
if not permission_policy.user_has_permission_for_instance(request.user, 'change', doc):
raise PermissionDenied

next_url = get_valid_next_url_from_request(request)

if request.method == 'POST':
original_file = doc.file
form = DocumentForm(request.POST, request.FILES, instance=doc, user=request.user)
Expand All @@ -176,10 +181,16 @@ def edit(request, document_id):
# Reindex the document to make sure all tags are indexed
search_index.insert_or_update_object(doc)

edit_url = reverse('wagtaildocs:edit', args=(doc.id,))
redirect_url = 'wagtaildocs:index'
if next_url:
edit_url = f"{edit_url}?{urlencode({'next': next_url})}"
redirect_url = next_url
Tijani-Dia marked this conversation as resolved.
Show resolved Hide resolved

messages.success(request, _("Document '{0}' updated").format(doc.title), buttons=[
messages.button(reverse('wagtaildocs:edit', args=(doc.id,)), _('Edit'))
messages.button(edit_url, _('Edit'))
])
return redirect('wagtaildocs:index')
return redirect(redirect_url)
else:
messages.error(request, _("The document could not be saved due to errors."))
else:
Expand Down Expand Up @@ -207,6 +218,7 @@ def edit(request, document_id):
'user_can_delete': permission_policy.user_has_permission_for_instance(
request.user, 'delete', doc
),
'next': next_url,
})


Expand All @@ -218,13 +230,16 @@ def delete(request, document_id):
if not permission_policy.user_has_permission_for_instance(request.user, 'delete', doc):
raise PermissionDenied

next_url = get_valid_next_url_from_request(request)

if request.method == 'POST':
doc.delete()
messages.success(request, _("Document '{0}' deleted.").format(doc.title))
return redirect('wagtaildocs:index')
return redirect(next_url) if next_url else redirect('wagtaildocs:index')

return TemplateResponse(request, "wagtaildocs/documents/confirm_delete.html", {
'document': doc,
'next': next_url,
})


Expand Down
Expand Up @@ -20,6 +20,7 @@
<p>{% trans "Are you sure you want to delete this image?" %}</p>
<form action="{% url 'wagtailimages:delete' image.id %}" method="POST">
{% csrf_token %}
<input type="hidden" value="{{ next }}" name="next">
<input type="submit" value="{% trans 'Yes, delete' %}" class="button serious" />
<a href="{% url 'wagtailimages:index' %}" class="button button-secondary">{% trans "No, don't delete" %}</a>
</form>
Expand Down
5 changes: 3 additions & 2 deletions wagtail/images/templates/wagtailimages/images/edit.html
Expand Up @@ -38,6 +38,7 @@

<form action="{% url 'wagtailimages:edit' image.id %}" method="POST" enctype="multipart/form-data" novalidate>
{% csrf_token %}
<input type="hidden" value="{{ next }}" name="next">
<div class="row row-flush nice-padding">
<div class="col6">
<ul class="fields">
Expand All @@ -54,7 +55,7 @@
<div class="u-hidden@xs">
<input type="submit" value="{% trans 'Save' %}" class="button" />
{% if user_can_delete %}
<a href="{% url 'wagtailimages:delete' image.id %}" class="button button-secondary no">{% trans "Delete image" %}</a>
<a href="{% url 'wagtailimages:delete' image.id %}{% if next %}?next={{ next|urlencode }}{% endif %}" class="button button-secondary no">{% trans "Delete image" %}</a>
{% endif %}
</div>
</div>
Expand Down Expand Up @@ -111,7 +112,7 @@ <h2 class="label no-float u-text-transform-uppercase">{% trans "Focal point" %}
<div class="col5">
<input type="submit" value="{% trans 'Save' %}" class="button" />
{% if user_can_delete %}
<a href="{% url 'wagtailimages:delete' image.id %}" class="button button-secondary no">{% trans "Delete image" %}</a>
<a href="{% url 'wagtailimages:delete' image.id %}{% if next %}?next={{ next }}{% endif %}" class="button button-secondary no">{% trans "Delete image" %}</a>
{% endif %}
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion wagtail/images/templates/wagtailimages/images/results.html
Expand Up @@ -22,7 +22,7 @@ <h2>{% trans "Latest images" %}</h2>
{% for image in images %}
<li>
{% include "wagtailadmin/bulk_actions/listing_checkbox_cell.html" with obj_type="image" obj=image aria_labelledby_prefix="select-image-label image_" aria_labelledby=image.pk|unlocalize aria_labelledby_suffix="_title" %}
<a class="image-choice" title="{% if collections %}{{ image.collection.name }} » {% endif %}{{ image.title }}" href="{% url 'wagtailimages:edit' image.id %}">
<a class="image-choice" title="{% if collections %}{{ image.collection.name }} » {% endif %}{{ image.title }}" href="{% url 'wagtailimages:edit' image.id %}{% if next %}?next={{ next|urlencode }}{% endif %}">
<figure>
{% include "wagtailimages/images/results_image.html" %}
{% trans "pixels" as translated_pixels %}
Expand Down