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

Add self_service bool to software_installers table #19122

Merged
merged 5 commits into from
May 21, 2024

Conversation

dantecatalfamo
Copy link
Member

@dantecatalfamo dantecatalfamo commented May 17, 2024

#19116

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
  • Added/updated tests
  • If database migrations are included, checked table schema to confirm autoupdate

@dantecatalfamo dantecatalfamo marked this pull request as ready for review May 17, 2024 19:53
@dantecatalfamo dantecatalfamo requested a review from a team as a code owner May 17, 2024 19:53
Copy link
Member

@roperzh roperzh left a comment

Choose a reason for hiding this comment

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

looks good! left a minor note

@dantecatalfamo dantecatalfamo changed the base branch from main to feat-software-self-service May 17, 2024 20:09
Copy link
Member

@roperzh roperzh left a comment

Choose a reason for hiding this comment

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

one more question, have you considered how we're going to distinguish self service "installs" vs "admin installs"?

Martin suggested in #18834 (comment) that we should maybe add a column to host_software_installs too, any thoughts?

@dantecatalfamo
Copy link
Member Author

That suggestion makes sense to me, will add 🙂

@dantecatalfamo dantecatalfamo merged commit b55e298 into feat-software-self-service May 21, 2024
8 checks passed
@dantecatalfamo dantecatalfamo deleted the self-service-migration branch May 21, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants