fix: Make new order email links correct for storefront

[focusaurus]

Resolves #4679
Impact: breaking|minor
Type: bugfix

Issue

New Order confirmation email was not updated yet to deal with storefront and core being on separate URLs yet.

Solution

We use the recently-added shop storefrontUrls settings to build the URL and pass that to the email template for rendering.

One complication we hit is dealing with access tokens for anonymous orders. We don't store them, only their hash, so while we can navigate to their order confirmation page immediately, later when we go to render their order confirmation email, we don't have the raw token. So we decided to generate a new one at that time which required a schema change so an order can now have an array of access tokens. Typically there will be one from when the order was placed via storefront and one from when the order confirmation email was rendered and sent.

Breaking changes

The new order confirmation email template has been updated. The new code is here but we also write these to mongodb so they can be edited in the GUI. There is no migration for that data. It would have some extra complexity to determine if the data had been modified or not. Here's the diff if anyone wants to apply manually:

-                                              <td width="33%" align="left" valign="top" style="font-size:14px; line-height:normal; color:#1999dd; font-family:Arial, helvetica;"><a href="{{homepage}}{{orderUrl}}" style="color:#1999dd;">{{order.referenceId}}</a></td>
+                                              <td width="33%" align="left" valign="top" style="font-size:14px; line-height:normal; color:#1999dd; font-family:Arial, helvetica;"><a href="{{orderUrl}}" style="color:#1999dd;">{{order.referenceId}}</a></td>

A migration was created and tested for the schema change.

Testing

1. Set up your install for outgoing emails (we have internal docs on this, ping Pete for a link)
2. Create some orders on your existing installation BEFORE loading the new code in this branch. Ideally both logged-in and anonymous orders.
   1. This will give you "before" data to test the migration
3. Configure your Shop Settings in catalyst with
   1. Single Order page URL: http://storefront.reaction.localhost:4000/checkout/order/:orderId?token=:token
   2. Homepage URL: http://storefront.reaction.locahost:4000
4. Checkout this branch and restart reaction
5. Verify the schema migration is run and you are up to 66 (you should see this in the startup logs for reaction core)
6. Verify the email links to your existing orders still work
7. Place new logged-in and anonymous orders and verify they still work

[kieckhafer]

One issue we are dealing with here seems to be stemming from the anonymousAccessToken field that is saved on an order. The same field is also saved on cart, so if we make changes it's possible we'll need to make adjustments in both places, just keep that in mind, but I'll just highlight the order here.

When an order is created, if you are an anonymous user (i.e. not logged into your account), the order is created with the following:

accountId: null,
anonymousAccessToken: {some-token-here}

If you are logged in, it's the opposite:

accountId: {some-account-id},
anonymousAccessToken: null

This makes sense for anonymous users. For logged in users, we need to think about why we aren't saving an anonymousAccessToken as well, or we need to re-think how we query for orders in the ordersByReferenceId query.

When an order email is sent out, the hashed anonymousAccessToken is sent along with the link. When the email is clicked, it goes to the order summary page, and as you can see in this code here, if an access code is available, we do some logic to query for an order with that access code, ignoring the accountId. However, since we don't save the access code if the user is logged in (see this code here), the anonymousAccessCode is null, and the query returns no result.

If you remove the code from the URL, you'll see the order summary page works fine, because we are now skipping the if (token) check here, and querying by accountId.

I'm not sure the reasoning behind not saving the anonymousAccessToken on a logged in order. @aldeed could you shed some light on that?

If there is no reason behind that, I think the simplest solution would be to add the anonymousAccessToken onto all orders by removing the accountId check here. This would also allow you to share an order link, if you wish, with friends / family and they wouldn't need to be logged into your account. The query here would also need to be updated to something like:

if (token) {
    selector.anonymousAccessToken = hashLoginToken(token);
  } else if (!userHasPermission(["orders"], shopId)) {
    // Unless you are an admin with orders permission, you are limited to seeing it if you placed it
    if (!contextAccountId) {
      throw new ReactionError("access-denied", "Access Denied");
    }
    selector.accountId = contextAccountId;
  }

[aldeed]

Looks wonderful @focusaurus . Just a few small things I noticed.

Reviewed code only. I'll let @kieckhafer test.

[focusaurus]

@kieckhafer I think I'm going to rework the logic to deal with permissions from most-privileged to least-privileged and I think it will work. Testing locally now.

  if (userHasPermission(["orders"], shopId)) {
    // admins with orders permissions can see any order in the shop
    // No need to adjust the selector
  } else if (contextAccountId) {
    // Regular users can only see their own orders
    selector.accountId = contextAccountId;
  } else if (token) {
    // If you have an anonymous access token for this order, OK to see it
    selector["anonymousAccessTokens.hashedToken"] = hashLoginToken(token);
  } else {
    throw new ReactionError("access-denied", "Access Denied");
  }
  return Orders.findOne(selector);

[aldeed]

@focusaurus Your solution seems like it will work, but the comment from @kieckhafer reminds me that really we have no reason to generate the token upon sending the email if it's an order with accountId. And in fact if we generate it and also rearrange the permission checks as you propose, the token wouldn't be used if they're logged in. But it would be used if they are not logged in, which means that account orders are now less secure.

@kieckhafer points out that making them less secure might be desirable if you want to share the link, but I think that's an edge case and I'd prefer if there is no way to access an account order unless you're logged in as that account.

So the upshot of all that is that, when generating the :token replacement for an order email, I think we should NOT do it if order.accountId is set. It should be just replaced with empty string I guess.

[focusaurus]

OK @aldeed updated to not generate a token if order.accountId is truthy and instead replace :token with empty string in the URL.

[focusaurus]

@manueldelreal Can you help us out with some testing on this please?

[kieckhafer]

👍 Seems to work as it should: Anon orders provide a token, and the order is correctly displayed. Logged in orders to not provide a token, and the correct order is displayed.

[kieckhafer]

@manueldelreal @aldeed this all looks good to me, I've signed off. I added some help text, so please look over that, but then feel free to merge when you give it the OK.

[manueldelreal]

[aldeed]

@focusaurus Changes look good but I just remembered there are 4 types of order emails with different templates for each, but they all have {{homepage}}{{orderUrl}} and need that same change. All 4 files in the same templates/orders folder.

The issue was about the "new order" email, but all order emails actually had the same incorrect link in them.

[willopez]

@aldeed @focusaurus
is changing {{homepage}}{{orderUrl}} => {{orderUrl}} in the templates

itemRefund
refunded
shipped

is all that is needed here?

[focusaurus]

@willopez Yes. I just pushed a commit that takes care of that.