[24.0] do not expand datasets that are known to be inaccessible
This now works better for hdas.
It does not fix existing bug for the corner case scenario of dataset collection that have some inaccessible element in it. This is due DCAs not having the accessible property and it is unclear to me whether including it would 1) make sense 2) impose a high performance penalty.
I was experimenting with making the inaccessible items unselectable, but there are actual operations like hide or delete that apply to these hdas (and are pretty likely to be invoked) even though you do not have access to its contents. Therefore I left it untouched. Note other operations will fail ungracefully - like collection building or tags.
also closes #16903
update: I've started reimplementing the inaccessible dataset state.
Notes:
- This is still broken for inaccessible datasets within collections
- Disable dragging of inaccessible items
- Investigate how this affects multiselect and our fancy keyboard selection
This is to show one approach how to address this -- I'd like to discuss this wider before going further.
I am not sure how much we should rely on the accessible property here, but if we do it makes it easier to catch, however we could possibly run into caching problems.
We could also alter the dataset store so the API call is not directly tied to a computed property (this may be an unwanted vue pattern?) which I hope would prevent the loop, and then catch the 403 and give user the latest information.
I have noticed similar issue also happens to data collections that contain some datasets inaccessible to current user. However in that case the collection is not supposed to expand, but open a different view, and that fails to load completely. So you do not even see the datasets you have access to.
edit: I forgot about it but @davelopez reminded me that ideally the history items would already render as inaccessible (white with padlock).
eventually closes https://github.com/galaxyproject/galaxy/issues/17757
How to test the changes?
(Select all options that apply)
- [ ] I've included appropriate automated tests.
- [ ] This is a refactoring of components with existing test coverage.
- [ ] Instructions for manual testing are as follows:
- [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]
License
- [x] I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.
Alright, I was too eager to get this in and overlooked your updated comments in the PR description :sweat_smile:
This is still broken for inaccessible datasets within collections
I think I have a potential fix for this. Should I push some commits to this branch to test it out?
Alright, I pushed a couple of commits to fix the remaining outstanding issue with inaccessible elements in collections.
Basically, I just serialized the accessible attribute for HDAObjects and make sure we don't even try to request details for inaccessible HDAs in the client. I don't know why this accessible field was never serialized before (maybe for performance reasons?) but it looks to me like it is the right thing to do because the UI state will be incorrect otherwise :thinking:
Before the changes
After the changes
Also, I patched the openapi_to_schema.mjs script to handle boolean constants.
Investigating test failures... of course, It couldn't be so easy... :laughing:
All green now, It wasn't that bad after all :sweat_smile:
lgtm, the only issue I still see is that the animation for expanding/collapsing looks clunky for the grey items. I assume that is because it is bound to use the same-length animation, but has less height, so everything looks sluggish. Not a big deal at all, I'd just ignore it for now.
thanks for pushing it over the finish line @davelopez !
Suspect Issues
This pull request was deployed and Sentry observed the following issues:
- ‼️ AttributeError: 'NoneType' object has no attribute 'dataset'
/api/dataset_collections/{hdca_id}/contents/{pa...View Issue
Did you find this useful? React with a 👍 or 👎
Nice catch Sentry!