Fixes and addition in InputMessegeContent and Parser modules

[anronin]

1. Bug fix in Nadia.Model.InputMessegeContent module where optional fields must be string instead of nil

2. Fixes and additions in Nadia.Parser after the new models were introduced

[zhyu]

What's the problem if optional fields of Nadia.Model.InputMessegeContent are nil?

[anronin]

If its nil and not used then you get error message like that:
"Bad request: Field "last_name" must be of type String" from telegram server

[anronin]

this is response from server when it's nil

{:ok, %HTTPoison.Response{body: "{\"ok\":false,\"error_code\":400,\"description\":\"Bad request: Field \\\"last_name\\\" must be of type String\"}", headers: [{"Server", "nginx/1.10.0"}, {"Date", "Tue, 31 May 2016 07:07:08 GMT"}, {"Content-Type", "application/json"}, {"Content-Length", "101"}, {"Connection", "keep-alive"}, {"Access-Control-Allow-Origin", "*"}, {"Access-Control-Expose-Headers", "Content-Length,Content-Type,Date,Server,Connection"}], status_code: 400}}

and params are like that
[inline_query_id: "13947328571332811", results: "[{\"type\":\"article\",\"title\":\"Test\",\"input_message_content\":{\"phone_number\":\"19232114412\",\"last_name\":null,\"first_name\":\"Arbuz\"},\"id\":\"1\"},{\"type\":\"article\",\"title\":\"Test2\",\"input_message_content\":{\"phone_number\":\"19232114412\",\"last_name\":null,\"first_name\":\"Dyna\"},\"id\":\"2\"}]"]

[zhyu]

Well, instead of using empty string for optional field, I prefer to remove nil fields in request params.

[anronin]

I do not quite understand where to do it: in Nadia.answer_inline_query or Nadia.API.build_request ?

[zhyu]

I think this issue relates to this line: https://github.com/zhyu/nadia/blob/master/lib/nadia.ex#L635

Only nil field in the top level has been removed, a nil field in a non-nil field will not been removed.
e.g.,

%{nil_field: nil, non_nil_field: %{nil_field: nil, non_nil_field: "something"}}

will be changed to

%{non_nil_field: %{nil_field: nil, non_nil_field: "something"}}

if we change it to

%{non_nil_field: %{non_nil_field: "something"}}

there won't be any problem.

So I think removing nil fields recursively will be a solution.
And since nil fields are very common, I think it's better to do it in Nadia.API.build_request.

[anronin]

not sure that is right solution)
EDIT: just a moment, this is not right solution
EDIT2: still not sure that is right solution, but at least it works. i'm think so

[anronin]

wdyt?

[zhyu]

Why not just use a map?

defp drop_nil_fields(value) when is_list(value), do: Enum.map(value, drop_nil_fields)

[anronin]

The result should look something like this?

defp drop_nil_fields(params) when is_list(params), do: Enum.map(params, &drop_nil_fields/1)
defp drop_nil_fields(params) when is_map(params) do
    params
    |> Map.from_struct
    |> Enum.filter(fn {_, v} -> v != nil end)
    |> Enum.into(%{})
    |> Poison.encode!
  end
defp drop_nil_fields(params), do: to_string(params)

EDIT: no, this is not working

[zhyu]

in drop_nil_fields for a map,
you should call drop_nil_fields on values of the map, not just return the non-nil value.

drop_nil_fields should remove any nil fields recursively,
it means you should call recursively on every item of a list, and every value of a map.

EDIT:
so just add |> Enum.map(&drop_nil_fields/1) after this line: |> Enum.filter(fn {_, v} -> v != nil end),
or you can use filter_map

[anronin]

so like this? test go fine, but i have a problem with this

defp drop_nil_fields(params) when is_list(params), do: Enum.map(params, &drop_nil_fields/1)
defp drop_nil_fields(params) when is_map(params) do
    params
    |> Map.from_struct
    |> Enum.filter_map(fn {_, v} -> v end, fn {k, v} -> {k, drop_nil_fields(v)} end)
    |> Enum.into(%{})
    |> Poison.encode!
  end
defp drop_nil_fields(params), do: to_string(params)

[anronin]

if i get it right |> Poison.encode! must be in drop_nil_fields for list

[anronin]

how about that?

[anronin]

only problem is that reply_markup: nil not removed

[anronin]

i could add |> Keyword.delete(:reply_markup, nil) after |> Keyword.update(:reply_markup, nil, &(Poison.encode!(&1))) in build_request of course

[anronin]

any thoughts?

[zhyu]

Sorry for keeping you waiting 😿
However, this p-r is outdated. I may check and merge it when conflicts are resolved.

[anronin]

np, i'm glad you are back :) will look at this asap

[zhyu]

Thanks for your reply, please take your time 😺
After resolving conflicts, please use the formatter to format your code too, you can find details in #60.
I'll update .travis.yml to check it 🔜