To produce this bug, define a requirements array without setting $requirements['value'], such as:

function example_code_requirements($phase) {
  if ($phase == 'runtime') {
      $requirements['example_code'] = array(
        'title' => t('Example'),
        'description' => "Blah blah blah",
        'severity' => REQUIREMENT_INFO,
      );  
    }
  return $requirements;
}

The API documentation for hook_requirements at http://api.drupal.org/api/drupal/modules--system--system.api.php/functio... states:

value: The current value (e.g., version, time, level, etc). During install phase, this should only be used for version numbers, do not set it if not applicable.

So I would expect that it not need be set, but lines 2499-2506 of /modules/system/system.admin.inc expects this value.

      // Output table row(s)
      if (!empty($requirement['description'])) {
        $output .= '<tr class="' . $severity['class'] . ' merge-down"><td class="status-icon">' . $severity['icon']
            . '</td><td class="status-title">' . $requirement['title'] . '</td><td class="status-value">'
            . $requirement['value'] . '</td></tr>';
        $output .= '<tr class="' . $severity['class'] . ' merge-up"><td colspan="3" class="status-description">' 
           . $requirement['description'] .  '</td></tr>';
      }
      else {
        $output .= '<tr class="' . $severity['class'] . '"><td class="status-icon">' . $severity['icon'] 
           . '</td><td class="status-title">' . $requirement['title'] . '</td><td class="status-value">' 
           . $requirement['value'] . '</td></tr>';
      }

Not sure if the code or the documentation is in error.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

FatherShawn’s picture

Here's a patch that prevents the error.

droplet’s picture

Version: 7.0 » 8.x-dev

if $requirement['value'] = NULL, then should not output "value" part HTML code

(d8 first and backport)

FatherShawn’s picture

That makes sense, but why move this to D8? At present the omission of an optional parameter throws an error. Shouldn't we fix that directly in D7?

droplet’s picture

@FatherShawn,
read it : http://drupal.org/node/767608

FatherShawn’s picture

@droplet
Thanks for the reference, that makes sense.

I don't agree, however about omitting the table cell if $requirements['value'] is null. The design of the Status Report really expects two columns and there's nothing wrong with an empty cell on a particular row when there is no data available for that cell.

Rerolled the patch against. 8.x

FatherShawn’s picture

Status: Active » Needs review
kscheirer’s picture

#5: requirements_params-1143922-5.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, requirements_params-1143922-5.patch, failed testing.

kscheirer’s picture

rerolled against HEAD. Does this need a test? And you're right to keep printing the whole table cell, that's just how html tables work.

kscheirer’s picture

Status: Needs work » Needs review

gah, for testbot.

FatherShawn’s picture

@kscheirer Thanks for the reroll!

kscheirer’s picture

Status: Needs review » Needs work

The last submitted patch, system-requirement_value-1143922-9.patch, failed testing.

FatherShawn’s picture

Status: Needs work » Needs review
FileSize
1.16 KB

Rerolled the patch. Applies cleanly to today's HEAD: commit e21c1a1

FatherShawn’s picture

Status: Needs review » Needs work

The last submitted patch, requirements_params-1143922-14.patch, failed testing.

FatherShawn’s picture

Status: Needs work » Needs review
FileSize
1.2 KB

Rerolled the patch. This is a little piddling error, but it would be nice if one of the followers would have a look and mark RTBC so we can get it in...
Applies cleanly to todays HEAD, commit f7a4182e2fe

Crell’s picture

Status: Needs review » Reviewed & tested by the community

Makes sense to me. Thanks, Shawn!

Crell’s picture

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 8.x, thanks! This is sufficiently minor that I don't think it's worth adding a test for.

Moving to 7.x for backport.

Sivaji_Ganesh_Jojodae’s picture

Issue summary: View changes
Status: Patch (to be ported) » Needs review
FileSize
2.63 KB

Patch for d7 attached.

parthipanramesh’s picture

Status: Needs review » Reviewed & tested by the community

works fine.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 21: drupal-system_requirements_value_optional-1143922-21.patch, failed testing.

  • catch committed df54550 on 8.3.x
    Issue #1143922 by FatherShawn, kscheirer: Fixed Undefined index: value...

  • catch committed df54550 on 8.3.x
    Issue #1143922 by FatherShawn, kscheirer: Fixed Undefined index: value...

  • catch committed df54550 on 8.4.x
    Issue #1143922 by FatherShawn, kscheirer: Fixed Undefined index: value...

  • catch committed df54550 on 8.4.x
    Issue #1143922 by FatherShawn, kscheirer: Fixed Undefined index: value...