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

[Connect] Fix API inconsistent naming + Add Dashboard-only support for Account Management component #4036

Merged

Conversation

mludowise-stripe
Copy link
Collaborator

@mludowise-stripe mludowise-stripe commented Sep 23, 2024

Summary

  • Adds the account management component with DashboardOnly access

  • Fixes inconsistent delegate naming in PayoutsViewController
    The account onboarding component had a delegate signature of:

    accountOnboarding(_ accountOnboarding: AccountOnboardingViewController,
                               didFailLoadWithError error: Error)
    

    However, the payouts component was using:

    payoutsLoadDidFail(_ payouts: PayoutsViewController,
                       withError error: Error)
    

    During API review, we had intended to use the account onboarding style signature (see papertrail)

Motivation

https://jira.corp.stripe.com/browse/MXMOBILE-2503

Testing

Unit tests

@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2502_uk_connect branch from 1802618 to cf3cbc1 Compare September 23, 2024 21:22
@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2503_dashboard_components branch from 990be64 to d639757 Compare September 24, 2024 01:00
Base automatically changed from mludowise/MXMOBILE-2502_uk_connect to master September 24, 2024 03:12
@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2503_dashboard_components branch from d639757 to fc2d6f3 Compare September 24, 2024 07:03
@mludowise-stripe mludowise-stripe changed the title [WIP] [Connect] Add Dashboard-only support for other components [Connect] Add Dashboard-only support for Account Management component + fix API discrepency Sep 26, 2024
@mludowise-stripe mludowise-stripe changed the title [Connect] Add Dashboard-only support for Account Management component + fix API discrepency [Connect] Add Dashboard-only support for Account Management component + fix API discrepancy Sep 26, 2024
@mludowise-stripe mludowise-stripe changed the title [Connect] Add Dashboard-only support for Account Management component + fix API discrepancy [Connect] Fix API discrepancy + Add Dashboard-only support for Account Management component Sep 26, 2024
@mludowise-stripe mludowise-stripe changed the title [Connect] Fix API discrepancy + Add Dashboard-only support for Account Management component [Connect] Fix API inconsistent naming + Add Dashboard-only support for Account Management component Sep 26, 2024
@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2503_dashboard_components branch from fc2d6f3 to efe90a3 Compare September 26, 2024 05:42
Comment on lines +57 to +58
func payouts(_ payouts: PayoutsViewController,
didFailLoadWithError error: Error)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We had intended to make this consistent with the AccountOnboarding delegate style, but it slipped past API review. Paper trail of API approval:
https://docs.google.com/document/d/195BaU6k2J-2CAs9anNE6e4OlZO34N-e2HTYc6pacpTw/edit?pli=1#bookmark=id.dk457iw2wtca

@@ -98,6 +98,13 @@ public class EmbeddedComponentManager {
.init(componentManager: self)
}

@_spi(DashboardOnly)
public func createAccountManagementViewController(
collectionOptions: AccountCollectionOptions = .init()) -> AccountManagementViewController {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure yet whether we plan to set account collection options or whether this does anything for Dashboard, but putting it in for now so we can test it

@mludowise-stripe mludowise-stripe marked this pull request as ready for review September 26, 2024 05:45
@mludowise-stripe mludowise-stripe requested review from a team as code owners September 26, 2024 05:45
@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2503_dashboard_components branch from dd968f0 to e844c0f Compare September 28, 2024 16:40
Copy link

github-actions bot commented Sep 28, 2024

🚨 New dead code detected in this PR:

AccountManagementViewController.swift: warning: Property 'collectionOptions' is assigned, but never used

Please remove the dead code before merging.

If this is intentional, you can bypass this check by adding the label skip dead code check to this PR.

ℹ️ If this comment appears to be left in error, double check that the flagged code is actually used and/or make sure your branch is up-to-date with master.

@mludowise-stripe mludowise-stripe marked this pull request as draft September 29, 2024 00:11
@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2503_dashboard_components branch from e844c0f to 13b9949 Compare September 29, 2024 00:12
@mludowise-stripe mludowise-stripe changed the base branch from master to mludowise/MXMOBILE-2796_onboarding_properties September 29, 2024 00:12
@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2503_dashboard_components branch from 13b9949 to a272b52 Compare September 29, 2024 00:14
Base automatically changed from mludowise/MXMOBILE-2796_onboarding_properties to master September 30, 2024 22:11
@mludowise-stripe mludowise-stripe force-pushed the mludowise/MXMOBILE-2503_dashboard_components branch from a272b52 to 62516e2 Compare October 1, 2024 00:28
@mludowise-stripe mludowise-stripe marked this pull request as ready for review October 1, 2024 00:37
@mludowise-stripe
Copy link
Collaborator Author

Dead code check has a false positive – adding label

Co-authored-by: Chris Mays <[email protected]>
@mludowise-stripe mludowise-stripe merged commit 9e06a39 into master Oct 2, 2024
6 checks passed
@mludowise-stripe mludowise-stripe deleted the mludowise/MXMOBILE-2503_dashboard_components branch October 2, 2024 19:33
porter-stripe added a commit that referenced this pull request Oct 3, 2024
## Summary
- Includes line numbers in the dead code check
- Retains `Codable` properties in dead code check
(#4036 (comment))
- Better format API diff output

## Motivation
Improve CI

## Testing
#4088

## Changelog
N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants