Issue267

Title Make the 'remove' buttons less annoying
Priority bug Status resolved
Superseder Nosy List ajaksu2, eric.araujo, ezio.melotti, hthompson, loewis, r.david.murray
Assigned To ezio.melotti Topics

Created on 2009-04-08.02:47:14 by ajaksu2, last changed 2011-07-26.14:56:47 by ezio.melotti.

Files
File name Uploaded Type Edit Remove
issue267-2.diff ezio.melotti, 2011-07-22.18:40:19 text/plain
issue267.diff ezio.melotti, 2011-07-22.07:28:39 text/plain
undo_and_audit_remove2.diff ajaksu2, 2009-04-13.17:38:26 text/plain
Messages
msg1313 (view) Author: ajaksu2 Date: 2009-04-08.02:47:13
I've seen many people delete messages by accident. Recently, Guilherme Polo
posted a workaround for re-adding them[1]. I've only added UI and a way to store
the issue ID on unlinking.

This patch adds an auditor to that stores the issue ID on a removed file or
message, and offers a 'restore' button on message or file item pages that allows
re-linking to the original issue.

The restore form is only shown for messages/files the unlink auditor marked with
an issue ID.

I'd also like to add JS guards (confirmations) to the remove buttons, will
provide a patch after most issue.item.html patches land.

[1] http://bugs.python.org/msg84430
msg1315 (view) Author: ajaksu2 Date: 2009-04-08.23:33:19
Cleaner auditor.
msg1321 (view) Author: ajaksu2 Date: 2009-04-12.23:24:08
Enhanced auditor to fix a minor security hole: any User can link/unlink files
and messages to/from any issue.
msg1322 (view) Author: ajaksu2 Date: 2009-04-12.23:56:41
Fix typo.
msg1324 (view) Author: loewis Date: 2009-04-13.12:46:25
What's wrong with allowing users to unlink arbitrary files? Anybody who has
write access to the files property should be allowed to do so.
msg1325 (view) Author: ajaksu2 Date: 2009-04-13.16:55:33
Martin v. Löwis wrote::
> What's wrong with allowing users to unlink arbitrary files? Anybody who has
> write access to the files property should be allowed to do so.

I thought the write access was about adding new files, while removing
files should be restricted to those the user has created. At least the
UI only renders the 'remove button' according to the permission below:

def may_edit_file(db, userid, itemid):
    return userid == db.file.get(itemid, "creator")
p = db.security.addPermission(name='Edit', klass='file', check=may_edit_file,
    description="User is allowed to remove their own files")
db.security.addPermissionToRole('User', p)

However, it's possible to perform the Edit action on any files even if
the remove button isn't shown:
http://localhost:9999/python-dev/issue2169?@action=edit&@remove@files=29

IMO this makes it easier to disrupt tracker work, besides making it
trivial replace valid files/patches with exploit-ish ones.
msg1326 (view) Author: ajaksu2 Date: 2009-04-13.17:38:25
This new version forbids re-linking an already linked file or message to
another issues.

If any part of this patch is desirable, maybe it'd be better to split it into
'allow restoring' and 'audit linking/unlinking' detectors?

Also tracked at http://issues.roundup-tracker.org/issue2550536
msg1458 (view) Author: hthompson Date: 2009-08-26.22:06:06
Not sure if you mind random folks from commenting on here.  I found your html
and python for the restore button and unlink auditor very helpful.  After a bit
of tweaking I got it working for me.  Thanks.  One problem I notice testing
this, is that the roundup message order "reverse" seems to fail after doing a
restore.  It seems that the order it is using is the date edited order rather
than the original date order (even though the original date is what is shown). 
Have you noticed this or is this just a problem I have because I don't know how
to do anything other than the default "reverse" ordering of messages?
msg1928 (view) Author: ezio.melotti Date: 2011-03-23.09:51:46
The remove button could be also moved to the msg page, so that it doesn't clutter the issue page and make accidental deleting less likely.  It could also require higher privileges to work, like the spam/ham buttons (currently developers can delete every message).
msg1985 (view) Author: eric.araujo Date: 2011-04-13.15:57:12
I have been telling contributors to clean up the list of files on some bugs.  Having the remove buttons on the main bug page or on message pages works equally well, I think (message pages can be opened in news tabs easily), so +1 on moving the buttons, as long as both the creator of the file and developers can remove (and maybe restore) them.
msg2143 (view) Author: ezio.melotti Date: 2011-07-22.07:28:39
The attached patch adds 
  "This message has been unlinked from issueXXX: [Restore]"
