feat(link-modules,modules-sdk,pricing): Medusa App Migrations + Core compatible migrations #5317
Conversation
🦋 Changeset detectedLatest commit: d9640c2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Ignored Deployments
|
riqwan
requested review from
carlos-r-l-rodrigues,
adrien2p,
olivermrbl and
pKorsholm
riqwan
force-pushed
the
feat/pricing-integration
branch
from
0a2693a to
e114b66
Compare
| // TODO: Remove this when product module is the default product service | ||
| isInternalService: true, |
There was a problem hiding this comment.
question: what does this do?
There was a problem hiding this comment.
This is to load it from the core and not as being a module.
There was a problem hiding this comment.
We need that if the product module is not required to run the pricing module, in that case the pricing modue is linked to the internal core service and the pivot table is managed in the core
| // TODO: Remove this when product module is the default product service | ||
| isInternalService: true, |
There was a problem hiding this comment.
We need that if the product module is not required to run the pricing module, in that case the pricing modue is linked to the internal core service and the pivot table is managed in the core
| // TODO: This is added here to make the schema compatible | ||
| // with the core. Remove this when core is no longer running migrations | ||
| @Property({ columnType: "boolean", nullable: true }) | ||
| includes_tax?: boolean |
There was a problem hiding this comment.
suggestion: If the module is shared and use the core connection, then the migration is managed by the core and the module should not run its migration. it should run its migrations only if it is isolated otherwise we rely on the core. wdyt? In that case we could remove this from here and the migration
There was a problem hiding this comment.
Not a big fan of that idea. The model and schema can easily be corrupted by one another. My ideal scenario would actually be to scope the tables by the name of the module. In a world of modules, I think this would actually be a good thing.
Additionally, we're making an assumption that the core models will be exactly the same as the module models. Which isn't the case with pricing models. There are other migrations that need to run.
There was a problem hiding this comment.
Sorry i thought i did remove this comment 🤣 but i wanted to start a discussion on how we would handle this. Cause another problem could be that when using the module the user might now expect to loose his current data in the core, so i think it is a larger topic that needs to be discussed
|
check if constraints needs to be removed, if any core table needs to be altered when the pricing module is shared, then create a feature flag and a migration in the core that only run under that flag and the migration will manage the alteration of the core tables instead of the module |
|
/snapshot-this |
…compatible migrations (#5317) * chore: remove skipping logic for migrations * chore: medusa app returns migrations to run for modules * chore: added migration for feature compatible * chore: added changelog * chore: create table only if it does not exist * chore: update migration to pluck from registered modules * chore: cleanup * chore: make product an internal service temp * chore: added options and deps to module * chore: added link module options * chore: remove duplicate * chore: added missing column names in core + remove from model * chore: scope migrations to only to create if not exist - money amount, currency --------- Co-authored-by: Sebastian Rindom <skrindom@gmail.com>
RESOLVES CORE-1514
RESOLVES CORE-1513