Add 'status report' for installer requirements

Here is a patch. Works fine for me, but should probably be reviewed by someone else.

Not tested, but small typo found :

$requirements['config file permissions']

Some reviews to the above patch:

- Note that drupal_verify_profile can also set error messages and the patch is not including them in the report.
- It uses hardcoded numbers instead of the constant REQUIREMENT_ERROR.

Does not seem to be a good idea to print and exit at install_check_requirements, I suggest to return an array of requirements and process all the collected requirements later. The same for the drupal_verify_profile.

The attached patch implements it in this way. It introduces a new function drupal_profile_modules that returns a list of modules required by a given profile and left drupal_verify_profile to verify the existence of the required modules. In consequence, simpletest should use this function as well. Both, install_check_requirements and drupal_verify_profile return an array of 'requirements'. The severity is checked and if there is a requirement error, a status report is displayed.

See attached screenshot, it happens when trying to install a profile that requires the views module but it was not found.

This looks great!

Couple of minor issues in the patch though. It removes the @drupal placeholder in messages so install profiles can't insert their own name. Also, there's a missing space here: needs write permissions to') . $file

Here is a update that fixes the two issues mentioned by catch.

Those changes look good. I also re-tested with 'views' added to the required modules and that works well too.

This could do with another set of eyes on it before being marked RTBC, but otherwise seems ready to go.

Except 'missing' is spelt 'mising' in the patch.

Darn! Ok. Here it is once again.

We don't write "Config File Permission", we write "Settings file permission". ;-)

Also, the patch no longer applies to CVS HEAD. Can you try to reroll?

Here is a patch rerolled against HEAD and fixing the message issue reported by Dries.

Also, other screen shoots where you can see how it looks like.

The patch still applies cleanly.

This is interesting. Subscribing.

Re-rolled for offset/fuzz/trailing whitespace.

The patch looks good. I have tested again and with other profiles with required modules and is working as expected. In this patch I am adding the admin.css that got removed by mistake in one of the above patches. That CSS file is required to show the icons in the report table.

As far as I can see, it is RTBC.

Didn't apply to HEAD anymore. Rerolling

Less code, more pretty, tested with various combinations of incorrect permissions/missing files. This is ready.

Looks good to me as well. Tested with PDO unavailable, resulting in a fatal error, this requirement must be included later in the same way the requirements are handled in this patch (#299308: Add PDO to installation requirements)

I think it looks good, I havn't been able to apply the patch myself but reviewing from the pictures. The contrast seems good, however is the red text(to highlight the required modules) we use accessible for those who have color disabilities / those in a hurry rushing over the page.

I like how you fixed the alignment and added space/indent, it made it completely more readable.

Here is an online color contrast checker: http://snook.ca/technical/colour_contrast/colour.html

Red text is #FF0000, background is #FFCCCC. The color difference is too low. A text color of #A30000 would do. (Too me it's easier on the eye too.)

BTW this is set in admin.css

There's an existing issue for colour contrast/accessibility here #8082: DROP Task: Improve accessibility of forms and messages.

The missing modules are themed using the CCS class .admin-missing from modules/system/admin.css, so the contrast/accessibility of the messages is other issue that should not block this one.

webchick asked for the settings file messages to be split into two in irc, I have to go out in a minute so this hasn't been tested extensively, but it ought to do it. If possible I'd love this change to go in before UNSTABLE-1 gets tagged so we can test it out on 'real people', so reviews welcome.

This patch actually works.

Last patch applies cleanly and works as expected. RTBC as far as I can see.

Attached another screen shot showing the last changes.

Ok, well. ;) We don't need the thing about the settings permission there twice. Please re-word the first error so it's only about copying the file.

I'm going to try and give this a thorough "webchick" review in a couple of hours, so if you want to hold off until then that's cool.

Sorry, ran out of steam for tonight ;( Will try again about 20 hours from now.

Reworded these so they only deal with one thing each. This means we have two 'Settings file' headings, one of which can have a tick, one an x, which isn't ideal. However, if possible it'd be nice to postpone any further textual changes to a followup patch, since this one is really only concerned with conversion from drupal_set_message() to the status report layout.

The internets is broken :(

changes via webchick in irc.

forgot one.

http://drupal.org/cvs?commit=143643

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