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

[DX][D9] Introduce a new hook_requirements_alter() hook, to allow altering entries defined in hook_requirements() implementations #6241

Open
klonos opened this issue Oct 2, 2023 · 2 comments · May be fixed by backdrop/backdrop#4528

Comments

@klonos
Copy link
Member

klonos commented Oct 2, 2023

I was working on some issues around https://github.com/backdrop-contrib/ckeditor5 (which is slated to eventually be added into core), and quickly realized that I cannot edit the status report page entries provided by the CKEditor module in core (v4). In general though, if you try to override the values of the existing $requirements[] array provided by hook_requirements() implementations of any module (core or otherwise), you are getting a php error about strcasecmp() in _system_sort_requirements() expecting a string and be provided with an array instead (which is true - that's what happens if you try to re-define an existing array key in $requirements[]).

A quick research surfaced #309040: Add hook_requirements_alter(), which has already been implemented in D9.5.x (change record: Third-party modules are able to alter the requirements entries, commit + follow-up commit to fix a typo + a currently RTBC follow-up issue to have the new alter hook work during update: https://www.drupal.org/project/drupal/issues/3324995), which I think we should introduce in Backdrop as well. From the dorg issue:

Problem/Motivation

Requirements are produced by implementing the hook_requirements() hook. But there's no way they can be altered. Consider some situations that requires alteration:

  • A distribution works only with a higher version of PHP than the underlying Drupal core it depends on.
  • A module wants to decrease or to increase the severity of a requirement.
  • Other modules want to add more details in the requirement description entry.
  • A CI/CD system checks for errors & warnings at the end of a production deployment. If there are some, it rolls-back the deployment. However, they want to define some exceptions that are acceptable.
  • A module simply wants to remove a requirement provided by other module.

Note that the Drush command core:requirements did already implement the --ignore option but that helps only to achieve removal of requirements and not alteration.

...

Original report by David_Rothstein

This patch introduces a hook_requirements_alter() hook to Drupal. A possible use case is discussed in http://drupal.org/node/295697 (allowing modules to modify the recommended minimum PHP memory limit provided by Drupal core), but there are others.

...

From #3184320: Improve DX for updates to status report requirements, which was closed as a duplicate of #309040: Add hook_requirements_alter()

Problem/Motivation

There are cases in which you may want to change the severity of a status page requirement (e.g., downgrade or upgrade) e.g. what we are doing at https://www.drupal.org/project/trusted_reverse_proxy to downgrade an error to info/checked.

Steps to reproduce

It's possible to adjust the variables for the status report page with hook_preprocess_HOOK(), e.g. hook_preprocess_status_report_page(), however this addresses the render array a little too late in the pipeline. ...

From #2869592: Disabled update module shouldn't produce a status report warning (which this change will help make easier):

On the status report, if you have uninstalled the Update module you will get a big scary warning:

Update notifications are not enabled. It is highly recommended that you enable the Update Manager module...

There are many situations where it's perfectly reasonable to disable the update module, such as in an enterprise environment where you monitor for updates via other dedicated means and don't want the performance impact of having Update enabled in production. In these cases, the warning is just a distraction and contributes to a "crying wolf" mentality.

Ideally, there would be a way for expert administrators to disable this warning.


Release notes snippet

Requirements defined in HOOK_requirements() implementations can now be altered by implementing HOOK_requirements_alter().

@klonos
Copy link
Member Author

klonos commented Nov 26, 2023

We have a couple of use cases for this in core:

@bugfolder
Copy link

It would also allow a more streamlined solution to backdrop-contrib/ubercart#450 (albeit at the cost of setting a more recent core version requirement for Ubercart, not to be taken lightly).

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

Successfully merging a pull request may close this issue.

2 participants