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.

Sign up for GitHub

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

Jump to bottom

Add constraints that account_number can't be an empty string #253

Merged
merged 4 commits into from
Sep 13, 2021
Merged

Add constraints that account_number can't be an empty string #253

merged 4 commits into from
Sep 13, 2021

Conversation

lavocatt
Copy link
Contributor

See #240

@lavocatt lavocatt requested a review from croissanne September 10, 2021 10:51
croissanne
croissanne requested changes Sep 10, 2021
View reviewed changes
Copy link
Member

@croissanne croissanne left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! Some thoughts

cmd/image-builder-db-test/main_test.go Show resolved Hide resolved
internal/db/migrations/3_add_alter_table_composes.up.sql Outdated Show resolved Hide resolved
internal/v1/server.go Outdated Show resolved Hide resolved
internal/tutils/idheader_utils.go Outdated Show resolved Hide resolved
internal/tutils/idheader_utils.go Outdated Show resolved Hide resolved
@lavocatt
DB: add constraint, AccountNumber non empty
a9b4bd1 
This commit adds a constraint on the database to make sure the account
number cannot be empty upon composes insertions.
Tests added with db tests on image-builder-db-test/main_test.go
@lavocatt
Tutils: add help to generate idHeader base64
914a0ef
@lavocatt
UnitTests: accountNumber cannot be empty
ab497cd 
Test the requirements of issue osbuild#240, the api cannot let an incoming
request pass without an empty accountNumber
@lavocatt lavocatt requested a review from croissanne September 10, 2021 16:17
croissanne
croissanne approved these changes Sep 13, 2021
View reviewed changes
Copy link
Member

@croissanne croissanne left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

For future reference, UnitTests: in the commit topic can point to the name of the module, so in this case v1, or server.

@croissanne croissanne enabled auto-merge (rebase) September 13, 2021 11:02
@croissanne croissanne merged commit 5c9f1c5 into osbuild:main Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@croissanne croissanne croissanne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lavocatt @croissanne