Problem/Motivation

As of 2022-12-27 / eu_cookie_compliance-8.x-1.24 (likely since #3264116: Migration from D7 to D9), I can confirm that this module properly migrates my EU Cookie Compliance settings from Drupal 7 to Drupal 9. Thank you very much for your hard work to make this easy for me!

As of Drupal core 8.8.x, modules can declare their Drupal 8 (or 9) upgrade status (changelog, issue #2936365: Migrate UI - allow modules to declare the state of their migrations). Failing to do so makes it seem as if this module has no migrations and/or is not ready to be upgraded... for example, see the following image...

A screenshot of the Drupal upgrade UI showing that EU Cookie Compliance will not be upgraded.

... this could be a barrier preventing other people from upgrading their sites to Drupal 8/9/10+.

Steps to reproduce

  1. Set up a Drupal 7 site. Download and install eu_cookie_compliance-7.x-1.43, and change some settings.
  2. Set up a Drupal 9 site with the Standard install profile. Download and install eu_cookie_compliance-8.x-1.24. Enable core's migrate, migrate_drupal, and migrate_drupal_ui modules.
  3. As an administrator, visit /upgrade to begin the Drupal-to-Drupal migration. Proceed through the wizard to the "What will be upgraded?" step.
    • Expected behaviour: "EU Cookie Compliance" is in the "Modules that will be upgraded" section.
    • Actual behaviour: "EU Cookie Compliance" is in the "Modules that will not be upgraded" section.
  4. Click "Perform upgrade" and wait for the upgrade to finish.
  5. Check the EU Cookie Compliance settings at /admin/config/system/eu-cookie-compliance - note the custom settings were migrated from D7 so the migration is working.

Proposed resolution

Declare the finished upgrade path from D7 by creating a migrations/state/eu_cookie_compliance.migrate_drupal.yml and populating it appropriately.

Remaining tasks

  1. Write a patch
  2. Review and feedback
  3. RTBC and feedback
  4. Commit
  5. Release

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#3 3329990-3--declare-eu_cookie_compliance-migration-status.patch1.9 KBmparker17
2022-12-28--eu_cookie_compliance--not-upgraded.png21.85 KBmparker17

Issue fork eu_cookie_compliance-3329990

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mparker17 created an issue. See original summary.

mparker17 opened merge request !114

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 3329990-3--declare-eu_cookie_compliance-migration-status.patch1.9 KB

I've made my changes in merge request !114; but here's a patch for folks who prefer that method.

Setting to Needs Review to see if Testbot has any suggested improvements.

Reviews are welcome!

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

(slight issue summary change)

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

Hmm... the test failure is in the other migration test, i.e.: the pre-existing migration test, i.e.: not the migration test added in this ticket.

By my own observation, the migrations work fine even if the old test doesn't, so I daresay that's outside of the scope of this issue.

Looking at the failure, it's a Uncaught AssertionError: The container was serialized. in /var/www/html/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php:160 being thrown before any tests are run. I don't see that when I run the migration manually, so it seems like this is a problem with the test, not the production code. It isn't particularly clear where this is happenign, but the only part that deals with the service container is in \Drupal\Tests\eu_cookie_compliance\Kernel\EuCookieComplianceMigrateTest::getFixtureFilePath(), so perhaps the fix is...

   protected function getFixtureFilePath() {
     return implode(DIRECTORY_SEPARATOR, [
-      \Drupal::service('extension.list.module')->getPath('eu_cookie_compliance'),
+      $this->container->get('extension.list.module')->getPath('eu_cookie_compliance'),
       'tests',
       'fixtures',

... but I'm not certain if that will work. And, I'm conscious that iterating on the problem in this thread may derail this issue.

Maintainers: given the above information, would you like me to try to fix that here, or in a separate ticket?

svenryen’s picture

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

I set up a D7 site, and migrated the settings of EU Cookie Compliance to D9. The interface properly tells me that the settings will be migrated and after migration I do see the same settings in the D9 site as I did in the D7 site.

Thanks for the patch!

  • svenryen committed b4b727ff on 8.x-1.x authored by mparker17 
    Issue #3329990 by mparker17, svenryen: Declare upgrade status from D7
svenryen’s picture

svenryen commented
Status: Reviewed & tested by the community » Fixed

svenryen closed merge request !114

Status: Fixed » Closed (fixed)

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