search

aesmail / kaffy

Powerfully simple admin package for phoenix applications
https://kaffy.fly.dev/admin/
MIT License
1.34k stars 155 forks source link

Unexpected error `Enumerable not implemented for nil of type Atom` #286

Open thiagomajesk opened 1 year ago

thiagomajesk commented 1 year ago

Versions Used

Kaffy: master Phoenix: 1.7 Elixir: 1.15.4

What's actually happening?

If you don't use the auto-discovery feature, that is, if you pass a function or a list to the resources config and forget to add one of the relationships to the resources list as well, you get an unexpected error while going to the edit page of a resource.

For instance, if I have a schema called Foo with a belongs_to :bar, Bar relationship and I only add Foo and not Bar to the resources configuration it raises an exception: protocol Enumerable not implemented for nil of type Atom when I edit it.

What should happen instead?

So, I understand that Kaffy needs all the relationships configured in the resources key to properly build the pages, but this is not immediately clear what it expects when you stumble upon this error... The error happens mostly because get_context_for_schema can return nil and get_schema_key expects a list, but IMHO it should actually raise an exception (fail fast) with an informative error message telling the user the schema was not found in the configuration.

PS.: Another thing that I wanted to ask is: If Kaffy needs a graph of all dependencies mapped as a resource, could we expose the auto setup function as a public API that can be used by users? I was omitting resources from the config to not show them in the menu, but it seems the correct way of doing that is setting the ìn_menu` flag for the resource. If Kaffy needs to know all the resources beforehand, It would help a lot if I could just iterate over the auto-discoved resources and just filter out what I don't want from the menu.

aesmail commented 1 year ago

Thanks for reporting this issue @thiagomajesk

I agree that Kaffy should provide a much better error message when there's a missing resource.

Kaffy master has recently added Kaffy.Utils.auto_detect_resources/0 which returns the list of auto-detected resources (Kaffy's attempt at discovering the list of schema/admin modules).

v0.10.0 final will probably be released by the end of this month.

thiagomajesk commented 1 year ago

Hi @aesmail, thank you for this awesome library.

Kaffy master has recently added Kaffy.Utils.auto_detect_resources/0 which returns the list of auto-detected resources (Kaffy's attempt at discovering the list of schema/admin modules).

I'm aware of this function... My problem with using it though is relying on a non-public API 😅 - @moduledoc false usually means it's an implementation detail that could change. Are you interested in perhaps moving this function and exposing it in another module as a user-facing API?

aesmail commented 1 year ago

@thiagomajesk you're right actually. I added this function specifically so users can use it instead of the private one that Kaffy uses internally. Feel free (and safe?) to use it since it should not change or go away. The original idea was that Kaffy.Utils should have been a public module, but it became a "dumpster" of internal utility functions.

I just got back to maintaining Kaffy again and the code needs to be cleaned up and re-organized 🙂 Hopefully things will get much better in the coming weeks 🙏🏻