-
Notifications
You must be signed in to change notification settings - Fork 252
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
Add Support for Custom JS/CSS on Job Schedule Form #1866
Add Support for Custom JS/CSS on Job Schedule Form #1866
Conversation
Solid start @tbotnz! Mind updating the |
@bryanculver done |
…ates-on-job-schedule-form
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.
Looking pretty good @tbotnz. Some nitpicks as well as please run black
.
...les/example_plugin/example_plugin/templates/example_plugin/example_with_custom_template.html
Show resolved
Hide resolved
nautobot/extras/views.py
Outdated
job_form = job_class.as_form(request.POST, request.FILES) if job_model.job_class is not None else None | ||
if hasattr(job_class, "template_name"): | ||
template_name = job_class.template_name | ||
except RuntimeError as err: |
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.
What's the case in which this RuntimeError was encountered?
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'm not exactly sure, i did not look into it. This was the in-place exception handling that was used in the codebase for the existing get
method when trying to access the job class. I can remove it if its not required
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 missed that this was patterned after the existing code above 🤦 . Fine to leave as-is!
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 removed this because the error condition this was handling is already being handled below and in fact changes the existing behavior:
https://github.com/nautobot/nautobot/blob/next/nautobot/extras/views.py#L999-L1000
When jobs were no longer installed it redirected to the list view and failed the tests previously for that. Instead of changing that behavior we have slightly duplicate check for the class existing.
The get
and post
could probably be made more succinct with their checks, error messaging, and return paths but I believe they are fine as-is at the moment.
...les/example_plugin/example_plugin/templates/example_plugin/example_with_custom_template.html
Outdated
Show resolved
Hide resolved
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.
Great stuff!
…ates-on-job-schedule-form
Closes: #1865
What's Changed
Just an idea to get the ball rolling on this, can tidy up if required
template_name
prop the to use custom Job templates with custom JS & CSS ( same as object edit view etc )