Problem/Motivation

The text “Is required in Drupal 10.0.” is shown next to the “Database support for JSON” section is the Status report. This should mention “Drupal 11.0” or “Since Drupal 10.0”.

Steps to reproduce

Visit the Status report page.

Proposed resolution

Change the requirement text to:

Drupal requires databases that support JSON storage.

Remaining tasks

User interface changes

Clarification.

Introduced terminology

None.

API changes

None.

Data model changes

None.

Release notes snippet

None.

Issue fork drupal-3472946

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:

About issue forks

Comments

xmacinfo created an issue. See original summary.

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Related issues: +#3203193: Fail install if JSON is not supported by the database in Drupal 10
xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented

The requirement check and message is in system.install.

quietone’s picture

quietone commented
Version: 11.0.x-dev » 11.x-dev

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

dpi’s picture

dpi
Location Perth, Australia
commented

Now this is a new major, it makes to remove it altogether?

Something more concrete, along the lines of

“Drupal requires databases support JSON”

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Issue summary: View changes

Agreed. Issue summary updated.

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Issue summary: View changes

xmacinfo opened merge request !9988

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Status: Active » Needs review
xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Assigned: Unassigned » xmacinfo
Status: Needs review » Needs work

/core/modules/system/tests/src/Functional/System/StatusTest.php fails.

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Status: Needs work » Needs review
xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Assigned: xmacinfo » Unassigned

An unrelated test fails. Leaving the status as Needs review for now.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Failure was random, all green after re-running test.

Change seems small enough and matches the summary I don't mind marking now.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs review

What about removing the REQUIREMENT_OK version of this? To me it's just wasted space on the status report, we should definitely error if this check fails but I don't feel like "Drupal requires databases that support JSON" actually tells the system administrator anything useful if their database already supports it.

smustgrave’s picture

smustgrave commented

Does removing something from the status report page require product manager review?

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented

Who is the database product manager?

On the MySQL side, Drupal 11 requires “MySQL/Percona 8.0”.

https://www.drupal.org/docs/getting-started/system-requirements/database...

If MySQL 8.x supports JSON by default, I tend to agree with longwage.

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented

Looking again at at the status page, we have:

Database support for JSON
Available
Is required in Drupal 10.0.

To be consistent with other requirements status we can simplify to:

Database support for JSON
Available

However, since this JSON support is “Required” for Drupal 10 and 11, we might prefer:

Database support for JSON
Enabled

As for the correct solution to implement (change of text) or the removal of the confirmation status, I would defer to the database product manager.

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented

It looks like that Drupal can be installed without JSON support.

if (!Database::getConnection()->hasJson()) {
  $requirements['database_support_json']['value'] = t('Not available');
  $requirements['database_support_json']['severity'] = REQUIREMENT_ERROR;
}

So the status report will display either:

Database support for JSON
Available
Is required in Drupal 10.0.

or

Database support for JSON
Not available
Is required in Drupal 10.0.

Based on that information, we should simplify to:

Database support for JSON
Available

or

Database support for JSON
Not available

Note that I did not try to install Drupal on top of a database without JSON support. But by reading the code, installing Drupal without JSON support is still possible, unless the installer halts early on with a warning.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue tags: +Needs subsystem maintainer review

@daffie is the database subsystem maintainer, tagging for review.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Component: system.module » database system
daffie’s picture

daffie commented
Status: Needs review » Needs work
Issue tags: -Needs subsystem maintainer review

I did as the subsystem maintainer a review and therefore I have removed the tag.
Changing the status to needs work for the nitpick on the MR.

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Issue summary: View changes

Thanks for the quick resposte, daffie.

Let's plan to implement this:

Database support for JSON
Available
Drupal requires databases that support JSON storage.

or

Database support for JSON
Not available
Drupal requires databases that support JSON storage.

That last one, hopefully, mostly not visible to users.

xmacinfo’s picture

xmacinfo
he/him
Primary language French
Location Canada
commented
Status: Needs work » Needs review

Text changed to match the subsystem maintainer review.

daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

The new message looks good to me.
It should be better for the site owner and it allow us to add checks on JSON functionality from the database at a later point in time.
For me it is RTBC.

  • longwave committed e2b97e56 on 11.x 
    Issue #3472946 by xmacinfo, daffie: Clarify "Database support for JSON"...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed e2b97e5 and pushed to 11.x. Thanks!

longwave closed merge request !9988

Status: Fixed » Closed (fixed)

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