Modifying custom fields for a node in hook_node_presave has no effect.

Created on 8 November 2023, about 1 year ago

Problem/Motivation

When unpublishing a previously published node via the Unpublish link on the node/%/moderation page, any custom fields that have their values modified within a hook_node_presave function will not retain the newly modified values after the node is saved.

NOTE: Whenever an Article node is unpublished from the admin/content page via UPDATE OPTIONS, by selecting Unpublish selected content and then selecting the checkbox beside the node, and lastly clicking the Update button, the functionality does work as expected and the updated value for the field is permanently stored.

Steps to reproduce

  • Using a Standard install of Drupal 7, install workbench_moderation, drafty and entity modules.
  • For the sake of replicating the moderation workflow on our system, modify the Workbench Moderation configuration under admin/config/workbench/moderation and remove the Needs review state, so that the only remaining states will be Draft & Published.
  • Edit the Article content type and under Publishing Options, check Create new revision and Enable moderation of revisions.
  • Set the Default moderation state to Published.
  • Click Save content type.
  • Click on manage fields for the Article content type, and add a Boolean field called toggle and select the Single on/off checkbox widget.
  • While editing the field settings, click the checkbox Use field label instead of the "On value" as label and then save the field settings. NOTE: This field will later be used in a custom hook_node_presave function for testing purposes.
  • Create a test module (I created a directory called test_custom and in that placed test_custom.info and test_custom.module), and implement a hook_node_presave function.
  • In that hook function, first add a conditional statement to check if the field is set for the current node object. Within that conditional statement, add another conditional statement to check if the node is NOT new (we don't want this executing on newly created nodes) and is unpublished. If it fulfills these two conditions, then set the value of our toggle field to ZERO.
  • Example code of my test_custom_node_presave hook function is below:
<?php

/**
 * @file
 * Test custom code using Drupal hooks.
 *
 */

/**
 * Implements hook_node_presave().
 */
function test_custom_node_presave($node) {
  if (isset($node->field_toggle)) {
    // Node is unpublished
    if (!$node->is_new && $node->status == 0) {
      $node->field_toggle[LANGUAGE_NONE][0]['value'] = 0;
    }
  }
}

Proposed resolution

On the surface and there's probably more debugging that may need to be done, as I don't know yet or how this would affect things further in the workbench_moderation module, I was able to track the surface cause down to line 1719 in the workbench_moderation.module file. If I comment out that assignment to node->original, then the updated toggle value does get saved, however as stated earlier, this may affect things further in the WM module and I am hoping those more knowledgeable can shed some light on this. Removing this line doesn't affect the workbench_moderation_state_current or workbench_moderation_state_new values, so the comment associated with that line doesn't appear to make any sense.

On line 1718, the comment states, Set the current and new moderation state value.
Then line 1719 assigns the current node object to the node->original value. Is there a particular reason why this was done here, instead of letting the the node_save function create node->original as it normally does from a completely unchanged entity? The new moderation state value is set in the workbench_moderation_moderate function and the current moderation state value is set in the workbench_moderation_set_node_state function.
Including the entire workbench_moderation_moderate function below for more context:

/**
 * Provide quick moderation of nodes.
 *
 * Access is controlled by the menu router to these pseudo-form callbacks.
 * This function is also abstracted so that it can be called from any node
 * context.
 *
 * @see _workbench_moderation_moderate_access()
 * @see workbench_moderation_menu()
 * @see workbench_moderation_node_update()
 *
 * @param $node
 *   The node being acted upon.
 * @param $state
 *   The new moderation state requested.
 */
function workbench_moderation_moderate($node, $state) {
  // Set the current and new moderation state value.
  $node->original = $node;

  // Ensure that published nodes are flagged properly.
  if ($state == workbench_moderation_state_published()) {
    $node->status = NODE_PUBLISHED;
  }
  // If the published version is being unpublished, account for that.
  elseif (isset($node->workbench_moderation['published']) && $node->workbench_moderation['published']->vid == $node->vid) {
    $node->status = NODE_NOT_PUBLISHED;
  }

  // Set the state property for saving.
  $node->workbench_moderation_state_new = $state;

  // Save the node and let drafty handle it.
  node_save($node);
}

Because of the forced update to node->original, the line from the node_save function that would normally load the unchanged entity will not be called, because the condition will never be met. Providing lines from node.inc for context below:

/**
 * Saves changes to a node or adds a new node.
 *
 * @param $node
 *   The $node object to be saved. If $node->nid is
 *   omitted (or $node->is_new is TRUE), a new node will be added.
 */
function node_save($node) {
  $transaction = db_transaction();

  try {
    // Load the stored entity, if any.
    if (!empty($node->nid) && !isset($node->original)) {
      $node->original = entity_load_unchanged('node', $node->nid);
    }
    // remainder of function omitted...
  }
}

Remaining tasks

  • Figure out the necessity of line 1719 to assign node->original to the current node object here.
  • Based on the answer to the above line item, figure out if there are any references to node->original in the workbench_moderation module, that would possibly be affected if line 1719 were removed. Since it is created right before executing the node_save function, which would ultimately restore it, I doubt this would be the case though. However, I would like to proceed with caution and get any additional information from those in the know. I also was able to recreate the same behavior of not being able to update the toggle field value, by updating my custom function and implementing a hook_menu function with a page callback that loaded a node object via nid, assigned $node->original = $node, and then saved it via the node_save function. The hook_node_presave function was again executed, and the toggle field value modified. However, again there was no change when viewing the node. So the line that sets $node->original = $node, exhibits the same unwanted behavior as the issue experienced in the WM module. Removing the line, gives the expected functionality with the toggle field value once again working.
  • Create a small patch to remove the line and fix this issue (if that is all that is needed).
πŸ› Bug report
Status

Active

Version

3.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States kevgrob

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024