Add functionality for activating/deactivating users in the users list #27
Conversation
Serneum
force-pushed
the
feature/toggle-user-active
branch
from
5599124 to
541e01b
Compare
mbramson
force-pushed
the
feature/toggle-user-active
branch
from
541e01b to
7275b50
Compare
|
|
||
| conn = get conn, user_path(conn, :index) | ||
| assert html_response(conn, 200) =~ "Inactive" | ||
| end |
There was a problem hiding this comment.
I think that these tests are testing too many things at once. You've even left whitespace where these tests should probably be broken out.
|
|
||
| def deactivate(conn, %{"user_id" => id}) do | ||
| update_user(conn, id, %{active: false}, :index) | ||
| end |
There was a problem hiding this comment.
We should definitely explore ways to accomplish this without adding new non-RESTful controller methods, as discussed.
|
@mbramson Commit one isn't RESTful, but cleaner in my opinion. Commit two is the more RESTful but I hate it. Tell me which you'd prefer. |
Serneum
force-pushed
the
feature/toggle-user-active
branch
from
d855acb to
2a9934d
Compare
| resources "/users", UserController | ||
| resources "/users", UserController do | ||
| put "/activate", UserController, :activate, as: :activate | ||
| put "/deactivate", UserController, :deactivate, as: :deactivate |
There was a problem hiding this comment.
This doesn't seem to follow RESTful type behavior. Why are 'activate' and 'deactivate' their own routes here?
|
Closing in favor of #85, since the /users route no longer exists. That issue is a great opportunity to introduce react into this project. |
Closes #25