[Workbench] Interface to list and untrash trashed collections
|Assignee:||Radhika Chippada||% Done:|
|Target version:||2017-06-07 sprint|
|Story points||2.0||Remaining (hours)||0.00 hour|
|Velocity based estimate||-|
Use case: Provide a simple interface for users to review what collections are trashed, and undelete them if desired.
- Add a /trash index page that lists collections with is_trashed=true.
- Display columns: checkbox, Name (with a link to object name / uuid), Contents (similar to /collections page), Trashed at (when it was trashed at), Created at, Un-delete button
- Selection drop down has one item to "un-delete" the selected items in the checkbox
- Search box
- Infinite scrolling (rather than paging - desirable)
#10 Updated by Radhika Chippada 4 months ago
Branch 9587-include-trash-in-group-contents @ 461b3f5a2edb53adda4f3b703d77e9efc0c262e9
Added support for include_trash param in groups_controller -> contents method. This is API server update only.
#12 Updated by Peter Amstutz 4 months ago
is_trashed is set once trash_at is in the past. In the case of temporary files, trash_at is in the future.
I think we want workbench treat files with trash_at in the future similarly to the trashed files (trashed_at in the past).
If we show temporary files (trash_at in the future) as "normal" files, then (a) it defeats the goal of decluttering the workbench listing and (b) means they can't actually be undeleted until trash_at passes.
So, instead of looking for is_trashed=true, Workbench should filter for "trash_at != nil" in the Trash tab and "trash_at == nil" in the regular Collection tab.
#13 Updated by Tom Morris 4 months ago
This is conflating two different things - 1) something which has been deleted/trashed and 2) something which is expiring and will be deleted/trashed in the future.
These aren't the same thing and they mean different things to the user. We need to stop thinking about this from the point of view of technical limitations cause by our particular token expiration strategy and look at it from the point of view of the user.
The story as written was fine. If we want to also implement a filtering strategy for things which will be expiring at some point in the future we can do that, but it isn't the same thing.
#16 Updated by Radhika Chippada 4 months ago
TomM and Peter, the more I look into it, the more this UI design is not making sense to me. Are we sure what we want is a project#Trash tab? It feels like an index page /trashed_collections (or something like that) would be lot more useful for the users.
This project#Trash tab is not making sense because if a user wants to look at trashed collections and untrash, he needs to go into each project and look for these collections. Adding another tab that is not very useful seems like a major wasted investment in a wrong place than doing the more useful /trashed_collections page. I think a /trashed_collections page and a link to it in the "Recent collections" panel on Dashboard might be a better UI design.
#17 Updated by Peter Amstutz 4 months ago
I had a similar thought. Once an object is trashed (not just future trash) it's going to be very hard to find in order to recover it, because the default behavior is to deny that it exists.
One thought I had is if you know the uuid and request the show page in workbench, it could request the record with include_trashed=true and present you with a "do you want to untrash this" page.
#21 Updated by Radhika Chippada 4 months ago
Branch 9587-trash-page @ 6c40c1c5b000b2eb0967df0f7ebaf613393501bc
- Added "Trash" to topnav (right justified to separate from the other Dashboard options)
- Trash page shows collections with is_trashed = true with Un-trash button with recycle icon and checkbox with Selection dropdown option to Un-trash
- Required to expose untrash api on server
- Since we do not want the user to be able to "view" the collection itself, the collection name is displayed as text than a link (with TomM's approval)
- The trash page lists files in the collection as "contents"
- Added search box that searches in trashed collections only
- Added infinite scrolling
- For the time being leaving in the updates from branch "9587-include-trash-in-group-contents", in case we decide to use this in some manner in future
#25 Updated by Radhika Chippada 4 months ago
Added owner to the display @ 88abf688d7545fc663e0422f0c586107d4c56e5a
Tests passed @ https://ci.curoverse.com/job/developer-run-tests/309/
#26 Updated by Lucas Di Pentima 4 months ago
Some comments & questions:
- When the user is on
/trash, how about having a title with the word “Trash” just like when the user is inside a project?
- Maybe it would be a good idea to also have API Server tests checking for the new endpoints.
- The order on which the collections are listed on the trash page is by its
create_atattribute, do you think it would be better if we order it by
modified_atso that the latest trashed collections appear first?
- I’ve been playing with it on arvbox and seen some weird behavior when trashing, then untrashing and then trashing again some collections, it repeats some collections on the trash page, see attached screenshot.
- The recycle button could have a tooltip indicating its function
- We’re showing only 3 files from every trashed collection manifest, isn’t it too expensive to ask for the manifests just to show so little? I’m thinking about production clusters with lots of intermediary trashed collections, the amount of data the api server will have to send when scrolling, I don’t know if it’s worth it.
- If the previous point is OK the way it is, how about taking advantage of having the manifest to show more data about the collection, for example file count and total size so that the user has more information?
#28 Updated by Radhika Chippada 4 months ago
When the user is on /trash, how about having a title with the word “Trash” just like when the user is inside a project?
Updated the display elements to display in "Recent" tab pane
Maybe it would be a good idea to also have API Server tests checking for the new endpoints.
Added controller tests
The order on which the collections are listed on the trash page is by its create_at attribute, do you think it would be better if we order it by trash_at or modified_at so that the latest trashed collections appear first?
I think sorting on created_at would be more meaningful and less confusing, but that is just my opinion. In any case, this is more straightforward implementation without needing extra changes to application_controller->next_page_filters method. Leaving this as is.
I’ve been playing with it on arvbox and seen some weird behavior when trashing, then untrashing and then trashing again some collections, it repeats some collections on the trash page, see attached screenshot.
I made an update around the "last_uuids" and hopefully this is better now
The recycle button could have a tooltip indicating its function
We’re showing only 3 files from every trashed collection manifest, isn’t it too expensive to ask for the manifests just to show so little? I’m thinking about production clusters with lots of intermediary trashed collections, the amount of data the api server will have to send when scrolling, I don’t know if it’s worth it.
Discussed this with TomM as well and we agreed to leave it for now. We can remove this column in future or reduce @limit if need be in the future
If the previous point is OK the way it is, how about taking advantage of having the manifest to show more data about the collection, for example file count and total size so that the user has more information?
Replaced the ellipse display to more useful info saying there are so many more files
#29 Updated by Lucas Di Pentima 4 months ago
- File trash_title_on_trash_page.png added
About the trash page title: I think using the word "Recent" is confusing because in the context of a trash page, it seems to me that "recent" would be referring to "recently trashed items", and this is not the case, because the listing is ordered by creation time.
I was thinking about something along the lines a simple title like in the attached screenshot, to make it clear to someone returning to this browser tab, that is not looking at a normal collection listing, just a personal opinion.
The rest LGTM. Thanks!
#30 Updated by Radhika Chippada 4 months ago
About the trash page title: I think using the word "Recent" is confusing ...
All our pages are sorted on CreatedAt time and use the title (mostly) "Recent". So, I think it probably will not be confusing to the users. I am going to leave it as is for now and we can consider renaming it if there is such a feedback by others. Thanks.
#32 Updated by Radhika Chippada 4 months ago
- Status changed from Resolved to In Progress
Branch 9587-trash-page @ f9ff1c5444f7030fa0b372446bd40d4feed203c8
Lucas: your earlier comment "The order on which the collections are listed on the trash page is by its create_at attribute, do you think it would be better if we order it by trash_at or modified_at so that the latest trashed collections appear first" and the comment about the title of the page, made me revisit this issue. In fact, I went back and saw the original description (where we were contemplating a project tab for trash) and that also said that we want to sort the rows by trash_at time.
Hence, I made one more update to this effect. The rows are now sorted by the "trash_at" time. In addition, I rearranged the columns to display the "Trashed at" column before the "Created at" column, because that makes more sense.
Please take another look of this particular update. Thanks.
#33 Updated by Lucas Di Pentima 4 months ago
Yes, it seems to me this way is more useful, just like a trash can! we remember the last things we throw there, the oldest one are less probable to be needed back :)
About the "Recent" tab, now it's not confusing in the sense of its meaning, but I keep my opinion that the user is not having any context as to where in the workbench this page is placed. For example, when the trash was thought as a "project tab", the project title was the context and to me at least, is enough information for the user to know where she/he is standing.
The way it is now, the user has to read carefully the listing to know that it's placed on the trash page, because the only clue there is to let the user know it's on the trash page, is the URL (I think the page title has lost its purpose now that we use to have tens of open tabs on the same browser window), so if you're revisiting that topic too, I have two suggestions:
- Add a "Trash" title
- Make the "Trash" button have a different background color to let the user know it is "pressed" to assume the page that is being shown is Trash
Sorry for being insistent about this, I just wanted to be clear about my concern about this topic.
The current updates look good, thanks!!
#34 Updated by Radhika Chippada 4 months ago
Renamed the pane name from "Recent" to "Recent trash" (even though I do not think it is necessary because the user got here by clicking on the Trash icon in the topnav) ):
Made the Trash icon slightly bigger (since the Dashboard icon is bigger and hence there is extra room). Didn't make any changes to pressed versus not, since we do not do that anywhere else.