absinthe-graphql / absinthe_plug

Plug support for Absinthe, the GraphQL toolkit for Elixir
https://hex.pm/packages/absinthe_plug
MIT License
261 stars 163 forks source link

Prevent config updates from overwriting map values #248

Closed ejscunha closed 3 years ago

ejscunha commented 3 years ago

Map values coming from init_options were overwriting the previous value instead of merging both maps.

This was removing the pubsub key from the config context map, which made subscription's triggers from mutations stop working.

I have no idea on how to test this fix.

benwilson512 commented 3 years ago

A test would really help me understand what the issue is, can you at least supply some examples from your own code and then I can try to write a test?

binaryseed commented 3 years ago

Will it also work to just swap the order of the update_config so that the pubsub one goes last?

benwilson512 commented 3 years ago

Oh I see, the issue is that if you set context: %{foo: :bar}, it nukes the context pubsub value that is set. @binaryseed I like your solution.

eduardo-cunha-bose commented 3 years ago

A test would really help me understand what the issue is, can you at least supply some examples from your own code and then I can try to write a test?

I will try to setup a test tomorrow for this.

Will it also work to just swap the order of the update_config so that the pubsub one goes last?

If we change the order we lose the added context information.

eduardo-cunha-bose commented 3 years ago

Looking into the issues, #247 is what I'm observing and this PR should fix it (it did on my project).

binaryseed commented 3 years ago

Here's a fix that works by re-ordering functions in update_config https://github.com/absinthe-graphql/absinthe_plug/pull/249 & a test for it

Thanks for the PR anyway!

eduardo-cunha-bose commented 3 years ago

Oh that makes sense, I didn't understand at first. Thank you for taking caring of that quickly. 🙏