D10: Update recommendation for Acquia Connector (drupal/acquia_connector)

The latest version of the Acquia Connector module is 4.0.5. This aligns with the value found in curated.json for Drupal 9 recommendations, and as such this should need no alterations/changes.

There are two migrations to test as well:

1. d7_acquia_connector.settings
2. d7_acquia_connector_susbcription_data

In Drupal 7, Acquia Connector settings are stored in the variable table. These are migrated into the state API (key_value table) in D9/D10.

For d7_acquia_connector_settings, the following mapping of state name (destination) to variable name (source) exists:

process:
  debug: acquia_agent_debug
  cron_interval: acquia_spi_cron_interval
  cron_interval_override: acquia_spi_cron_interval_override
  hide_signup_messages: acquia_agent_hide_signup_messages

These can be viewed with a query in D7 database like: select * from variable where name like "%acquia_%" and not name="acquia_subscription_data";

When processing this migration, it completes but i'm not sure how to get the source data in place as I did not have any of the settings in the mapping in my version of the module. It appears these:

1. acquia_agent_debug
2. acquia_agent_hide_signup_messages

are legacy variables and thus installing a newer version of the module won't implement them. These ones:

1. acquia_spi_cron_interval
2. acquia_spi_cron_interval_override

are part of the acquia_spi module, which does this on hook_install:

/**
 * Implements hook_install().
 */
function acquia_spi_install() {
  module_disable(['acquia_spi']);
  drupal_uninstall_modules(['acquia_spi']);
}

And then hook_uninstalls all of the related variables. So it would appear I cannot test migrating these values unless I have legacy system in place.

For d7_acquia_connector_susbcription_data , the following mapping of state name (destination) to variable name (source) exists:

process:
  acquia_subscription_data: acquia_subscription_data

and the migration works:

When processing this migration, it completes but i'm not sure how to get the source data in place as I did not have any of the settings in the mapping in my version of the module. It appears these:

Which version of the Drupal 7 module did you install?

Which older version of the Drupal 7 module did contain those variables?

So it would appear I cannot test migrating these values unless I have legacy system in place.

It looks like this module's migrations have been poorly written, documented and especially poorly maintained.

Generally speaking, it's always recommended for sites to first update to the latest stable version of all modules on Drupal 7. And then start the migration. Many times in the past did we see users of AM:A trying to use alpha/beta module versions that had gotten stable releases years ago 🙈

That also means it's recommended to update the migration to stop referring to variables that no longer exist (and per your link have not existed for well over a year).

Which version of the Drupal 7 module did you install?

I was using 7.x-4.6 version of the module, the latest stable release.

Which older version of the Drupal 7 module did contain those variables?

They are actually spread across 2 sub-modules. These

1. acquia_agent_debug
2. acquia_agent_hide_signup_messages

are part of acquia_agent and these

1. acquia_spi_cron_interval
2. acquia_spi_cron_interval_override

are part of acquia_spi. Im not sure the best way to determine when these variables were last in use other than like, parsing the entire git commit history which seems a bit daunting. Or checkout every version locally and do CTRL + F for those variables.

That also means it's recommended to update the migration to stop referring to variables that no longer exist (and per your link have not existed for well over a year).

As far as I can tell, this would mean we dont even need the d7_acquia_connector_settings migration anymore since none of these variables appear in use:

1. acquia_agent_debug
2. acquia_spi_cron_interval
3. acquia_spi_cron_interval_override
4. acquia_agent_hide_signup_messages

I think I need to understand what the D9/D10 version of Acquia Connector is setting and if those values are being populated with just the subscription data migration or if we need to update the settings migration to target different variables. These variables are present in D7 after a fresh install and set up...and do not appear to get migrated:

