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

Quick fix to hide NOL pay from Drop-In #893

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

borisprimer
Copy link
Contributor

I raised this PR just to pinpoint the problem and to fix it quickly, but there should be a better way to do it.

I also created this JIRA ticket with findings.

@borisprimer borisprimer requested review from jnewc and NQuinn27 June 6, 2024 17:25
@borisprimer borisprimer self-assigned this Jun 6, 2024
@borisprimer borisprimer marked this pull request as draft June 6, 2024 17:25
Copy link
Contributor

github-actions bot commented Jun 6, 2024

Fails
🚫

Please use a conventional commit title for this PR. See Conventional Commits and SemVer

Warnings
⚠️ This PR doesn't seem to contain any updated Unit Test 🤔. Please consider double checking it.🙏
⚠️ PR is classed as Work in Progress

Generated by 🚫 Danger Swift against d23bdf3

Copy link

sonarqubecloud bot commented Jun 6, 2024

Copy link
Contributor

github-actions bot commented Jun 6, 2024

@NQuinn27
Copy link
Contributor

NQuinn27 commented Jun 7, 2024

Nice quick find.

On PrimerUniversalHeadlessCheckout we have unsupportedPaymentMethodTypes.
I would advocate for moving this and the new logic in Drop-in to PrimerPaymentMethodType

internal enum PrimerPaymentMethodType {
    ...
    
    var supportsHeadless: Bool {
        switch self {
            ...
        }
    }
    
    var supportsDropIn: Bool {
        switch self {
            ...
        }
    }

WDYT. Its not much more work and I think it could be the end state here. Simple and consolidated to one place.

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

Successfully merging this pull request may close these issues.

3 participants