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

Fix: ANCV ignoring existing orders & not working as intended via custom payButton #2655

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

m1aw
Copy link
Contributor

@m1aw m1aw commented Apr 23, 2024

Summary

This PR was original intended to fix a bug in ANCV where it always made new order calls. This was an issue if ANCV was not the first partial payment method, ie: Giftcard -> ANCV, or ANCV -> ANCV.

To fix this issue a check was introduce to use an existing order. The problem is that in the process it was notice that passing a different Class instance function to payButton was causing it to dereference this.props and use the one from payButton instead of UIElement/ANCVElement.

The solution was to use submit in the payButton. The problem is that submit only really gets reimplemented in express payment methods. This also lead to realise that contrary to what we do in gift card, this is probably the most correct way. It was because of this also necessary to change onSubmit access modifier, from private, to protected. This makes so we can use onSubmit implementation on the component. I believe this change is more in line what we are doing in V6.

Possible further discussion can be held on this point about .submit() being more consistent for partial payment methods.

Tested scenarios

  • ANCV was tested with the help of Jakub making multiple payments

Fixed issue:

Copy link

changeset-bot bot commented Apr 23, 2024

🦋 Changeset detected

Latest commit: fb89ae8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@adyen/adyen-web Patch

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

Copy link
Contributor

github-actions bot commented Apr 23, 2024

Size Change: +260 B (0%)

Total Size: 1.15 MB

Filename Size Change
packages/lib/dist/adyen.js 302 kB +70 B (0%)
packages/lib/dist/cjs/index.js 263 kB +58 B (0%)
packages/lib/dist/es.modern/index.js 125 kB +97 B (0%)
packages/lib/dist/es/index.js 146 kB +35 B (0%)
ℹ️ View Unchanged
Filename Size
packages/lib/dist/es.modern/ar.js 6.97 kB
packages/lib/dist/es.modern/cs-CZ.js 6.21 kB
packages/lib/dist/es.modern/da-DK.js 5.62 kB
packages/lib/dist/es.modern/de-DE.js 6.1 kB
packages/lib/dist/es.modern/el-GR.js 7.94 kB
packages/lib/dist/es.modern/es-ES.js 5.75 kB
packages/lib/dist/es.modern/fi-FI.js 5.75 kB
packages/lib/dist/es.modern/fr-FR.js 5.98 kB
packages/lib/dist/es.modern/hr-HR.js 6.01 kB
packages/lib/dist/es.modern/hu-HU.js 6.3 kB
packages/lib/dist/es.modern/it-IT.js 5.83 kB
packages/lib/dist/es.modern/ja-JP.js 6.71 kB
packages/lib/dist/es.modern/ko-KR.js 6.34 kB
packages/lib/dist/es.modern/nl-NL.js 5.78 kB
packages/lib/dist/es.modern/no-NO.js 5.59 kB
packages/lib/dist/es.modern/pl-PL.js 6.28 kB
packages/lib/dist/es.modern/pt-BR.js 5.79 kB
packages/lib/dist/es.modern/pt-PT.js 5.92 kB
packages/lib/dist/es.modern/ro-RO.js 6.07 kB
packages/lib/dist/es.modern/ru-RU.js 7.41 kB
packages/lib/dist/es.modern/sk-SK.js 6.38 kB
packages/lib/dist/es.modern/sl-SI.js 5.92 kB
packages/lib/dist/es.modern/sv-SE.js 5.61 kB
packages/lib/dist/es.modern/zh-CN.js 6.14 kB
packages/lib/dist/es.modern/zh-TW.js 6.23 kB
packages/lib/dist/es/ar.js 6.97 kB
packages/lib/dist/es/cs-CZ.js 6.21 kB
packages/lib/dist/es/da-DK.js 5.62 kB
packages/lib/dist/es/de-DE.js 6.1 kB
packages/lib/dist/es/el-GR.js 7.94 kB
packages/lib/dist/es/es-ES.js 5.75 kB
packages/lib/dist/es/fi-FI.js 5.75 kB
packages/lib/dist/es/fr-FR.js 5.98 kB
packages/lib/dist/es/hr-HR.js 6.01 kB
packages/lib/dist/es/hu-HU.js 6.3 kB
packages/lib/dist/es/it-IT.js 5.83 kB
packages/lib/dist/es/ja-JP.js 6.71 kB
packages/lib/dist/es/ko-KR.js 6.34 kB
packages/lib/dist/es/nl-NL.js 5.78 kB
packages/lib/dist/es/no-NO.js 5.59 kB
packages/lib/dist/es/pl-PL.js 6.28 kB
packages/lib/dist/es/pt-BR.js 5.79 kB
packages/lib/dist/es/pt-PT.js 5.92 kB
packages/lib/dist/es/ro-RO.js 6.07 kB
packages/lib/dist/es/ru-RU.js 7.41 kB
packages/lib/dist/es/sk-SK.js 6.38 kB
packages/lib/dist/es/sl-SI.js 5.92 kB
packages/lib/dist/es/sv-SE.js 5.61 kB
packages/lib/dist/es/zh-CN.js 6.14 kB
packages/lib/dist/es/zh-TW.js 6.23 kB

compressed-size-action

Copy link

sonarcloud bot commented Apr 26, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@m1aw m1aw merged commit cfc8d29 into main Apr 26, 2024
12 of 13 checks passed
@m1aw m1aw deleted the fix/ancv-multiple-payments branch April 26, 2024 16:22
@github-actions github-actions bot mentioned this pull request Apr 29, 2024
m1aw added a commit that referenced this pull request May 7, 2024
…om payButton (#2655)

* allow ancv to use existing order

* simplify submit handling

* add changeset

(cherry picked from commit cfc8d29)
m1aw added a commit that referenced this pull request May 15, 2024
…om payButton (#2655)

* allow ancv to use existing order

* simplify submit handling

* add changeset

(cherry picked from commit cfc8d29)
m1aw added a commit that referenced this pull request May 22, 2024
…s intended via custom pay button (#2675)

* Fix: ANCV ignoring existing orders & not working as intended via custom payButton (#2655)

* allow ancv to use existing order

* simplify submit handling

* add changeset

(cherry picked from commit cfc8d29)

* fix ANCV cherry pick
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants