Nik Kantar

Wednesday, May 6, 2015

Drupal Is a Nightmare and Done Is Better than Perfect

Sometimes you have to take your pride in doing things well and distract it while you do something badly. This was one of those times.

This piece is about two things: how much I dislike Drupal and the fact that getting something done beats perfecting it without deliverable results.

Let's start with the negative.

Drupal Is a Nightmare

The thing I understand the least about Drupal is why otherwise competent people continue to choose it.

Past the in vogue ripping on PHP, the way Drupal goes about things strikes me as particularly inconvenient for any use case. Its a massive bloat makes it an equal pain for users and developers. It has way too many moving pieces. Ability to construct complex views/templates/whatevers in the CMS itself is a huge red flag.

But maybe I just don't understand Drupal itself. Maybe it really is as good as its proponents claim it to be, and I'm just unable to think past the developer-friendly world of Django. Maybe it is me.

What I know for sure is that I dislike dealing with Drupal about as much as I disliked dealing with osCommerce nine years ago, at the beginning of my career.

Unfortunately, I have to deal with it these days, which brings me to my second point...

Done Is Better than Perfect

I try my best to write Good Code™ whenever possible. I consider that to be a required trait in any developer, as little code has a lifespan so short that maintainability isn't relevant. Even when the code is going away shortly, it's best to stay away from bad practices in order to avoid forming questionable habits.

However, there are times when it's appropriate to write Bad Code™.

The reason I've encountered most frequently is deadlines. Given that code I write at work is meant to serve a purpose other than its own existence, external factors mandate that I eventually deliver something that works, which occasionally requires compromising my desire to create nothing but beautiful, elegant code.

Yesterday I was working on a client request for an enhancement of a Drupal 7 site. One of the content types has a phone number field which only allows properly formatted numbers to be entered. The client wanted to use letters in the number (e.g., 1-555-YAY-TEAM), and I set about figuring out how to best accomplish this, keeping in mind the site's end of life is some three months from now, and we're only doing absolutely minimal maintenance in the meantime.

Drupal (at least version 7) doesn't provide a way to change field types, so I was left with two options:

Option 1: create a new text field, copy phone number data to it, drop the original phone number field, and rename the new field to that.

Option 2: create a new text field, copy phone number data to it, and change all appropriate content blocks to use the new field.

I didn't like either one of these ideas. Option 1 involves dropping a column from the production database, which for someone who knows very little about Drupal is a downright frightening thought. Option 2 requires understanding the whole site and its content organization quite well. Ugh.

After significant time spent researching, deciding on #2, and making some progress, I started wondering if it would be possible to disable the error checking of the phone number module itself. After all, the database field is just a plain text one, so the formatting requirements are only enforced in the code.

It turned out there's no way to disable it with configuration. The phone number module doesn't actually have any configuration in the CMS, and some minor things can be changed in the code itself, though that requires altering the source.

And then it hit me: what if I literally hack the module's source to prevent it from doing certain parts of its job?

A few minutes of searching later, I found these two functions:

<?
/**
 * Implements hook_field_validate().
 */
function phone_field_validate($entity_type, $entity, $field, $instance, $langcode, $items, &$errors) {
  foreach ($items as $delta => $item) {
    if (isset($item['value']) && $item['value'] != '') {
      $ccode = $field['settings']['country'];
      $value = $item['value'];
      if (!valid_phone_number($ccode, $value)) {
        $country = phone_country_info($ccode);
        $errors[$field['field_name']][$langcode][$delta][] = array(
          'error' => 'phone_invalid_number',
          'message' => t($country['error'], array('%value' => $value)),
        );
      }
    }
  }
}
<?
/**
 * Convert a valid North American phone number into standard (444) 867-5309 x1234 format
 *
 * @param $phonenumber must be a valid ten-digit number (with optional extension)
 *
 */
