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

RUN-551: Vue conversion of job option editor #8922

Merged
merged 167 commits into from Mar 18, 2024
Merged

RUN-551: Vue conversion of job option editor #8922

merged 167 commits into from Mar 18, 2024

Conversation

gschueler
Copy link
Member

Is this a bugfix, or an enhancement? Please describe.
Vue conversion of Job Option edit section

Copy link
Contributor

@smartinellibenedetti smartinellibenedetti left a comment

Choose a reason for hiding this comment

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

Thank you for tackling this piece of the conversion, it was huge!

From the FE perspective, I tested the following:

  • creating new options with all the input types (it's working);
  • editing and saving (it's not working 100% due to data mismatches);
  • duplicating the options (working fine, but if you duplicate an option that UI isn't handling correctly, the duplicate will have the same issues)
  • changing the order of options by clicking the arrows/drag and drop (working 100%)
  • downloading and uploading the job definition (working, tested json and yaml)
  • saving a job that has a schedule with options with missing properties (working)
  • undo & redo (working)

Majority of my comments are small nits/things that can be done at a later time, but marked as blockers the ones that might be confusing/problematic for the users (I got myself confused a few times until realizing what was going on).

Also, is it intentional to have the whole object output in the top error message?
Screenshot 2024-03-12 at 2 55 30 PM

Copy link
Contributor

@smartinellibenedetti smartinellibenedetti left a comment

Choose a reason for hiding this comment

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

Thank you so much for accepting the suggestions and implementing them. I confirmed all the fixes and the validation toggling the job schedule.

Left a suggestion to add the errorList component on the required radio buttons, but other than that g2g!

@gschueler gschueler merged commit 972488f into main Mar 18, 2024
4 checks passed
@gschueler gschueler deleted the RUN-551 branch March 18, 2024 22:50
@gschueler gschueler changed the title RUN-551: Vue conversion of job option editor (WIP) RUN-551: Vue conversion of job option editor Mar 19, 2024
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.

None yet

2 participants