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.

Sign up for GitHub

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

Jump to bottom

feat(medusa, types, utils, workflow): Migrate medusa workflow to the workflow package #4682

Merged
merged 29 commits into from
Aug 5, 2023
Merged

feat(medusa, types, utils, workflow): Migrate medusa workflow to the workflow package #4682

merged 29 commits into from
Aug 5, 2023

Conversation

adrien2p
Copy link
Member

@adrien2p adrien2p commented Aug 3, 2023 •
edited
Loading

What
Migrate medusa create products workflow to the workflow package using the new API.

@changeset-bot
Copy link

changeset-bot bot commented Aug 3, 2023 •
edited
Loading

⚠️ No Changeset found

Latest commit: 98ee60d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Aug 3, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
medusa-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 2:37pm

@adrien2p adrien2p marked this pull request as ready for review August 3, 2023 15:44
@adrien2p adrien2p requested a review from a team as a code owner August 3, 2023 15:44
carlos-r-l-rodrigues
carlos-r-l-rodrigues reviewed Aug 4, 2023
View reviewed changes
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.

Overall it looks good before applying the changes to the pipe function.
We just need to fix the tests/build.

adrien2p added 3 commits August 4, 2023 15:43
@adrien2p
fix
cdd6344
@adrien2p
Merge branch 'feat/migration-create-products-workflow' of github.com:…
de64cf0 
…medusajs/medusa into feat/migration-create-products-workflow
@adrien2p
fix
4116ad9
@adrien2p
Copy link
Member Author

adrien2p commented Aug 4, 2023

@olivermrbl can I merge

olivermrbl
olivermrbl approved these changes Aug 5, 2023
View reviewed changes
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.

LGTM, with questions and comments. Also, I think we should consider adding integration tests of workflows - not through API routes. I have started doing that in my PR. Let's discuss next week :)

...ges/workflows/src/handlers/middlewares/create-products-prepare-create-prices-compensation.ts
}

createProductsPrepareCreatePricesCompensation.aliases = {
payload: "payload",
Copy link
Contributor

Choose a reason for hiding this comment

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

comment: This does not match the input, nor the defined alias in the handler

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure it is used, ill remove it

@olivermrbl
Copy link
Contributor

Feel free to merge as you please

@adrien2p
Copy link
Member Author

adrien2p commented Aug 5, 2023

Yes definitely need to add some tests for the handlers, i was discussing that with carlos as well, lets discuss this next week?

@adrien2p adrien2p merged commit dc46927 into develop Aug 5, 2023
@carlos-r-l-rodrigues carlos-r-l-rodrigues deleted the feat/migration-create-products-workflow branch August 25, 2023 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlos-r-l-rodrigues carlos-r-l-rodrigues carlos-r-l-rodrigues approved these changes

@olivermrbl olivermrbl olivermrbl approved these changes

@riqwan riqwan Awaiting requested review from riqwan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adrien2p @olivermrbl @carlos-r-l-rodrigues