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

Wizard: Remove first boot script from services when no script (HMS-5481) #2871

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

regexowl
Copy link
Collaborator

@regexowl regexowl commented Feb 10, 2025

There is a bug that makes custom-first-boot service stay in services when the first boot script is removed.

How to reproduce:

  1. create a blueprint with a first boot script
  2. download the blueprint and confirm custom-first-boot was added to enabled services
  3. click on "Edit blueprint"
  4. go to First boot step and remove the script
  5. save edited blueprint

Current behaviour:

  • custom-first-boot service should be still enabled even with the removed first boot script

Updated behaviour:

  • custom-first-boot is no longer in the blueprint after first boot script got removed

JIRA: HMS-5481

@regexowl regexowl marked this pull request as draft February 10, 2025 13:39
@regexowl regexowl marked this pull request as ready for review February 10, 2025 13:50
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.09%. Comparing base (bf704af) to head (0226386).

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2871   +/-   ##
=======================================
  Coverage   82.09%   82.09%           
=======================================
  Files         207      207           
  Lines       23293    23300    +7     
  Branches     2286     2291    +5     
=======================================
+ Hits        19122    19129    +7     
  Misses       4144     4144           
  Partials       27       27           
Files with missing lines Coverage Δ
...nents/CreateImageWizard/utilities/requestMapper.ts 94.93% <100.00%> (+0.05%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf704af...0226386. Read the comment docs.

lucasgarfield
lucasgarfield previously approved these changes Feb 10, 2025
Copy link
Collaborator

@lucasgarfield lucasgarfield left a comment

Choose a reason for hiding this comment

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

Nice catch! I wonder if this type of thing would be worth checking for in our tests? 🤔

@regexowl
Copy link
Collaborator Author

/jira-epic HMS-5279

@schutzbot schutzbot changed the title Wizard: Remove first boot script from services when no script Wizard: Remove first boot script from services when no script (HMS-5481) Feb 11, 2025
@regexowl
Copy link
Collaborator Author

Nice catch! I wonder if this type of thing would be worth checking for in our tests? 🤔

It would be for sure. Making the code block work properly with the tests is a bit of pain, but tried to add one which uses the upload function with an empty script (body of the code block does not seem reachable via testing-library methods).

There is a bug that makes `custom-first-boot` service stay in services when the first boot script is removed.

How to reproduce:
1. create a blueprint with a first boot script
2. download the blueprint and confirm `custom-first-boot` was added to enabled services
3. click on "Edit blueprint"
4. go to First boot step and remove the script
5. save edited blueprint

Current behaviour:
- `custom-first-boot` service should be still enabled even with the removed first boot script

Updated behaviour:
- `custom-first-boot` is no longer in the blueprint after first boot script got removed
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.

2 participants