feat(auth): Add copy for account-linking verification #28931
Conversation
| organization: Organization | ||
| provider_name: str | ||
| email: str | ||
| identity_id: str |
There was a problem hiding this comment.
I hope this refactoring doesn't seem gratuitous. Otherwise, it would have been necessary to duplicate both the org and provider name between the parameter lists for the two functions, which tripped my personal "time to gather everything into a class" threshold.
The "hide whitespace changes" setting should cut down on the diff noise.
|
The new confirmation prompt page:
A mockup of the email body (HTML):
|
src/sentry/auth/idpmigration.py
Outdated
| organization=self.organization, user=self.user | ||
| ).id | ||
|
|
||
| verification_code = get_random_string(32, string.ascii_letters + string.digits) |
There was a problem hiding this comment.
would it be worth moving verification_code to init (or post_init i think since its a dataclass), and also perhaps moving the redis functionality into its own class / function?
There was a problem hiding this comment.
Both sound good. @maxiuyuan, we could work on those changes together?
There was a problem hiding this comment.
moving the redis functionaility to its own function is already done in another pr
There was a problem hiding this comment.
Yep, I'll be waiting to merge this until one or two of those dev branches are finished. I'll have to shake out a few merge conflicts but it won't be a problem.
|
|
Yes, the only detail I wanted to run by you is how it changes the unit tests. |
Pass additional arguments from AuthIdentityHandler to make the message more informative. Refactor into AccountConfirmLink class to avoid passing too many params between functions.
ca91865 to
81b73ab
Compare
|
Ready for review. I believe we're done revising the mock-ups. @maxiuyuan The unit test changes on 81b73ab turned out not to be too complicated -- mainly it was about trading the mock injection in |
Pass additional arguments from AuthIdentityHandler to make the message more informative.