In the issue converting nodes to classed entities some questions around the behavior of the log property came up: #1361234-84: Make the node entity a classed object

Does log need it's own special handling? It's a string of text associated with a node that can vary on revisions. And a given user may or may not have access to edit it's value. That sounds like a field to me.

There is also a thread discussing how to make node titles more field-like: #1188394: Make title behave as a configurable, translatable field (again)

Is there a strong reason to keep log as special case property? If so, I think this issue # can be used to follow up on any changes prompted by #1361234: Make the node entity a classed object


moshe weitzman’s picture

Good idea. The only other specialness of log that I can think of is that its value always starts as NULL on the edit form. Most other field values persist across revisions.

yched’s picture

Also log is required only if the 'create revision' box is checked (can be done through hook_field_attach_validate(), I guess)

sun’s picture

Title:Clean up node log handling or convert node log to a field» Clean up node revision log message handling or convert it to a field

I'm not sure whether a field is the proper thing for this.

A field gets revisioned, ok. But is the node revision log message as optional and customizable as other fields? Can it be multiple? Is it multilingual? Can you add a revision log message field to a bundle to get revisions?

The handling of the revision log message is special - whether a value is processed and stored is directly tied to the condition of

  1. whether the node/entity (type) is revisionable
  2. whether revisions are enabled for the (node type) bundle, and
  3. whether a new node/entity revision will be created or not:
    if ($node->is_new || !empty($node->revision)) {
      // When inserting either a new node or a new node revision, $node->log
      // must be set because {node_revision}.log is a text column and therefore
      // cannot have a default value. However, it might not be set at this
      // point (for example, if the user submitting a node form does not have
      // permission to create revisions), so we ensure that it is at least an
      // empty string in that case.
      // @todo: Make the {node_revision}.log column nullable so that we can
      // remove this check.
      if (!isset($node->log)) {
        $node->log = '';
    elseif (!isset($node->log) || $node->log === '') {
      // If we are updating an existing node without adding a new revision, we
      // need to make sure $node->log is unset whenever it is empty. As long as
      // $node->log is unset, drupal_write_record() will not attempt to update
      // the existing database column when re-saving the revision; therefore,
      // this code allows us to avoid clobbering an existing log entry with an
      // empty one.

As such, I see the revision log message as a small part of a larger business logic that is applied for entity revisions. Whether the revision log message input element will appear and will be processed depends on many factors. On top of that, it is only applicable in the first place, if the storage of a particular entity type supports revisions.

Admittedly, the copypasted code snippet is about quirks that we badly need to fix. But nevertheless, even with them being resolved and simplified, the log message rather ties into a more complex concept and business logic of entity revisions, and it would feel a bit odd to me to have the revision log message singled out of that concept into a separate field.

As a property on the entity like now, it is stored and part of the data that actually makes up the primary entity revision data. If the entity does not support revisions, the entire concept does not apply, and thus, there's also no log message in the entity properties. The concept lives in one place.

OTOH, since #1498634: [meta] Define revision/translation SQL schema for core entities (no patch) is not a reality yet, and we thus don't know how revisions for entity translations are going to be handled, someone could make the point that a field would come with language support built-in, so the Spanish revisions of a translation could have entirely different (and possibly more, or less) revisions and thus revision log messages than the hypothetical English original/source. However, I'm not even sure whether that is or might be technically possible, so the amount of unknown factors make that argument moot for me at this point in time.

andypost’s picture

hey, this kind of issues could lead to make revisions for fields are optional

xjm’s picture

Component:node.module» node system
Issue summary:View changes

(Merging "node system" and "node.module" components for 8.x; disregard.)