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

Issue #5014: Soften token status report messages, split into individual items with different severity values. #4489

Open
wants to merge 2 commits into
base: 1.x
Choose a base branch
from

Conversation

klonos
Copy link
Member

@klonos klonos commented Aug 4, 2023

Fixes backdrop/backdrop-issues#5014

See: https://git.drupalcode.org/project/token/-/commit/b189c11160e8ef4a517f3e749ac8ebb791c29a99

NOTE!!!: This includes some of the changes in #4488 (which is an earlier commit than this one here) - pay attention to not revert changes, depending on which PR gets merged first.

Example of a status warning before:
image

After this change it becomes an "info" message:
image

@backdrop-ci
Copy link
Collaborator

Related to: backdrop/backdrop-issues#5014

@dragonbot
Copy link
Collaborator

Tugboat has finished building a preview for this pull request!

Website: https://pr4489-a88fm1kejef2fvlp67qozssxrssuxum4.tugboatqa.com/
Username: admin
Password: cb29b87da36f

This preview will automatically expire on the 4th of October, 2023.

@klonos klonos changed the title Issue #5014: Soften token status report messages, split into individu… Issue #5014: Soften token status report messages, split into individual items with different severity values. Aug 4, 2023
@klonos klonos marked this pull request as ready for review August 4, 2023 06:07
@kiamlaluno
Copy link
Member

I comment here because the related issue is for multiple PRs. I wonder if it makes sense to show, on /admin/reports/status, code errors as errors.

For an error caused by a missing dependency, I could install the module or the PHP extension that is required; in the case the error is about a PHP extension I could not install (for example because my hosting provider does not include it between the available extensions), I can at least uninstall the module showing the error on the reports page.
If the error is about the $settings['trusted_host_patterns'] value that is not defined, I can open the settings.php file and change the lines that define $settings['trusted_host_patterns'].

For a Tokens or token types not defined as arrays error, which does not identify the module causing it, I would not know what to do. I should disable all the modules that define tokens, one by one, and check all the times if the error vanished, which is not that easy if the module that causes the error is required by another module (and I am not allowed to disable it before disabling all the dependent modules).

@klonos
Copy link
Member Author

klonos commented Aug 6, 2023

I wonder if it makes sense to show, on /admin/reports/status, code errors as errors.

Sorry @kiamlaluno, can you please clarify? Are you saying that all these problems should be flagged as errors? Before this change, everything was flagged as a warning. With this change, some will be shown as errors, some as warnings, and some as info. Is that what you are suggesting to be changed?

Since the respective 7.x commit was not linked to any issue in d.org, I couldn't find any discussion around this change 🤷🏼 ...do others think that we should omit it?

@kiamlaluno
Copy link
Member

I meant that I would not show, on the reports, errors that say that some hook did not return the right value, especially when the error does not make clear which module caused it. If showing those errors on the reports page is mandatory, I would rather show them as warnings.

@kiamlaluno
Copy link
Member

The issue that introduced that code in the Token module is Improve checking for token definition problems, where the last comment is the following one.

Committed #᠎5 to Git. Still needs improved reporting on 'which' modules are the violators.
http://drupalcode.org/project/token.git/commit/6cfd59b

That issue's status is still Need work.

Drupal 8, although it has tokens in core, does not contain code similar to that one.

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