smpallen99 / coherence

Coherence is a full featured, configurable authentication system for Phoenix
MIT License
1.27k stars 224 forks source link

Updating the User Model #204

Closed newterminator closed 7 years ago

newterminator commented 7 years ago

I followed the instructions as given here and added the following code in the post controller where I am updating my post: apply(Config.auth_module, Config.update_login, [conn, user, [id_key: Config.schema_key]])

like so:

user = Repo.get(User, post.user_id)
user = Ecto.Changeset.change user, review_count: (user.review_count + 1)
case Repo.update(user) do
   {:ok, _struct} ->
        apply(Config.auth_module, Config.update_login, [conn, user, [id_key: Config.schema_key]])
   {:error, _changeset} ->
        IO.puts("error")
end

But when later in the template when I do: @current_user.review_count the value is not getting updated to reflect the updated value from the database.

smpallen99 commented 7 years ago

Are you trying to access current_user on the same request that it was changed? It looks like I don't store the new data in the conn.assigns in the update_login function. This is a bug that will need to be fixed. Good catch.

For now, you can set it manually as a work around.

apply(Config.auth_module, Config.update_login, [conn, user, [id_key: Config.schema_key]])
|> assign(Config. assigns_key, user)  # add this
newterminator commented 7 years ago

Hey @smpallen99 Thanks for responding back. In the post controller, I updated the user who made the review on the post. In the browser, I am logged in as the user who made the review, and after making the review which the controller successfully updates, the review count does not reflect accurately, even though checking the postgres database confirms that the controller completed the action successfully.

So for a small example: if User A posts a review then his review count is updated to be incremented by 1 in the controller action. But the @current_user.review_count in the template displays 0.

I used your code to get it work manually, but then it breaks code at all places where @current_user is used.

smpallen99 commented 7 years ago

New to functional programming? :) I missed your problem the first time around. I was focused on the update_login function not putting the new user into the conn.assigns map. You're ignoring the updated user _struct below. The user variable is bound to the changeset and not updated after calling Repo.update. See my annotations below and the code that should fix your problem.

user = Ecto.Changeset.change user, review_count: (user.review_count + 1)
case Repo.update(user) do
   {:ok, _struct} ->  # _struct contains the updated user
       # the user below contains the changeset
        apply(Config.auth_module, Config.update_login, [conn, user, [id_key: Config.schema_key]])
   {:error, _changeset} ->
        IO.puts("error")
end

Try this...

user = Ecto.Changeset.change user, review_count: (user.review_count + 1)
case Repo.update(user) do
   {:ok, user} ->  # user now gets bound to the updated user schema
        apply(Config.auth_module, Config.update_login, [conn, user, [id_key: Config.schema_key]])
       |> assign(conn, Config.assigns_key, user)  # not sure this is needed. See below
   {:error, _changeset} ->
        IO.puts("error")
end

The assign call would only be required if you are doing a render at the end of the controller action. If you are doing a redirect, its not needed since its a new request which will call the auth plug again and load current_user.

The reason why things are breaking is that User is not an Ecto.Schema. Its a Ecto.Changeset.

Here's a tip in debugging these types of errors. Add an IO.inspect or a Logger call to verify the value that is not being set right. Something like:

case Repo.update(user) do
   {:ok, _struct} ->  # user now gets bound to the updated user schema
        IO.inspect user, label: "user: "
        apply(Config.auth_module, Config.update_login, [conn, user, [id_key: Config.schema_key]])

With that, you will see that user is a changeset and not a %MyApp.User{} struct.

newterminator commented 7 years ago

Hi @smpallen99 , Thanks for writing such a detailed and accurate response to my question, including nailing the reality that I am new to functional programming. What a thorough breakdown you gave including the comments with the code. Reading this I understood that the readme was also most of your writing, which I found to be very clear at the time of installing the package.

So yes this part right here as you suggested resolved the issue:

user = Ecto.Changeset.change user, review_count: (user.review_count + 1)
case Repo.update(user) do
   {:ok, user} ->  # user now gets bound to the updated user schema
        apply(Config.auth_module, Config.update_login, [conn, user, [id_key: Config.schema_key]])
       |> assign(conn, Config.assigns_key, user)  # not sure this is needed. See below
   {:error, _changeset} ->
        IO.puts("error")
end

I didn't need the assign part in my case, as you guessed it right that I was doing a redirect.

Thank you for your continued improvements towards the package.

As a side note I wanted to ask you that I wanted that both the login and the register form be displayed on the sidebar in the application layout template file (web/templates/layout/app.html.eex). How could I accomplish that.

I will close the original issue since its been resolved.

smpallen99 commented 7 years ago

@newterminator Thanks for the nice feedback. Checkout the [exadmin demo][demo.exadmin.info] for examples on how to create custom side panels. One of the examples shows you have to render a template in the side panel. You will need to setup the form yourself and ensure you get the routes correct.

I've never tried it personally, but I can't think of a reason why it won't work.

newterminator commented 7 years ago

@smpallen99 Thanks again for your prompt response. The exadmin demo looks superb. Now this is an awesome package for anyone coming from rails which it seems if I am not wrong at first glance something like Rails_Admin gem. And it's also your baby...so the mark of your work will continue there ... woot woot ... Thanks Steve...you rock. So I am gonna check it out...

smpallen99 commented 7 years ago

@newterminator Actually, ExAdmin was based off of Ruby's ActiveAdmin. I started developing it when I was moving a Rails project that used ActiveAdmin to Phoenix. Its very much inspired by ActiveAdmin, right down to a very similar DSL. However, that is going to change with the next version of ExAdmin that I'm currently flushing out the new architecture. Will be a much better product, but it won't resemble ActiveAdmin anymore.

It will be much easier to use since you won't have to learn a new DSL language :)

newterminator commented 7 years ago

@smpallen99 I see the resemblance to ActiveAdmin and now as you mention that you are working on making it better and even easier to use, I am very much looking forward to the new version. Thanks again Steve for increasing productivity for new and old Elixir/Phoenix users with these 2 awesome packages of yours.