8000 Parent object layer for flexible file by koopmant · Pull Request #3821 · comic/grand-challenge.org · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Parent object layer for flexible file #3821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send yo 8000 u account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Feb 10, 2025
Merged

Parent object layer for flexible file #3821

merged 5 commits into from
Feb 10, 2025

Conversation

koopmant
Copy link
Contributor
@koopmant koopmant commented Feb 7, 2025

Add a layer for selecting the parent object type (algorithm job, display set or archive item) to the flexible file widget.

The parent object is used to make the query more specific and smaller.

related to https://github.com/DIAGNijmegen/rse-roadmap/issues/363

@koopmant koopmant self-assigned this Feb 7, 2025
@koopmant koopmant requested a review from amickan February 7, 2025 12:24
@koopmant koopmant marked this pull request as ready for review February 7, 2025 12:24
@koopmant koopmant requested a review from jmsmkn as a code owner February 7, 2025 12:24
@koopmant
Copy link
Contributor Author
koopmant commented Feb 7, 2025

You can test the widget manually on this branch:
https://github.com/comic/grand-challenge.org/tree/integrate-flexible-file-widget

amickan
amickan reviewed Feb 10, 2025
View reviewed changes
amickan
amickan reviewed Feb 10, 2025
View reviewed changes
amickan
amickan previously approved these changes Feb 10, 2025
View reviewed changes
Copy link
Contributor
@amickan amickan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@amickan
Copy link
Contributor
amickan commented Feb 10, 2025 •
edited
Loading

You can test the widget manually on this branch: https://github.com/comic/grand-challenge.org/tree/integrate-flexible-file-widget

I gave it a quick test: looks great! Two things I noticed (maybe you're still working on it, since this is for the next PR, but leaving the comments here anyway):

  1. I think you are still loading the full file queryset for the user on initial widget load (i.e before parent object selection) and on form validation. I think the queryset should be empty at the beginning and appropriately subset at validation, so as to force the user to make a parent object choice. I only took a quick look at the integration branch, so it's possible that I missed the code that does this somewhere, but thought I'd mention it.
  2. The upload widget didn't load fully for me. I didn't get the draf-drop area and hence could not actually upload a new file. You're probably missing some uppy initialization magic somewhere after an htmx load event.

@koopmant koopmant mentioned this pull request Feb 10, 2025
Merged
@koopmant
Copy link
Contributor Author

I gave it a quick test: looks great! Two things I noticed (maybe you're still working on it, since this is for the next PR, but leaving the comments here anyway):

  • I think you are still loading the full file queryset for the user on initial widget load (i.e before parent object selection) and on form validation. I think the queryset should be empty at the beginning and appropriately subset at validation, so as to force the user to make a parent object choice. I only took a quick look at the integration branch, so it's possible that I missed the code that does this somewhere, but thought I'd mention it.
  • The upload widget didn't load fully for me. I didn't get the draf-drop area and hence could not actually upload a new file. You're probably missing some uppy initialization magic somewhere after an htmx load event.

I´ll address these in the next PR.

@koopmant koopmant merged commit 5a8e745 into main Feb 10, 2025
8 checks passed
@koopmant koopmant deleted the parent-object-layer-for-flexible-file branch February 17, 2025 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amickan amickan amickan approved these changes

@jmsmkn jmsmkn Awaiting requested review from jmsmkn jmsmkn is a code owner

Assignees

@koopmant koopmant

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@koopmant @amickan
0