Problem/Motivation

No confirmation message is shown when saving the views settings forms (basic and advanced).

Proposed resolution

ConfigFormBase::submitForm() includes a message but we are not calling the parent anymore, so let's do that. I thought we used to be doing this...

Remaining tasks

Fix + tests

User interface changes

Message shown when these forms are saved.

API changes

No

CommentFileSizeAuthor
#6 interdiff-2254765-6.txt1.63 KBdamiankloip
#6 2254765-6.patch3.5 KBdamiankloip
#1 2254765-PASS.patch3.29 KBdamiankloip
#1 2254765-tests-only-FAIL.patch1.92 KBdamiankloip

Comments

damiankloip’s picture

damiankloip commented
Status: Active » Needs review
StatusFileSize
new 2254765-tests-only-FAIL.patch1.92 KB
new 2254765-PASS.patch3.29 KB

The last submitted patch, 1: 2254765-tests-only-FAIL.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2254765-PASS.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review

1: 2254765-PASS.patch queued for re-testing.

olli’s picture

olli commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/SettingsTest.php
@@ -117,6 +121,23 @@ function testEditUI() {
+    // Test the advanced settings form.
+
+    // Test the confirmation message.
+    $this->drupalPostForm('admin/structure/views/settings', array(), t('Save configuration'));

    This should use admin/structure/views/settings/advanced. 

  2. +++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/SettingsTest.php
@@ -117,6 +121,23 @@ function testEditUI() {
+    $this->assertFieldChecked('edit-skip-cache', t('The skip_cache option is checked.'));
+    $this->assertFieldChecked('edit-sql-signature', t('The skip_cache option is checked.'));
+    $this->assertFieldChecked('edit-no-javascript', t('The skip_cache option is checked.'));

    No need for t() in assert messages.

Should we add a few lines for the "Clear views' cache" button in the test?

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 2254765-6.patch3.5 KB
new interdiff-2254765-6.txt1.63 KB

Thanks olli! Fixed those things.

olli’s picture

olli commented
Status: Needs review » Reviewed & tested by the community

The confirmation message is shown now, thank you!

dawehner’s picture

dawehner
Primary language German
commented

+1

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Thanks, this has really been bugging me!

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/SettingsTest.php
@@ -117,6 +121,23 @@ function testEditUI() {
+    $this->assertFieldChecked('edit-skip-cache', t('The skip_cache option is checked.'));
+    $this->assertFieldChecked('edit-sql-signature', t('The skip_cache option is checked.'));
+    $this->assertFieldChecked('edit-no-javascript', t('The skip_cache option is checked.'));

Oopsie. ;) A little too much "copy pasta" here. Fixed on commit.

Committed and pushed to 8.x. Thanks!

  • Commit e88f22a on 8.x by webchick: 
    Issue #2254765 by damiankloip: No confirmation messages when Views UI...
damiankloip’s picture

damiankloip commented

Thanks for fixing my negligent copy pasting! :)

Status: Fixed » Closed (fixed)

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