Config entity in save assumed as updated when it's new

Probably there's no test, got this issue working on #1552396: Convert vocabularies into configuration
When vocabulary saved it's always displayed message about Update on new vocabulary

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

Status: Active » Needs review
Issue tags: -Needs tests
FileSize
994 bytes

Simple fix

andypost’s picture

FileSize
1.08 KB

Finally enforceNew() is moved too

webchick’s picture

Really? how on earth can we not need tests for this? The logic was completely bass-ackwards.

andypost’s picture

Changed a test to get this trouble

Attached 1789722-core-save-4-test.patch should fail

tim.plunkett’s picture

+++ b/core/modules/config/lib/Drupal/config/ConfigStorageController.phpundefined
@@ -283,18 +283,18 @@ class ConfigStorageController implements EntityStorageControllerInterface {
+      $entity->enforceIsNew(FALSE);

I'm curious if this passes tests without this one line?

tim.plunkett’s picture

Priority: Critical » Major
FileSize
2.31 KB

D'oh.

Status: Needs review » Needs work

The last submitted patch, drupal-config-1789722-6.patch, failed testing.

tim.plunkett’s picture

Priority: Major » Critical
Status: Needs work » Needs review

I don't fully understand that, I'll have to look at the tests tomorrow. Back to CNR for #4. Also, I didn't mean to change the priority.

andypost’s picture

Assigned: Unassigned » andypost

This is a simple bug and there's already a test - Use patch from #4
let's get this in and continue with conversion to ConfigEntity patches

tim.plunkett’s picture

DatabaseStorageController does the same thing, but in the reverse order. Here's a patch that is identical to #4 but with the order of the conditionals matching DatabaseStorageController.

I'd say this is RTBC.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

+1 RTBC and -1 to criticals, btw we should minimize commit thresholds to unfreeze commit features

sun’s picture

Issue tags: +Configuration system

Looks good to me.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome!!! :D Thanks so much for the quick turnaround on this.

Committed and pushed to 8.x.

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