Default values?

[lasseebert]

Thanks for a great library. I'm just learning Elixir, so I might miss something obvious here.

Is it possible to somehow specify default values in a schema?

I have something like this:

defparams index_params %{
  filter: %{
    foo_id: :integer,
    bar_id: :integer
  },
  page: %{
    number: :integer,
    size: :integer
  },
  sort: :string
}

filter is not required, but it should default to %{}.
page is not required, but it should default to e.g. %{number: 1, size: 20}.
sort is not required, and should default to e.g. "id"

I could of course write a function that adds the default values, but since Ecto.Schema supports this it could be nice to somehow specify it directly in the schema.

[vic]

Hey, just added support for actually passing any Ecto.Schema option, you need to give a keyword list instead of just a symbol. So for example, the page field would be:

page: %{
   number: [field: :integer, default: 1],
   size: [field: :integer, default: 20]
}

Note that if an embedded schema like page has any field with default value set, the struct will be created even if no data was present for it on request-params. However if the embed schema has no field with default, likefilterin your example, then it will just be nil, as ecto doesn't allow to set default values for embed schemas. You can work around this by having a dummy field with a default value set inside filter, just to force the whole embed schema to be generated.

I've already fixed this in master and the ecto-2 branch. I'm unable to publish the new version to hex.pm but will try later today. Hope this fixes this issue.

[vic]

Just for reference, the tests are here

[lasseebert]

Wow, that was fast, thanks.
It seems like exactly what I need. I will test it later on my own project and provide some feedback.

[lasseebert]

Hi @vic

Again thanks for the new feature.
I've had a chance to test it in my own project and the default values works as expected.

However, I have build some of my app relying on maps and not structs, so I used Params.changes instead of Params.data. And changes returns a map of only the changes, which does not include default values.
This is technically correct, but somehow useless in the context of this package.

I tried to change my app to use structs instead, but it revealed another problem: Structs returned by data contains id: nil, which does not make sense for this kind of data. This was a problem for me where I recusively pattern match on the key-value pairs of a nested schema.

Do you agree that:

• changes should return a map of all values including default values (maybe rename the function?)
• Structs from data should not have an id (could be accomplished by setting @primary_key false on each schema)

I will be happy to help with the implementation.

[vic]

Hey @lasseebert

Yeah, both things you mentions seem right to me.

changes should return the same content as data except that it should be just Maps. perhaps we should use data and then Map.from_struct: the struct constructor ensures default values get populated so then we just need to convert that to nested Maps. And yeah perhaps changes needs to be renamed to something else, any suggestion?

As for the structs from data, yes they should not include id unless it's explicitly declared on the schema by user.

So glad you want to help with the implementation :), give it a go. Tell me if you need some help with (my shameful) code.

Cheers.

[lasseebert]

It seems that we do need a @primary_key (id) if we want to have relations like embeds_one, so we can't just use @primary_key false. (ref: elixir-ecto/ecto#420 (comment))

AFAICS, the primary key is set when the adapter saves the schema to a datastore, which I think will not happen in the use case of this package.

Would it be ok with you that

• The primary key is renamed from id to _id
• changes return a Map of all attributes from data except _id

[vic]

I'm ok with both, go ahead with it, thank you very much :) Btw, should changes be named something different ? perhaps to_map. Initially I named it changes because it was extracting the values that changed in the changeset, but I suppose a better name would be nice. What you think?

[lasseebert]

I think to_map is a good name.

On Tue, Jun 14, 2016, 03:35 Victor Hugo Borja notifications@github.com
wrote:

I'm ok with both, go ahead with it, thank you very much :) Btw, should
changes be named something different ? perhaps to_map. Initially I named
it changes because it was extracting the values that changed in the
changeset, but I suppose a better name would be nice. What you think?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAsb6gijvDVouBhLU-z-jFqrqAphLjl3ks5qLgV9gaJpZM4IuD-w
.

[vic]

@lasseebert merged your PR #5.

Looks like ecto-2 will be released next week, when that happens I'm going to make the 2.x branch become master so I guess it's fine if we don't backport your changes to the ecto-1 branch.

Thanks for your contribution!

[lasseebert]

Thanks for you guidance :)