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

feat: Enable ACH via Stripe Vaulting flows #1056

Merged
merged 44 commits into from
Dec 17, 2024
Merged

Conversation

NQuinn27
Copy link
Contributor

@NQuinn27 NQuinn27 commented Dec 4, 2024

Description

ACC-4384
ACC-4461
ACC-4515

  • Adds vaultOnAgreement to the Debug App UI
  • Adds necessary deviceInfo to metadata for ACH flows
  • Show vaulted ACH payment methods in Drop-in UI
  • Enable paying with a vaulted ACH payment method on both Drop-in and Headless

Other Notes

Manual Testing

Add manual testing notes here if applicable, otherwise remove this section

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

Copy link
Contributor

github-actions bot commented Dec 4, 2024

Warnings
⚠️ > Pull Request size seems relatively large. If this Pull Request contains multiple changes, please split each into separate PR will helps faster, easier review.
⚠️ Please assign someone aside from CODEOWNERS (@checkout-pci-reviewers) to review this PR.
⚠️

Sources/PrimerSDK/Classes/Core/PrimerHeadlessUniversalCheckout/Composable/ACH/Services/ACHTokenizationService.swift#L16 - Delegate protocols should be class-only so they can be weakly referenced. (class_delegate_protocol)

⚠️

Sources/PrimerSDK/Classes/Core/PrimerHeadlessUniversalCheckout/Composable/ACH/Services/ACHTokenizationService.swift#L23 - Delegate protocols should be class-only so they can be weakly referenced. (class_delegate_protocol)

⚠️

Sources/PrimerSDK/Classes/Core/PrimerHeadlessUniversalCheckout/Composable/ACH/Services/ACHTokenizationService.swift#L28 - Lines should not have trailing whitespace. (trailing_whitespace)

⚠️

Sources/PrimerSDK/Classes/Core/PrimerHeadlessUniversalCheckout/Composable/ACH/Services/ACHTokenizationService.swift#L49 - Lines should not have trailing whitespace. (trailing_whitespace)

⚠️

Sources/PrimerSDK/Classes/Core/PrimerHeadlessUniversalCheckout/Composable/ACH/Services/ACHTokenizationService.swift#L64 - Lines should not have trailing whitespace. (trailing_whitespace)

⚠️

Sources/PrimerSDK/Classes/Core/PrimerHeadlessUniversalCheckout/Composable/ACH/Services/ACHTokenizationService.swift#L132 - Lines should not have trailing whitespace. (trailing_whitespace)

⚠️

Sources/PrimerSDK/Classes/User Interface/Vault/VaultPaymentMethodViewController.swift#L112 - Line should be 150 characters or less: currently 156 characters (line_length)

Generated by 🚫 Danger Swift against e4e1ad5

Copy link
Contributor

github-actions bot commented Dec 6, 2024

@primer-io primer-io deleted a comment from sonarqubecloud bot Dec 6, 2024
@NQuinn27 NQuinn27 marked this pull request as ready for review December 6, 2024 13:04
@NQuinn27 NQuinn27 requested a review from borisprimer December 13, 2024 15:42
semirp
semirp previously approved these changes Dec 16, 2024
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.

PCI OK.

* - Returns: A `Promise<Void>` that resolves if the payment is completed successfully, or rejects if there is
* an error during the API call.
*/
func completePayment(clientToken: DecodedJWTToken, completeUrl: URL) -> Promise<Void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

[q]: wouldn't it make more sense to send some params to be mapped to request body or maybe request body itself? This seems to be a generic function that creates body for only one use case.

@NQuinn27 NQuinn27 requested a review from semirp December 17, 2024 09:27
semirp
semirp previously approved these changes Dec 17, 2024
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.

PCI OK

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
26.2% Coverage on New Code (required ≥ 80%)
12.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@NQuinn27 NQuinn27 requested a review from semirp December 17, 2024 10:40
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.

PCI OK

@NQuinn27 NQuinn27 merged commit e0cb7e9 into master Dec 17, 2024
14 of 15 checks passed
@NQuinn27 NQuinn27 deleted the nq/update_ach_vaulting branch December 17, 2024 10:47
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.

4 participants