Problem/Motivation

Notice: Trying to get property of non-object in Drupal\Core\Entity\RevisionableContentEntityBase->getRevisionLogMessage() (line 104 of /vagrant/app/core/lib/Drupal/Core/Entity/RevisionLogEntityTrait.php)

Proposed resolution

Return an empty string for missing log messages as per the interface:

  /**
   * Returns the entity revision log message.
   *
   * @return string
   *   The revision log message.
   */

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#27 2827701-27.patch770 bytesneclimdul
#9 revisionlogentitytrait_getrevisionlogmessage_displays_a_notice-2827701-9.patch613 bytesRobin Monks
#6 revisionlogentitytrait_getrevisionlogmessage_displays_a_notice-2827701-6.patch615 bytesRobin Monks
#3 Issue__2827701__RevisionLogEntityTrait__getRevisionLogMessage_displays_a_notice_when_no_re.patch689 bytesAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sam152 created an issue. See original summary.

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Issue tags: +Novice, +Needs tests
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous as a volunteer commented
FileSize
Issue__2827701__RevisionLogEntityTrait__getRevisionLogMessage_displays_a_notice_when_no_re.patch689 bytes
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous as a volunteer commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: Issue__2827701__RevisionLogEntityTrait__getRevisionLogMessage_displays_a_notice_when_no_re.patch, failed testing.

Robin Monks’s picture

Robin Monks CreditAttribution: Robin Monks at Appnovation commented
FileSize
revisionlogentitytrait_getrevisionlogmessage_displays_a_notice-2827701-6.patch615 bytes

Re-roll & update for coding standards.

Robin Monks’s picture

Robin Monks CreditAttribution: Robin Monks at Appnovation commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: revisionlogentitytrait_getrevisionlogmessage_displays_a_notice-2827701-6.patch, failed testing.

Robin Monks’s picture

Robin Monks CreditAttribution: Robin Monks at Appnovation commented
Status: Needs work » Needs review
FileSize
revisionlogentitytrait_getrevisionlogmessage_displays_a_notice-2827701-9.patch613 bytes
Chi’s picture

Chi CreditAttribution: Chi commented 
+++ b/core/lib/Drupal/Core/Entity/RevisionLogEntityTrait.php
@@ -101,6 +101,9 @@ public function setRevisionUserId($user_id) {
+    if (empty($this->revision_log_message->value)) {

If the log message consists of just one character "0" this would replace it with an empty string.

Can we use isset here?
return isset($this->revision_log_message->value) ? $this->revision_log_message->value : NULL;

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

How exactly do you get this? THis can't happen if the *message* is empty, only if there is no field named like that. Which would mean that it is explicitly unset from the trait, which might not be a supported thing to do?

See also #2248983: Define the revision metadata base fields in the entity annotation in order for the storage to create them only in the revision table

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented

Maybe possible with a programatically created node with no explicit log message? I seemed to remember triggering it on an entity that had the field.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Postponed (maintainer needs more info)

I think we need some proper steps to reproduce before attempting any patches here :)

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Issue tags: -Novice, -Needs tests +Bug Smash Initiative

@Sam152, Thank you for reporting this problem.

Is this issue still a problem?

There has been no activity here for 5 years.

Since we need more information to move forward with this issue, I am keeping the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul at APQC commented
Status: Postponed (maintainer needs more info) » Needs work
Issue tags: +PHP 8.1

Looking at the report, probably not but I think its come back in a different way. Not matching the interface is still causing problems.

Technically another solution would be to update the interface but... I think this solution is probably right? Does getting a null for the revision log have a useful meaning?

NW because looking at the trait, it looks nothing like the code in the patch so this definitely needs a reroll.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul at APQC commented
Status: Needs work » Needs review
FileSize
2827701-27.patch770 bytes

Here's a patch. Much easier with Null coalescing. Could have been a one line patch but made it a bit longer to document what was going on.

No interdiff because the patch conflict is so old its painful to make. Summary, converted to "new" metadata to get field name and used the null coalescing to condense the return logic.

Status: Needs review » Needs work

The last submitted patch, 27: 2827701-27.patch, failed testing. View results

ameymudras’s picture

ameymudras CreditAttribution: ameymudras at Salsa Digital commented

May be we can reduce this to:

// Ensure null values return as a string.
return $this->{$this->getEntityType()->getRevisionMetadataKey('revision_log_message')}->value ?? '';
neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul at APQC commented

Because it makes the line very long and the documentation less clear.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.