function format_ca_phone_number($phonenumber, $field) {

  // define regular expression
  $regex = '/
    \D*            # ignore non-digits
    (\d*)          # an optional 1
    \D*            # optional separator
    ([2-9][0-8]\d) # area code (Allowed range of [2-9] for the first digit, [0-8] for the second, and [0-9] for the third digit)
    \D*            # optional separator
    ([2-9]\d{2})   # 3-digit prefix (cannot start with 0 or 1)
    \D*            # optional separator
    (\d{4})        # 4-digit line number
    \D*            # optional separator
    (\d*)          # optional extension
    \D*            # ignore trailing non-digits
    /x';

  // get digits of phone number
  preg_match($regex, $phonenumber, $matches);

  $separator = isset($field['ca_phone_separator']) ? $field['ca_phone_separator'] : '-';

  // construct ten-digit phone number
  $phonenumber =
    ( $field['ca_phone_parentheses'] ?
      '(' . $matches[2] . ') ' :
      $matches[2] . $separator ) .
      $matches[3] . $separator . $matches[4];

  // Optional extension
  if ($matches[5] != '') {
      $phonenumber .= ' x' . $matches[5];
  }

  if ($field['phone_country_code']) {
    // This condition check is pointless.
    if ($matches[1] != '1') {
    $phonenumber = '1' . ' ' . $phonenumber;
    }
  }
  return $phonenumber;
}

From what I could tell, phone_field_validate() validates input, and format_ca_phone_number() formats valid input consistently for storage.

I changed them so they wouldn't interact with the data at all:

<?
/**
 * Implements hook_field_validate().
 */
function phone_field_validate($entity_type, $entity, $field, $instance, $langcode, $items, &$errors) {
  // foreach ($items as $delta => $item) {
  //   if (isset($item['value']) && $item['value'] != '') {
  //     $ccode = $field['settings']['country'];
  //     $value = $item['value'];
  //     if (!valid_phone_number($ccode, $value)) {
  //       $country = phone_country_info($ccode);
  //       $errors[$field['field_name']][$langcode][$delta][] = array(
  //         'error' => 'phone_invalid_number',
  //         'message' => t($country['error'], array('%value' => $value)),
  //       );
  //     }
  //   }
  // }
}
<?
/**
 * Convert a valid North American phone number into standard (444) 867-5309 x1234 format
 *
 * @param $phonenumber must be a valid ten-digit number (with optional extension)
 *
 */
function format_ca_phone_number($phonenumber, $field) {

  // // define regular expression
  // $regex = '/
  //   \D*            # ignore non-digits
  //   (\d*)          # an optional 1
  //   \D*            # optional separator
  //   ([2-9][0-8]\d) # area code (Allowed range of [2-9] for the first digit, [0-8] for the second, and [0-9] for the third digit)
  //   \D*            # optional separator
  //   ([2-9]\d{2})   # 3-digit prefix (cannot start with 0 or 1)
  //   \D*            # optional separator
  //   (\d{4})        # 4-digit line number
  //   \D*            # optional separator
  //   (\d*)          # optional extension
  //   \D*            # ignore trailing non-digits
  //   /x';

  // // get digits of phone number
  // preg_match($regex, $phonenumber, $matches);

  // $separator = isset($field['ca_phone_separator']) ? $field['ca_phone_separator'] : '-';

  // // construct ten-digit phone number
  // $phonenumber =
  //   ( $field['ca_phone_parentheses'] ?
  //     '(' . $matches[2] . ') ' :
  //     $matches[2] . $separator ) .
  //     $matches[3] . $separator . $matches[4];

  // // Optional extension
  // if ($matches[5] != '') {
  //     $phonenumber .= ' x' . $matches[5];
  // }

  // if ($field['phone_country_code']) {
  //   // This condition check is pointless.
  //   if ($matches[1] != '1') {
  //    $phonenumber = '1' . ' ' . $phonenumber;
  //   }
  // }
  return $phonenumber;
}

Now the text field is really just a text field, and I don't have to worry about existing data, variable references, or function calls.

It's not perfect, but it's done.

Want to respond with a comment or question, or have some feedback?
Email and Twitter are good direct options, but you could also write a piece of your own and send me the link. :)