Skip to content

Conversation

matt-bernhardt
Copy link
Member

@matt-bernhardt matt-bernhardt commented Apr 30, 2025

Why are these changes being introduced:

There are several paths which have proven problematic under our current caching setup (where content lasts for a week before re-rendering)

  1. The hours grid, which accepts a URL parameter to highligh specific days.
  2. The news site's events page
  3. The exhibit site's homepage
  4. The exhibit site's event index, which contains lists of current, future, and past exhibits.

Each of these pages doesn't work well when cached for more than a day, because content can get out of date after 24 hours.

Relevant ticket(s):

https://mitlibraries.atlassian.net/browse/web-2050
https://mitlibraries.atlassian.net/browse/web-2052

How does this address that need:

This adds a function to the parent theme which implements the add_filter command provided by WordPress. This allows us to define a conditional that assigns the cache length according to defined logic - logic which can be pretty flexible in the checks it performs. The default response from this function is the existing cache length of one week.

Document any side effects to this change:

  • CodeClimate is flagging this function for complexity. An alternative structure might be to refactor this to use a switch statement, which I'm willing to do if something like this pseudocode seems clearer:
function override_cache_default_max_age() {
  $complete_path = parse_url( get_site_url(), PHP_URL_PATH) + '/' + get_site_slug();
  switch ($complete_path):
    case '/hours':
    case '/news/events/':
    case '/exhibits/':
    case '/exhibits/current-upcoming-past-exhibits':
      return 1 * DAY_IN_SECONDS;
      break;
    default:
      return 1 * WEEK_IN_SECONDS;
}
  • The function is being defined within the parent theme, which gives it reach across the network only because everything on the network uses either the parent theme or one descended from it. Content within a site other than the parent site is a little awkward to identify, so there is a $site variable defined that seems to use PHP's parse_url to extract the site slug reliably.

  • Because we are implementing this filter to occasionally define the max cache lifetime, the existing admin UI that also allows site admins to set a default cache length will get disabled. This moves the cache length management firmly into the realm of engineers and version control, and away from site admins.

  • Code Climate is flagging the next bit of functions.php for a poorly formatted function comment - so I'm fixing it here, even though it isn't strictly necessary.

To confirm these changes

The Pantheon documentation about its CDN caching includes a helpful command to poke at cache status via a terminal:

% curl -L -Is -H "accept-encoding: gzip, deflate, br" "https://web-2050-mitlib-wp-network.pantheonsite.io/hours/?d=2025-04-28" | egrep '(HTTP|cache-control|age:)'
HTTP/2 200 
cache-control: public, max-age=86400
age: 4

Poking at various URLs on the multidev attached to this branch should allow you to observe the max-age parameter changing as different branches of this function are activated.

Developer

Stylesheets

  • Any theme or plugin whose stylesheets have changed has had its version
    string incremented.
  • No theme or plugin assets are changed that warrant a new version string.

Secrets

  • All new secrets have been added to Pantheon tiers
  • Relevant secrets have been updated in Github Actions
  • All new secrets documented in README
  • No secrets are affected

Documentation

  • Project documentation has been updated
  • No documentation changes are needed

Accessibility

  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • There are no UI changes in this work

Stakeholder approval

  • Stakeholder approval has been confirmed
  • Stakeholder approval is not needed

Dependencies

NO dependencies are updated

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • The changes have been verified
  • The documentation has been updated or is unnecessary
  • New dependencies are appropriate or there were no changes

** Why are these changes being introduced:

There are several paths which have proven problematic under our current
caching setup (where content lasts for a week before re-rendering)

1. The hours grid, which accepts a URL parameter to highligh specific
   days.
2. The news site's events page
3. The exhibit site's homepage
4. The exhibit site's event index, which contains lists of current,
   future, and past exhibits.

Each of these pages doesn't work well when cached for more than a day,
because content can get out of date after 24 hours.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/web-2050
* https://mitlibraries.atlassian.net/browse/web-2052

** How does this address that need:

This adds a function to the parent theme which implements the add_filter
command provided by WordPress. This allows us to define a conditional
that assigns the cache length according to defined logic - logic which
can be pretty flexible in the checks it performs. The default response
from this function is the existing cache length of one week.

** Document any side effects to this change:

* The function is being defined within the parent theme, which gives it
  reach across the network only because everything on the network uses
  either the parent theme or one descended from it. Content within a
  site other than the parent site is a little awkward to identify, so
  there is a $site variable defined that seems to use PHP's parse_url
  to extract the site slug reliably.

* Because we are implementing this filter to occasionally define the
  max cache lifetime, the existing admin UI that also allows site
  admins to set a default cache length will get disabled. This moves
  the cache length management firmly into the realm of engineers and
  version control, and away from site admins.

* Code Climate is flagging the next bit of functions.php for a poorly
  formatted function comment - so I'm fixing it here, even though it
  isn't strictly necessary.
** Why are these changes being introduced:

We define some custom Composer commands to remind engineers of the
syntax to use for multidev management. One of those commands is focused
on the search-replace commands to manipulate database contents, but this
workflow has recently found a second step to more accurately manage db
contents.

Specificially, after the blunt tool to update domain names, there needs
to be a second, more targeted, command to revert some email addresses
that need to preserved with the original value in order for WordPress to
still be able to send emails.

** Relevant ticket(s):

n/a

** How does this address that need:

This updates the existing multidev-search-replace-syntax command to also
provide the suggested syntax for this second command.

Like before, this command is only echoed to the terminal, and not run
directly.

** Document any side effects to this change:

* The search-replace command does not currently adapt to multidevs that
  have a custom domain assigned (live, test, dev, and stage). That is
  a known issue with this command, which I've chosen not to try and
  solve here.
@matt-bernhardt matt-bernhardt requested a review from JPrevost April 30, 2025 19:04
@JPrevost JPrevost self-assigned this Apr 30, 2025
Copy link
Member

@JPrevost JPrevost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like what we discussed. Thanks for implementing and 🤞🏻 it works as we think it will!

@matt-bernhardt matt-bernhardt merged commit 3f6f946 into master Apr 30, 2025
4 of 5 checks passed
@matt-bernhardt matt-bernhardt deleted the web-2050 branch April 30, 2025 19:18
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