Skip to content
This repository has been archived by the owner on Apr 8, 2023. It is now read-only.

Add discard button on document creation #767

Merged
merged 4 commits into from
May 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions _1327/documents/templates/documents_edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ <h2 class="document-title">{% trans "Edit page" %}</h2>
<button type="submit" class="btn btn-primary">
{% trans 'Save' %}
</button>
{% if document|num_revisions > 0 %}
{% if document|num_revisions == 0 and not form.autosave_id %}
<button type="button" class="btn btn-danger" id="discardDocumentButton" disabled="true">
{% trans 'Discard' %}
</button>
{% elif document|num_revisions > 0 %}
<button type="button" class="btn btn-danger" id="deleteDocumentButton">
{% trans 'Delete' %}
</button>
Expand Down Expand Up @@ -259,6 +263,7 @@ <h4 class="modal-title" id="deleteAutosaveHeader">{% trans "Do you really want t
const destinationElement = $('#shareText');
destinationElement.attr('href', url);
destinationElement.removeClass('hidden');
$("#discardDocumentButton").attr("disabled", false);
},
error: function(jqXHR, textStatus, errorThrown) {
const reasonDisplay = $('.autosaveErrorDialogExplanation');
Expand Down Expand Up @@ -307,7 +312,18 @@ <h4 class="modal-title" id="deleteAutosaveHeader">{% trans "Do you really want t
$('#image-upload-area').removeClass('hidden');
});

{% if document|num_revisions > 0 %}
{% if document|num_revisions == 0 and not form.autosave_id %}
$("#discardDocumentButton").attr("disabled", true);
$('#discardDocumentButton').on('click', function (event) {
$.post(
"{% url "documents:delete_document" document.url_title %}",
{},
function (data, textStatus, jqXHR) {
window.location.replace("/");
}
);
});
{% elif document|num_revisions > 0 %}
function createCascadeList(cascade, domObject) {
for(let i = 0; i < cascade.length; i++) {
const cascadeItem = cascade[i];
Expand Down
40 changes: 39 additions & 1 deletion _1327/documents/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,14 +640,28 @@ def test_autosave_delete_user_different_user(self):
def test_autosave_delete_user_insufficient_permissions(self):
user = baker.make(UserProfile)
autosave = baker.make(TemporaryDocumentText, document=self.document, author=user)

user2 = baker.make(UserProfile)
response = self.app.post(
reverse("documents:delete_autosave", args=[self.document.url_title]),
user=user,
user=user2,
expect_errors=True,
params={"autosave_id": autosave.id}
)
self.assertEqual(response.status_code, 403)

def test_autosave_can_be_deleted(self):
user = baker.make(UserProfile)
autosave = baker.make(TemporaryDocumentText, document=self.document, author=user)
response = self.app.post(
reverse("documents:delete_autosave", args=[self.document.url_title]),
user=user,
expect_errors=True,
params={"autosave_id": autosave.id}
)
self.assertEqual(response.status_code, 302)
self.assertEqual(TemporaryDocumentText.objects.count(), 0)

def test_autosave_delete_not_existing_autosave_id(self):
baker.make(TemporaryDocumentText, document=self.document, author=self.user)
response = self.app.post(
Expand Down Expand Up @@ -1426,6 +1440,12 @@ def test_delete_button_not_present_if_creating_document(self):
self.assertEqual(response.status_code, 200)
self.assertNotIn("deleteDocumentButton", response.body.decode('utf-8'))

def test_discard_button_if_creating_document(self):
# test that the discard button exists if the document that gets edited has no revisions or autosaves
response = self.app.get(reverse(self.document.get_edit_url_name(), args=[self.document.url_title]), user=self.user)
self.assertEqual(response.status_code, 200)
self.assertIn("discardDocumentButton", response.body.decode('utf-8'))

def test_delete_button_present_if_editing_already_existing_document(self):
# test that the delete button is visible if the document has at least one revision
response = self.app.get(reverse(self.document.get_edit_url_name(), args=[self.document.url_title]), user=self.user)
Expand All @@ -1439,6 +1459,24 @@ def test_delete_button_present_if_editing_already_existing_document(self):
self.assertEqual(response.status_code, 200)
self.assertIn("deleteDocumentButton", response.body.decode('utf-8'))

def test_delete_document_in_creation_fails_without_autosave(self):
user = baker.make(UserProfile)
document = baker.make(InformationDocument)
self.assertEqual(Document.objects.count(), 2)
response = self.app.post(reverse("documents:delete_document", args=[document.url_title]), user=user, expect_errors=True)
self.assertEqual(response.status_code, 403)

def test_delete_document_in_creation_with_autosave(self):
user = baker.make(UserProfile)
document = baker.make(InformationDocument)
self.assertEqual(Document.objects.count(), 2)
baker.make(TemporaryDocumentText, document=document, author=user)
self.assertEqual(TemporaryDocumentText.objects.count(), 1)
response = self.app.post(reverse("documents:delete_document", args=[document.url_title]), user=user)
self.assertEqual(response.status_code, 200)
self.assertEqual(Document.objects.count(), 1)
self.assertEqual(TemporaryDocumentText.objects.count(), 0)


class TestPreview(WebTest):
csrf_checks = False
Expand Down
30 changes: 26 additions & 4 deletions _1327/documents/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,24 @@ def change_attachment(request):

def delete_document(request, title):
document = get_object_or_404(Document, url_title=title)
check_permissions(document, request.user, [document.edit_permission_name])

if document.is_in_creation:
try:
# check super user permissions
check_permissions(document, request.user, [document.edit_permission_name])
except PermissionDenied:
# check if an autosave has already been created
autosaves_for_document = TemporaryDocumentText.objects.filter(document=document)
if autosaves_for_document.exists():
# with an unsaved document, only one user can have autosaves
if autosaves_for_document.first().author != request.user:
raise PermissionDenied
else:
# no permission check possible if no autosave was saved (current behavior is not ideal)
raise PermissionDenied
else:
check_permissions(document, request.user, [document.edit_permission_name])

document.delete()

messages.success(request, _("Successfully deleted document: {}").format(document.title))
Expand Down Expand Up @@ -519,14 +536,19 @@ def delete_autosave(request, title):
if request.method != 'POST':
raise Http404

# first check that the user actually may change this document
# check that the user may change this document
document = get_object_or_404(Document, url_title=title)
check_permissions(document, request.user, [document.edit_permission_name])

# second check that the supplied autosave id matches to the document and has been created by the user
# check that the supplied autosave id matches to the document and has been created by the user
autosave_id = request.POST['autosave_id']
autosave = get_object_or_404(TemporaryDocumentText, id=autosave_id)
autosaves_for_object_and_user = TemporaryDocumentText.objects.filter(document=document, author=request.user)

# a new document does not have permissions, just check if the autosave author is correct
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment does not explain why a permission check is done nevertheless if the author does not match

if autosave.author != request.user:
# if the autosave author is not correct, only proceed when the user has superuser privileges by checking permissions
check_permissions(document, request.user, [document.edit_permission_name])

if autosave not in autosaves_for_object_and_user:
raise SuspiciousOperation

Expand Down