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 all factor sources screens #1303

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

giannis-rdx
Copy link
Contributor

Description

This PR:

  • adds all security factor sources screens in "Security Factors" settings screen
  • renames "passhprase" to "mnemonic"
  • adds info dialog for all factor source types
  • several renamings and refactorings

⚠️ Security factor warnings have not been implemented yet!

Initially, the idea was to introduce a single generic screen and ViewModel for all factor source screens (e.g., Biometrics/PIN, Arculus Card, etc.). However, this approach didn’t work well because each factor source type either already has, or might later require, business logic specific to its type. For example, consider LedgerDevicesViewModel, which contains business logic related to linked connectors.

To avoid spending more time trying to figure out ‘generic’ ideas or patterns, I opted for a simpler and faster approach: one ViewModel and screen per factor source type. This approach leverages some already implemented shared logic, such as composables and the newly introduced GetFactorSourcesOfTypeUseCase in this PR.

How to test

  1. Test it with several factor source types. You can use the profile from here.

Video

paste link here

PR submission checklist

  • I have tested several security factor sources of different types.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube Cloud

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.

2 participants