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

click_link wipes current user in assigns #38

Closed
drumusician opened this issue Feb 29, 2020 · 9 comments
Closed

click_link wipes current user in assigns #38

drumusician opened this issue Feb 29, 2020 · 9 comments

Comments

@drumusician
Copy link

Not sure if I'm doing something wrong or that this is a bug, but I am passing an authenticated conn struct into a test pipeline (conn.assigns.current_user is set). This works fine on the first assertion, but when I try to continue the path to the next page it wipes the current_user...

Here is the test:

test "Basic page flow", %{conn: conn, student: student} do
    conn
    |> get(Routes.student_path(conn, :index))
    |> follow_link(student.first_name)
    |> IO.inspect # => at this point conn.assigns.current_user is nil 
    |> assert_response(status: 200, path: Routes.student_path(conn, :show, student))
  end

I have tracked it down to here:

request_path(conn, href, opts.method)

Basically that return does not keep the current_user intact. I am not sure what is happening, but it seems it might have something to do with the macro implementation in ConnTest.

Hope you are able to understand and track it down. If you have pointers for me on how to fix it, I'd be happy to contribute!

Thanks!

Tjaco

@boydm
Copy link
Owner

boydm commented Mar 1, 2020

Don't have a solution, but the issue is has something to do with Phoenix.ConnTest.recycle, which is building a new conn without copying in the old assigns. Confirmed this doesn't work in 0.7.0, and I could swear it used to. (obvs missing a test...)

I'll dig around and see what I find.

@drumusician
Copy link
Author

Ok, yes that was the thing I had been looking at as well. I'll try to dig a bit myself too to see if I can find a solution (or why it could have worked before).

At least good to know that you can reproduce it 👍

@boydm
Copy link
Owner

boydm commented Mar 1, 2020

It's just been a while since I was in this part of the code. Need to remember the expected behavior correctly...

@boydm
Copy link
Owner

boydm commented Mar 2, 2020

OK. Took me a bit to remember how it all works. This is one those things that once you (meaning I...) get it set up, you tend to not mess with it again.

The behavior you are seeing is actually OK and expected. The assigns field of a conn should get erased and rebuilt by the plug pipeline in your app with ever request. This is part of the benefit of the phoenix_integration approach - it exercises the full plug pipe with every request.

In other words, treat each follow, or click or whatever as a new request going to your server. The assigns are wiped and rebuilt each time. The reason you reuse the conn between call is to retain the cookies, not the assigns.

So the solution is to treat your integration test set up more like you are making calls to the web server. In my working setup, to log in a user, I literally have it go to the sign-in form, then fill in the user information. When I follow that form, the assigns are wiped, but the session the cookies are kept.

Any given call to the server starts with an empty assigns field, but good session cookies. The plug chain then does whatever you normally do on a web request to build the assigns field for that request from the stored cookies.

I have a function test_sign_in_user that I keep in my conn_case.exs file that does this for me, so I can call that at the top of any given integration test. Don't rely on the :assigns field between calls, but the cookies are good.

defmodule ProtoServerWeb.ConnCase do

  use ExUnit.CaseTemplate

  using do
    quote do
      # Import conveniences for testing with connections
      use Phoenix.ConnTest
      alias ProtoServerWeb.Router.Helpers, as: Routes

      # The default endpoint for testing
      @endpoint ProtoServerWeb.Endpoint

      #----------------------------------------------------
      def test_sign_in_user( conn, user ) do
        token = ProtoServer.Account.User.gen_token!(user)
        post( conn, ProtoServerWeb.Router.Helpers.session_path(conn, :create_token, token) )
      end

    end
  end

  setup tags do
    :ok = Ecto.Adapters.SQL.Sandbox.checkout(ProtoServer.Repo)

    unless tags[:async] do
      Ecto.Adapters.SQL.Sandbox.mode(ProtoServer.Repo, {:shared, self()})
    end

    {:ok, conn: Phoenix.ConnTest.build_conn()}
  end
end

By the way, the strategy for that test_sign_in_user function is to generate an encrypted token, which is then presented to an api function. This is for speed and simplicity during tests. I also have integration tests that literally fill out the user sign-in form and log in that way...

@drumusician
Copy link
Author

drumusician commented Mar 2, 2020

awesome! Thanks for the detailed investigation. I actually came to about the same conclusion after looking again at the Phoenix docs for ConnTest

I was actually using the method provided by Pow to set the current_user like this:

setup %{conn: conn} do
  user = %User{email: "test@example.com"}
  conn = Pow.Plug.assign_current_user(conn, user, otp_app: :my_app)

  {:ok, conn: conn}
end

And that of course works fine if you use something like Wallaby to perform a feature test as that creates a session for you to keep this in scope. But indeed here we are using stateless requests, so that makes sense that the assigns are not carried over.

Thanks a lot for the feedback. I'll close this, but will reference it from a post I'm writing :)

btw, this is what my test looks like now. I'm actually just logging in:

test "View student contract listing", %{
    conn: conn,
    student: student,
    contract: contract,
    user: user
  } do
    conn
    |> get(Routes.pow_session_path(conn, :new))
    |> follow_form(%{
      user: %{
        email: user.email,
        password: user.password
      }
    })
    |> get(Routes.student_path(conn, :index))
    |> follow_link(student.first_name)
    |> assert_response(
      status: 200,
      path: Routes.student_path(conn, :show, student),
      html: "Contracts",
      body: contract.start_date |> to_string
    )
end

@boydm
Copy link
Owner

boydm commented Mar 2, 2020

Cool. Post a link to the post here and I'll tweet it out. Or @ me on twitter and I'll retweet it. Or both.

@boydm
Copy link
Owner

boydm commented Mar 2, 2020

If you end up with a bunch of integration tests, you will probably want to wrap the sign-in flow you have above into a function that you can just call from other tests.

For example, I have tests where I alternate between a user and and admin hitting the site. This lets me test things like "user does not have access to thing", then "admin makes change", then "user now has access".

It would be a pain to have user and admin login code replicated all over...

@drumusician
Copy link
Author

yes I agree, for this single example I left it in there for now. But will add it as another sidenote 👍

@drumusician
Copy link
Author

here is the writeup of this: https://realworldphoenix.com/blog/2020-03-03/feature_tests

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

No branches or pull requests

2 participants