Arvados

Branch 9587-wb-trash-tab pushed so Radhika can continue from there.
The parameter include_trash is on the collection's controller, so I was going to see if it should be added to the projects controller too, when I had to switch focus to other stories.

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.

Test run @ https://ci.curoverse.com/job/developer-run-tests/291/

Branch 9587-include-trash-in-group-contents LGTM, thanks.

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.

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.

Should the Trash tab show collections with "trash_at" in the future? Because if not, there won't be any way in the UI to stop a collection from being trashed in the future.

(The description, as written, does not define precisely the criteria for a collection to appear under the "Trash" tab)

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.

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.

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

Test run @ https://ci.curoverse.com/job/developer-run-tests/308/

Added owner to the display @ 88abf688d7545fc663e0422f0c586107d4c56e5a

Tests passed @ https://ci.curoverse.com/job/developer-run-tests/309/

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_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’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?

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

Added

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

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.

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!!

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.

Thanks.