Hi all,

I am not sure if it is a bug of migrate or not, I think yes, and I also think that it is a problem with the use of property is_new.
Migrate set this value correctly, but, as you can see, node_object_prepare set status or other properties not depends on the value of is_new, but if this value is present.

In second migrations, when you try to update a node, node_object_prepare set status values as true or "1", as you prefer, even when you removeFieldMapping for this scenario. I think "is_new" should be set only when node is new, and It shouldn´t be set if it is an old node.

function node_object_prepare($node) {
  // Set up default values, if required.
  $node_options = variable_get('node_options_' . $node->type, array('status', 'promote'));
  // If this is a new node, fill in the default values.
  if (!isset($node->nid) || isset($node->is_new)) {
    foreach (array('status', 'promote', 'sticky') as $key) {
      // Multistep node forms might have filled in something already.
      if (!isset($node->$key)) {
        $node->$key = (int) in_array($key, $node_options);
      }
    }
    global $user;
    $node->uid = $user->uid;
    $node->created = REQUEST_TIME;
  }
  else {
    $node->date = format_date($node->created, 'custom', 'Y-m-d H:i:s O');
    // Remove the log message from the original node object.
    $node->log = NULL;
  }
  // Always use the default revision setting.
  $node->revision = in_array('revision', $node_options);

  node_invoke($node, 'prepare');
  module_invoke_all('node_prepare', $node);
}

As you can see, this not respect the old property of status, and It is a nightmare.

I could set the correct value recovering the old value by node_load, by I think It is not a correct approach.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Daniel Royo created an issue. See original summary.

nicholasThompson’s picture

Version: 7.x-2.8 » 7.x-2.9

I can confirm this bug/feature too. We just had unpublished content randomly published because of this.

Alternatively, setting the migrate like this changes the behaviour.


$this->setSystemOfRecord(Migration::DESTINATION);

This is also present int 2.9 too.

nicholasThompson’s picture

Version: 7.x-2.9 » 7.x-2.x-dev
Status: Active » Needs review
FileSize
1.03 KB

This patch moves is_new further down the method and sets it to TRUE if nid is not set.

Status: Needs review » Needs work

The last submitted patch, 3: node-status-bug-2680385.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pifagor’s picture

The patch does not pass the tests

nicholasThompson’s picture

@pifagor - thanks. Looks like that error is a symptom of fixing the bug. It tries to update the node, but the node no longer has a UID.

I'm starting to think the issue is about how we update nodes; instead of considering the current state to be the default, we look like we're trying to "Reset" it each time.

Destination as a systemOfRecord type behaves better, but suffers from not being able to create nodes for new rows. Is this (systemOfRecord) documented anywhere?