Update backfill migration to also set label in dataReceived column #1048
Conversation
test/integration/other/migrations.js
Outdated
| // Create two entity defs | ||
| await container.run(sql` | ||
| INSERT INTO entity_defs ("entityId", "createdAt", "creatorId", "current", "label", "data") | ||
| VALUES (${newEntity.id}, now(), 5, true, 'some label', '{"foo": "bar"}')`); |
There was a problem hiding this comment.
Seems like current should be false if another def is about to be created.
There was a problem hiding this comment.
it's not the best/most coherent entity def 😅 but at least current is not a relevant part of the migration.
There was a problem hiding this comment.
I will make these improvements in case anyone (future me?) decides to "borrow" this entity-insertion code in the future...
| VALUES (${newEntity.id}, now(), 5, true, 'some label', '{"foo": "bar"}')`); | ||
|
|
||
| await container.run(sql` | ||
| INSERT INTO entity_defs ("entityId", "createdAt", "creatorId", "current", "label", "data", "version") |
There was a problem hiding this comment.
Should version be specified for both defs? The baseVersion of the first def will be null after the migration, but I think its version should be 1.
There was a problem hiding this comment.
The column has a default value of 1, which I visually saw, and is how the baseVersion gets set to null.
Closes getodk/central#540
I debated between adding a new migration that would fill in this data, or changing the old migration file. I decided to change the old migration file, the one that added
baseVersionand that introduced the columndataReceived, to also populate the label field indataReceived.I was nervous about getting that change right so I wrote a test, but I've committed the code with the test skipped.
What has been done to verify that this works as intended?
A test.
Why is this the best possible solution? Were any other approaches considered?
I think this is tidier. The change is simple, it'll mean fewer migrations have to run on upgrade. The only downside is some of our test data may not have the label populated.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
no
Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.
no
Before submitting this PR, please make sure you have:
make test-fulland confirmed all checks still pass OR confirm CircleCI build passes