Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer registration of admin models until lookup time #483

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

spohlenz
Copy link
Member

@spohlenz spohlenz commented Sep 2, 2024

In order to generate the admin routes, the Trestle reloader must read each admin file when routes are loaded (which is at startup in most cases). However we can defer the lookup and constantization of the admin resource's model until the Trestle::Registry#lookup_model method is called. This fixes issues such as #482 where an error in loading a model can mask the root cause of the exception.

Unfortunately an error in an admin definition will still cause a failure at startup time. I don't know at this point whether that is resolvable (in a way that is desirable, i.e. don't just rescue any exceptions).

This PR also pushes the logic of whether an admin resource should register itself for model lookup out of the Trestle::Registry class and down to the Trestle::Resource itself where it is a better fit.

@spohlenz spohlenz merged commit dfd4362 into main Sep 2, 2024
44 checks passed
@spohlenz spohlenz deleted the defer-admin-model-registration branch September 2, 2024 10:27
@coveralls
Copy link

Coverage Status

coverage: 91.494% (+0.006%) from 91.488%
when pulling 845c3c6 on defer-admin-model-registration
into dfc360a on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants