Skip to content

LG 11141 Break up MFA selection presenter classes for Backup codes#9532

Merged
kevinsmaster5 merged 6 commits intomainfrom
kmas-lg-11141-break-up-backup-code-presenter
Nov 6, 2023
Merged

LG 11141 Break up MFA selection presenter classes for Backup codes#9532
kevinsmaster5 merged 6 commits intomainfrom
kmas-lg-11141-break-up-backup-code-presenter

Conversation

@kevinsmaster5
Copy link
Contributor

🎫 Ticket

LG-11141

🛠 Summary of changes

Divide Backup codes presenter classes into Sign In and Sign Up classes following the example of #9211

📜 Testing Plan

  • Create a new account at http://localhost:3000
  • Choose Backup codes as MFA and some other method to complete signup
  • Log out, log back in using Backup codes
  • Observe that no errors occur and login is successful

@kevinsmaster5 kevinsmaster5 marked this pull request as ready for review November 3, 2023 12:38
@kevinsmaster5 kevinsmaster5 force-pushed the kmas-lg-11141-break-up-backup-code-presenter branch from 324f7fd to a59bb74 Compare November 3, 2023 14:06
@aduth aduth requested a review from a team November 3, 2023 14:36
case type
when 'backup_code'
t('two_factor_authentication.login_options.backup_code')
when 'personal_key'
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we changing backup_code to personal_key permanently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not as such. I think personal_key gets removed from this switch statement on a different branch that's ahead of mine. Those label cases are moved to their respective set up/sign in selection presenter. I'll double check to make sure I'm not somehow adding that back in.
It should be gone from line ~72 as of this PR #9528

Copy link
Contributor

@aduth aduth Nov 6, 2023

Choose a reason for hiding this comment

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

There may have been a merge conflict where the personal_key statement got added back accidentally. I wouldn't expect it to be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed that from my branch.

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM 👍

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.

3 participants