to the msg page when a message has been unlinked and not relinked yet.
When the [Restore] button is pressed the message is added back where it was and the user is sent back to the issue page.

Something like:
  "This message is linked to issueXXX: [Remove]"
could be added as well and the [Remove] buttons could be removed from the issue pages.
msg2147 (view) Author: r.david.murray Date: 2011-07-22.13:34:35
+1 for moving the remove button to the message page.  I think I've used it accidentally more often than I've used it intentionally.
msg2148 (view) Author: eric.araujo Date: 2011-07-22.15:11:28
+1.  What about files?
msg2149 (view) Author: ezio.melotti Date: 2011-07-22.18:40:19
New patch that:
 * adds "This message/file is linked to issueXXX: [Unlink]" when the msg/file is linked;
 * adds "This message/file has been unlinked from issueXXX: [Restore]" when the msg/file is unlinked;
 * removes the [remove] buttons for messages and files in the issue page.
msg2153 (view) Author: eric.araujo Date: 2011-07-22.22:07:58
That sounds very good!  I think you can just go ahead and commit, we’ll get feedback on python-dev after real use just like for previous tracker improvements.

On a related note, I find the use of GETs distasteful, but that’s Roundup.
msg2155 (view) Author: ezio.melotti Date: 2011-07-22.22:45:48
Fixed in r88867.
One think I noticed is that restoring a message seems to repeat all (some of?) the changes done when it was submitted the first time (e.g. adding the user to nosy).  A mail from the original user is also sent again.
The CSS might get some tweaking to center the Author and right-align the date too.
msg2182 (view) Author: eric.araujo Date: 2011-07-26.14:32:03
This is a great change, thanks a lot.  I find it a bit strange that we jump to the bug page after clicking a button on a file page; would it be hard to stay on the file page and get the confirmation message there?
msg2183 (view) Author: ezio.melotti Date: 2011-07-26.14:56:47
That's because the issue page is the target of the POST request, and I don't think that can't be changed.  I actually find it convenient (even if indeed somewhat unexpected), since the issue page is where I would want to go next anyway (to check if the message got remove or came back).
History
Date User Action Args
2011-07-26 14:56:47ezio.melottisetstatus: chatting -> resolved
messages: + msg2183
2011-07-26 14:32:04eric.araujosetstatus: resolved -> chatting
messages: + msg2182
2011-07-25 01:12:17ezio.melottisetstatus: testing -> resolved
2011-07-22 22:45:48ezio.melottisetstatus: chatting -> testing
assignedto: ezio.melotti
messages: + msg2155
2011-07-22 22:07:58eric.araujosetmessages: + msg2153
2011-07-22 18:40:20ezio.melottisetfiles: + issue267-2.diff
messages: + msg2149
2011-07-22 15:11:28eric.araujosetmessages: + msg2148
2011-07-22 13:34:35r.david.murraysetnosy: + r.david.murray
messages: + msg2147
2011-07-22 07:28:39ezio.melottisetfiles: + issue267.diff
messages: + msg2143
2011-04-13 15:57:12eric.araujosetnosy: + eric.araujo
messages: + msg1985
2011-03-23 09:51:46ezio.melottisetnosy: + ezio.melotti
messages: + msg1928
2009-08-26 22:06:07hthompsonsetnosy: + hthompson
messages: + msg1458
2009-04-28 13:05:34ajaksu2setfiles: - undo_and_audit_remove.diff
2009-04-28 13:05:20ajaksu2setfiles: - undo_remove2.diff
2009-04-13 17:38:26ajaksu2setfiles: + undo_and_audit_remove2.diff
messages: + msg1326
2009-04-13 16:55:35ajaksu2setmessages: + msg1325
2009-04-13 12:46:25loewissetnosy: + loewis
messages: + msg1324
2009-04-12 23:56:41ajaksu2setfiles: + undo_and_audit_remove.diff
messages: + msg1322
2009-04-12 23:55:54ajaksu2setfiles: - undo_and_audit_remove.diff
2009-04-12 23:24:08ajaksu2setfiles: + undo_and_audit_remove.diff
messages: + msg1321
2009-04-08 23:33:38ajaksu2setfiles: - undo_remove.diff
2009-04-08 23:33:20ajaksu2setfiles: + undo_remove2.diff
status: unread -> chatting
messages: + msg1315
2009-04-08 02:47:14ajaksu2create