MySQL [drupal7]> select * from variable where name like "%acquia_%" and not name="acquia_subscription_data";
+-------------------------+----------------------------------------------+
| name                    | value                                        |
+-------------------------+----------------------------------------------+
| acquia_application_uuid | s:36:"<redacted>"; |
| acquia_identifier       | s:11:"<redacted>";                          |
| acquia_key              | s:32:"<redacted>";     |
+-------------------------+----------------------------------------------+
3 rows in set (0.013 sec)

In 4.x of Acquia Connector it seems like these values are needed - https://git.drupalcode.org/project/acquia_connector/-/blob/4.x/src/Form/...

The settings in the migration:

acquia_agent_debug
acquia_spi_cron_interval
acquia_spi_cron_interval_override
acquia_agent_hide_signup_messages

are no longer needed for migration as they are unsettable (via normal means) in the latest stable Drupal 7 version and are legacy/unused in new versions of Acquia Connector.

This MR - https://www.drupal.org/project/acquia_connector/issues/3416551 🐛 Update settings migration to target state API Needs work - changes the destination of Connector Settings to the State API, and changes the variable naming conventions to match those of the latest Drupal 7 module.
-----
Here is my Drupal 7 site, connected:

And here is my brand new D10 site, after running the migrations:

#7:

As far as I can tell, this would mean we dont even need the d7_acquia_connector_settings migration anymore since none of these variables appear in use:

👍

I think I need to understand what the D9/D10 version of Acquia Connector is setting and if those values are being populated with just the subscription data migration or if we need to update the settings migration to target different variables.

Indeed!

These variables are present in D7 after a fresh install and set up...and do not appear to get migrated:

Nice job on this detective work! 🕵️ I reviewed the upstream issue and provided extra pointers: #3416551-3: Update settings migration to target state API → .

(The original migration from #3204533: Migrate Acquia Connector configurations from Drupal 7 → was fine, but the new 7.x-4.x branch was created after that migration, and they forgot to update the migration accordingly 🙈)

#8: Looks great 🤩

The one merge-blocking thing is the reliance on the automagic "MR-as-patch" functionality: that's unsafe. MR comment: https://git.drupalcode.org/project/acquia_migrate/-/merge_requests/23#no....

On second thought, I think it'd be safer to address #3416551-3: Update settings migration to target state API → too before merging this — otherwise we risk breaking migrations from Drupal 7 sites that are still using an older version of Acquia Connector.

Or … is that not possible for some reason — is it guaranteed that all Drupal 7 versions are using version 4 of the module?

What do you think about this recommendation? https://git.drupalcode.org/project/acquia_migrate/-/merge_requests/23/diffs

#13: are you proposing to not fix 🐛 Update settings migration to target state API Needs work ? 🤔 See my comments on the MR, I think I see what's wrong 🤓

This also needs a rebase after 📌 D10: Re-vet Token Filter (drupal/token_filter) Fixed .

#13: are you proposing to not fix #3416551: Update settings migration to target state API? 🤔 See my comments on the MR, I think I see what's wrong 🤓

Thanks! Also thank you for the guidance on rebasing to remove all the make commits interactively - that make the whole process much smoother.

I made some adjustments to https://www.drupal.org/project/acquia_connector/issues/3416551 🐛 Update settings migration to target state API Needs work - not sure about how to do the minimum_version approach via annotation. Its like i would need a maximum_version as well which would be applied to the v3 source plugin.

The patch fails to apply cleanly for me for some reason → despite passing all tests. Not sure what is going on there but trying to sort it out locally.

The patch fails to apply cleanly for me for some reason despite passing all tests. Not sure what is going on there but trying to sort it out locally.

Any chance it's this? https://github.com/cweagans/composer-patches/issues/326 😅

Ah, no, found it: #3416551-16: Update settings migration to target state API → . 🏓

Nice! I think that resolved the issue😅🤞. I updated the recommendation and tested it locally. Video here 😎 →

1. Screencast shows the migration working end-to-end
2. The applied patch is passing tests and RTBC: #3416551-20: Update settings migration to target state API →

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