🇳🇮Nicaragua @baltowen

Reviewed the latest fixes by @akulsaxena and tested the commands again. Things are working as expected.

baltowen → made their first commit to this issue’s fork.

Hi @rohan_singh, thanks for the MR. I tested it and notice that there was an undefined variable. I replaced the wrong name with the proper one. I also replaced the usage of empty because Drupal PHPStan rules recommend against it. And finally I fixed a typo in the error message.

baltowen → made their first commit to this issue’s fork.

I worked on removing the empty statements.

I noticed an error logic in the code. When selecting the content types, if only pages were selected it would trigger an error that at least one content type should be selected. The error message suggests that it should not be possible to have an import without content types, so I implemented that. If we want to allow that, we can remove the validateForm method altogether.

I also fixed a coding standard issue. It seems there are still static calls to Drupal. Not sure how to address those.

Moved config action to drupal_cms_starter recipe. Rebased after 📌 Merge drupal_cms_dashboard into the starter recipe Active was merged. Tests are passing. Please review.

I noticed the following in CODEOWNERS file:

[dashboard track] @penyaskito @mtift @plopesc
/recipes/drupal_cms_dashboard/

Now that the recipe has been removed, should those lines be removed from CODEOWNERS too?

baltowen → made their first commit to this issue’s fork.

baltowen → made their first commit to this issue’s fork.

hongpong → credited baltowen → .

Yes, $tempstore_id is a string See https://git.drupalcode.org/project/ctools/-/blob/8.x-3.x/src/Wizard/Form...

This problem is caused because we are using #default_value in a #machine_name render element. Per the documentation of the exist callback...

exists: A callable to invoke for checking whether a submitted machine name value already exists. The arguments passed to the callback will be:

• The submitted value.
• The element array.
• The form state object.

In most cases, an existing API or menu argument loader function can be re-used. The callback is only invoked if the submitted value differs from the element's initial #default_value. The initial #default_value is stored in form state so AJAX forms can be reliably validated.

Because we are using the #default_value property, the callback to check whether an entity exists is never called. Later on when the migration group is being created, it fails with a duplicate error.

We can either remove the #default_value property altogether. Or as suggested in the MR, change it to a HTML placeholder attribute. For the latter, we need to consider some accessibility concerns. See image below:

baltowen → made their first commit to this issue’s fork.

I created a MR mapping the uid, please review.

I made a MR, please review.

baltowen → made their first commit to this issue’s fork.

baltowen → made their first commit to this issue’s fork.

I tested all commands and they work as expected. @dinarcon asked to rename the ignore-auto-increment-column option to unfiltered, but the actual name only-with-auto-increment-column was different from the instruction. As a suggestion it would be better to rename this option to all, since it's concise and clear to understand.

baltowen → made their first commit to this issue’s fork.

Please review the MR.

baltowen → created an issue.

An array key was being access before checking that it exists. I pushed a commit to fix this. It might be worth to create a follow up issue to make sure that all expected configurations exist or define a default value for them. Back to needs review.

I tested the MR #23, and the migration warning is gone. But there is another warning still present.

Warning: Undefined array key "default_author" in Drupal\wordpress_migrate\WordPressMigrationGenerator->createMigrations() (line 112 of modules/contrib/wordpress_migrate/src/WordPressMigrationGenerator.php).

I will work on this.

I tested the MR with the following commands. I also updated the readme file.

drush php:eval "\Drupal::service('auto_increment_alter.mysql')->getTableAutoIncrementValue('node');"
drush php:eval "print_r(\Drupal::service('auto_increment_alter.mysql')->getAllTableAutoIncrementValues());"
drush php:eval "print_r(\Drupal::service('auto_increment_alter.mysql')->getAllTableAutoIncrementValues(FALSE));"

baltowen → made their first commit to this issue’s fork.

Please review.

baltowen → made their first commit to this issue’s fork.

Thanks @gaurav_manerkar to start working on this. I moved the implementation of getTableList method to the AutoIncrementAlterMysql class. SHOW TABLES is specific to MySQL. It would not work in PostgreSQL nor SQLite.

baltowen → made their first commit to this issue’s fork.

Back to needs review.

Thanks @benjifisher for the review. I have reverted the change of $storage.

Hi, I added the default value, please review.

baltowen → made their first commit to this issue’s fork.

Hi, I attempted to resolve the merge conflict, please review.

baltowen → made their first commit to this issue’s fork.

ricardoamaro → credited baltowen → .

jlbellido → credited baltowen → .