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 3 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
19 changes: 19 additions & 0 deletions _1327/documents/templates/documents_edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +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 and not form.autosave_id %}
<button type="button" class="btn btn-danger" id="discardDocumentButton" disabled="true">
{% trans 'Discard' %}
</button>
{% endif %}
{% if document|num_revisions > 0 %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{% endif %}
{% if document|num_revisions > 0 %}
{% elif document|num_revisions > 0 %}

<button type="button" class="btn btn-danger" id="deleteDocumentButton">
{% trans 'Delete' %}
Expand Down Expand Up @@ -259,6 +264,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,6 +313,19 @@ <h4 class="modal-title" id="deleteAutosaveHeader">{% trans "Do you really want t
$('#image-upload-area').removeClass('hidden');
});

{% 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("/");
}
);
});
{% endif %}

{% if document|num_revisions > 0 %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{% endif %}
{% if document|num_revisions > 0 %}
{% elif document|num_revisions > 0 %}

function createCascadeList(cascade, domObject) {
for(let i = 0; i < cascade.length; 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
25 changes: 23 additions & 2 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 @@ -521,12 +538,16 @@ def delete_autosave(request, title):

# first check that the user actually may change this document
Copy link
Collaborator

Choose a reason for hiding this comment

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

the comments ("first ..." and "second ...") need to be updated

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
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:
check_permissions(document, request.user, [document.edit_permission_name])

if autosave not in autosaves_for_object_and_user:
raise SuspiciousOperation

Expand Down