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

Implement an option to choose a job type on relaunch (issue #14177) #15249

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

Sasa993
Copy link
Contributor

@Sasa993 Sasa993 commented Jun 4, 2024

SUMMARY

Supersedes #14837 - thought removing the issue ID from the feature branch would make the api-schema check in the workflow successful.

Fixes 14177 issue.
This PR adds a feature for users to change the Job Type (run type) on relaunching the job.

ISSUE TYPE
  • New or Enhanced Feature
COMPONENT NAME
  • API
  • UI
AWX VERSION
23.6.1.dev334+ge2758724b1
ADDITIONAL INFORMATION

Before this change, modifying the run type required creating a new job, which could be inconvenient, especially for jobs with numerous prompts. The aim is to improve the user experience by allowing users to choose the run type while relaunching the job.

I decided to use a dropdown instead of modal (modal was suggested in the issue).
By implementing a dropdown instead of a modal, we avoid potential negative impacts on the user experience. The dropdown was chosen as it is already used for failed jobs, providing a consistent interface.

The drop-down menu with the options for the job type will only appear when the job is relaunched from the Job Details page (as agreed in the previous PR discussion)

@Sasa993 Sasa993 marked this pull request as ready for review June 5, 2024 07:37
@Sasa993 Sasa993 changed the title Restart check job as run job Implement an option to choose a job type on relaunch (issue #14177) Jun 5, 2024
@fosterseth
Copy link
Member

fosterseth commented Jun 5, 2024

thanks, this is a nice addition to the job relaunch feature

it would be nice to add a functional api test around this

  • mock a job with job type check
  • post to the api relaunch endpoint with job check run
  • assert new job created with proper job type

take a look at some other tests have that creates/mocks job runs to get a good example to work with, and let me know if you need help with it

@fosterseth fosterseth self-assigned this Jun 5, 2024
@Sasa993
Copy link
Contributor Author

Sasa993 commented Jun 7, 2024

thanks, this is a nice addition to the job relaunch feature

it would be nice to add a functional api test around this

  • mock a job with job type check
  • post to the api relaunch endpoint with job check run
  • assert new job created with proper job type

take a look at some other tests have that creates/mocks job runs to get a good example to work with, and let me know if you need help with it

Thank you. I have now added a working API test.

@fosterseth
Copy link
Member

I'll also mention that we have a new UI that is going to replace the old UI eventually

You may wish to create a PR (or an issue) here as well once the api changes land https://github.com/ansible/ansible-ui

@Sasa993
Copy link
Contributor Author

Sasa993 commented Jun 14, 2024

I'll also mention that we have a new UI that is going to replace the old UI eventually

You may wish to create a PR (or an issue) here as well once the api changes land https://github.com/ansible/ansible-ui

Yeah, I noticed that and it looks really good. Will do :)

@Sasa993 Sasa993 force-pushed the restart-check-job-as-run-job branch 2 times, most recently from 905450f to ea49da0 Compare June 24, 2024 10:03
@Sasa993 Sasa993 force-pushed the restart-check-job-as-run-job branch from ea49da0 to 00a015f Compare July 1, 2024 15:03
@djyasin
Copy link
Member

djyasin commented Jul 3, 2024

Hello @Sasa993! Thank you so much for making the requested changes. We have gone ahead and re-run CI checks. Would you mind rebasing this again when you have a chance?

@Sasa993 Sasa993 force-pushed the restart-check-job-as-run-job branch from 00a015f to a7f600d Compare July 4, 2024 06:25
@Sasa993
Copy link
Contributor Author

Sasa993 commented Jul 4, 2024

Hello @Sasa993! Thank you so much for making the requested changes. We have gone ahead and re-run CI checks. Would you mind rebasing this again when you have a chance?

Done 😃
I can't figure out why api-schema keeps failing. Do you have any ideas?

@Sasa993 Sasa993 force-pushed the restart-check-job-as-run-job branch 2 times, most recently from e5dac84 to 177f4ae Compare July 14, 2024 16:15
@Sasa993 Sasa993 force-pushed the restart-check-job-as-run-job branch from 177f4ae to 91700f2 Compare July 18, 2024 10:04
@Sasa993
Copy link
Contributor Author

Sasa993 commented Jul 18, 2024

Hi @fosterseth,
Any idea when this might be merged? I'd like to implement this on the new UI as well.

Copy link

@Sasa993 Sasa993 force-pushed the restart-check-job-as-run-job branch from db4e8b8 to b8dcbd9 Compare December 10, 2024 15:12
@Sasa993 Sasa993 force-pushed the restart-check-job-as-run-job branch from b8dcbd9 to dc2fa0e Compare January 14, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restart a check job as a run job
3 participants