Replace ckeditor with ckeditor5 in the 9.4.x database dumps in Drupal 10.0.x & 10.1.x

Per #3304736-26: Provide a good UX to ensure the CKEditor 4 to 5 update is always done before upgrading to Drupal 10 unless the contrib module is installed, a decision has been made 👍

Bumping to critical given it's blocking criticals (#3304736: Provide a good UX to ensure the CKEditor 4 to 5 update is always done before upgrading to Drupal 10 unless the contrib module is installed and #3270438: Remove CKEditor 4 from core).

I found this issue along with #3304326: Deprecate CKEditor 4 module in 9.5, #3270438: Remove CKEditor 4 from core and #3304736: Provide a good UX to ensure the CKEditor 4 to 5 update is always done before upgrading to Drupal 10 unless the contrib module is installed extremely confusing. So … updated issue summary that explains why we're updating the 9.4 DB dumps this way, by synthesizing all discussion in the past ~week into a (hopefully 🤞) cohesive explanation.

Title updated too:

• 9.4.0 in the title is wrong, because per #3306897: Fix the 9.4.4 database dumps it's actually 9.4.4.
• 10.1.0 is wrong because per @catch in #3304326-14: Deprecate CKEditor 4 module in 9.5.

HTML fix 😬

@xjm and @catch definitely deserve credit here! 😊

Let's run only the Update tests while figuring this out.

Ugh. phpunit does have a --group argument. But this us actually using core/scripts/run-tests.sh …

3rd time's a charm?

1. HEAD of 9.4.x is currently f20c0a2030cb19bafa2f54d7e5da8c75459410fd.

2. #
   # 1. Get current dumps
   #
   git checkout 10.0.x
   gunzip core/modules/system/tests/fixtures/update/drupal-9.4.0.bare.standard.php.gz
   git checkout core/modules/system/tests/fixtures/update/drupal-9.4.0.bare.standard.php.gz
   gunzip core/modules/system/tests/fixtures/update/drupal-9.4.0.filled.standard.php.gz
   git checkout core/modules/system/tests/fixtures/update/drupal-9.4.0.filled.standard.php.gz
   
   #
   # 2. Update the "bare" dump
   #
   git checkout 9.4.x
   # manual: wipe DB
   php -d memory_limit=-1 core/scripts/db-tools.php import core/modules/system/tests/fixtures/update/drupal-9.4.0.bare.standard.php 
   # manual: log in with drupal/drupal
   # manual: navigate to /admin/config/content/formats/manage/basic_html, switch to CKEditor 5, save
   # manual: navigate to /admin/config/content/formats/manage/full_html, switch to CKEditor 5, save
   # manual: navigate to /admin/modules/uninstall, uninstall CKEditor (4)
   php -d memory_limit=-1 core/scripts/db-tools.php dump-database-d8-mysql > some-temporary-file
   
   # 
   # 3. Update the "filled" dump
   #
   # repeat 2. but with s/bare/filled/
   

   Wow, this is painful! We really really need #3305003: Create standard steps for creating database dumps 🤯

3. Note that diff or git diff seem to be pointless: the order in which things are dumped is different 🤷‍♀️
4. Note that the first time I created those dumps, they were causing changes in PHP's serialize() output — it was because I was using PHP 7.4 instead of 7.3 — previously seen in #3275093: Drupal 9.3.0 dumps were created with PHP 8, needs to be PHP 7.3.
5. Note that as I described in #3304736-6: Provide a good UX to ensure the CKEditor 4 to 5 update is always done before upgrading to Drupal 10 unless the contrib module is installed, the dumps have broken Text Format/Editor configuration — they don't match what was in the Standard install profile! So a step I didn't list above was to re-import the Basic & Full HTML text format + editor.
6. Note that the test_text_format appears to be designed to test that a text format with all filters enabled still has all filters enabled after applying the update path. It also had CKEditor 4 enabled, but this made no sense, given that the filter_html_escape filter was enabled. So, disabled CKEditor 4 for it. This was wrong before, and during the CKE 4 → 5 upgrade that an end user would do as per #3304736: Provide a good UX to ensure the CKEditor 4 to 5 update is always done before upgrading to Drupal 10 unless the contrib module is installed, they'd do the same.
7. I expect both of CKEditor4To5UpgradeTest's test cases to fail because it specifically accounted for the broken fixtures; if I'm right and they do fail, we'll be able to remove 10 lines of code from that file, making it also more maintainable in the long run 👍

Now … how does one go about reviewing this? Since this involved manual steps? I do see a lot more changes than I'd like 😬 I don't know how to solve the different ordering of identical data. I can't find docs about it. I also see there's more LoC than before, but I didn't do anything besides the above so … 🤷‍♀️

Really curious about reviews on this. The above took hours to figure out already 🙃

EDIT: #12: too late 😅

We really need #3213633: Improve DX of maintaining migration database fixtures: provide an option for creating per-table database fixtures in DbDumpcommand for our own sanity 😬

Update UpdatePathTestBaseFilledTest expectation.

Hoping that this will address the otherwise inexplicable Undefined variable $theme_list errors 😬

#18:

So import what from where, I could have a stab at updating the dumps.

Re-import these:

core/profiles/standard/config/install/filter.format.basic_html.yml
core/profiles/standard/config/install/filter.format.full_html.yml
core/profiles/standard/config/install/editor.editor.basic_html.yml
core/profiles/standard/config/install/editor.editor.full_html.yml

I've lost that long ago

🤣

Down to 4 failures in #16! 🚀

But … Undefined variable $theme_list all over the place. Let's investigate! It comes from the last line in this chunk of code:

      if (!empty($extensions['theme'])) {
        $theme_list_scan = $listing->scan('theme');
        foreach (array_keys($extensions['theme']) as $theme) {
          if (isset($theme_list_scan[$theme])) {
            $theme_list[$theme] = $theme_list_scan[$theme];
          }
        }
        $this->folders += $this->getComponentNames($theme_list);
      }

… that actually looks like a genuine bug. How is it possible that this has never been surfaced?! 🤯

Yes, 9.4.4. Or 9.4.x — it doesn't matter (git diff 9.4.4 origin/9.4.x -- core/profiles/standard/config/install is empty).

P.S.: I'd really love to know what I did wrong 😬

I definitely violated 4) and 2), I probably violated 1), and possibly 3). Clearly a case of PEBKACAC (classical PEBKAC with the addition of And Cthulhu).

I manually gunzipped and diffed the drupal-9.4.0.bare.standard.php.gz and drupal-9.4.0.filled.standard.php.gz before and after. The same observations I had about my own patch in #13 applies:

I do see a lot more changes than I'd like 😬 I don't know how to solve the different ordering of identical data. I can't find docs about it. I also see there's more LoC than before, but I didn't do anything besides the above so … 🤷‍♀️

I see the exact same number of lines before vs after, the same ordering differences. The only difference appears to be that @Spokje did it with PHP 8.1 instead of PHP 7.3. 👍