dev/drupal#137 - Alternate PR - On drupal status report need different check when civi is already installed

[demeritcowboy]

Overview

https://lab.civicrm.org/dev/drupal/-/issues/137

This is an alternate to civicrm/civicrm-drupal-8#46

On drupal 8's system status report page, the module calls Civi\Setup's requirements checks, which mostly are still valid but there are some like the writeability of civicrm.settings.php that are backwards when civi is already installed and you're looking at the status report. This addresses that particular one.

Before

If civi is already installed and you've locked down the civicrm.settings.php file, the status report page incorrectly gives an error telling you that it needs to be writeable.

After

Different message when civi is already installed, where it warns you if it is writeable.

Technical Details

The other PR works too but addresses this by bypassing all the checks and doing its own. This PR still allows the other valid checks to run. And it seems appropriate for other CMS's that might call this too if civi is already installed.

Comments

There's a similar one for the templates compile dir, but I've left that for another PR.

@stesi561

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[maxtsero]

I think this is the right direction to go in!

[demeritcowboy]

Just updated to add some code comments.

[demeritcowboy]

Jenkins retest this please (intermittent cache deletekey fail)

[eileenmcnaughton]

test this please

[eileenmcnaughton]

@stesi561 - are you able to test this proposed alternative? I'll merge it if you are comfortable with it

[stesi561]

@eileenmcnaughton I've run an install using this against d8 5.30.0 and works fine. Do I assume from the checks passing above that this doesn't need to be tested against other cms install processed? Not clear on coverage over the actual install process verses testing post install.

Otherwise it seems fine.

[demeritcowboy]

Thanks @stesi561. Against another CMS for install it should be no change. Theoretically IF they call this function somewhere for an already-installed site they'd want the same thing, that it warn you about a writable file instead of incorrectly telling you it SHOULD be writable. Having said that I haven't looked at Joomla/Wordpress.

[sunilpawar]

Tested functionality on Drupal 8.9.7 and CiviCRM 5.27, Error message get disappear with above patch for existing installation.

[seamuslee001]

Haven't seen any issues with this and people's r-runs are showing this is working merging