Multi facilities #189
Multi facilities #189
Conversation
|
Hello, @fcapovilla! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information. |
![sourcelevel-bot[bot]](https://avatars.githubusercontent.com/in/597?s=60&v=4){kind=small}
| alias OpenPantry.Repo | ||
| alias OpenPantry.User | ||
|
|
||
| def for_token(user = %User{}), do: { :ok, "User:#{user.id}" } | ||
| def for_token(_), do: { :error, "Unknown resource type" } | ||
|
|
||
| def from_token("User:" <> id), do: { :ok, Repo.get(User, id) } | ||
| def from_token("User:" <> id), do: { :ok, User |> Ecto.Query.preload(:managed_facilities) |> Repo.get(id) } |
There was a problem hiding this comment.
There is no whitespace around parentheses/brackets most of the time, but here there is.
| end | ||
|
|
||
| @doc false | ||
| def changeset(%UserManagedFacility{} = user_facility, attrs) do |
There was a problem hiding this comment.
File has the variable name after the pattern while most of the files have the variable name before the pattern when naming parameter pattern matches
| @@ -0,0 +1,19 @@ | |||
| defmodule OpenPantry.UserManagedFacility do | |||
There was a problem hiding this comment.
Modules should have a @moduledoc tag.
lib/open_pantry/user_from_auth.ex
Outdated
| case Repo.get_by(User, email: auth.info.email) do | ||
| user = | ||
| from(u in User, where: u.email == ^auth.info.email, preload: [:managed_facilities]) | ||
| |> Repo.one |
There was a problem hiding this comment.
Use a function call when a pipeline is only one function long
|
Ebert has finished reviewing this Pull Request and has found:
You can see more details about this review at https://ebertapp.io/github/openpantry/open_pantry/pulls/189. |
|
Ugh I failed myself! Forgot to do this Monday or Tuesday, just looked at the other open PR tonight... Prefer to test locally before merging since test suite is for shit, but also trust you if you say it works, so... I'm just gonna merge this and if anything is broken, we'll fix in a follow up PR! I'm slightly tipsy, but I feel this is still a responsible decision ;-) Thanks so much @fcapovilla ! |
|
Don't worry, I also forgot to remind you like you asked me. :D |
Fixes issues #163
This feature adds the "managed_facilities" field to Users, so an admin can manage multiple facilities.