Problem/Motivation

In ContentEntityBase::setNewRevision():throw new \LogicException(String::format('Entity type @entity_type does support revisions.'));

Proposed resolution

Add the entity type label/ID.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#8 setnewrevision-message-2445175-7-TESTS_ONLY.patch885 bytesarla
#7 setnewrevision-message-2445175-7.interdiff.txt919 bytesarla
#7 setnewrevision-message-2445175-7.patch1.6 KBarla
#6 setnewrevision-message-2445175-6.interdiff.txt1.03 KBarla
#6 setnewrevision-message-2445175-6.patch1.7 KBarla
#5 setnewrevision-message-2445175-4.interdiff.txt887 bytesarla
#5 setnewrevision-message-2445175-4.patch1.6 KBarla
#2 setnewrevision-message-2445175-2.patch753 bytesarla
setNewRevision message.png32.94 KBarla

Comments

arla’s picture

arla commented

Oh... and the word "not" is missing from the message ;)

arla’s picture

arla commented
Status: Active » Needs review
StatusFileSize
new setnewrevision-message-2445175-2.patch753 bytes
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

Yeah, looks good. Although... No test coverage for this situation? ;-)

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: +Needs tests

I like the missing not ;)

We already have ContentEntityBaseTest::testIsNewRevision(), shouldn't be too hard to have a version of that method that results in throwing that exception.

arla’s picture

arla commented
Issue tags: -Needs tests
StatusFileSize
new setnewrevision-message-2445175-4.patch1.6 KB
new setnewrevision-message-2445175-4.interdiff.txt887 bytes

Thanks. Here's a test.

arla’s picture

arla commented
StatusFileSize
new setnewrevision-message-2445175-6.patch1.7 KB
new setnewrevision-message-2445175-6.interdiff.txt1.03 KB

The PHPUnit annotation @expectedExceptionMessageRegExp uses preg_match(), where \w includes "any letter or digit or the underscore character" (Escape sequences), so it fails if the placeholder is not replaced.

But still, checking for the exact message is more strict so here's that.

arla’s picture

arla commented
StatusFileSize
new setnewrevision-message-2445175-7.patch1.6 KB
new setnewrevision-message-2445175-7.interdiff.txt919 bytes

Using setExpectedException to do the same thing but nicer.

arla’s picture

arla commented
StatusFileSize
new setnewrevision-message-2445175-7-TESTS_ONLY.patch885 bytes

Tests-only patch, for the sake of completion.

Status: Needs review » Needs work

The last submitted patch, 8: setnewrevision-message-2445175-7-TESTS_ONLY.patch, failed testing.

arla’s picture

arla commented
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Thanks.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 5fe56e2 and pushed to 8.0.x. Thanks!

  • alexpott committed 5fe56e2 on 8.0.x 
    Issue #2445175 by Arla: Exception message in setNewRevision() is missing...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.