Export single simple configuration name has no empty value, resulting in confusing UI

makes much sense, the only 'annoying' part is that if you would select '- Select - ' again, you get 'false', not sure if we should protect against that.

Rerolled against 8.6.x.
Minor, but IMO clearly an omission and RTBC if tests passing.

Thanks for bringing this up. Actually, even without the patch, this happens when switching back to "- Select -".
Enclosed patch fixes that as well. Do we need a test?

It's a bug, I think it should have a test.

It's not clear if #3 was addressed.

+++ b/core/modules/config/src/Form/ConfigSingleExportForm.php
@@ -160,9 +168,6 @@ public function updateExport($form, FormStateInterface $form_state) {
-    $names = [
-      '' => $this->t('- Select -'),
-    ];

@@ -194,7 +199,7 @@ protected function findConfiguration($config_type) {
-    return $names;
+    return ['' => $this->t('- Select -')] + $names;

The need for this change is not clear.
Also, there is a possibility that $names will not have been defined and this will trigger a warning.

Note that the first set of changes is confusing to read because of the change to use local variables. None of the code is actually changing there; it is being wrapped in a !empty() check.

Wondering why it still reproducible after related is fixed #3225381: PHP Notice logged when switching "Configuration type" in single configuration export screen

This is more of a UI issue then an error thrown. If you go to the export single item page the dropdown is selected the first item but the config is not loaded by default.

1. +++ b/core/modules/config/src/Form/ConfigSingleExportForm.php
   @@ -144,18 +144,26 @@ public function updateConfigurationType($form, FormStateInterface $form_state) {
   +    if (!empty($config_name)) {
   ...
        else {
   +      $form['export']['#value'] = '';
   +      $form['export']['#description'] = '';
   

   80% of this patch is switching to local variables and indenting. These are the new actual lines.
   (no feedback, just pointing it out to other reviewers)

2. +++ b/core/modules/config/src/Form/ConfigSingleExportForm.php
   @@ -163,9 +171,6 @@ public function updateExport($form, FormStateInterface $form_state) {
   -    $names = [
   -      '' => $this->t('- Select -'),
   -    ];
        // For a given entity type, load all entities.
        if ($config_type && $config_type !== 'system.simple') {
          $entity_storage = $this->entityTypeManager->getStorage($config_type);
   @@ -197,7 +202,7 @@ protected function findConfiguration($config_type) {
   
   @@ -197,7 +202,7 @@ protected function findConfiguration($config_type) {
            }
          }
        }
   -    return $names;
   +    return ['_none_' => $this->t('- Select -')] + $names;
   

   Why is this moved to the bottom? The change from `''` to `'_none_'` could be done where it was before, no?

3. +++ b/core/modules/config/tests/src/Functional/ConfigExportUITest.php
   @@ -92,6 +92,12 @@ public function testExport() {
   +    $option_node = $this->assertSession()->optionExists("config_name", '_none_');
   

   If this test was changed to assert that optionExists('config_name', '');, wouldn't that pass already? (also no need for double quotes here)
   The test coverage should help expose the bug.

@tim.plunkett originally was just a reroll. Picking up tickets that needed tests.

regarding #27.2 it looked like it was cleaner code to me. Changed to _none_ though because setting to nothing was difficult to pickup in the tests. Which answers #27.3

Verified and tested patch#26 on drupal 9.5.x-dev. Patch applied successfully.

Testing Steps:

1. Install drupal 9.5.x-dev version.
2. Go to admin/config/development/configuration/single/export.
3. Observe Export single simple configuration name has no empty value.
4. Apply patch.
5. Clear cache and reload the page.

Testing result:
I observed that when switching back to the '- Select' state: the text area shows `false` and the description below shows `Filename: _none_.yml`.

Needs review.

2 reviews so going to say RTBC

It would be nice to have this changed.

Code reviews were done in #14 and #27 which have not been addressed. Setting to Needs Work for those to be done.

The testing done in #29 onwards on this patch seems pre-mature when there are changes to the patch to be made. Remember to read the comments before working on an issue.

Still true, updated the remaining tasks.

Hello,

I made a new patch and in this case I am correcting both problems:

• The initial problem of the issue: adding for export single simple configuration name the empty value
• The problem that when we switching back to the '- Select-' state the text area shows 'false' value

I added a gif where you can check that is working.

If there's anything I can improve just let me know!

Thanks

I have verified and tested patch #36 on Drupal 9.5 and it working fine.
Both of the issues were addressed.
@Leticia Gauna provide an interdiff file.

Sorry delay @omkar_yewale!

Here is the interdiff file, the patch is in the comment #36 https://www.drupal.org/project/drupal/issues/2622468#comment-15027683

Thanks

Will need a simple assertion to show the bug is fixed please.

Added test, please review

Confirmed applying patch #40 that going to the configuration export page the first option is -Select-

+++ b/core/modules/config/src/Form/ConfigSingleExportForm.php
@@ -154,8 +154,8 @@ public function updateExport($form, FormStateInterface $form_state) {
+    $form['export']['#value'] = !$this->configStorage->read($name) ? NULL : Yaml::encode($this->configStorage->read($name));
+    $form['export']['#description'] = !$this->configStorage->read($name) ? NULL : $this->t('Filename: %name', ['%name' => $name . '.yml']);

we're calling $this->configStorage->read($name) 3 times here now

How about we do something like this instead

$exists = $this->configStorage->exists($name)

And then use that for our logic instead

Other than that, this looks great - thanks!

Hey - Here is the patch with test and larowlan suggestion!

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Removed extra + symbols from previous patch.

Seems point #44 has been addressed.

Seems to be unrelated failure.

Committed 288f761 and pushed to 11.x. Thanks!