Defect #28953
open
Issue content gets lost if user deletes an attachment
0%
Description
Given is an already existing issue with one or more attachments. A user wants to edit this issue and add a comment, so he clicks "Edit". While editing and adding the issue's text, he notices an unwanted attachment in the issue head (in the yellow box on top of an issue) - so he clicks the small trash bin icon right to the attachment. The user gets asked if he is sure to delete this file, so he clicks "Yes". Now all of his text changes are gone because Redmine reloads the issue page! And it doesn't even warn the user that all of his changes will get lost.
To resolve this issue, I suggest a solution similar to when a user wants to close a browser tab while there is still an unsaved issue edit. Here the user gets asked if he really wants to close this tab.
The Redmine version is 3.4.5, and I also confirmed this with an older Redmine 2.6.2.
Files
Updated by Takenori TAKAKI over 6 years ago
- File diff-r17439.patch diff-r17439.patch added
I propose a patch to asynchronize deletion of attachment.
With this change, user will not lose his text changes.
This patch works with the latest Redmine trunk (r17439).
Updated by Go MAEDA over 6 years ago
Thank you for posting the interesting patch. But I found a behavior that needs to be fixed.
After deleting all attachments, the "Edit attached files" icon remains displayed (see the screenshot below). I think the icon should not appear if there are no attached files. If you click the icon when there are no attachments, you will see 404 error.
Could you fix the patch?
Updated by Marius BĂLTEANU over 6 years ago
I thought too on removing the attachments via ajax, but there are some cases that must be handled. For example, in addition to the problem reported by Go Maeda, in the issue page, we need to remove the attachment also from the "Edit attached files" section from the edit form.
To reproduce, go to an issue, delete an attachment, click "Edit" and then click "Edit attached files" and you'll see the attachment still in the page.
We can handle also this case in the app/views/attachments/destroy.js.erb, but the code will become quite complex (even with the proposed solution, when you delete an attachment from issue page, the response contains some code that is related to the wiki page). I think that we need a better way to handle these cases and I have in mind the following:
1. in the delete request, send also the entity (issue, wiki, etc) and treat the response depending on this
2. implement the delete logic on each view that has "special" needs by listening on the delete with success event (http://guides.rubyonrails.org/working_with_javascript_in_rails.html#rails-ujs-event-handlers)
Updated by Takenori TAKAKI over 6 years ago
Go MAEDA wrote:
After deleting all attachments, the "Edit attached files" icon remains displayed (see the screenshot below). I think the icon should not appear if there are no attached files. If you click the icon when there are no attachments, you will see 404 error.
Could you fix the patch?
Thank you for pointing out the problem.
I would like to fixe the problem and recreate the patch.
Updated by Takenori TAKAKI over 6 years ago
- File diff-r17442.patch diff-r17442.patch added
Marius BALTEANU wrote:
I thought too on removing the attachments via ajax, but there are some cases that must be handled. For example, in addition to the problem reported by Go Maeda, in the issue page, we need to remove the attachment also from the "Edit attached files" section from the edit form.
I could not realize that attachments and "Edit attached files" is out there...
Thank you for pointing out the problem.
Marius BALTEANU wrote:
We can handle also this case in the
app/views/attachments/destroy.js.erb, but the code will become quite complex (even with the proposed solution, when you delete an attachment from issue page, the response contains some code that is related to the wiki page). I think that we need a better way to handle these cases and I have in mind the following:
1. in the delete request, send also the entity (issue, wiki, etc) and treat the response depending on this
2. implement the delete logic on each view that has "special" needs by listening on the delete with success event (http://guides.rubyonrails.org/working_with_javascript_in_rails.html#rails-ujs-event-handlers)
Thanks for great suggestions!
I recreated the patch with the following way.
1. In the delete request, switch the response data with js / json depending on the presence of "@attachment.container"
2. Implement the delete logic that has required on each view (The way Marius Ionescu suggested)
This patch works with the latest Redmine trunk (r17442).