Problem/Motivation
The signature of RevisionableInterface::preSaveRevision() is SQL-specific as it has a $record parameter, that wouldn't make sense for a MongoDB implementation.
That was introduced only to address an obscure issue with the revision_log field:
if (!$this->isNewRevision() && isset($this->original) && (!isset($record->revision_log) || $record->revision_log === '')) {
// If we are updating an existing node without adding a new revision, we
// need to make sure $entity->revision_log is reset whenever it is empty.
// Therefore, this code allows us to avoid clobbering an existing log
// entry with an empty one.
$record->revision_log = $this->original->revision_log->value;
}
Proposed resolution
- Move this logic to the storage layer and address it "generically" for any entity type.
- Remove
RevisionableInterface::preSaveRevision() or at least make it not SQL-specific.
Remaining tasks
- Validate the proposed solution
- Write a patch
- Review it
User interface changes
None
API changes
Sort of BC API change: the logic will be moved where it can always be executed for any entity type. Then it should be safe to remove the method, since there are probably not many use cases for it left. We could also leave an empty implementation in place to be strictly BC.
Data model changes
None
Comments
Comment #1
plachBetter title
Comment #2
plachand better IS
Comment #13
catchMaybe this will get done if it's re-framed as a task.
Comment #17
longwaveJust run into this in a roundabout way. I have a site which has altered the revision_log field and
$record->revision_logis not a valid column in the table any more. Agree that this is making assumptions about the data structure that is in use here, and it should be done in a more generic way instead.Comment #18
longwaveI also think this logic could be simplified:
If
$record->revision_logis not set then we don't need to set it at all for the purposes of the SQL statement.