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

Order link in email does not take you to Order #4679

Closed
brent-hoover opened this issue Sep 25, 2018 · 6 comments
Closed

Order link in email does not take you to Order #4679

brent-hoover opened this issue Sep 25, 2018 · 6 comments
Assignees
Labels
bug For issues that describe a defect or regression in the released software

Comments

@brent-hoover
Copy link
Collaborator

brent-hoover commented Sep 25, 2018

Issue Description

If you click on the order link in the shipping confirmation email it just takes you to a screen that says "Order Not Found". It might/probably is all emails but I found it in the shipping one.

Steps to Reproduce

  1. Place an order
  2. Approve/Capture/Ship the order
  3. Open the email and click on the link
  4. Observe it takes you to a page that says "Order Not Found"

Versions

Reaction: 2.0.0-rc1

UPDATE: 2.0.0-rc11

@brent-hoover brent-hoover added bug For issues that describe a defect or regression in the released software impact-minor labels Sep 25, 2018
@hfmw
Copy link
Contributor

hfmw commented Oct 11, 2018

Issue here is with the completedCartOrder publication.

  return Orders.find({
    accountId,
    cartId
  }, { limit: 1 });

Is accountId required from a security perspective? I would be happy submit a PR removing the requirement for accountId, if that's a valid solution for you.

@brent-hoover
Copy link
Collaborator Author

Yes accountid is required because we can't allow anyone besides the creator of the order to see the order, so removing accountId is not a valid solution

@aldeed
Copy link
Contributor

aldeed commented Apr 10, 2019

This is still an issue, but in a different way. Now the link should be going to the storefront order page, but it's going to the operator UI domain. Keeping this open to track this new variation of the issue.

@aldeed
Copy link
Contributor

aldeed commented Apr 11, 2019

Some clarity on what needs to be done here:

In sendOrderEmail.js, when building dataForEmail:

homepage: Reaction.absoluteUrl()

Replace Reaction.absoluteUrl() with the storefront base URL (new ENV variable?)

And here:

orderUrl: `cart/completed?_id=${order.cartId}`,

Replace this with the storefront route.

Because this is likely changed by each implementation, a better long-term approach might be to call a query function to get the URL, so that a custom plugin can override that query function.

@focusaurus
Copy link
Contributor

OK I paired a bit with aldeed on this.

Changes to data provided to template

  • homepage: shop.storefrontUrls.storefrontHomeUrl
  • orderUrl based on shop.storefrontUrls.storefrontOrderUrl, but with templatization of an :orderId placeholder variable
    • The operator should put :orderId in the URL they configure in the email settings
    • Reaction core code will replace that with the relevant order reference id when preparing the template data

Handling anonymous checkout

There's some additional scope to deal with anonymous checkouts and the associated token. Will propose solution there as I dig more into it.

Some UI changes we should also make

  • Add inline help text explaining the :orderId and :token placeholders
  • Provide a sample value for reference like: http://shop.example.com/my-orders/:orderId?token=:token

@kieckhafer
Copy link
Member

@focusaurus can we close this based on #5251?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For issues that describe a defect or regression in the released software
Projects
None yet
Development

No branches or pull requests

5 participants