Problem/Motivation

The flattening approach works fine for simple label fields like credits, etc, but it has caused problems breaking from standard Drupal architecture as this module gets more usage.

Proposed resolution

Would suggest instead we have a loader that renders site settings entities in a standard way, respecting manage display.

It then cannot inject the site settings to every template, so keeping with the goal of the module to add labels very simply to eg footer, header, other regions where site settings are useful, suggest we add a Twig Filter and make that the recommended way. Similar to what Twig Tweak offers but without the dependency on that module perhaps, or it could be a dependency of that loader?

Remaining tasks

  1. Provide annotation for Loader; id + label + whether to autoload or not
  2. Provide new loader with autoload disabled
  3. Provide update hook to maintain status quo for loader
  4. Let site settings config form choose loader
  5. Only auto-load site settings into templates if autoload is enabled on the loader
  6. Remove calls to site_settings.loader service, replace with plugin manager load definition
  7. Move SiteSettingsLoaderBase functionality into FlattenedSiteSettingsLoader
  8. Move SiteSettingsRenderer into FlattenedSiteSettingsLoader
  9. Change /admin/content/site-settings to a View
  10. Update hook to install the View maintaining roughly the status quo for the listing
  11. Add 'Create setting' to View header for not yet created site settings
  12. Add 'Add another' to operations for site settings that allow multiple
  13. Build FullSiteSettingsLoader
  14. Add tests
  15. Update documentation

Follow-up issues

  1. https://www.drupal.org/project/site_settings/issues/3398584
  2. https://www.drupal.org/project/site_settings/issues/3399190

User interface changes

Site settings config form allows choice of loader

Data model changes

Multiple site settings loaders will be available + developers can optionally override/provide their own.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

scott_euser created an issue. See original summary.

scott_euser’s picture

Would welcome feedback on the proposed steps.

scott_euser’s picture

Assigned: Unassigned » scott_euser
Status: Active » Needs work
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Am thinking this probably needs a 8.x-2.x branch in case anyone is extending/overriding services from this module, its probably unavoidable to cause breaks there. For those not extending/overriding services, should not be a problem to have a smooth upgrade path maintaining the status quo.

scott_euser’s picture

Progress is happening in the MR, crossing things off as I get through them.

scott_euser’s picture

Issue summary: View changes

scott_euser’s picture

Adding credit to bobi-mel for type hints from https://www.drupal.org/project/site_settings/issues/3176342 - thanks!

scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Nearly there with this. Need to resolve existing test failures, add new tests to cover new functionality, and update documentation. Then follow-up with this probably https://www.drupal.org/project/site_settings/issues/3398584

Would appreciate feedback on approach and suggestions before we eventually lock in to this direction of travel.

scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Issue summary: View changes
scott_euser’s picture

Okay tests are all passing now, including new tests for twig extension for the full loader.

One warning from phpstan which is ignored in core; support request raised here for help ignoring that in contrib as well: https://www.drupal.org/project/drupal/issues/3399282

Just the documentation updates to be made otherwise this is ready now for 2.0.x branch.

scott_euser’s picture

Assigned: scott_euser » Unassigned
Issue summary: View changes
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.