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

[Bug]: Order Workflow Doesn't Support Creating Multiple Orders #10054

Closed
AlexAntonides opened this issue Nov 12, 2024 · 12 comments
Closed

[Bug]: Order Workflow Doesn't Support Creating Multiple Orders #10054

AlexAntonides opened this issue Nov 12, 2024 · 12 comments

Comments

@AlexAntonides
Copy link

Package.json file

{
    [...]
    "scripts": { [...] },
    "dependencies": {
        "@medusajs/admin-sdk": "latest",
        "@medusajs/framework": "latest",
        "@medusajs/medusa": "latest",
        "@medusajs/cli": "latest",
        "@mikro-orm/core": "5.9.7",
        "@mikro-orm/knex": "5.9.7",
        "@mikro-orm/migrations": "5.9.7",
        "@mikro-orm/postgresql": "5.9.7",
        "awilix": "^8.0.1",
        "pg": "^8.13.0",
        [...]
    },
    "devDependencies": {
        "@mikro-orm/cli": "5.9.7",
        "@medusajs/test-utils": "latest",
        [...]
    }
}

Node.js version

v20.17.0

Database and its version

PostgreSQL v10.4

Operating system name and version

macOS Sequoia 15.1

Browser name

No response

What happended?

I'm creating a system that is capable of bulk importing data from another datasource into my MedusaJS app. I'm currently busy with importing orders, and I have noticed that the createOrdersWorkflow only supports creating a single order rather than multiple orders, similarly to other workflows.

Expected behavior

Similarly to other workflows, I expect the order workflow to be able to create multiple orders, especially since the workflow is plural (createOrder-s-workflow).

Actual behavior

Only accepts a single order.

Link to reproduction repo

@AlexAntonides
Copy link
Author

Also, for importing orders, would it be possible to have an external_id to the orders entity?
And is it possible to supply a created_at/last_updated date when creating these entities, so that we don't lose data from when the order was actually made?

@thetutlage
Copy link
Contributor

@AlexAntonides Can you please share some code or a repo that we can use to reproduce the issue? Feel free to reply here with reproduction and we will re-open the issue and provide the needed help 🙂

@SalahAdDin
Copy link

@AlexAntonides man?

@AlexAntonides
Copy link
Author

@thetutlage, thank you for responding.

I wasn't entirely sure whether to include a code snippet, considering the workflow in itself only supports a single entity (input: WorkflowData<CreateOrderDTO & [...]> rather than input: WorkflowData<CreateOrderDTO[] & [...]>).

However, if you create a route which tries to create multiple orders,

export const POST = async(
    req: AuthenticatedMedusaRequest<{
          orders: OrderDTO[],
    } & AdditionalData>,
    res: MedusaResponse
) => {
      const { additional_data, ...data} = req.validatedBody
      await createOrdersWorkflow(req.scope).run({
           input: { data.orders, additional_data },
      })
}

you'll receive a typescript error that Type '(OrderDTO[] | WorkflowData<OrderDTO[]>) & OrderDTO[]' is not assignable to type '(CreateOrderDTO & AdditionalData) | WorkflowData<CreateOrderDTO & AdditionalData>', because the workflow input only accepts a single order, rather than multiple ones, like the rest of the workflows.

@thetutlage thetutlage reopened this Nov 13, 2024
@thetutlage
Copy link
Contributor

Hello @AlexAntonides 👋

Okay, so we discussed this internally and decided that createOrdersWorkflow will not support creating multiple orders in one go. However, the naming of the workflow has to be fixed in this case and the workflow should be renamed to createOrderWorkflow.

@thetutlage
Copy link
Contributor

Fixed the naming issue in this PR https://github.com/medusajs/medusa/pull/10134/files. The plural name is deprecated

@AlexAntonides
Copy link
Author

AlexAntonides commented Nov 19, 2024

Hi @thetutlage, thank you for addressing this.
If the decision is to keep the createOrderWorkflow workflow 'singular', is there a way for us to extend this in such a way that we can create multiple orders in a request, without making a HTTP-Request for each product, or to for-loop over all orders in a controller and calling the workflow per order?
This is quite important for us, because we need it to migrate our orders from our current e-commerce system to Medusa which will replace it.

@ghardin1314
Copy link

Hi @thetutlage, thank you for addressing this. If the decision is to keep the createOrderWorkflow workflow 'singular', is there a way for us to extend this in such a way that we can create multiple orders in a request, without making a HTTP-Request for each product, or to for-loop over all orders in a controller and calling the workflow per order? This is quite important for us, because we need it to migrate our orders from our current e-commerce system to Medusa which will replace it.

Have you tried running the order create steps in parallel? I have no idea if this would actually work or if there are limitations to them, but this is what I would try next

https://docs.medusajs.com/learn/advanced-development/workflows/parallel-steps#main

@AlexAntonides
Copy link
Author

@ghardin1314 thanks for your help. I've tried this before,

const orders: CreateOrderDTO[] / = ** workflow input  **/

const requests = transform(
   { orders }, 
   (data) => data.orders.map((order) => createOrderWorkflow.runAsStep({ input: order })),
)

const result = parallelize(
   ...requests
)

and it doesn't seem to work, because the internal representation assumes I'm merging one action here.

error:   Error in loading config: Cannot merge less than two actions
error:   Error: Cannot merge less than two actions
at OrchestratorBuilder.mergeActions (/node_modules/@medusajs/orchestration/dist/transaction/orchestrator-builder.js:172:19)
at Object.<anonymous> (/node_modules/@medusajs/workflows-sdk/dist/utils/composer/parallelize.js:52:19) 
at parallelizeBinder (/node_modules/@medusajs/workflows-sdk/dist/utils/composer/create-workflow.js:107:36)
at parallelize (/node_modules/@medusajs/workflows-sdk/dist/utils/composer/parallelize.js:50:12)

@sradevski
Copy link
Member

@AlexAntonides you can always add a custom endpoint that takes a list of orders, and then you can do something like this:

await Promise.all(
  orders.map(async (order) => {
    await createOrderWorkflow(req.scope).run({
      input: { ...order, additional_data },
    })
  })
)

Or, to not overload the server, you can create batches of 5 and run those in parallel.

Closing the ticket as batch creation is not something we'll be adding anytime soon, and there is a simple implementation to support it for those who need it.

@SalahAdDin
Copy link

@ghardin1314 thanks for your help. I've tried this before,

const orders: CreateOrderDTO[] / = ** workflow input  **/

const requests = transform(
   { orders }, 
   (data) => data.orders.map((order) => createOrderWorkflow.runAsStep({ input: order })),
)

const result = parallelize(
   ...requests
)

and it doesn't seem to work, because the internal representation assumes I'm merging one action here.

error:   Error in loading config: Cannot merge less than two actions
error:   Error: Cannot merge less than two actions
at OrchestratorBuilder.mergeActions (/node_modules/@medusajs/orchestration/dist/transaction/orchestrator-builder.js:172:19)
at Object.<anonymous> (/node_modules/@medusajs/workflows-sdk/dist/utils/composer/parallelize.js:52:19) 
at parallelizeBinder (/node_modules/@medusajs/workflows-sdk/dist/utils/composer/create-workflow.js:107:36)
at parallelize (/node_modules/@medusajs/workflows-sdk/dist/utils/composer/parallelize.js:50:12)

Is that feature implemented in other medusa alternatives?

@AlexAntonides
Copy link
Author

@SalahAdDin, I'm not entirely sure what you mean, but I'm trying to create a module that allows us to migrate the products/orders of another e-commerce platform into MedusaJS, because we don't want to lose our data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants