search

mathieuprog / polymorphic_embed

Polymorphic embeds in Ecto
Apache License 2.0
340 stars 62 forks source link

Add :by_module option to :types to allow passing in the embedded_struct in the type_field. #65

Open Ivor opened 2 years ago

Ivor commented 2 years ago

This PR is based on the idea and work from https://github.com/pzingg/polymorphic_embed/tree/type-module-fun referenced in https://github.com/mathieuprog/polymorphic_embed/pull/18

To make for the smallest change, I have only added the :by_module option. Please let me know if I can make this better or if I have missed anything. The tests that I added pass, but I am not sure whether I might be overlooking something.

mathieuprog commented 2 years ago

Wow. I need some time though, can you depend on your branch for now?

Can you explain your use case please?

Ivor commented 2 years ago

Hi, the use case we have is a centralised way of capturing notes. This is used by different dashboards for different industries. Each industry has different extra fields to capture. So different contexts want to capture different nested structures. The Notes don't know about the different dashboards so I can't pass the keyword list.

I can use my fork and see how it pans out. If there are any specific short comings please let me know and I can learn how to fix those.

mathieuprog commented 2 years ago

Did you try it on your project already? Does it cover all of your use case that you described? If you don't know yet, when will you be able to share some feedback?

Ivor commented 2 years ago

I haven't tried it. Just added the tests here. We use the current version with hard-coded types. I can implement it and provide feedback by Wednesday next week. Maybe sooner if I have something to report. Thanks for the quick response!

Ivor commented 2 years ago

@mathieuprog I added tests for most of the remaining cases. In doing the tests I realise my previous attempt did not quite have everything that is needed.

I tried in our project but we're using Surface and at this point it feels like a bridge too far getting the changesets to work properly with Surface and Phoenix forms. One problem is that when inputs_for needs to get the fields from the polymorphic embed, with the :by_module option the information is simply not there.

I do not have the time to dive deep enough into the forms to get that working. In our case we can build the form without needing inputs_for since the parent record is mostly a container for some metadata around our notes.

I will leave this here in case someone else has the opportunity to delve into the changesets and forms and inputs.