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

getMeでDBを叩くように変更 #763

Open
wants to merge 2 commits into
base: v2
Choose a base branch
from
Open

Conversation

comavius
Copy link
Contributor

@comavius comavius commented Jun 7, 2024

#725

  • getMeでセッションからユーザーを取ってくる実装からDBからに変更
  • DBにユーザーが存在しない場合にセッションのユーザーをDBに追加

@comavius comavius requested a review from H1rono June 7, 2024 14:10
@@ -82,11 +82,27 @@ func (h Handlers) GetMe(c echo.Context) error {
h.Logger.Error("failed to get session", zap.Error(err))
return echo.NewHTTPError(http.StatusInternalServerError, err)
}
user, ok := sess.Values[sessionUserKey].(User)
user_in_session, ok := sess.Values[sessionUserKey].(User)
Copy link

Choose a reason for hiding this comment

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

めちゃ横槍で申し訳ないけど、プライベートな変数はローワーキャメルケースが標準な気がします

Suggested change
user_in_session, ok := sess.Values[sessionUserKey].(User)
userInSession, ok := sess.Values[sessionUserKey].(User)

Copy link
Member

Choose a reason for hiding this comment

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

:koreni_natteru:

router/user.go Outdated
Comment on lines 91 to 100
users, err := h.Repository.GetUsers(c.Request().Context())
if err != nil {
h.Logger.Error("failed to get users from repository", zap.Error(err))
return echo.NewHTTPError(http.StatusInternalServerError, err)
}
for _, user_i := range users {
if user_i.ID == user_in_session.ID {
return c.JSON(http.StatusOK, user_i)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

おかしい
UserRepository.GetUserByIDがあるのでそちらを使うべきです

GetUserByID(ctx context.Context, userID uuid.UUID) (*User, error)

userが存在しない場合は何かしらのerrが返ってくるはずなので、それに応じて後に処理を続けてください

@H1rono
Copy link
Member

H1rono commented Jun 8, 2024

とりあえずこれ
あとテストの方を直す必要があります

@comavius comavius requested a review from H1rono July 9, 2024 07:04
Copy link
Member

@H1rono H1rono left a comment

Choose a reason for hiding this comment

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

まずは変数名をcamelCaseにしてください

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