Skip to content

Feat: tags + User auth implementation#10

Open
JackMaarek wants to merge 4 commits intomasterfrom
feat_tags-implementation
Open

Feat: tags + User auth implementation#10
JackMaarek wants to merge 4 commits intomasterfrom
feat_tags-implementation

Conversation

@JackMaarek
Copy link
Collaborator

No description provided.

Copy link

@qraimbault qraimbault left a comment

Choose a reason for hiding this comment

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

Still some work to do but nice job 👍

CMD go run main.go
RUN go mod download

EXPOSE 8080 No newline at end of file

Choose a reason for hiding this comment

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

Missing line break

"time"
)

type UserSession struct {

Choose a reason for hiding this comment

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

Your user session should not be stored in DB. You can either store it in an in-memory cache (redis/memcached/consul for example) or use a manager with a mutex (see https://astaxie.gitbooks.io/build-web-application-with-golang/content/en/06.2.html for more details and example)

"net/http"
)

func CheckAuthorization(c *gin.Context) {

Choose a reason for hiding this comment

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

Missing comment for this exported function

if err != nil {
if err == http.ErrNoCookie {
c.Redirect(http.StatusPermanentRedirect, "/login")
c.Abort()

Choose a reason for hiding this comment

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

IIRC you just need to return but there is no need to abort

# Run command to nstall the dependencies
RUN go install

CMD go run main.go

Choose a reason for hiding this comment

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

If you remove your go run, you then need to compile and start the binary or nothing will happen

"github.com/jasongauvin/wikiPattern/services"
)

func ValidateArticle(article *services.ArticleForm) error {

Choose a reason for hiding this comment

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

Missing comment for this exported function

"github.com/jasongauvin/wikiPattern/services"
)

func ValidateComment(comment *services.CommentForm) error {

Choose a reason for hiding this comment

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

Missing comment for this exported function

Comment on lines +9 to +14
if article.Title == "" {
return errors.New("Required title")
}
if article.Content == "" {
return errors.New("Required content")
}

Choose a reason for hiding this comment

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

What if we are missing both ? Maybe return a slice of errors

"strings"
)

func ValidateUser(user *models.User, action string) error {

Choose a reason for hiding this comment

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

Missing comment for this exported function

Choose a reason for hiding this comment

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

Also, comment about slice of errors applies here too


func ValidateUser(user *models.User, action string) error {
switch strings.ToLower(action) {
case "update":

Choose a reason for hiding this comment

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

Don't use a switch. ValidateUser should validate the entire user, and does not depend of it being updated or created or whatever. Also you user validator is not meant to also validate your login form, these are 2 completely different things

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