If running update.php (in-browser) and the 'user has the same email as the site email address' requirement is flagged, it prevents the update scripts from running (because any requirement warning or error does). I propose to hide this warning if running update.php.

CommentFileSizeAuthor
#3 2184049-requirement-runtime-phase.patch513 bytesdave reid
#1 2184049-requirement-runtime-phase.patch513 bytesdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
StatusFileSize
new 2184049-requirement-runtime-phase.patch513 bytes

Looks like it's as simple as checking the $phase variable passed into hook_requirements(). Stuff like this should only be checked if $phase == 'runtime' as a best practice. Note that update_check_requirements() passes in 'update' so this would be skipped in that case.

q0rban’s picture

q0rban commented

Shouldn't this be if $phase != 'runtime'?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 2184049-requirement-runtime-phase.patch513 bytes

I would say I was testing you but I just made this too quickly. My bad.

q0rban’s picture

q0rban commented

It seems odd that Drupal would be blocking an update from a REQUIREMENT_WARNING, since the docs specifically say that *shouldn't* be happening: https://api.drupal.org/api/drupal/includes!install.inc/constant/REQUIREM...

Requirement severity -- Warning condition; proceed but flag warning.

q0rban’s picture

q0rban commented

Either way, I think this is a safe enough change.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

It doesn't reference what should be proceeding though. I think it's also meant to reference the installer only, looking at drupal_check_module() (which is used during install process).

Now that I take a hard look at the update.php warning, it's deceptive as well. It shows the warnings at first and only has the following text at the bottom:

Check the error messages and try again.

To mean this means I need to resolve the warnings and re-attempt update.php, but in reality the 'try again' link actually just lets me continue the update.php script ignoring the warnings. I still think it's proper to hide this warning except when $phase == 'runtime'.

q0rban’s picture

q0rban commented

Oh interesting.

At any rate, this is fixed now. Thanks for the patch! :)

http://drupalcode.org/project/email_noreply.git/commitdiff/11f01ad9e6354...

q0rban’s picture

q0rban commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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