ConfigStorageController::importChange() doesn't work for protected properties.
Tests and fix coming tomorrow.

Files: 
CommentFileSizeAuthor
#11 config-1889854-10-FAIL.patch2.51 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 49,678 pass(es), 3 fail(s), and 3 exception(s). View
#11 config-1889854-10-PASS.patch3.28 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 50,098 pass(es). View
#11 interdiff.txt3.54 KBtim.plunkett
#7 config-1889854-7.patch4.13 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 49,542 pass(es). View
#7 interdiff.txt2.22 KBtim.plunkett
#5 config-1889854-5-FAIL.patch3.51 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 49,625 pass(es), 27 fail(s), and 68 exception(s). View
#5 config-1889854-5-PASS.patch4.27 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 49,671 pass(es), 24 fail(s), and 68 exception(s). View
#5 interdiff.txt1.89 KBtim.plunkett
#3 config-1889854-2-FAIL.patch2.65 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 50,686 pass(es), 5 fail(s), and 0 exception(s). View
#3 config-1889854-2-PASS.patch3.41 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 50,707 pass(es), 3 fail(s), and 0 exception(s). View
#1 config-1889854-1-FAIL.patch1 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View
#1 config-1889854-1-PASS.patch1.77 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
1.77 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View
1 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Okay, this needs a test using the config_test config entity, so I'll work on that, but at least I have something.

Status: Needs review » Needs work

The last submitted patch, config-1889854-1-PASS.patch, failed testing.

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Priority: Normal » Major
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
3.41 KB
FAILED: [[SimpleTest]]: [MySQL] 50,707 pass(es), 3 fail(s), and 0 exception(s). View
2.65 KB
FAILED: [[SimpleTest]]: [MySQL] 50,686 pass(es), 5 fail(s), and 0 exception(s). View

Oh, that actually wasn't that hard.

This is a pretty big one, since Views cannot be updated at all.

Status: Needs review » Needs work

The last submitted patch, config-1889854-2-PASS.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
1.89 KB
4.27 KB
FAILED: [[SimpleTest]]: [MySQL] 49,671 pass(es), 24 fail(s), and 68 exception(s). View
3.51 KB
FAILED: [[SimpleTest]]: [MySQL] 49,625 pass(es), 27 fail(s), and 68 exception(s). View

Had to adjust the expectations of other tests, and reorder the properties.

Status: Needs review » Needs work

The last submitted patch, config-1889854-5-PASS.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
2.22 KB
4.13 KB
PASSED: [[SimpleTest]]: [MySQL] 49,542 pass(es). View

Ahh, had to adjust for the config entity query commits.

EclipseGc’s picture

Status: Needs review » Reviewed & tested by the community

eyup

sun’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
@@ -128,4 +129,22 @@ function prepareSiteNameUpdate($new_site_name) {
+  /**
+   * Tests updating existing configuration.
+   */
+  public function testUpdate() {
+    // Create a staged version of the configuration object with different data.

Why is this part of the UI test?

Let's move it into the API import test.

+++ b/core/modules/config/tests/config_test/config/config_test.dynamic.default.yml
@@ -1,2 +1,3 @@
+description: Default

+++ b/core/modules/config/tests/config_test/lib/Drupal/config_test/Plugin/Core/Entity/ConfigTest.php
@@ -62,4 +62,25 @@ class ConfigTest extends ConfigEntityBase {
+  protected $description;

Can we rename this property to 'protected_property', please?

sun’s picture

Also, can we make sure to check for existing issues first? :)

Marked #1892558: Fatal error when importing a view, using set() rather than accessing properties directly might work as duplicate.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
3.54 KB
3.28 KB
PASSED: [[SimpleTest]]: [MySQL] 50,098 pass(es). View
2.51 KB
FAILED: [[SimpleTest]]: [MySQL] 49,678 pass(es), 3 fail(s), and 3 exception(s). View

It turned out we already had test coverage for this in ConfigImportTest, and my addition to ConfigImportUITest was redundant (I put it in there initially because I was writing test coverage to mirror my interaction in the UI).

I also did s/description/protected_property, that's a nice clarification.

tim.plunkett’s picture

That was tagged neither Configurables nor VDC, I can't look everywhere.

sun’s picture

Title: Configurables with exported protected properties cannot react to config sync » Config import breaks on protected entity properties
Status: Needs review » Reviewed & tested by the community

Thanks, looks great. :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, config-1889854-10-PASS.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

Retested through qa.d.o

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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