Follow-up to #2247291: Reorder tabs in configuration UI. This was originally addressed in that issue but there is ongoing discussion about the other changes in that issue. The confirmation dialog part of the issue is being separated here so that it isn't held back whilst the correct solution for the other changes is sought.

Problem/Motivation

There is no confirmation form and the operation is certainly destructive

Proposed resolution

Add a confirm dialog before performing a full import

Remaining tasks

#2642420: Selectively remove files from full import

User interface changes

As listed in Proposed Resolution

API changes

None.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because current UI works but is not as usable as it could be
Issue priority Major because having a usable UI is important for D8
Prioritized changes The main goal of this issue is usability.
Disruption None.
CommentFileSizeAuthor
#18 add_confirm_dialog-2572503-18.patch31.51 KBEli-T
#2 add_confirm_dialog-2572503-2.patch30.27 KBEli-T
#5 add_confirm_dialog-2572503-5.patch32.35 KBEli-T
#6 interdiff-2247291-2572503.txt3.03 KBEli-T
#15 sync-confirm.png20.11 KBswentel
#15 interdiff.txt848 bytesswentel
#15 add_confirm_dialog-2572503-15.patch31.54 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Eli-T created an issue. See original summary.

Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
add_confirm_dialog-2572503-2.patch30.27 KB

Status: Needs review » Needs work

The last submitted patch, 2: add_confirm_dialog-2572503-2.patch, failed testing.

The last submitted patch, 2: add_confirm_dialog-2572503-2.patch, failed testing.

Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented
Status: Needs work » Needs review
FileSize
add_confirm_dialog-2572503-5.patch32.35 KB
Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented
FileSize
interdiff-2247291-2572503.txt3.03 KB

Adding interdiff to configuration-2247291-98.patch, the most recent patch on the parent issue from which this was split and has previously been RTBC.

Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented
Assigned: Eli-T » Unassigned
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon as a volunteer commented

Hm... I have a few questions/concerns about this patch:

a) Why is the confirm form being generated from a form state redirect? That is not the normal way to get to a confirm form. Normally, when you have a destructive operation, the trigger for getting to the confirm form is an ordinary link, while here for some reason it is a form submit with redirect set, which doesn't make sense to me and might not always work (sometimes the form system doesn't redirect the way you think it should, like for instance if some URL query parameters are set).

I guess I don't understand why the original page is still a form at all, since the submit just redirects. Couldn't it be an ordinary page with a link that would be used to import the shown config?

b) The new confirm form class has no documentation header. It definitely must have one.

c) For future reviews, a screen shot would be helpful. It's not so easy to set up a site to test this.

Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented

Just to manage expectations, I'm away for the next two weeks so will address points above on my return.

tkoleary’s picture

tkoleary CreditAttribution: tkoleary at Acquia commented
Issue summary: View changes
Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented

Is that a remaining task, or a related issue?

tkoleary’s picture

tkoleary CreditAttribution: tkoleary at Acquia commented

Added to config management usability meta #2642404: [meta] Usability improvements to configuration management post 8.0

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Parent issue: #2247291: Reorder tabs in configuration UI » #2642404: [meta] Usability improvements to configuration management post 8.0
Related issues: +#2247291: Reorder tabs in configuration UI
cilefen’s picture

cilefen CreditAttribution: cilefen commented

From the issue summary:

There is no confirmation form and the operation is certainly destructive

The argument can be made that this is a critical bug.

swentel’s picture

swentel CreditAttribution: swentel commented
Issue summary: View changes
FileSize
add_confirm_dialog-2572503-15.patch31.54 KB
interdiff.txt848 bytes
sync-confirm.png20.11 KB

Reroll + diff + screenshot

The last submitted patch, 15: add_confirm_dialog-2572503-15.patch, failed testing.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea at Webikon commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/config/src/Form/ConfigSync.php
@@ -7,18 +7,12 @@
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;

    Not in use. Remove. 

  2. +++ b/core/modules/config/src/Form/ConfigSync.php
@@ -115,35 +74,20 @@ class ConfigSync extends FormBase {
-  public function __construct(StorageInterface $sync_storage, StorageInterface $active_storage, StorageInterface $snapshot_storage, LockBackendInterface $lock, EventDispatcherInterface $event_dispatcher, ConfigManagerInterface $config_manager, TypedConfigManagerInterface $typed_config, ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, ThemeHandlerInterface $theme_handler, RendererInterface $renderer) {
+  public function __construct(StorageInterface $sync_storage, StorageInterface $active_storage, StorageInterface $snapshot_storage, ConfigManagerInterface $config_manager, RendererInterface $renderer, KeyValueStoreExpirableInterface $key_value_expirable) {

    Well, I guess no contrib tried to extend this because this is a BC break. In a conservative approach I would suggest to keep the signature and create a protected getter for the key-values store. Also we add a @todo to inject the service in 9.0.x. But normally there should be no use-case ti extend this form. Opinions? 

  3. +++ b/core/modules/config/src/Form/ConfigSync.php
@@ -247,8 +186,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-        // @todo A table caption would be more appropriate, but does not have the
-        //   visual importance of a heading.
+        // @todo A table caption would be more appropriate, but does not have
+        //   the visual importance of a heading.

@@ -298,7 +237,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-                'width' => 700
+                'width' => 700,

    Look as not-related? 

  4. +++ b/core/modules/config/src/Form/ConfigSyncConfirmForm.php
@@ -0,0 +1,290 @@
+          'title' => t('Synchronizing configuration'),
+          'init_message' => t('Starting configuration synchronization.'),
+          'progress_message' => t('Completed @current step of @total.'),
+          'error_message' => t('Configuration synchronization has encountered an error.'),

    $this->t() 

  5. +++ b/core/modules/config/src/Form/ConfigSyncConfirmForm.php
@@ -0,0 +1,290 @@
+            array(get_class($this), 'processBatch'),
+            array($config_importer, $sync_step),

    s/array()/[] 

  6. +++ b/core/modules/config/src/Form/ConfigSyncConfirmForm.php
@@ -0,0 +1,290 @@
+        drupal_set_message(\Drupal::translation()->translate('The configuration was imported with errors.'), 'warning');
...
+        drupal_set_message(\Drupal::translation()->translate('The configuration was imported successfully.'));
...
+      $message = \Drupal::translation()->translate('An error occurred while processing %error_operation with arguments: @arguments', array(

    $this->t() 

  7. +++ b/core/modules/config/src/Form/ConfigSyncConfirmForm.php
@@ -0,0 +1,290 @@
+      // An error occurred.
+      // $operations contains the operations that remained unprocessed.

    Why line break? The 2nd phrase should continue on the 1st line. 

  8. +++ b/core/modules/config/src/Tests/ConfigExportImportUITest.php
@@ -173,6 +173,11 @@ public function testExportImport() {
+    $this->drupalPostForm('admin/config/development/configuration/sync/confirm', array(), t('Import'));

    s/array()/[]

Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented
FileSize
add_confirm_dialog-2572503-18.patch31.51 KB

Reroll against latest 8.1.x before addressing points in #2572503-17: Add confirm dialog before full configuration import

Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented

https://www.drupal.org/files/issues/add_confirm_dialog-2572503-18.patch is bad as it applies cleanly but doesn't take the changed code that made it failed to apply into account when it moves it elsewhere. I will fix later.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.