Arvados

16811-public-favs @ arvados|3c893d29ad7a9acda0c25b47fd0c9ba5a4a5193a

16811-public-favs @ arvados-workbench2|5145f5d248a59bd87869096fddf0365766249575

This works, but

1. I didn't add any tests yet
2. We might want to reconsider the representation

The part to reconsider: previously, a public favorite was a "star" link owned by the "all users" group. The "all users" group is now a "role" and "role" groups cannot own things, only have outgoing links. So now adding a public favorite now involves a two step process of creating a "star" link owned by the system user, and the creating a can_read permission link from "all users" to that individual link object.

A less confusing solution would be a create a new special "Public favorites" project. This would have a well known UUID (zzzzz-j7d0g-publicfavorites) and "all users" would have can_read permission. Then, public favorites would be owned by that project. This would be more like it worked before, but it does require a database migration.

16811-public-favs @ arvados|7e358c01226d55a44ca7b6224f9c33c38510572b

16811-public-favs @ arvados-workbench2|e44d47bdea01e25926f4f3ff750f2d1cb4fd8204

• DatabaseSeeds creates public favorites project and permission link
• Migration ensures public favorites project and permission link exists and adjusts links to new public favorites
• Workbench & Workbench 2 updated to use new publicfavorites project
• Also added check to disallow system users/groups from being destroyed
• On workbench2 the "public favorites" project shows up under "shared with me" but it will appear to be empty. I'm not sure if it should be (1) filtered out from "shared with me", (2) special-cased to display the public favorites view, or (3) the standard project panel view should include display of "star" links. I kind of like (3) the best as it would give us the option to simplify the left panel by removing "favorites" as separate panels.
• Still needs tests

Some comments:

• I’m not sure the migration is reversible the way it’s written. I’ve read the documentation and it seems that only the commands listed on https://api.rubyonrails.org/v5.2.1/classes/ActiveRecord/Migration/CommandRecorder.html are auto-reversible.
• Should the workbenches check the API version so that they can work with older clusters?
• Regarding the UI question on wb2 fav panels: For the time being I guess the less involved option would be the first one, and then we may revisit this topic after all pending tasks for replacing wb1 are ready, wdyt?

Lucas Di Pentima wrote:

Some comments:

• I’m not sure the migration is reversible the way it’s written. I’ve read the documentation and it seems that only the commands listed on https://api.rubyonrails.org/v5.2.1/classes/ActiveRecord/Migration/CommandRecorder.html are auto-reversible.

Instead of 'change' it runs on "up", and "down" doesn't do anything. I don't think trying to do anything in "down" is useful.

• Should the workbenches check the API version so that they can work with older clusters?

If we were going to roll out an updated workbench without updating the API server, we would need to check the API version. But we don't support that in general, and I don't think we're going to do that for on the playground or dev clusters, either.

• Regarding the UI question on wb2 fav panels: For the time being I guess the less involved option would be the first one, and then we may revisit this topic after all pending tasks for replacing wb1 are ready, wdyt?

Yes, that turned out to be pretty easy, I just tweaked the 'shared' query to filter out the publicfavorites project.

I added some tests. These are the first cypress tests I've written. I'm very impressed by the interactive mode, it is a great framework.

16811-public-favs arvados|a13547aec78a75da2174e083f6015280787cd597

16811-public-favs @ arvados-workbench2|91ec75d2d0e388a8818d0d4d2d7c2990cccf7f62

• I think we could do a reversible migration (as it seems to me it’s a trivial task) allowing a correct downgrade process in case it's needed for whatever reason, but if you don’t think that’s needed, I suppose we should be adding an upgrade note warning that public favorites may not be visible in case of a downgrade?
• I ran wb2 tests and got a failure: developer-tests-workbench2: #93

The rest LGTM.

Lucas Di Pentima wrote:

• I think we could do a reversible migration (as it seems to me it’s a trivial task) allowing a correct downgrade process in case it's needed for whatever reason, but if you don’t think that’s needed, I suppose we should be adding an upgrade note warning that public favorites may not be visible in case of a downgrade?

It actually isn't trivial.

The real problem is that fix_roles_projects migration is non-reversible, and that's the one that actually breaks public favorites (because 'star' links are no longer be owned by the 'all users' group). So I would have to adjust that migration as well to be able to fully revert the links back to the 2.0 scheme.

I don't really want to do that. However, you're right that we should document the change in the upgrade notes.

• I ran wb2 tests and got a failure: developer-tests-workbench2: #93

I believe that is because the arvados repo changes need to be merged first.

The rest LGTM.