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

docs: updates to digital product recipe #9165

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

shahednasser
Copy link
Member

  • Create workflow for fulfilling digital products that sends a notification + create a shipment to mark the fulfillment as fulfilled / shipped.
  • Update the subscriber handling the digital_product_order.created event to use the workflow
  • Add section on using the local notification module provider for testing purposes.

@shahednasser shahednasser requested a review from a team as a code owner September 17, 2024 14:32
Copy link

vercel bot commented Sep 17, 2024

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

Name Status Preview Comments Updated (UTC)
api-reference 🔄 Building (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 2:46pm
api-reference-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 2:46pm
docs-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 2:46pm
docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 2:46pm
medusa-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 2:46pm
resources-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 2:46pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
medusa-docs ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 2:46pm

Copy link

changeset-bot bot commented Sep 17, 2024

⚠️ No Changeset found

Latest commit: 8e93926

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

]
1. Retrieve the digital product order's details. For this, you'll use the `useRemoteQueryStep` imported from `@medusajs/core-flows`.
2. Send a notification to the customer with the digital products to download.
3. Create a shipment for the fulfillment to indicate that the item is fulfilled. For this step, you'll use from the `@medusajs/core-flows` package the `createOrderShipmentWorkflow` as a step.
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: I think we should consider removing the third step here. Digital products are not shipped and should ideally be created with a requires_shipping: false option, which can be used in the storefront to build the appropriate checkout flow. The item processing flow for digital products should end after fulfillment.

Copy link
Member Author

Choose a reason for hiding this comment

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

@olivermrbl ah makes sense, where would requires_shipping: false be passed? when the shipping method is set on the cart?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just checked and that's the property of an inventory item, but digital products don't have inventory items (unless that's the better approach?)

Copy link
Contributor

Choose a reason for hiding this comment

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

but digital products don't have inventory items

Hmm, I guess that depends on the case. For example, you might have a limited amount of licenses. Conversely, it is also very likely that you have an unlimited amount of licenses 😄

For the recipe, my take would be that we should not have the shipment step. This is likely also how we will build the digital products starter (when we get around to it).

@420coupe
Copy link
Contributor

420coupe commented Sep 18, 2024

Was the recipe changed otherwise?

Was having an issue when uploading, had to add multer on the api path for upload. Now i'm having an issue with req.validatedBody coming back as undefined. specifically this piece of the code below.

type CreateRequestBody = z.infer<typeof createDigitalProductsSchema>;

export const POST = async (
  req: AuthenticatedMedusaRequest<CreateRequestBody>,
  res: MedusaResponse
) => {
  const { result } = await createDigitalProductWorkflow(req.scope).run({
    input: {
      digital_product: {
        name: req.validatedBody.name,
        medias: req.validatedBody.medias.map((media) => ({
          fileId: media.file_id,
          mimeType: media.mime_type,
          ...media,
        })) as Omit<CreateDigitalProductMediaInput, "digital_product_id">[],
      },
      product: req.validatedBody.product,
    },
  });

  res.json({
    digital_product: result.digital_product,
  });
};

Not sure why middleware.ts wasn't applying the multer, but Modified api to use multer, now uploads work, but it dies on verificationBody as mentioned above

import multer from "multer";
import { AuthenticatedMedusaRequest, MedusaResponse } from "@medusajs/medusa";
import { uploadFilesWorkflow } from "@medusajs/core-flows";
import { MedusaError } from "@medusajs/utils";

const upload = multer({ storage: multer.memoryStorage() });

export const uploadMiddleware = upload.array("files");

export const POST = async (
  req: AuthenticatedMedusaRequest,
  res: MedusaResponse
) => {
  uploadMiddleware(req as any, res as any, async (err: any) => {
    if (err) {
      console.error("Error handling file upload:", err);
      return res.status(500).json({ error: "File upload failed" });
    }

    const access = req.params.type === "main" ? "private" : "public";
    const input = req.files as Express.Multer.File[];

    if (!input?.length) {
      throw new MedusaError(
        MedusaError.Types.INVALID_DATA,
        "No files were uploaded"
      );
    }

    const { result } = await uploadFilesWorkflow(req.scope).run({
      input: {
        files: input?.map((f) => ({
          filename: f.originalname,
          mimeType: f.mimetype,
          content: f.buffer.toString("binary"),
          access,
        })),
      },
    });

    res.status(200).json({ files: result });
  });
};

@shahednasser
Copy link
Member Author

Was having an issue when uploading, had to add multer on the api path for upload.

@420coupe This is already part of the recipe.

Not sure why middleware.ts wasn't applying the multer, but Modified api to use multer, now uploads work, but it dies on verificationBody as mentioned above

there was an issue with middlewares in a preview version we released yesterday. Can you update and try again?

@420coupe
Copy link
Contributor

420coupe commented Sep 18, 2024

Was having an issue when uploading, had to add multer on the api path for upload.

@420coupe This is already part of the recipe.

Not sure why middleware.ts wasn't applying the multer, but Modified api to use multer, now uploads work, but it dies on verificationBody as mentioned above

there was an issue with middlewares in a preview version we released yesterday. Can you update and try again?

Tyvm, been chasing my tail all day trying to figure out why.

found another small bug, when deleting a digital product from the product section, it doesn't delete the digital product and then causes an error when going to digital products on adminui

@420coupe
Copy link
Contributor

also you never attach the digital product in the notification.

@shahednasser
Copy link
Member Author

also you never attach the digital product in the notification.

@420coupe We instead retrieve the URLs of all digital products and pass them in the notification's payload. You're free to do it differently if that's your use case.

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