Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Add not-null constraint for column trackedentitytypeid in trackedentity table [DHIS2-15066] #19112
base: master
Are you sure you want to change the base?
feat: Add not-null constraint for column trackedentitytypeid in trackedentity table [DHIS2-15066] #19112
Changes from 23 commits
4627513
adf3d6a
962dae2
972f8d5
e642c35
290ad52
bee4104
900f3d7
62ef7d6
ea2402b
81daef6
4f26b27
adc0326
4ac4a44
13024a3
2baca3a
38edde3
b0797bc
e62c46b
72868f4
746ec8b
1484545
c3359ce
b265862
d885171
964ed52
91189c9
6dcdf05
f2adebf
cdb5bf8
659a986
cfea998
2682751
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
setTrackedEntityType
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.
Here we shouldn't update any record.
We should only apply the constraint.
The we can suggest such a script in the migration notes
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.
there is a lower possibility of errors if we handle the task ourselves rather than leaving it to the user. Therefore, we should aim to complete as much of the migration process as possible and leave as little as necessary for the user to manage.
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.
For this kind of process that deal with data we need to be much more clear with the user and very very careful on any step.
Here you are fixing inconsistent data in a correct way, getting the tracked entity type from any underling enrollment but this is not the only issue that could happen and this SQL is running in a transaction (even without specifically start the transaction as done here, flyway will wrap this in a transaction), so if there is any inconsistent data that cannot be fixed in the way you provided, no data is fixed.
Than the user is sent to the guide and there we are not providing any explanation of what we attempted in the migration and we are not providing any option of fixing the data, only a script to delete tracked entitities and with all the enrollments and events.
To resume:
If the user has a number x of "fixable" tracked entities and one that is not automatically fixable, we are deleting ALL the tracked entities with inconsistent data.