-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat: Add HTTP endpoints and workflows for price preference management #7960
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, clean PR 💪
integration-tests/http/__tests__/price-preference/admin/price-preference.spec.ts
Outdated
Show resolved
Hide resolved
10c5b63
to
b91d7e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥇
async (pricePreferences, { container }) => { | ||
if (!pricePreferences?.length) { | ||
return | ||
} | ||
|
||
const pricingModule = container.resolve<IPricingModuleService>( | ||
ModuleRegistrationName.PRICING | ||
) | ||
|
||
await pricingModule.deletePricePreferences(pricePreferences) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async (pricePreferences, { container }) => { | |
if (!pricePreferences?.length) { | |
return | |
} | |
const pricingModule = container.resolve<IPricingModuleService>( | |
ModuleRegistrationName.PRICING | |
) | |
await pricingModule.deletePricePreferences(pricePreferences) | |
} | |
async (ids, { container }) => { | |
if (!ids?.length) { | |
return | |
} | |
const pricingModule = container.resolve<IPricingModuleService>( | |
ModuleRegistrationName.PRICING | |
) | |
await pricingModule.deletePricePreferences(ids) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
damn the automerge was faster 😄 I'll open a new PR now anyway and address this.
ModuleRegistrationName.PRICING | ||
) | ||
|
||
await service.upsertPricePreferences(prevData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: shouldn't this be update as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We pass the prevData as an array of entities, and only the Upsert method accepts such input. update
is either an ID or a selector, which doesn't work for the reversal (we do this everywhere basically). I do think we might need to revisit the compensation methods in a lot of workflows and test them at the very least
REF CORE-2376
Remaining pieces are adding UI to manage the flag, showing the flag in price editor, plugging it in cart calculations, and #7827