ex_machina icon indicating copy to clipboard operation
ex_machina copied to clipboard

values are not strings in string_params_for

Open tielur opened this issue 6 years ago • 7 comments

I may be making a bad assumption but I would assume that id's would be strings when called with string_params_for. I have controller tests that I'm testing that pass the data through as integers. Which causes my tests to pass but in reality when you submit a POST/CREATE request via phoenix on a form it comes through as a string. So if you do anything in the controller action that assumes one the params is an integer it will pass the test but actually fail in execution.

I can create an example if this doesn't make sense.

As an example, I would assume that this test would return the ratingas "5" and "4" when using string_params_for?

tielur avatar Jul 17 '18 16:07 tielur

Okay I believe I understand more of what's going on now. string_params_for only converts the keys to strings. It doesn't convert the values. I'm wondering it it makes sense to have a controller_params_for or something that converts both the keys and values to strings. I ran into an issue when testing a create route on a phoenix application.

My action looks like this:

def create(conn, %{"patient" => %{"account_id" => account_id} = patient_params}) do
  account = Enum.find(conn.assigns.accounts, &(&1.id == account_id))
  # more controller stuff
end

When using my application, because browsers are using HTTP any time I submit a form the account_id value will always be a string. It doesn't seem to be the case when I'm testing though.

My controller test looks like this:

test "create redirects to show when data is valid", %{conn: conn} do
  [account] = conn.assigns.current_user.accounts
  params = params_for(:patient, account: account)
  conn = post(conn, patient_path(conn, :create), patient: params)

In this case, the params looks like this:

%{
  account_id: 2072,
  #other stuff
}

So back in the controller, it is also an integer and the above create controller code works. But during run time because the browser is submitting the request, it will always be a string and this will fail. So we have a situation in which the tests pass but the actual logic is wrong.

tielur avatar Jul 17 '18 19:07 tielur

Moving forward if I wanted to fix the actual implementation I would change the controller to something like this:

account = Enum.find(conn.assigns.accounts, &(to_string(&1.id) == account_id))

This would work as implemented and my application would be fixed because I am now comparing a two strings. My test would also correctly fail. But now I need an elegant way to make sure when I'm testing a phoenix controller that my values, specifically in this case my account_id is a string.

I could hack something together quick to make it work like so:

params = params_for(:patient, account: account)
params = %{params | account_id: "#{account.id}"}

But I'm wondering if it would make sense for something in ExMachina to help with this?

tielur avatar Jul 17 '18 19:07 tielur

Hi @tielur, thanks for submitting this issue.

I just tested this in an internal elixir app we use, and I'm seeing the id come in as an integer, not as a string.

I may just be missing something, but could you confirm you're still seeing that behavior? And if so, do you have a way to reproduce the error?

germsvel avatar Oct 19 '18 16:10 germsvel

@germsvel that was the issue, I expected it to be a string not an integer.

tielur avatar Oct 23 '18 21:10 tielur

@tielur sorry, I think my last message was confusing. Phoenix seems to pass them as integers (that's what I was seeing in the internal app), so I was thinking ex_machina's implementation was okay. I'm wondering how you're getting string ids.

Could you copy your form's code here or is there a sample app that you could point me to so I can reproduce the error?

germsvel avatar Oct 24 '18 10:10 germsvel

@germsvel sorry for the late reply, I'll throw together a sample app and put a link to it here

tielur avatar Nov 01 '18 16:11 tielur

Hi @germsvel, I can confirm that phoenix sends form param values as strings by default, and that this behaviour would be much preferred in string_params_for

Put together a very quick demo using our reference app: https://github.com/civilcode/acme-ex/tree/chore/reproduce-form-string-values

Steps to reproduce:

  • start the app
  • go to: http://localhost:4000/nesting/1/reproduce_form_string_params/new
  • submit the form
  • look at the server console to see the params -- the values are all strings

Looking at your example (Constable), I'm wondering if you were seeing an integer ID because of this plug https://github.com/thoughtbot/constable/blob/66f964dcb9eac5abe2f7080362c22d5b3f496a12/lib/constable/plugs/deslugifier.ex#L20

bgracie avatar Apr 08 '19 20:04 bgracie