Add cache dependency on module settings

Created on 6 April 2023, over 1 year ago
Updated 25 May 2023, over 1 year ago

Problem/Motivation

The AddToAny share block provided by this module currently required a cache clear to reflect any changes to the default config in addtoany.settings.

Steps to reproduce

Configure the settings, add a block to a page, then change the settings. You won't see changes (e.g. change in button size) until after a cache clear.

Proposed resolution

Add a cache tag to the block render array so it depends on config:addtoany.settings.

Remaining tasks

Review?

User interface changes

None

API changes

None

Data model changes

None

πŸ› Bug report
Status

Postponed: needs info

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States xaqrox Washington, D.C.

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @xaqrox
  • @xaqrox opened merge request.
  • πŸ‡ΊπŸ‡ΈUnited States xaqrox Washington, D.C.
  • Status changed to Needs review over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States xaqrox Washington, D.C.
  • πŸ‡ΊπŸ‡ΈUnited States xaqrox Washington, D.C.

    A couple notes on this and other topics β€”

    1. This dependency is probably needed in more places, but I'm not sure what is the most Drupaly way to add a cache dependency to a theme hook everywhere. template_preprocess_HOOK()? That would make the merge request obsolete.
    2. In looking thru this, I started to feel like addtoany_create_data() and addtoany_create_entity_data() should be moved into a service and decomposed into functions for each of the variables. Don't have a *great* reason for that, just a code smell instinct.
    3. If there was a template_preprocess_HOOK() implementation, that is where the defaults from the settings could be filled in. For example, the extra field would likely always provide URL and title, but if the user doesn't specify icon size, then it could get filled in from the default config. The block could provide icon size and custom_html, and if URL/title was not provided, they could get filled in from the current route in the hook. In the process, if there is a need to hit the default config, that is when the cache dependency could be added. (It may also be that there are settings that will always be needed to build the markup, in which case the dependency should be added in every case.)
  • Issue was unassigned.
  • Status changed to Postponed: needs info over 1 year ago
  • πŸ‡¦πŸ‡ΊAustralia VladimirAus Brisbane, Australia

    Checking if committed πŸ› Block Display is random for anonymous Fixed resolved the issue?

  • πŸ‡ΊπŸ‡ΈUnited States xaqrox Washington, D.C.

    I tested this on simplytest.me: I added the share block to the block layout, made a change to the global config. Reloaded the page, my change was not reflected. Cleared the cache and reloaded, change was reflected. I did not expect the code added in https://www.drupal.org/project/addtoany/issues/3212302 πŸ› Block Display is random for anonymous Fixed to fix this, because it doesn't account for the dependency on the global config.

Production build 0.71.5 2024