Do not show MFA button when the project does not have MFA enabled#1338
Do not show MFA button when the project does not have MFA enabled#1338megastep wants to merge 2 commits intofirebase:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces conditional visibility for the MFA management button in SignedInView based on the authentication configuration. Feedback includes a suggestion to mark the internal helper property as private for better encapsulation and a recommendation to add UI or view-level tests to verify the button's presence in the hierarchy, as the current unit tests only validate the underlying logic.
| @Test("MFA management button is hidden when MFA is disabled") | ||
| func mfaManagementButtonHiddenWhenMfaDisabled() { | ||
| let configuration = AuthConfiguration(mfaEnabled: false) | ||
|
|
||
| #expect(SignedInView.showsMfaManagementButton(configuration: configuration) == false) | ||
| } | ||
|
|
||
| @Test("MFA management button is shown when MFA is enabled") | ||
| func mfaManagementButtonShownWhenMfaEnabled() { | ||
| let configuration = AuthConfiguration(mfaEnabled: true) | ||
|
|
||
| #expect(SignedInView.showsMfaManagementButton(configuration: configuration) == true) | ||
| } |
There was a problem hiding this comment.
The PR description mentions adding a UI test, but the implementation provides unit tests for a helper method. While testing the logic is a good practice, these tests do not verify that the button is actually hidden or shown in the view hierarchy. Consider adding a UI test or a view-level test (e.g., using ViewInspector) to verify the actual presence of the mfa-management-button based on the configuration.
….swift Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
This is a simple change to the SignedIn view to conditionally disable showing the manage MFA button when this feature is not enabled in the Firebase project, to prevent user confusion.
Also added a corresponding UI test for this behavior.