Catalog schema migration #4272
Catalog schema migration #4272
Conversation
|
This is ready for review. @spencern do you want to review this or should I assign to someone else? |
|
@aldeed I'll check this one out. |
There was a problem hiding this comment.
@aldeed I ran this migration against a db that was loaded with 1k products with images via dev-tools from master to this branch. No products were added to the catalog during the migration.
Realized that's actually not what this PR is doing.
Reset everything, loaded up a master branch, loaded 1k products, hit "publish all" which only publishes the products loaded on the grid at the time, not everything in a particular query. Added a few images and published.
Loaded up your branch, the migration ran successfully, but the products that were published had their order reversed.
My guess is that it's due to the updatedAt date being set by your do while.. loop and it's essentially flipping which products were updated most recently.
Not sure if this is a blocker, as updatedAt is not a very consistent merchandising sort anyway.
Otherwise, things looked good.
| let items; | ||
|
|
||
| do { | ||
| items = Catalog.find({ product: { $exists: false } }, { limit: LIMIT }).fetch(); |
There was a problem hiding this comment.
I think adding a sort: { updatedAt: 1 } to the options here would keep everything in the same order.
There was a problem hiding this comment.
I was on the fence about whether a migration should change updatedAt anyway. Thoughts? It is technically updating the doc, but it might also be misleading for all of the updatedAt to update since they were more "rearranged" than "updated"
|
@spencern Grid is sorted by manual positions, then createdAt. So I added sort by My changes are pushed but I didn't retest manually yet. If you retest before I do and it looks fixed, I'm cool with merging this. |
Resolves #4197
Impact: minor
Type: feature
Changes
PR #4218 changed the catalog schema but did not migrate existing data. This is the migration.
Breaking changes
The breaking changes were outlined in PR #4218. This PR does not break anything further.
Testing
masterbranch and reset the database.