-
Notifications
You must be signed in to change notification settings - Fork 8
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
Rework job form #416
Rework job form #416
Conversation
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.
It is a bit odd to me to have the params etc then click next and do job name and trigger... I assume it helps with code reuse between command and job? Visually it might be better to have all the tabs together (optional, required, job trigger) and bucket together everything else above the tabs. Or have the trigger tabs below the regular tabs... But I'm fine being over-ruled on this given the discussion this morning at standup.
When I made a interval job and then went to edit it, not all data was pre-populated (instance, interval). Then after switching to optional tab, everything was gone when I switched back to trigger tab. Cron job was fine to edit.
I can't seem to make a job with a date
trigger:
marshmallow.exceptions.ValidationError: {'trigger': {'run_date': ['Not a valid datetime.']}}
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.
Creating a date trigger job in the past still has a not helpful error. But I'm able to make a job for a valid date so - pass?
Was able to update interval job successfully. Comment doesn't show on job detail page except in the template box, unsure if that is intentional...
LGTM, just remove debugs and make ticket for tests
The date trigger is working the same as old ui. It could be more of a problem on the backend since it allows a successful creation of a job it cannot schedule. I can make a ticket just to track it but since we are going for operation parity with the old ui it should be fine as is for now.
The comment is not for the job detail page but is for the request view page, which is why it is in the template box. |
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.
LGTM
Closes #415
Reworked job form to use react hook form. The form should be able to create a job and update a job.