if (!$storage_comparer->createChangelist()->hasChanges()) {
        $form_state->setErrorByName('import', $this->t('There are no changes to import.'));
      }
      else {

This reads badly and is poor DX. Callling createChangelist() is an essential step in setting up the $storage_comparer. If you don't to it, you get errors when you try to do the config import.

But that code makes it look like it's just something that's necessary for checking for changes.

The call to createChangelist() should be moved to its own separate line here and in several other places in the config module.

CommentFileSizeAuthor
#6 3123472-6.patch2.34 KBneslee canil pinto
#2 3123472-2.patch2.42 KBneslee canil pinto

Comments

joachim created an issue. See original summary.

neslee canil pinto’s picture

neslee canil pinto
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 3123472-2.patch2.42 KB
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
+++ b/core/modules/config/src/Form/ConfigSingleImportForm.php
@@ -354,7 +354,8 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
-      if (!$storage_comparer->createChangelist()->hasChanges()) {
+      $storage_has_changes = $storage_comparer->createChangelist()->hasChanges();
+      if (!$storage_has_changes) {

I am not sure this makes the difference any more visible. How about:

$storage_comparer->createChangelist();
if (!$storage_comparer->hasChanges()) {
joachim’s picture

joachim commented 
$storage_comparer->createChangelist();
if (!$storage_comparer->hasChanges()) {

That was what I had in mind.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work
neslee canil pinto’s picture

neslee canil pinto
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 3123472-6.patch2.34 KB
joachim’s picture

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

That's perfect, thanks!

  • catch committed 81c6dc6 on 9.1.x 
    Issue #3123472 by Neslee Canil Pinto, joachim, longwave: calls to...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.0.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 81c6dc6 and pushed to 9.1.x. Thanks!

Status: Fixed » Closed (fixed)

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