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

Add Optional merchantAccountID to PayPalVaultEditRequest #1383

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

KunJeongPark
Copy link
Contributor

Summary of changes

  • Add optional merchantAccountID and docstrings to PayPalVaultEditRequest

Checklist

  • Added a changelog entry

Authors

List GitHub usernames for everyone who contributed to this pull request.

@KunJeongPark KunJeongPark requested a review from a team as a code owner August 5, 2024 17:08
@@ -5,13 +5,16 @@ import Foundation
public class BTPayPalVaultEditRequest {

private let editPayPalVaultID: String
public var merchantAccountID: String?
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public var merchantAccountID: String?
private let merchantAccountID: String?

I agree with Jax. Also any reason we can't make this a data class?

Copy link
Contributor Author

@KunJeongPark KunJeongPark Aug 5, 2024

Choose a reason for hiding this comment

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

data class? Not sure what you mean. Did you mean struct?
I don't think there is a reason except for pre-existing pattern of using classes for requests.
I think that's from objective-c maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wow yeah I'm getting confused I thought this was Kotlin lol. Sorry. I forgot that Objective-C and Swift structs don't mix well. Plz disregard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries. You do bring up a good point about use of classes for requests.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could be a struct. For new features, we're trying to experiment w/ seeing if we get merchant feedback by not offering Objective-C support (ex: ShopperInsights & CreditMessaging don't support Obj-C). If we need to switch it to class later for ObjC bridging we can, but I like the idea of using a struct!


// TODO: specify endpoint for merchant to retrieve the token
/// Initializes a PayPal Edit Request for the edit funding instrument flow
/// - Parameters:
/// - editPayPalVaultID: Required: The `edit_paypal_vault_id` returned from the server side request
/// merchantAccountIdD: optional ID of the merchant account; if one is not provided the default will be used
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// merchantAccountIdD: optional ID of the merchant account; if one is not provided the default will be used
/// merchantAccountID: optional ID of the merchant account; if one is not provided the default will be used

@KunJeongPark KunJeongPark merged commit be06e4a into edit-fi-feature Aug 7, 2024
8 checks passed
@KunJeongPark KunJeongPark deleted the edit-fi-merchantAccountID branch August 7, 2024 16:51
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.

5 participants