After #1826602: Allow all configuration entities to be enabled/disabled it has become apparent that the status is not updated correctly when views objects are loaded from the user tempstore.

Here is the fix, but we need tests.

I'm on it. I will write some tests in the morning.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

Issue tags: +Needs tests
damiankloip’s picture

Status: Active » Needs review
Issue tags: -Needs tests
FileSize
3.52 KB
2.96 KB
dawehner’s picture

Awesome!

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/CachedDataUITest.phpundefined
@@ -0,0 +1,75 @@
+    $this->assertFalse($view_cache->locked, 'The view is not locked.');

We could refactor that in the future to use unit tests, as they are just function calls.

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/CachedDataUITest.phpundefined
@@ -0,0 +1,75 @@
+    $this->assertText('* All changes are stored temporarily. Click Save to make your changes permanent. Click Cancel to discard your changes.', 'The view has been changed.');
...
+    $this->assertText('The view test_view has been saved.');

I guess a t() function would help here.

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/CachedDataUITest.phpundefined
@@ -0,0 +1,75 @@
+    $this->assertTrue($view_cache->locked, 'The vierw is locked.');

vierwrrr, harharhar!

damiankloip’s picture

FileSize
1.11 KB
3.52 KB

Nice, thanks for the review!

I'm not sure about unit tests in the future, as I think we should keep testing this based on UI events, but not sure on that.

I made those other changes, see interdiff.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!

tim.plunkett’s picture

Issue tags: -VDC

#4: 1909878-4.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +VDC

The last submitted patch, 1909878-4.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
3.56 KB

Rerolled to use the new stuff from the views UI routes patch.

tim.plunkett’s picture

damiankloip’s picture

x post? I think #9 might fail. Didn't you remove views_ui_cache_load? :)

Status: Needs review » Needs work

The last submitted patch, vdc-1909878-8.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

Ah yes, I didn't see that in the test coverage. Your patch is clearly better :)

dawehner’s picture

#9: vdc-1909878-8.patch queued for re-testing.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed #8 to 8.x. Thanks.

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.