The base field types that are provided with cck implement hook_diff() in includes/content.diff.inc

Comments

markus_petrux’s picture

markus_petrux commented

Good point. Not sure when, but I'll try.

I plan to create a new release (and I'll try to add diff support as well) as soon as this patch in CCK is committed: #131953: Expose additional database columns to Views (not just 'value') (related issue in Money CCK queue: #335546: Expose separate fields to views, not just the amount)

markus_petrux’s picture

markus_petrux commented
Status: Active » Needs review

Would you mind trying this?

Please, open money.module and add the following to the end of the file. Then try diff comparing revisions for nodes with money fields. Works? How does it look?

/**
 * Implementation of hook_diff().
 */
function money_diff(&$old_node, &$new_node) {
  $result = array();
  $cck_info = content_types($new_node->type);
  if (isset($cck_info['fields']) && is_array($cck_info['fields'])) {
    foreach ($cck_info['fields'] as $field) {
      if ($field['type'] != 'money') {
        continue;
      }
      $old_values = array();
      $new_values = array();
      if (isset($old_node->$field['field_name'])) {
        $old_values = _money_diff_values($old_node, $field);
      }
      if (isset($new_node->$field['field_name'])) {
        $new_values = _money_diff_values($new_node, $field);
      }
      $result[$field['field_name']] = array(
        '#name' => $field['widget']['label'],
        '#old' => $old_values,
        '#new' => $new_values,
        '#weight' => $field['widget']['weight'],
        '#format' => array(
          'show_header' => FALSE,
        ),
      );
    }
  }
  return $result;
}

function _money_diff_values(&$node, &$field) {
  $result = array();
  foreach ($node->$field['field_name'] as $item) {
    if (!money_content_is_empty($item, $field)) {
      $result[] = theme('money_formatter_default', array(
        '#field_name' => $field['field_name'],
        '#type_name' => $node->type,
        '#formatter' => 'default',
        '#item' => $item,
      ));
    }
  }
  return $result;
}
BrightLoudNoise’s picture

BrightLoudNoise commented

That appears to work, however it also throws the following warning.

warning: preg_match() expects parameter 2 to be string, array given in /var/www/drupal/drupal-6.6/includes/bootstrap.inc on line 737.

BrightLoudNoise’s picture

BrightLoudNoise commented

That particular line is from the drupal_validate_utf8 function in bootstrap.inc, if that helps at all.

function drupal_validate_utf8($text) {
  if (strlen($text) == 0) {
    return TRUE;
  }
  return (preg_match('/^./us', $text) == 1);
}
markus_petrux’s picture

markus_petrux commented

Problem described in #3 and #4 is caused by a bug in CCK module. Please, try the patch I attached here: #344004: Content diff integration causing problems when other fields implement diff support

markus_petrux’s picture

markus_petrux commented
Status: Needs review » Fixed

Feature committed to CVS. Though, it is implemented in a separate file that is loaded only if diff module is enabled. It still requires the bugfix in content module mentioned in #5

markus_petrux’s picture

markus_petrux commented
Status: Fixed » Closed (fixed)
BrightLoudNoise’s picture

BrightLoudNoise commented

Thanks Markus!

yched’s picture

yched commented

CCK bug is fixed.
I also changed the way it works to a more hook-based solution - you now don't need to implement hook_diff(), but only need to provide hook_content_diff_values() (basically the previous _money_diff_values() in the code above. The current code should still work, though.
See content.diff.inc for more info.

Also, I'm having a hard time seeing how the _money_diff_values() above can work for a money field with multiple values.
[edit: forget it, my bad, it rightfully works :-).]