We've previously been using this module, but recently noticed an issue when using a dev version that this issue was occurring with save(), which actually appears to have been fixed by:
https://www.drupal.org/project/name/issues/3163645

And is now committed into the latest dev version.
After pulling in latest dev into my codebase, and running database updates, I am seeing this error:

ArgumentCountError: Too few arguments to function Drupal\Core\Config\Entity\ConfigEntityStorage::save(), 0 passed in /home/vagrant/docroot/docroot/modules/contrib/name/name.post_update.php on line 38 and exactly 1 expected in /home/vagrant/docroot/docroot/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php on line 248 #0 /home/vagrant/docroot/docroot/modules/contrib/name/name.post_update.php(38): Drupal\Core\Config\Entity\ConfigEntityStorage->save()
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joshua.boltz created an issue. See original summary.

joshua.boltz’s picture

I've attached a patch that appears to fix this issue.

I think the problem was the previous code was calling ->save() on a ConfigEntityStorage instance, which expected one parameter provided to the function.

Starting around line 15 of name.post_update.php, the previous code was

  $default_list = $name_list_format_storage->load('default');
  if ($default_list) {
    if (!$default_list->locked) {
      $default_list->locked = TRUE;
      $default_list->save();
      $message = t('Default name list format was set to locked.');
    }
    else {
      $message = t('Nothing required to action.');
    }
  }
  else {
    $name_list_format_storage->create([
      'id' => 'default',
      'label' => 'Default',
      'locked' => TRUE,
      'status' => TRUE,
      'delimiter' => ', ',
      'and' => 'text',
      'delimiter_precedes_last' => 'never',
      'el_al_min' => 3,
      'el_al_first' => 1,
    ]);
    $name_list_format_storage->save();
    $message = t('Default name list format was added.');
  }

My patch updates the code to be this instead:

  $default_list = $name_list_format_storage->load('default');
  if ($default_list) {
    if (!$default_list->locked) {
      $default_list->locked = TRUE;
      $default_list->save();
      $message = t('Default name list format was set to locked.');
    }
    else {
      $message = t('Nothing required to action.');
    }
  }
  else {
    $name_list_format = $name_list_format_storage->create([
      'id' => 'default',
      'label' => 'Default',
      'locked' => TRUE,
      'status' => TRUE,
      'delimiter' => ', ',
      'and' => 'text',
      'delimiter_precedes_last' => 'never',
      'el_al_min' => 3,
      'el_al_first' => 1,
    ]);
    // So, instead of calling ->save() on a ConfigEntityStorage instance, it's being called on an EntityInterface like it was in the other conditional case.
    $name_list_format->save();
    $message = t('Default name list format was added.');
  }

The patch I added works similarly to the above if() conditional in that same post update function, with how the $default_list works. Whereas that code is loading a default list, if else conditional where this problem was occurring was creating a list first, and then calling save() on it.

joshua.boltz’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 3165930-name-config-entity-save-error.patch, failed testing. View results

Alan D.’s picture

Just push it through, that update hook is super old ;)

JeremySkinner’s picture

Status: Needs work » Needs review
FileSize
1.04 KB

I'm also running into this problem. The original patch resolves the issue, although the tests were complaining about an unrelated coding standards issue in the same file (doc comment line was too long). The attached patch includes the original fix and also fixes the coding standards issue. Please can this be merged?

Status: Needs review » Needs work

The last submitted patch, 6: 3165930-name-config-entity-save-error.patch, failed testing. View results

JeremySkinner’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: 3165930-name-config-entity-save-error.patch, failed testing. View results

nkoporec’s picture

@JeremySkinner the tests are still failing ... once those are fixed, I will merge the patch.

JeremySkinner’s picture

Hi @nkoporec the reason that the build isn't marked as successful isn't related to this patch (phpunit says "ERROR: No valid tests were specified.") - this regardless of whether this patch is applied or not - please see the build output. It would be great if this could be merged - it doesn't affect the tests one way or the other and actively prevents this module from being upgraded.

If the test issue is a problem, then that needs to be handled as a separate patch/issue as it's not related to this, but this is actively blocking the module's db updates from being able to run so would be good if this could go in sooner rather than later. Thanks!

JeremySkinner’s picture

Quick followup: looking at here https://www.drupal.org/pift-ci-job/1700584 tests stopped working on 13th August (and affects all patches/commits since that date). I'll open a separate issue for it.

  • nkoporec committed 51b7eef on 8.x-1.x authored by JeremySkinner
    Issue #3165930 by JeremySkinner, joshua.boltz: Too few arguments to save...
nkoporec’s picture

Status: Needs work » Fixed

oh missed the test output, we should create a seperate issue for that ... commited #8 to 8.x-1.x branch. Thanks for the patch @JeremySkinner.

JeremySkinner’s picture

Perfect, thanks!

I'm assuming the testing issue is because the tests are still using simpletest rather than phpunit (I believe drupalci stopped supporting simpletest). I've created #3172223

Status: Fixed » Closed (fixed)

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