Skip to content

feat: [MP-2900-2905] - allow removal promotion rows in express checkout and prevent close modal#6980

Open
cristhianDt wants to merge 11 commits into
mainfrom
feature/mp-2900-2905-allow-removal-promotion-rows-in-express-checkout
Open

feat: [MP-2900-2905] - allow removal promotion rows in express checkout and prevent close modal#6980
cristhianDt wants to merge 11 commits into
mainfrom
feature/mp-2900-2905-allow-removal-promotion-rows-in-express-checkout

Conversation

@cristhianDt

@cristhianDt cristhianDt commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

MP-2900-2905

Summary

  1. allows removal promotion rows in express checkout modal
  2. initialize checkout to apply promotion balance
  3. prevent closing modal when user is paying

https://www.figma.com/design/ARJWLwdnFJ84FNSvPX4wYz/Goals-V2?node-id=5831-98211&t=QSdzKl5WSyTatxBr-0

How test it

  • 'kiva credit', add some credit to the user.

    • add new credit amount using the type 'm_credit', any value works
  • 'promotion', there are two types 'bonu' and 'kiva promotion', in other words free credit and kiva card redemption

    • for free credit: login in our admin site and go to the user page > click on Add Free Credit > add new free credit amount using the type '147 - kiva', any value works
    • buy a new kiva card then redeem the card

Evidences

Captura de pantalla 2026-06-11 a la(s) 13 50 33

@cristhianDt cristhianDt requested a review from a team June 11, 2026 18:49
@cristhianDt cristhianDt added the b2c Sends B2C team a message in Slack on PR creation label Jun 11, 2026
* caller can surface a tip message.
*/
export async function removeBasketCredit({ apollo, basketId, creditType }) {
const { errors } = await apollo.mutate({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/kiva/kv-ui-elements/blob/main/%40kiva/kv-shop/src/validatePreCheckout.ts#L7 may also be useful as you get closer to the actual checkout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, yeah I'm trying to get the same or similar flow in checkout, I will check how we are using the validate and what point there, thanks matt

@cristhianDt cristhianDt Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @mcstover , I can't use your recomendation https://github.com/kiva/kv-ui-elements/blob/main/%40kiva/kv-shop/src/basketCredits.ts#L32, because that method is not using the creditType and I need to specify the promo to remove, we could add a new ticket to update it later, what do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on adding the parameter now? It would only take a few mins, and the package deploys quickly.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you try to pass null there?

It's true that UPC and redemption codes do have a value we can pass there... wonder what the php is doing within this mutation when it see's the new "admin_credit" type...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok to just skip using the global method if it's not working. We also use that mutation without any value in the ui version of checkoutUtils... so if that's working cool

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I did, I get the error: the mutation does not expect to receive redemptionCode argument

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking we could use the mutation as already defined in this PR since it's relatively simple, and this feature is part of an experiment. It would definitely be good to move this entire feature (express checkout) into the component library if/when the experiment does well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, updating the global or using my new one, I'll just drop a comment in case we fix the global method and we could start using it,

I'll let you know @mcstover when I add validatePreCheckout query in this flow

thanks team!

@dyersituations dyersituations left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cristhianDt changes are looking good. I'll functionally test once the validatePreCheckout method is in place and the dev DB sync is complete.

validateBasket() {
checkInjections(this, injections);

return new Promise((resolve, reject) => {

@cristhianDt cristhianDt Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcstover @dyersituations I moved this logic to this util file > src/util/checkout/checkoutValidationUtils.js, shared by this mixin and the new modal, lmk any doubt or comment, thanks

logFormatter(`ExpressCheckoutModal validation failed: ${validationMessage}`, 'error');
paying.value = false;
closeLightbox();
router.push('/basket');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm now seeing an error initially or a redirect to /basket when I test the latest changes locally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you see any message in the console?, could you share a screenshot please

the idea is: if something happen during the verification step we redirect so I would like to check whether the reservation service or checkout service is failing again

did you try in the checkout page too?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the checkout page, testing express checkout. No specific error, but now I just get redirected to basket each time, meaning errors are happening. Please see if you can reproduce locally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on it

@cristhianDt cristhianDt Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dyersituations I tested with 2 users and both pages are working correctly for those users, what is your user id?, let me try with your user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

b2c Sends B2C team a message in Slack on PR creation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants