Skip to content
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: CartRegion link, definition + workflow #6392

Merged
merged 15 commits into from
Feb 16, 2024

Conversation

olivermrbl
Copy link
Contributor

No description provided.

Copy link

changeset-bot bot commented Feb 13, 2024

🦋 Changeset detected

Latest commit: 8aaab81

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@medusajs/medusa Patch
@medusajs/core-flows Patch
@medusajs/link-modules Patch
@medusajs/modules-sdk Patch
@medusajs/types Patch

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

Copy link

vercel bot commented Feb 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
medusa-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 16, 2024 1:42pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Feb 16, 2024 1:42pm
docs-ui ⬜️ Ignored (Inspect) Visit Preview Feb 16, 2024 1:42pm
medusa-docs ⬜️ Ignored (Inspect) Visit Preview Feb 16, 2024 1:42pm

@olivermrbl olivermrbl changed the title feat: Cart <> Region link feat: CartRegion link, definition + workflow Feb 14, 2024
Copy link
Collaborator

@srindom srindom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be changed to readonly link

Copy link
Member

@sradevski sradevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not 100% sure how all of this ties together, so the comments are mostly on style/correctness of the code itself

@olivermrbl olivermrbl requested a review from sradevski February 16, 2024 07:59
@olivermrbl
Copy link
Contributor Author

olivermrbl commented Feb 16, 2024

Made a couple of additional changes in 1597853:

  • Removed old create-cart workflow from the create cart endpoint, because it was messing with some test suites. This is theoretically a breaking change, as people could be using the it, but since the feature is in beta, and we've communicated that things are subject to change, I decided to get rid of it
  • Added a transformer to add region currency to cart in case currency code is not passed to the workflow
  • Moved workflow input type to @medusajs/types

@olivermrbl olivermrbl requested a review from srindom February 16, 2024 11:42
Comment on lines 18 to 25
serviceName: Modules.REGION,
relationship: {
serviceName: Modules.CART,
primaryKey: "id",
foreignKey: "cart_id",
alias: "carts",
isList: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
serviceName: Modules.REGION,
relationship: {
serviceName: Modules.CART,
primaryKey: "id",
foreignKey: "cart_id",
alias: "carts",
isList: true,
},
serviceName: Modules.REGION,
relationship: {
serviceName: Modules.CART,
primaryKey: "region_id",
foreignKey: "id",
alias: "carts",
isList: true,
},

Copy link
Contributor

@carlos-r-l-rodrigues carlos-r-l-rodrigues Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can cover these configs on the tests, querying these new properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this: 2225f15

Copy link
Contributor

@carlos-r-l-rodrigues carlos-r-l-rodrigues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kodiakhq kodiakhq bot merged commit 24fb102 into develop Feb 16, 2024
17 checks passed
@kodiakhq kodiakhq bot deleted the feat/create-cart-with-region branch February 16, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants