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

Datetime serialization differs between params_for/2 and string_params_for/2 #362

Closed
christianjgreen opened this issue Aug 4, 2019 · 6 comments · Fixed by #420
Closed

Comments

@christianjgreen
Copy link

christianjgreen commented Aug 4, 2019

Given a factory that produces a struct Foo with a UTC datetime field published_at, the follow is produced.

params_for(:foo)
-> %Foo{published_at: ~U[2019-08-04 17:25:42.638664Z]}

string_params_for(:foo)
-> %Foo{"published_at" => %{ "calendar" => Calendar.ISO, "day" => 4, "hour" => 17, "microsecond" => {657863, 6}, "minute" => 25, "month" => 8, "second" => 42, "std_offset" => 0, "time_zone" => "Etc/UTC", "utc_offset" => 0, "year" => 2019, "zone_abbr" => "UTC" }}

I was assuming this function would return the same formatted datetime; is this correct?

@elieteyssedou
Copy link

Same constatation for us at work... Something like this could lead us to give up on using ex_machina.

It would be great if string_params_for could return iso_8601 dates, don't you think ?

@germsvel
Copy link
Collaborator

germsvel commented Sep 4, 2019

@christianjgreen thanks for opening this issue. I see why that is a bit surprising.

string_params_for/2 turns all the keys of nested structs/maps into strings. Since the ~U sigil is a convenience for creating a %DateTime{} struct, ExMachina just treats it like any other struct.

I originally thought that changing this behavior to match that of params_for/2 would make sense. But after thinking about it some more, I realized that the way it works now is the expected behavior, and making the change would be an unexpected breaking change.

If you consider that string_params_for/2 is used (at least in part) for generating params for testing Phoenix controllers, then it makes sense that we would generate params like Phoenix does in forms.

I just took a look at how Phoenix's form helper datetime_select sends params to a controller, and I see them coming through as a map like this,

  "born_at" => %{
    "day" => "3",
    "hour" => "8",
    "minute" => "3",
    "month" => "2",
    "second" => "2",
    "year" => "2018"
  },

So I think being able to test the controller like this is a valid use case,

test "sets user's birth datetime", %{conn: conn} do
  user = string_params_for(:user, born_at: ~D[2018-11-15 11:00:00Z])

  conn |> post(path_to_user(conn, :create, %{"user" => user})

  # some assertion

I tend to use string_params_for/2 to test controllers because we work with Phoenix a lot, but that doesn't mean everyone does. I am curious, what is your use case for string_params_for/2?

@mojidabckuu
Copy link

@germsvel Yes, the use case is valid, but when it comes to build an api doc basing on tests the devs who use this api start sending "as it is" or complaining about bad api.
Probably there could be a setting to specify scalar types aka use the ISO standard or pass the custom transform/fn doing struct->string encoding

@germsvel
Copy link
Collaborator

germsvel commented Nov 11, 2020

Just revisiting this. I think if we wanted to change how string_params_for handles dates, we could do that. I see how the current behavior is surprising. It would be a breaking change, so I'd like to ship it along with other breaking changes (if there are any), but if someone is interested in doing this, I'd be happy to review a PR for it.

@cschilbe
Copy link
Contributor

@germsvel take a look at my PR - non breaking change with config option.

@robsonpeixoto
Copy link

A simple hack is encode and decode to json:

def json_decoded_for(factory_name, attrs \\ %{}) do
  factory_name
  |> params_for(attrs)
  |> json_decoded()
end

def json_decoded_with_assocs(factory_name, attrs \\ %{}) do
  factory_name
  |> params_with_assocs(attrs)
  |> json_decoded()
end

defp json_decoded(map) do
  map
  |> Jason.encode!
  |> Jason.decode!
end

brian-penguin pushed a commit that referenced this issue Oct 26, 2022
…t a breaking change yet (#420)

Resolves #362 #362

This PR introduces a configuration option to preserve dates when using the string_params_for function, making it a non breaking change for now.

Add the following to your config/test.exs file to enable this:
`config :ex_machina, preserve_dates: true`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants