Skip to content

BE-18 Email confirmation #1

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

Merged
merged 11 commits into from
Dec 9, 2021
Merged

BE-18 Email confirmation #1

merged 11 commits into from
Dec 9, 2021

Conversation

elffjs
Copy link
Member

@elffjs elffjs commented Dec 7, 2021

No description provided.

@@ -0,0 +1,78 @@
### Running the app
Copy link
Member

Choose a reason for hiding this comment

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

👍 for readme

@@ -50,7 +50,7 @@ func startWebAPI(logger zerolog.Logger, settings *config.Settings, pdb database.
},
DisableStartupMessage: true,
})
usersController := controllers.NewUsersController(settings, pdb.DBS, &logger)
usersController := controllers.NewUserController(settings, pdb.DBS, &logger)
Copy link
Member

Choose a reason for hiding this comment

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

if you're going to name it singular may also wish to change the name of the variable to userController for consistency

Copy link
Member Author

Choose a reason for hiding this comment

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

Missed that one.

Comment on lines 131 to 133
token := c.Locals("user").(*jwt.Token)
claims := token.Claims.(jwt.MapClaims)
userID := claims["sub"].(string)
Copy link
Member

Choose a reason for hiding this comment

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

could this be in a helper method.. could definitely see that one being part of our library too

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it should be middleware. Coming!

return nil
}

func init() {
Copy link
Member

Choose a reason for hiding this comment

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

do we really need an init function? Kinda frowned upon these days - they were even trying to remove them

Copy link
Member Author

@elffjs elffjs Dec 8, 2021

Choose a reason for hiding this comment

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

I try to not use them. I'm glad the linked proposal was shot down, though.

My use is that I have a constant time.Duration that I want to keep using, and I don't want to keep re-parsing the string that defines it. It wouldn't hurt to shove the difference in the controller struct.

@elffjs elffjs merged commit 49d7ee9 into main Dec 9, 2021
@elffjs elffjs deleted the BE-18-email-confirmation branch December 9, 2021 00:14
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.

2 participants