Skip to content
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.

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

Already on GitHub? Sign in to your account

(GUI) Set a new resource's default group selection to the user's group #771

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

mtrapnell-nist
Copy link
Collaborator

This GUI change sets the group field on the different resource creation pages to automatically populate with the user's group.

@henrychoy
Copy link
Collaborator

Looks great to me! One minor detail is that when editing an experiment or entrypoint, then leaving without saving, then returning, it'll prompt you if you want to load the previous values, if you say cancel, then we probably shouldn't clear out the group. You can fix this by editing the clearForm() function in those files.

@keithmanville
Copy link
Collaborator

LGTM as well. I'd recommend this for merging once you address Henry's comment.

The git commit message will need to be re-written to pass gitlint.

I saw the sorting tags test fail. I saw this crop up somewhere before, so I created an issue on it here with some thoughts on the problem and suggestions for fixing it: #772

@mtrapnell-nist
Copy link
Collaborator Author

Thanks for the review! When attempting to implement the suggestion to not clear the group field in clearForm(), I noticed a different unexpected behavior which I am now looking into.

  • Click create new entrypoint and enter data,
  • leave without saving,
  • click create an entrypoint again,
  • click cancel on the prompt to load previous values,
  • then immediately leave the page without saving again,
  • click create entrypoint a third time,
  • it prompts a second time to load saved values

It only happens on the first 2 times after trying to create the entrypoint, then everything works as expected. This is consistent for both experiments and entrypoints.

@henrychoy
Copy link
Collaborator

Thanks for the review! When attempting to implement the suggestion to not clear the group field in clearForm(), I noticed a different unexpected behavior which I am now looking into.

As discussed on the call, this PR is good to merge as-is. And I can look at not clearing out the group separately here: #774

This GUI change sets the group field on the different resource creation pages to automatically
populate with the user's group.
@keithmanville keithmanville merged commit 10eff3f into dev Feb 28, 2025
11 checks passed
@keithmanville keithmanville deleted the mt-545 branch February 28, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QoL Improvement for GUI - Set groups field to current user group
3 participants