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(medusa,types): add promotion list/get endpoint #6110

Merged
merged 16 commits into from
Jan 18, 2024

Conversation

riqwan
Copy link
Contributor

@riqwan riqwan commented Jan 17, 2024

what:

  • adds get promotion endpoint (RESOLVES CORE-1677)
  • adds list promotions endpoint (RESOLVES CORE-1676)
  • uses new API routes

Copy link

vercel bot commented Jan 17, 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 Jan 18, 2024 3:42pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Visit Preview Jan 18, 2024 3:42pm
docs-ui ⬜️ Ignored (Inspect) Visit Preview Jan 18, 2024 3:42pm
medusa-docs ⬜️ Ignored (Inspect) Visit Preview Jan 18, 2024 3:42pm

Copy link

changeset-bot bot commented Jan 17, 2024

🦋 Changeset detected

Latest commit: ac04047

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

This PR includes changesets to release 2 packages
Name Type
@medusajs/medusa 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
Member

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@adrien2p
Copy link
Member

Could you just add a description to the pr for the changelog please 💪

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

todo: If we can, we should use the new API Routes. Did you try that?

@adrien2p
Copy link
Member

todo: If we can, we should use the new API Routes. Did you try that?

The new API is not supported by the core @olivermrbl, it goes through a separated loader etc

@olivermrbl
Copy link
Contributor

todo: If we can, we should use the new API Routes. Did you try that?

The new API is not supported by the core @olivermrbl, it goes through a separated loader etc

Yeah – maybe now is a good time to revisit and gauge how much effort it would require to support it, wdyt?

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Really clean 😻

Left a couple of questions

packages/medusa/src/loaders/api.ts Outdated Show resolved Hide resolved
packages/medusa/src/loaders/api.ts Outdated Show resolved Hide resolved
Copy link
Member

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

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

Fantastic work man 🔥 last couple of comments from oli to discuss

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.

3 participants