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

Feature/ja 117 activation emails #107

Merged
merged 25 commits into from
Jun 26, 2024
Merged

Conversation

skrawus
Copy link
Collaborator

@skrawus skrawus commented Jun 6, 2024

I added sending activation emails. Still in progress.

@skrawus skrawus marked this pull request as ready for review June 7, 2024 21:59
Copy link
Collaborator

@Zjyslav Zjyslav left a comment

Choose a reason for hiding this comment

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

  • Move activation link to configuration
  • Move activation code generation to UserAuthenticationService (return the activation code as part of result of RegisterUser, instead of putting it in as a parameter)

TutorLizard.Web/Services/UserAuthenticationService.cs Outdated Show resolved Hide resolved
Copy link
Collaborator

@Zjyslav Zjyslav left a comment

Choose a reason for hiding this comment

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

  • Move all logic touching db to UserService.

I'm thinking about putting the IsUserActive logic as part of LogInAsync. It already recevices the user from UserService, so it could return not just a bool, but a result object containing the code and we wouldn't need to call the db twice.
Now that I think of it, we log the user in, then we check if the account is active and if it isn't, they are still logged in (even though they are shown a failure message). I think this check should be done as part of logging in and we could return a result with an enum or something telling the controller what was the reason of failure (wrong data, inactive account, etc.).

TutorLizard.Web/Services/UserAuthenticationService.cs Outdated Show resolved Hide resolved
Copy link
Collaborator

@Zjyslav Zjyslav left a comment

Choose a reason for hiding this comment

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

I approve, but please resolve merge conflicts.

and resolve conflicts
@Zjyslav Zjyslav merged commit dce3b36 into develop Jun 26, 2024
1 check passed
@Zjyslav Zjyslav deleted the feature/ja-117-activation-emails branch June 26, 2024 11:09
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.

None yet

2 participants