Drop fields that has nil values

[Fs02]

This will drop all key that contains nil values which remain untouched previously.
This also fixes error when using ex_machina and arc_ecto on testing. which is caused by cast_attachments/3 on arc_ecto doesn't expect the value of the fields to be nil.

[DavidVII]

I like this 👍

I feel that using Enum.reject would make the intention a bit clearer. What do you think?

[Fs02]

Yeah, I agree on that. I will update it 😃

[paulcsmith]

Instead of adding this to drop_ecto_fields what do you think of adding it as the last pipe in params_for and params_with_assocs? I think that's more clear than adding it to drop_ecto_fields

[paulcsmith]

@fladson This looks really awesome. Just a couple small comments and then I'll merge this in :D

[fladson]

oops, I think you meant @Fs02 😁

[paulcsmith]

@fladson Oops! I need to be more careful about using GitHub autocomplete :) Thanks!

[Fs02]

I've updated the PR, this is what you meant right?

and I'm wondering whether we should rename drop_nil_values/1 to drop_nil_fields or maybe drop_empty_fields?

[paulcsmith]

How about "params_for/2 removes fields with nil values"?

[paulcsmith]

@Fs02 Yeah that's a good idea to clarify that function name. I left a comment with a suggestion. Once that's addressed I'll go ahead and merge this in!

[Fs02]

Updated, thanks!!

[paulcsmith]

Looks awesome! Thanks @Fs02

[barberj]

New to elixir, but this PR breaks how I was using ex_machina. Curious what the expected approach should be.

I'm setting some valid attrs for testing on a module attribute

@valid_attrs FieldGuide.Factory.params_for(:person)

And then later on in a test I mutate just what I'm testing and add the required things that the factory didn't provide.

changeset = Person.changeset(%Person{}, %{@valid_attrs | deleted: true, user_id: user.id})

Now that user_id is stripped i can't sub it in for my test. Is there a better practice I should be following?

Thanks

[paulcsmith]

@barberj I just took a quick looks, but instead of using the update syntax, I think you could use Map.merge

changeset = Person.changeset(%Person{}, Map.merge(@valid_attrs, %{deleted: true, user_id: user.id}))

Alternatively you could use a function instead of an attribute

changeset = Person.changeset(%Person{}, valid_attrs(deleted: true, user_id: user.id))

def valid_attrs(overrides \\ []) do
  FieldGuide.Factory.params_for(:person, Map.new(overrides))
end

Final option would be to use params_for(:person) directly. Then you can do params_for(:person, deleted: true, user_id: user.id)

Does that help?

[barberj]

It does, thanks for the help and the awesome elixir libraries

[paulcsmith]

Awesome, I'm glad that helped. Thanks for the kind words!