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

[FC] Removes mavericks from repositories and more panes. #8154

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Mar 25, 2024

Summary

  • Removes mavericks repositories, replacing them by regular state flows.
  • Removes mavericks from Manual Entry, Partner Auth, Success and Account Picker.

Motivation

Testing

  • Added tests
  • Modified tests
  • Manually verified

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review March 25, 2024 16:57
@carlosmuvi-stripe carlosmuvi-stripe requested review from a team as code owners March 25, 2024 16:57
@carlosmuvi-stripe carlosmuvi-stripe requested review from awush-stripe and tillh-stripe and removed request for a team and awush-stripe March 25, 2024 16:57
Copy link
Contributor

github-actions bot commented Mar 25, 2024

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │     2 MiB │     2 MiB │  0 B │   4.2 MiB │   4.2 MiB │  0 B 
     arsc │   1.5 MiB │   1.5 MiB │  0 B │   1.5 MiB │   1.5 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │   8.1 KiB │   8.1 KiB │  0 B 
      res │ 363.2 KiB │ 363.2 KiB │  0 B │ 490.6 KiB │ 490.6 KiB │  0 B 
   native │   7.3 MiB │   7.3 MiB │  0 B │  18.4 MiB │  18.4 MiB │  0 B 
    asset │   1.6 MiB │   1.6 MiB │  0 B │   1.6 MiB │   1.6 MiB │  0 B 
    other │ 116.1 KiB │ 116.1 KiB │  0 B │ 273.1 KiB │ 273.1 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  12.8 MiB │  12.8 MiB │  0 B │  26.4 MiB │  26.4 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 21414 │ 21414 │ 0 (+0 -0) 
   types │  6641 │  6641 │ 0 (+0 -0) 
 classes │  5408 │  5408 │ 0 (+0 -0) 
 methods │ 31079 │ 31079 │ 0 (+0 -0) 
  fields │ 18096 │ 18096 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  262 │  262 │  0   
 entries │ 5514 │ 5514 │  0

Comment on lines -15 to -20
/**
* The active auth session id. Used across process kills to prevent re-creating the session
* if one is already active.
*/
@PersistState
val activeAuthSession: String? = null,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

realized we don't need this anymore. When loading partner auth, we'd check the active session in the cached manifest, and if there's no manifest cached (i.e. process kill) we'd refetch it from backend, getting the existing auth session as well.

): Job {
return viewModelScope.launch {
setState { reducer(Loading) }
setState { reducer(Loading(value = retainValue?.get(this)?.invoke())) }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Took this from mavericks: This allows us to optionally keep the current Async value if moving to a Loading state from Success (so that we can keep the screen content while loading)


suspend fun get() = awaitState().error
private val state = MutableStateFlow(State())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need a flow here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -180,7 +181,7 @@ internal class AccountPickerViewModel @Inject constructor(
)
}

fun onAccountClicked(account: PartnerAccount) = withState { state ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about adding a withState method to our base ViewModel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@carlosmuvi-stripe carlosmuvi-stripe merged commit 6ca0025 into carlosmuvi/i/remove-mavericks Mar 26, 2024
11 of 12 checks passed
@carlosmuvi-stripe carlosmuvi-stripe deleted the carlosmuvi/remove-mavericks-part-2 branch March 26, 2024 16:31
carlosmuvi-stripe added a commit that referenced this pull request Mar 27, 2024
* [FC] Moves Activities and some Panes out of Mavericks (#8125)

* Removes mackericks references on Consent screen.

* Updates files.

* Uses viewmodel factory builder.

* Updates functions.

* Updates execute.

* Updates compose util.

* Adds missing side effects.

* Simplifies code.

* Renames viewmodel.

* Updates async.

* Removes mavericks from institution picker.

* Reverts rename.

* Updates baseline.

* Adds setState and persists state.

* Renames result error to fail.

* Migrates Initial activity out of mavericks.

* Removes persist state.

* Updates tests.

* Regenerates API.

* filterNotNull.

* Nits.

* Updates async.

* use suspend block.

* Update financial-connections/src/main/java/com/stripe/android/financialconnections/core/FinancialConnectionsViewModel.kt

Co-authored-by: Till Hellmund <tillh@stripe.com>

* Tries onAsync.

* Moves activity to stripe ui core.

* PR feedback.

* Regenerates deps.

---------

Co-authored-by: Till Hellmund <tillh@stripe.com>

* [FC] Removes mavericks from repositories and more panes. (#8154)

* Migrates more screens out of mavs.

* Migrates partner auth.

* Removes active auth session field.

* Updates tests.

* Updates attach payment viewmodel.

* PR feedback.

* Api dump.

* [FC] Removes mavericks from all viewmodels and tests (#8155)

* Migrates more screens out of mavs.

* Migrates partner auth.

* Removes active auth session field.

* Updates tests.

* Updates attach payment viewmodel.

* PR feedback.

* Api dump.

* Migrates missing viewmodels.

* Updates dependencies.

* PR feedback.

* Merge master.

* [FC] Removes mavericks dependency (#8160)

* Migrates more screens out of mavs.

* Migrates partner auth.

* Removes active auth session field.

* Updates tests.

* Updates attach payment viewmodel.

* PR feedback.

* Api dump.

* Migrates missing viewmodels.

* Updates dependencies.

* Removes mavericks dependency.

* Merge with integration.

* Uses collect.

* Updates dependencies

* Updates Changelog.

* Update CHANGELOG.md

Co-authored-by: Till Hellmund <tillh@stripe.com>

---------

Co-authored-by: Till Hellmund <tillh@stripe.com>
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.

2 participants