-
Notifications
You must be signed in to change notification settings - Fork 50
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
Harmony 1892 - Add/remove existing labels to jobs via Workflow UI #656
Conversation
handle mulitple users editing a job's labels
deployments that didn't already have labels
TODO - hide labels if too many
…stays in the same spot
Curious to see if anyone else experiences the database "hanging" behavior I was seeing locally after making a few labeling (or pause/resume/cancel) requests via the UI. (The behavior was also reproduced via the API endpoints.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - just have a couple things that I think should be addressed.
- A feature toggle would be good so that we can turn on in test environments and get feedback prior to enabling in production.
- Long labels cause the dropdown to not fit in the browser display - should probably do some kind of truncation similar to the label display.
Other feedback:
3. Selecting the label feature without any rows selected on the page is a bit confusing seeing a greyed out list of labels. I'm thinking we probably don't want to show the labels until at least one row is selected.
4. Now that we can select jobs in terminal states the cancel and pause buttons will not appear if you select a mix of active and complete jobs. I think my preference would be to show the buttons if any of the jobs you select are active and only send cancel/pause events to those active jobs.
I also verified harmony in a box works correctly.
…ns to be union instead of intersection
James and I decided that we should also reject any labels that contain commas. We can discuss this further next week if necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested out the changes I requested and everything looks good.
Jira Issue ID
HARMONY-1892
Description
This pull request allows users to select jobs from the jobs page of the Workflow UI (using the non-admin page) and to subsequently label them using the label drop-down. For MVP, it currently only uses the set of labels that have been created via the
label
request parameter. I'll update the tickets' AC and descriptions once we are OK with merging this as an MVP.Local Test Steps
1 - Make requests with different labels http://localhost:3000/C1233800302-EEDTEST/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?granuleId=G1233800343-EEDTEST&format=image%2Ftiff&label=one&label=two&label=three
2 - Use the workflow UI to add and remove labels in bulk
UPDATE:
Also test that a request with labels rejects a label with a comma in it.
PR Acceptance Checklist