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

string_params_for does not stringify keys in associations #222

Closed
zwippie opened this issue Jun 1, 2017 · 4 comments
Closed

string_params_for does not stringify keys in associations #222

zwippie opened this issue Jun 1, 2017 · 4 comments

Comments

@zwippie
Copy link

zwippie commented Jun 1, 2017

  • elixir 1.4.1
  • phoenix 1.3.0-rc.2
  • ecto 2.1.4
  • ex_machina 2.0.0

Given a Phoenix + Ecto app with the following factory:

def album_factory do
  %Album {
    name: sequence(:name, &"Album #{&1}"),
    items: build_list(5, :item)
  }
end

def item_factory do
  %Item {
    name: sequence(:name, &"item-#{&1}")
  }
end

The produced output of string_params_for(:album) is:

iex(13)> string_params_for(:album)
%{"items" => [%{name: "item-49"}, %{name: "item-50"}, %{name: "item-51"},
   %{name: "item-52"}, %{name: "item-53"}], "name" => "Album 48"}

The expected output should have the name key of every associated item as a string, not an atom:

%{"items" => [%{"name": "item-49"}, %{"name": "item-50"}, %{"name": "item-51"},
   %{"name": "item-52"}, %{"name": "item-53"}], "name" => "Album 48"}

Which is understandable because of the call to build_list in album_factory, but it's confusing none the less.

@zwippie
Copy link
Author

zwippie commented Jun 1, 2017

I just realized that the usage of stringified keys in associations within Phoenix controller tests (the apparent use case for the string_params functions) is quite weak; ecto does not accept associations as flexible as for example ActiveRecord does.

So this might just be a non-issue, but it's a little confusing none the less.

@germsvel
Copy link
Collaborator

germsvel commented Jun 9, 2017

@zwippie thanks for reporting this. Could you take a look at #224 and see if it solves the issue?

@germsvel
Copy link
Collaborator

I believe #224 closes this issue. If not, please re-open. Thank you for reporting this!

@zwippie
Copy link
Author

zwippie commented Jun 23, 2017

Seems to work perfect, thanks for fixing this!

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