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: Validate PENDING in resume if showSuccessCheckoutOnPendingPayment flag is true #957

Merged
merged 7 commits into from
Aug 6, 2024

Conversation

NQuinn27
Copy link
Contributor

@NQuinn27 NQuinn27 commented Jul 31, 2024

Description

This PR respects the showSuccessCheckoutOnPendingPayment flag when validating Payment response on the /resume call.

  • If the Payment response is in state PENDING and showSuccessCheckoutOnPendingPayment is false, we will throw the payment failed error, as we would if the state was FAILED
  • If the Payment response is in state PENDING and showSuccessCheckoutOnPendingPayment is true, we will continue on the success path

Manual Testing

Test with Fintechture as outlined here

Screenshots

If applicable, otherwise remove this section

Contributor Checklist

  • All status checks have passed prior to code review
  • I have added unit tests to a reasonable level of coverage where suitable
  • I have added UI tests to new user flows, if applicable
  • I have manually tested newly added UX
  • I have open a documentation PR, if applicable

Reviewer Checklist

  • I have verified that a suitable set of automated tests has been added
  • I have verified that the title prefix aligns to the code changes + whether a release is expected after merging the PR
  • I have verified the documentation PR aligns with this PR, if applicable

Before Merging

  • If introducing a breaking change, I have communicated it internally
  • Any related documentation PRs are ready to merge

Other Stuff

  • You can find out more about our automation checks here
  • Find out more about conventional commits here

@NQuinn27 NQuinn27 requested a review from a team as a code owner July 31, 2024 13:46
Copy link
Contributor

github-actions bot commented Jul 31, 2024

Warnings
⚠️ Please assign someone aside from CODEOWNERS (@checkout-pci-reviewers) to review this PR.
⚠️

Sources/PrimerSDK/Classes/Core/Payment Services/CreateResumePaymentService.swift#L41 - Lines should not have trailing whitespace. (trailing_whitespace)

⚠️

Sources/PrimerSDK/Classes/Data Models/API/PaymentAPIModel.swift#L125 - Line should be 150 characters or less: currently 173 characters (line_length)

⚠️

Sources/PrimerSDK/Classes/Services/Network/PrimerAPIClientProtocol.swift#L13 - Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (colon)

⚠️

Sources/PrimerSDK/Classes/Services/Network/Protocols/PrimerAPIClientCreateResumePaymentProtocol.swift#L10 - Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)

Generated by 🚫 Danger Swift against 133300d

Copy link
Contributor

Copy link
Contributor

@semirp semirp left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

sonarqubecloud bot commented Aug 5, 2024

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarCloud

@NQuinn27 NQuinn27 force-pushed the nq/respect_pending_success_flag branch from f609972 to 133300d Compare August 5, 2024 14:46
@NQuinn27 NQuinn27 merged commit 88122fa into master Aug 6, 2024
10 of 12 checks passed
@NQuinn27 NQuinn27 deleted the nq/respect_pending_success_flag branch August 6, 2024 08:31
NQuinn27 added a commit that referenced this pull request Aug 7, 2024
…t flag is true (#957)

* Validate PENDING in resume if flag is true

* Remove unused code

* Added unit tests for CreateResumeService

* Make new flag a bool

* Added strings for fintechture

* Remove redundant enum

* Split out createresume into seperate protocol
NQuinn27 added a commit that referenced this pull request Aug 8, 2024
* Added poc of config caching

* Extend network service to bubble up headers when wanted

* Dont utilise headers in klarna configs

* Added Swift-friendly Cache

* Refactored caching logic to dedicated class

* use refactored cache

* Extract ttl from headers

* Process images also for actions

* Upgrade config endpoints to 2.3

* Add config cache tests

* Fix tests for new interface

* update fallback to 0

* add tests for config module

* Add responses to mocks

* Formatting: Update ConfigurationCache.swift

* Formatting: Update PrimerAPIConfigurationModule.swift

* Formatting: Update PrimerAPIConfigurationModule.swift

* Formatting: Update DefaultNetworkService.swift

* Formatting: Update NetworkService.swift

* Clear cache after tests

* Update apiVersion in tests

* Added events for loading

* Clear cache on cleanup

* Add checkout session active

* Record load of vault manager

* Record headless loading event

* fix: Validate PENDING in resume if showSuccessCheckoutOnPendingPayment flag is true (#957)

* Validate PENDING in resume if flag is true

* Remove unused code

* Added unit tests for CreateResumeService

* Make new flag a bool

* Added strings for fintechture

* Remove redundant enum

* Split out createresume into seperate protocol

* Fix conflicts

* Release 2.28.0 (#963)

[create-pull-request] automated change

Co-authored-by: NQuinn27 <3179752+NQuinn27@users.noreply.github.com>

* Add clientSessionCachingEnabled flag

* Add unit test to test for flag

* move cache clean to cleanup

* Implement LogReporter

* Fix use cached config test

---------

Co-authored-by: Security Integrations <security-integrations@primer.io>
Co-authored-by: NQuinn27 <3179752+NQuinn27@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants