-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: populate version in assets_versions for 1st entry #221
Conversation
Pull Request Test Coverage Report for Build 4593350100Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
@@ -0,0 +1 @@ | |||
UPDATE assets_versions SET version = '0.1' WHERE version = ''; |
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.
Instead of modifying the db for it, can we handle it in go? Like if no version, assume 0.1?
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.
How will we query the database for an asset's first version? Currently we're unable to do so.
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.
In that case, instead of running migrations for version 0.1, I can add an extra case to query depending on the version number 0.1 like this:
+ if version == "0.1" {
+ ast, err := r.getByVersion(ctx, id, version, r.GetByID, sq.And{
+ sq.Eq{"a.urn": urn},
+ sq.IsNull{"a.version"},
+ }
+ else{
ast, err := r.getByVersion(ctx, id, version, r.GetByID, sq.Eq{
"a.asset_id": id,
"a.version": version,
})
+ }
Should we proceed with this?
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.
Okay make sense, then I think it's better we put the db
migration call.
assets_versions table.
column was not populated.
Closes: #215