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

[4.x]: Should the Order query number method support 'not' syntax #3290

Closed
samhibberd opened this issue Oct 4, 2023 · 3 comments
Closed

[4.x]: Should the Order query number method support 'not' syntax #3290

samhibberd opened this issue Oct 4, 2023 · 3 comments
Labels
commerce4 Issues related to Commerce v4 💡 enhancement Ideas and suggestions

Comments

@samhibberd
Copy link

samhibberd commented Oct 4, 2023

What happened?

I had wrongly assumed that this would work, should number accept the same syntax as id for example?

$existingCarts = Order::find()
    ->isCompleted(false)
    ->customer($user)
    ->number('not ecc9d51f885906b1d0084c6272212559')
    ->orderBy('dateCreated DESC');

Craft CMS version

4.5.6.1

Craft Commerce version

4.3

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@samhibberd samhibberd added commerce4 Issues related to Commerce v4 bug labels Oct 4, 2023
@nfourtythree
Copy link
Contributor

Hi @samhibberd

Thank you for the message. My assumption is that you are trying to retrieve the other carts/orders apart from the one that is currently being viewed.

If this is the case you could do a quick update to your code to get this working for now

// Assumption that the $currentCart variable has been retrieved/instantiated earlier in the process

$existingCarts = Order::find()
    ->isCompleted(false)
    ->customer($user)
    ->id('not ' . $currentCart->id)
    ->orderBy('dateCreated DESC');

And from our end, we can look at the viability of updating number() to have the same functionality.

Thanks!

@nfourtythree nfourtythree added 💡 enhancement Ideas and suggestions and removed bug labels Oct 4, 2023
@lukeholder
Copy link
Member

lukeholder commented Apr 3, 2024

The number is used as unguessable param to retrieve the order, and many developers use it in URLs to retrieve the order without a login. We decided to make the number query param basic, and immune from being parsed with special things like not prefixes since the order contains personal information.

For example a user could change the number to any chars and get all orders in the system.

If you need to do a not query etc, retrieve the order with the number and use the not method with an ID like Nathaniel suggested. This will ensure you have a real order returned first before doing the not.

@samhibberd
Copy link
Author

That makes sence and thanks for the explanation, although we set a best practice of not exposing these apis to user input and use a shopify style authentication for guests to access their own orders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commerce4 Issues related to Commerce v4 💡 enhancement Ideas and suggestions
Projects
None yet
Development

No branches or pull requests

3 participants