Shopify / dawn

Shopify's first source available reference theme, with Online Store 2.0 features and performance built-in.
Other
2.49k stars 3.35k forks source link

CRITICAL BUG - customer page does not work accross domains #739

Closed gregjotau closed 2 years ago

gregjotau commented 2 years ago

We have several domains.

If a customer log in to fammestore.dk, but go into the order, then they are sent to famme.no which is the main domain.

All links must be relative to the current domain, now the user is logged out.

Screenshot 2021-10-06 at 10 48 02

gregjotau commented 2 years ago

The link to the order is: https://famme.no/account/orders/7dbfee7b9fcd6a0313151e80f34c4a8b

The domain should have been fammestore.dk

gregjotau commented 2 years ago

@ludoboludo if this is not prioritized to fix today as a hotfix we will fix it ourselves πŸ€“

ludoboludo commented 2 years ago

Hey @gregjotau, thanks for bringing this up πŸ‘ Seems like it could be a limitation of the liquid value {{ order.customer_url }} (doc reference). I might be mistaking but it looks like this issue would be the same on another theme (like Debut). I will look into it and get back to you when I find more information.

gregjotau commented 2 years ago

@ludoboludo out of the sandbox had the exact same bug on their turbo theme :p They have still not fixed it, but on that theme a "hack" was implemented to fix it.

This was the fix:

{% for order in customer.orders %}
                <tr class="{% cycle 'odd', 'even' %} {% if order.cancelled %}cancelled_order{% endif %}">
                  {% assign order_number = order.customer_url | split : '/' | last %}
                  {% assign relative_url = routes.account_url | append: '/orders/' | append: order_number %}
                  <td data-label="{{ 'customer.orders.order_number' | t }}">{{ order.name | link_to: relative_url }}</td>
                  <td data-label="{{ 'customer.orders.date' | t }}">{{ order.created_at | date: format: "month_day_year" }}</td>
                  <td data-label="{{ 'customer.orders.payment_status' | t }}">{{ order.financial_status_label }}</td>
                  <td data-label="{{ 'customer.orders.fulfillment_status' | t }}">{{ order.fulfillment_status_label }}</td>
                  <td data-label="{{ 'customer.orders.total' | t }}">
                    <span class="money">{% render 'price-element', price: order.total_price %}</span>
                  </td>
                </tr>
              {% endfor %}
ludoboludo commented 2 years ago

Ah interesting! Thanks a bunch for sharing this πŸ‘Œ

gregjotau commented 2 years ago

No problems, we have recently started doing returns via the order page for international orders, so customers have to login to return and go into the order page. But that does not work now, so a fix like that would be greatly appreciated :)

gregjotau commented 2 years ago

We will also merge it in ASAP to test live πŸš€

bakura10 commented 2 years ago

This fix looks a bit hacky. It would be better if it could be fixed at Shopify level. If tomorrow Shopify allows to change the patterns of the URL we’re screwed up. @Thibaut would it be possible to have this customer variable be locale aware ?

gregjotau commented 2 years ago

I agree that it should be on the Shopify level, but I would say the probability of this working 10 years from now is very high IMO. Changing URL pattern = something people ask for but will "never" be allowed. But I might be wrong.

So it is better to merge in this fix and be realistic about URL pattern change, than to wait 1 year for a bugfix. But hopefully they can fix it sooner 😎

tjoyal commented 2 years ago

Had a quick look. Laying down some facts:

I am hesitant to say what should be done here, it need to be looked at in depth.

Skudo commented 2 years ago

We shipped a fix for the storefront that creates order.customer_url with the current domain. That should address the issue OP was facing.

If we've missed something and we need to have another look at this, please don't hesitate to reopen this issue or create a new one! πŸ™‡β€β™‚οΈ