I found this when creating a module that extends the support module. At first I thought it was a bug but actually the problem was caused by my module accidentally unsetting the $form['support']['client'] field in hook_form_alter. Nevertheless, the problem I noticed is still true so thought I would mention it.

Basically on line 922 of support.module:

    $client = db_query('SELECT name FROM {support_client} WHERE clid = :clid', array(':clid' => $node->client))->fetchField();
    if (!isset($node->client) || $node->client == 0) {
      form_set_error('client', t('You must select a client'));
    }

I think this should be more like:

    if (isset($node->client) && $node->client != 0) {
      $client = db_query('SELECT name FROM {support_client} WHERE clid = :clid', array(':clid' => $node->client))->fetchField();
      // probably all of the other checks in this function should be moved inside this bracket since they use the result of this query
    }
    else {
      form_set_error('client', t('You must select a client'));
    }

This is the same logic but the check is done before the variable is used, so there won't be problems or strict warnings if $node->client isn't set. The way it is currently: if the client has not been set it still tries to do the database query using $node->client (and gives errors), and then afterwards checks whether $node->client is set.

Comments

Purencool’s picture

Status: Active » Closed (outdated)