craftcms / commerce

Fully integrated ecommerce for Craft CMS.
https://craftcms.com/commerce
Other
219 stars 170 forks source link

[4.x]: Querying order with .customer(null) expected to return no order but does #3551

Closed svondervoort closed 3 months ago

svondervoort commented 3 months ago

What happened?

Description

We have this order complete step in the checkout where the checkout allows for guest and logged in to finish an order. The query we use for the complete step is:

{% set orderId = craft.app.request.queryParam('order') %}
{% set user = user ?? currentUser %}
{% set order = craft.orders().isCompleted(true).number(orderId).customer(user).one() %}
{% dd(order) %}

In this case the user completing the order is a guest the part .customer(user) simply equals to .customer(null) and no matter what orderId is queried it always returns an order.

In the case the user completing the order is logged in the query only returns an order if the currentUser matches with the order, else it will simply return no order.

Steps to reproduce

  1. Create a page with with the above queries
  2. Create or complete a few orders under different users
  3. Replace the order queryParam with the orderNumber or add the orderNumber to the url query
  4. Be sure you're not logged in as one of the users

Expected behavior

If the user variable is null I would expect to not be able to match any order.

Actual behavior

The user variable is null but still returns an order even when the order has a user linked.

Craft CMS version

4.10.2

Craft Commerce version

4.6.2

linear[bot] commented 3 months ago

PT-1881 [4.x]: Querying order with .customer(null) expected to return no order but does

lukeholder commented 3 months ago

This is most likely because you are getting null from the {% set orderId = craft.app.request.queryParam('order') %} and passing null to the query which is the same as querying for orders without setting the param.

Try testing you have a orderId before querying for orders with it.

Will reopen if this is not the case. Maybe show me the output of orderId before running the query.

svondervoort commented 3 months ago

Hi @lukeholder in this case the orderId is not empty since the url is http://redacted.test/checkout/complete?order=d20f813d23649a624abb140b4085b959 which is the number of an order visible as completed in the Control Panel.

The output of the query is a big chunk, how would you like me to share it?

lukeholder commented 3 months ago

@svondervoort if you pass null to .user(null) its the same as .number(null) - for both, it is as if you have no condition on that param since you are passing null.

In Commerce 4, if a user has entered an email, then there is a user attached to the order, even if the user is a guest/not logged in. We make an inactive user if a user with that email doesn't exist, or we attach the real user with that same email address. So unless they have not set their email (which is required for a completed order) you will always have a user/custom for the order.

So when you say:

If the user variable is null I would expect to not be able to match any order.

this is not correct for how .user(null) works. If you pass null it puts no user/custom condition on query.

The user variable is null but still returns an order even when the order has a user linked.

This would be correct if you passed only an order number.

Also please note: .user() in Commerce 4 was deprecated, you should be using .customer() instead on the order query.

svondervoort commented 3 months ago

@lukeholder ah ok that makes sense.

So I guess the best approach regarding privacy, this is the main reason I submitted this question, is that if there is no currentUser in the session to just show basic order information, and if there is a currentUser we can use that to get the order if the user and number match on the order and show detailed information of the order.