Fix/user object on session #125
Conversation
![sourcelevel-bot[bot]](https://avatars.githubusercontent.com/in/597?s=60&v=4){kind=small}
web/controllers/retro_controller.ex
Outdated
| @@ -9,10 +9,9 @@ defmodule RemoteRetro.RetroController do | |||
| conn = put_session conn, "requested_endpoint", conn.request_path | |||
| redirect conn, to: "/auth/google" | |||
| user -> | |||
| user_from_db = Repo.get_by(User, email: user["email"]) | |||
| query = from p in Participation, where: p.user_id == ^user_from_db.id and p.retro_id == ^params["id"] | |||
| query = from p in Participation, where: p.user_id == ^user.id and p.retro_id == ^params["id"] | |||
There was a problem hiding this comment.
Line is too long (max is 80, was 99).
web/controllers/retro_controller.ex
Outdated
| @@ -1,6 +1,6 @@ | |||
| defmodule RemoteRetro.RetroController do | |||
| use RemoteRetro.Web, :controller | |||
| alias RemoteRetro.{Retro, Participation, User} | |||
| alias RemoteRetro.{Retro, Participation} | |||
There was a problem hiding this comment.
Most of the time you are using the multiple single line alias/require/import/use directives but here you are using the multi-alias/require/import/use syntax
web/models/user.ex
Outdated
| :last_login | ||
| ] | ||
|
|
||
| @derive {Poison.Encoder, only: @required_fields } |
There was a problem hiding this comment.
There is no whitespace around parentheses/brackets most of the time, but here there is.
| changeset = User.changeset(%User{}, user_params) | ||
| Repo.insert!(changeset) | ||
| else | ||
| changeset = User.changeset(user, user_params) | ||
| Repo.update!(changeset) | ||
| end | ||
|
|
||
| conn = put_session(conn, :current_user, user_info) | ||
| user = Map.delete(user, :__meta__) | ||
| user = Map.delete(user, :__struct__) |
There was a problem hiding this comment.
Variable "user" was declared more than once.
web/controllers/auth_controller.ex
Outdated
| } | ||
|
|
||
| user_params = Map.merge(user_params, user_info) | ||
|
|
||
| if !user do | ||
| user = if !user do |
There was a problem hiding this comment.
Avoid negated conditions in if-else blocks.
vanderhoop
force-pushed
the
fix/user-object-on-session
branch
from
8854603
to
06ba6db
Compare
| redirect conn, to: "/auth/google" | ||
| user -> | ||
| user_from_db = Repo.get_by(User, email: user.email) | ||
| query = from p in Participation, where: p.user_id == ^user_from_db.id and p.retro_id == ^params["id"] |
There was a problem hiding this comment.
Line is too long (max is 100, was 107).
| @@ -13,15 +13,18 @@ defmodule RemoteRetro.AuthController do | |||
|
|
|||
| user_params = User.build_user_from_oauth(user_info) | |||
|
|
|||
| if !user do | |||
| user = if !user do | |||
There was a problem hiding this comment.
Avoid negated conditions in if-else blocks.
|
Ebert has finished reviewing this Pull Request and has found:
You can see more details about this review at https://ebertapp.io/github/stride-nyc/remote_retro/pulls/125. |
- because we're relying on a new data structure that gets set in the authentication callback, we have to ensure users with existing sessions don't have their sessions invalidated
vanderhoop
force-pushed
the
fix/user-object-on-session
branch
from
8f9e3fa
to
3fabfda
Compare
Add %User{} to session instead of oauth user data.