Add support for nested changeset error messages

[simonprev]

This adds basic support for errors in nested changesets. I’m open to change the formatting for nested fields, currently it’s: association.field but it could also be associationField or something else. The dot notation fitted our use case.

Results in a GraphQL payload

Client belongs to tutor with validation for the tutor association

%{
  data: %{
    "updateClient" => %{
      "messages" => [
        %{"code" => "required", "field" => "firstname"},
        %{"code" => "required", "field" => "tutor.email"},
        %{"code" => "required", "field" => "tutor.firstname"},
        %{"code" => "required", "field" => "tutor.lastname"},
      ]
    }
  }
}

I also fixed a warning and a failing test in test/test_helper_test.exs

[lauraannwilliams]

This is a good start - I want to make sure that when there are many children we support that too.

I'm thinking of two different paths -

1. Number each nested field, like tutors.0.email tutors.1.email and so forth. Will need to be sure the order in the mutation matches the order in the errors

2. Pass in an errorId field that is then used to generate the error keys when there are nested errors. In this case, behaviour as specified in point one could be a default behaviour.

[simonprev]

@lauraannwilliams I just pushed changes to allow custom field constructor in simple validation message but nested one too. Kronky provides a "default" one that matches the parent.0.child format. You can see the test in test/changeset_parser_custom_field_constructor_test.exs

[kapilpipaliya]

When will this available? I require it. Thanks

[Eiji7]

@simonprev: Both original and your code have a small bug.

Please replace:

String.replace(acc, "%{#{key}}", to_string(value))

to:

key_pattern = "%{#{key}}"

if String.contains?(acc, key_pattern) do
  String.replace(acc, key_pattern, to_string(value))
else
  acc
end

Example values to reproduce problem:

message = "is invalid"
opts = [validation: :embed, type: {:array, :map}]

Found in:

https://github.com/mirego/kronky/blob/0076cde1cac3dc9ab465afec743540f6b08d3e7c/lib/kronky/changeset_parser.ex#L103

[simonprev]

@Eiji7 Done! I’ve made the requested changes 😄

[Eiji7]

@simonprev Also I forgot to mention that you should set app env (default library configuration), because it fails without any configuration extra. I have tested it and I think that's ready to deploy, but unfortunately I do not have a permission to merge it.

[simonprev]

@Eiji7 What do you mean by set app env (default library configuration)?

I added the config in config/config.exs:

config :kronky,
  ecto_repos: [],
  field_constructor: Kronky.FieldConstructor

[Eiji7]

@simonprev: Please take a look at stackoverflow question.

[simonprev]

@Eiji7 Is my latest commit enough as the default library config?

[Eiji7]

@simonprev: Yes, now all good! Good job!

[ssbb]

Is there any updates on this?

[Eiji7]

@ssbb Looks like owner does not made any update for months. I think that @simonprev's version works without any problem, so you should use his fork version instead.

[Eiji7]

@simonprev After updating Elixir new warning come:

warning: found quoted keyword "coveralls" but the quotes are not required. Note that keywords are always atoms, even when quoted, and quotes should only be used to introduce keywords with foreign characters in them

It looks pretty simple, so can you update it?

[quolpr]

@lauraannwilliams could you please merge it? 🙏

[d-webs]

Any updates on this? Will it get merged anytime soon?

[Eiji7]

@dwebster17 It's almost 6 months after it's 100% ok for me. I don't see any arguments to not merge it especially that's not conflicting with original repository. Unfortunately 6 months should say enough … looks like this repository is dead.

Fortunately it does not mean that this project is dead. You can still use code from this PR in any Elixir project (just use github option when defining dependency in mix.exs). Maybe @simonprev would release this as hex package under his own account (so it would have same name, but different author).

The other way is to get in touch with author and change permissions or even change owner for this repository. I could take it and merge PR then, but looks like that its owner is not replying here, so we would need to find other communication way with him.

[simonprev]

Hey @lauraannwilliams, would it be OK if I officially fork this repo and publish a new package to hex.pm (I would rename it). You can see from this PR that the fork is already used by some people 😄 The fork would live in the Mirego organization on GitHub.

If I have not heard from you by the end of the month, I will assume that this repo is unmaintained and I will proceed with the fork. If you don’t want me to publish your code on hex.pm that’s ok too 😄 Thank you for this library!

[Eiji7]

@simonprev I don't think you need even ask about permissions. Just take a look at LICENSE file - it's MIT license. As long as you use somebody work in way which license allows then you should not have any law problems.

Also there is no need to rename it as you can use same name: https://hex.pm/docs/private

[simonprev]

For everyone interested in switching their Kronky dependency to our fork: https://github.com/mirego/absinthe-error-payload

It supports nested changesets, uses mix format to simplify collaboration and is used in many projects already. Bonus: We run tests in travi to prevent broken releases and code coverage to maintain quality 😄 And the new name fits nicely in your mix.exs below absinthe, absinthe_plug, absinthe_phoenix, etc

Thank you again Ethelo for building Kronky!

[lauraannwilliams]

Hi Simon - I havn't had any time to devote to this, so the fork is definitly a good idea. :) Thanks for taking this on.