allow backorder #4051
allow backorder #4051
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
🦋 Changeset detectedLatest commit: eb8622d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| allow_backorder, | ||
| }), | ||
| { | ||
| ...removeNullish({ |
There was a problem hiding this comment.
todo: Should we update removeNullish instead to check != null or != '' intead in order to prevent stripping false values? wdyt?
There was a problem hiding this comment.
renaming existing utility to falsy and updating remove nullish to be == null since that evaluates to true for null and undefined (source: https://dorey.github.io/JavaScript-Equality-Table/)
...min-ui/ui/src/components/organisms/product-variants-section/edit-variant-inventory-modal.tsx
Outdated
Show resolved
Hide resolved
...min-ui/ui/src/components/organisms/product-variants-section/edit-variant-inventory-modal.tsx
Outdated
Show resolved
Hide resolved
…-section/edit-variant-inventory-modal.tsx Co-authored-by: Oliver Windall Juhl <[email protected]>
| @@ -74,8 +74,9 @@ const useEditProductActions = (productId: string) => { | |||
| updateVariant.mutate( | |||
| { | |||
| variant_id: id, | |||
| ...removeNullish(payload), | |||
| ...removeFalsy(payload), | |||
There was a problem hiding this comment.
question: @adrien2p @pKorsholm I am currently fiddling around with a reported bug around updating inventory quantities. Can you remember the reason for us using removeFalsy here? It will remove all number properties (e.g. inventory_quantity) that are attempted to be set to 0.
I have a PR ready, but I just want to make sure I don't introduce unexpected issues.
There was a problem hiding this comment.
Currently, you can't update any number properties to 0, including all measurements + inventory, because they are stripped from the payload.
There was a problem hiding this comment.
I dont recall but indeed it strips every falsy values, 0 being considered as false as well.
There was a problem hiding this comment.
RemoveNullish will strip all null or undefined which seems better no? Unless we want to erase if undefined in that case we need to check !== null
There was a problem hiding this comment.
I want to revert this to removeNullish.
There was a problem hiding this comment.
In that case null or undefined will not be assigned, is it what we want? Looks alright to me, but what are we really trying to achieve here? Undefined is stripped by the api i think and null could allow to reset a field that is nullable right?
There was a problem hiding this comment.
I can't immediately think of any properties that we would like to remove with undefined on variants. If the API strips undefined values, I would also argue we could just pass the entire payload through without stripping anything.
Maybe we'll have to go a PR further back, and check why we added removeNullish 😅
What
allow_backorderissueHow
removeNullish