userfrosting / UserFrosting

Modern PHP user login and management framework
https://www.userfrosting.com
Other
1.64k stars 366 forks source link

Remove index named route dependency in Admin / AdminLTE sprinkle #1244

Closed lcharette closed 9 months ago

lcharette commented 9 months ago

AdminLTE and Admin Sprinkle's both reference the index routes in their Twig templates, but none of them actually implements it. When testing both sprinkle on their own, no error is shown because the "develop" test sprinkle bundled with those DO define the route.

The skeleton also defines the index route. However, if a dev removes it from their project, instead of a 404 page, they get a fatal error.

This is because AdminLTE defines the urlFor('index') in app/templates/pages/error/http.html.twig. Remove it and you get the default 404 page. However, others pages of AdminLTE (eg. app/templates/pages/register.html.twig) also defines it, and will throw an error.

The solution would be for those particular routes to define a fallback route.

The difficulty here is

  1. Routes can't easily be replace in 5.0 (so it's not a simple matter of defining the index in core)
  2. Adequate fallback needs to be done when a route name is not found, while preserving the ability to throw an error when needed for actual coding mistake
  3. It's not a build-in function of Slim's
  4. Twig function is only a passthrough / TwigFunction referenced in core app/src/Twig/Extensions/RoutesExtension.php

The ideal solution here would be to create our own pathFor method, wrapped around slim's, and with the ability to define a fallback route (string) when an exception is thrown.

lcharette commented 9 months ago

Fixed by :