🇺🇸United States @dan612

dan612 → created an issue.

I believe this issue can be recreated with any Text (plain) input field, regardless of profile/installation method. After some digging around, I think the problem lies in the validation surrounding the StringItem field type.

We have a subform being created for max_length but there is no restriction on the highest allowed value for that input. Could we simply provide a #max on the subform element? For instance:

  /**
   * {@inheritdoc}
   */
  public function storageSettingsForm(array &$form, FormStateInterface $form_state, $has_data) {
    $element = [];

    $element['max_length'] = [
      '#type' => 'number',
      '#title' => $this->t('Maximum length'),
      '#default_value' => $this->getSetting('max_length'),
      '#required' => TRUE,
      '#description' => $this->t('The maximum length of the field in characters.'),
      '#min' => 1,
      '#max' => 255, // adding this line
      '#disabled' => $has_data,
    ];

    return $element;
  }

This leads me to another question...what is the correct max to set here? My inclination is to set a cap of 255, however, I don't think such a restriction exists for the varchar data type. Looking at the MySQL reference manual:

Values in VARCHAR columns are variable-length strings. The length can be specified as a value from 0 to 65,535. The effective maximum length of a VARCHAR is subject to the maximum row size (65,535 bytes, which is shared among all columns) and the character set used. See Section 10.4.7, “Limits on Table Column Count and Row Size”.

Now, this changes based on the character set in use. In the example screenshots we can see:

Column length too big for 'field_test_value' (max=16383)

The 16,383 comes from the fact that the database is using utfmb4 character encoding which requires 4 bytes per character (65,535 / 4 = 16,383.75 - can't support three-quarters of a character so drop the .75).

If I set this locally to 16,383 I am still met with the error. The highest value I can input for a max appears to be 16,339. If I set it to more than that I get an error. Why this value? Not sure...seems too arbitrary.

Additionally, it looks like the storage can specify if ASCII or not - if these characters were limited to ASCII then the max would be much greater as each character only requires 1 byte instead of 4. So if I change this to TRUE, I can then enter a value up to 65,358.

Right now the questions I have outnumber the solutions 😁

1. Where is the Ajax validation coming from on the subform input?
2. Why is the Ajax validation bypassed on second save?
3. What is an appropriate max value to set?

This should also be something that is testable in the StringItemTest...once we know what to set it at 🙃.

Hi! 👋 No problem, made an adjustment to the code and attached picture for reference. 🙂

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

Ah! Looking at the $data variable passed in from hook_tokens(), I see I have the current node already in there. So I can change:

$cur_node = \Drupal::routeMatch()->getParameter('node');

to

$cur_node = $data['node'] ?? FALSE;

Now it is working! 😃 -- closing.

I think I have narrowed this down to the:

$cur_node = \Drupal::routeMatch()->getParameter('node');

portion. It seems when first creating a node this is a null value.

dan612 → created an issue.

dan612 → created an issue.

I would not use the lateruntime processor on any semi-large scale production site. It will hijack/piggyback a request and can cause performance issues. This is recommended from Acquia here - https://acquia.my.site.com/s/article/360005311513--Drupal-8-Acquia-Purge...

Busy sites should DISABLE the lateruntime processor, because it runs when users are logged in, it can increase load on your site(s).

And further emphasized by this issue to remove it from Acquia Purge - https://www.drupal.org/project/acquia_purge/issues/2925381 → .

The issue then becomes, on Acquia Cloud (non-classic), you can only configure cron at 5 minute intervals. So if you set up your p:queue-work to run at this minimum it will mean editors looking for their changes will have to wait a variable amount of time (up to 5 mins) to see the changes. For some sites this may not be an acceptable in which case you would need to set up custom purging from actions on the site - https://docs.acquia.com/acquia-cloud-platform/performance/varnish/manual.... It becomes a tradeoff:

1. Rely on late runtime processor and deal with some slow requests / performance issues + spikes
2. Rely on cron to purge the queue every 5 mins
3. Custom develop a solution to make BAN requests and invalidate necessary items in real time

Maybe there are more strategies than listed above, but that is where I have netted out after dabbling with this!

dan612 → created an issue.

Resource: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →

Started the work here. Need to actually run the tests in Gitlab CI, but locally am getting the following to pass:

$ ../vendor/bin/phpunit modules/contrib/jsonapi_permission_access/tests/src/Functional/JsonApiPermissionAccessTest.php --verbose -c ./core/phpunit.xml.dist
PHPUnit 9.6.19 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.15
Configuration: ./core/phpunit.xml.dist

Testing Drupal\Tests\jsonapi_permission_access\JsonApiPermissionAccessTest\JsonApiPermissionAccessTest
.                                                                   1 / 1 (100%)

Time: 00:04.327, Memory: 4.00 MB

OK (1 test, 10 assertions)

dan612 → created an issue.

assigning to me! will make updates and release new tag that includes the update from https://www.drupal.org/project/jsonapi_permission_access/issues/3425408 📌 Modify the package name as you've specified 'Package: Custom' Fixed

Looks like the pipeline is throwing a warning related to this:

[responsivewrappers] #3382402: package installability
FAILING 

This means that issue - https://www.drupal.org/project/responsivewrappers/issues/3382402 📌 Drupal 10 Version Release? Fixed - has been resolved. Not sure if this is considered a blocker for this issue or not. I broke that ticket out here 📌 D10: Update recommendation for responsivewrappers (drupal/responsivewrappers) Active

dan612 → created an issue.

dan612 → created an issue.

I took care of

1. this needs a reroll
2. Also: can you please create a "test MR"? 🙏

Will get going on the latter half of comment #8 📌 D10: Update recommendation for Bean (drupal/bean) Needs work

swapped out node_type for entity_bundle to see if that helps things along

I took what was in #67 🐛 Custom aliases (which are not generated with the actual patterns) can be lost during the migration Needs review and then changed out some things to avoid errors. The issue label conflicts with the patch name, at the moment...but I just wanted to see what this would do in CI.

trying something...

I'd like to call attention to this patch - https://www.drupal.org/project/pathauto/issues/3079275#comment-15271879 🐛 Custom aliases (which are not generated with the actual patterns) can be lost during the migration Needs review

I believe work from the other patches is now looped into this one. For example, the patch from this issue - https://www.drupal.org/project/pathauto/issues/3179835#comment-15474081 ✨ Migrate forum pattern to taxonomy term forums if forum is enabled on the source site Needs review - introduces a hook_migrate_process_info_alter() to do

+
+/**
+ * Implements hook_migrate_process_info_alter().
+ */
+function pathauto_migrate_process_info_alter(&$definitions) {
+  if (
+    \Drupal::moduleHandler()->moduleExists('taxonomy') &&
+    !empty($definitions['pathauto_forum_vocabulary'])
+  ) {
+    $definitions['pathauto_forum_vocabulary']['class'] = ForumVocabulary::class;
+  }
+}

This same snippet is in the patch from #67 🐛 Custom aliases (which are not generated with the actual patterns) can be lost during the migration Needs review .
. I spot checked a few other classes/code changes and they were all present in the "combined" patch.

So...I think this means we can omit the first 3 patches and just use #67 🐛 Custom aliases (which are not generated with the actual patterns) can be lost during the migration Needs review . However, that patch also will fail validation and need to be updated - for example the setUp functions lack a : void

Need to test more.

Attaching reroll with updates to tests to avoid the following errors:

PHP Fatal error:  Declaration of Drupal\Tests\pathauto\Kernel\Migrate\d7\MigratePathautoTest::setUp() must be compatible with Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase::setUp(): void in /app/docroot/modules/contrib/pathauto/tests/src/Kernel/Migrate/d7/MigratePathautoTest.php on line 91

Drupal\Tests\pathauto\Kernel\Migrate\d7\MigratePathautoTest::testPathautoMigrations with data set "Disabled forum on source site" (false)
Error: Call to undefined function Drupal\Tests\pathauto\Kernel\Migrate\d7\drupal_get_path()

Updated! Thanks - will cut a new tag soon.

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

Wim Leers → credited dan612 → .

There is quite a bit of red that we need to debug in the CI...but I think the critical point of emphasis is here:

Added a field group to ARTICLE content type in Drupal 7:

Here it is on the node add form:

Then I ran AM:A and completed the migrations, which resulted in the following on D10:

And the node add/edit form:

So this seems to be working pretty well! Next, to create a sibling MR.

dan612 → created an issue.

dan612 → created an issue.

Now CI can feel my pain as well 😁

  Problem 1
    - drupal/drupal[8.4.0-alpha1, ..., 8.7.x-dev] require composer/installers ^1.0.24 -> found composer/installers[v1.0.24, ..., 1.x-dev] but it conflicts with your root composer.json require (^2.0).
    - drupal/bean_migrate 1.0.0 requires drupal/core ^8.8 || ^9 -> satisfiable by drupal/drupal[8.4.8, 8.5.15, 8.6.18, 8.7.14].
    - Root composer.json requires drupal/bean_migrate 1.0.0 -> satisfiable by drupal/bean_migrate[1.0.0].

I installed the latest version of Bean on my Drupal 7 site and set up a new "Hero banner" block type:

Here it is on the front end attached to my home page Main Content region:

There is no newer release of Bean Migrate → and as such the recommendation should be the same. When I went to run the acli command I was met with the following:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - zendframework/zend-feed[dev-master, dev-develop, 2.11.0, ..., 2.13.x-dev] require php ^5.6 || ^7.0 -> your php version (8.1.27) does not satisfy that requirement.
    - laminas/laminas-feed[2.12.0, ..., 2.12.x-dev] require php ^5.6 || ^7.0 -> your php version (8.1.27) does not satisfy that requirement.
    - symfony/psr-http-message-bridge[v1.1.0, ..., 1.1.x-dev] require php ^5.3.3 || ^7.0 -> your php version (8.1.27) does not satisfy that requirement.
    - symfony/psr-http-message-bridge[v1.2.0, ..., v1.3.0] require php ^7.1 -> your php version (8.1.27) does not satisfy that requirement.
    - drupal/core[8.9.11, ..., 8.9.x-dev] require php ^7.0.8 -> your php version (8.1.27) does not satisfy that requirement.
    - laminas/laminas-feed[2.13.0, ..., 2.14.x-dev] require php ^7.3 || ~8.0.0 -> your php version (8.1.27) does not satisfy that requirement.
    - drupal/core[9.0.10, ..., 9.0.x-dev] require php ^7.3 -> your php version (8.1.27) does not satisfy that requirement.
    - Root composer.json requires drupal/bean_migrate 1.0.0 -> satisfiable by drupal/bean_migrate[1.0.0].
    - Root composer.json requires drupal/color ^1 -> satisfiable by drupal/color[1.0.1, 1.0.2, 1.0.3, 1.x-dev].
    - drupal/bean_migrate 1.0.0 requires drupal/core ^8.8 || ^9 -> satisfiable by drupal/core[8.8.0-alpha1, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev].
    - You can only install one version of a package, so only one of these can be installed: drupal/core[8.0.0-beta6, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.3.x-dev, 11.x-dev].
    - You can only install one version of a package, so only one of these can be installed: drupal/core[8.7.0-alpha1, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.3.x-dev, 11.x-dev].
    - drupal/core-recommended 10.2.3 requires drupal/core 10.2.3 -> satisfiable by drupal/core[10.2.3].
    - Only one of these can be installed: drupal/core[8.0.0-beta12, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.4.3, 10.0.0-alpha1, ..., 10.0.0-alpha6], drupal/color[1.0.1, 1.0.2, 1.0.3, 1.x-dev]. drupal/core replaces drupal/color and thus cannot coexist with it.
    - drupal/core[9.0.0-alpha1, ..., 9.0.0-alpha2] require symfony/psr-http-message-bridge ^1.2.0 -> satisfiable by symfony/psr-http-message-bridge[v1.2.0, v1.3.0].
    - drupal/core[8.9.0-beta1, ..., 8.9.10] require symfony/psr-http-message-bridge ^1.1.2 -> satisfiable by symfony/psr-http-message-bridge[v1.1.2, 1.1.x-dev, v1.2.0, v1.3.0].
    - drupal/core[8.8.0-alpha1, ..., 8.8.x-dev] require zendframework/zend-feed ^2.12 -> satisfiable by laminas/laminas-feed[2.12.0, ..., 2.23.x-dev], zendframework/zend-feed[2.12.0, 2.12.x-dev, 2.13.x-dev].
    - Root composer.json requires drupal/core-recommended 10.2.3 -> satisfiable by drupal/core-recommended[10.2.3].

It appears this module cannot be installed via composer at the moment without Composer Lenient using this patch 📌 Drupal 10 compatibility Needs work

🤔 not sure what to do here. Is this hard blocked until the Bean Migrate composer.json file is updated?

dan612 → created an issue.

Well...I got it to be green but I am not sure if it is quite working as I see this in the output:

Uploading artifacts...
recommendations-next-*.json: found 1 matching artifact files and directories 
WARNING: recommendations-pinned-issue-fork/3424401-update-pathauto.json: no matching files. Ensure that the artifact path is relative to the working directory (/builds/issue/acquia_migrate-3424401) 

So I think changing the filename was OK to get past the first part, but now when we build the artifact to export it with the same name because when we try to run ACLI later on we encounter this:

🤖 Scanning Drupal 7 site.
👍 Found Drupal 7 site (7.82 to be precise) at sites/default, with 28 modules enabled!
In NewFromDrupal7Command.php line 83:
                                                                               
  /builds/issue/acquia_migrate-3424401/recommendations-pinned-*.json could no  
  t be located. Check that the path is correct and try again.   

I was trying to get this to run locally but am encountering this:

www-data@2030d29ffb9a:/app/docroot/core$ ../../vendor/bin/phpunit ../modules/contrib/acquia_migrate/tests/src/FunctionalJavascript/PathautoMigrationTest.php -vvv
PHPUnit 9.6.17 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.16
Configuration: /app/docroot/core/phpunit.xml

Testing Drupal\Tests\acquia_migrate\FunctionalJavascript\PathautoMigrationTest
S                                                                   1 / 1 (100%)

Time: 01:09.909, Memory: 10.00 MB

There was 1 skipped test:

1) Drupal\Tests\acquia_migrate\FunctionalJavascript\PathautoMigrationTest::testPathautoMigrations
The test wasn't able to connect to your webdriver instance. For more information read core/tests/README.md.

The original message while starting Mink: Could not open connection: Curl error thrown for http POST to http://localhost:4444/session with params: {"desiredCapabilities":{"browserName":"chrome","name":"Behat Test"}}

Failed to connect to localhost port 4444: Connection refused

/app/docroot/core/tests/Drupal/FunctionalJavascriptTests/WebDriverTestBase.php:58
/app/docroot/core/tests/Drupal/Tests/BrowserTestBase.php:372
/app/docroot/modules/contrib/acquia_migrate/tests/src/FunctionalJavascript/PathautoMigrationTest.php:52
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

OK, but incomplete, skipped, or risky tests!
Tests: 1, Assertions: 1, Skipped: 1.

Which I believe is a problem with my local setup. I did have chromedriver running but not inside the container which I think was the ultimate issue. That might take a while to sort out, so perhaps easier to just test in the CI 😇

After adding the snippet and regenerating the recommendations file, I ran the acli command and was met with:

  - Applying patches for drupal/pathauto
    https://www.drupal.org/files/issues/2022-01-06/pathauto-migrate_forum_pattern_if_forum_is_enabled_on_source-3179835-22--complete.patch ([AMA_MIGRATE_FIX] Issue #3179835: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2022-01-06/pathauto-migrate_forum_pattern_if_forum_is_enabled_on_source-3179835-22--complete.patch
    https://www.drupal.org/files/issues/2021-04-06/pathauto-derive_pathauto_migrations_per_entity_type-3179865-25.patch ([AMA_MIGRATE_FIX] Issue #3179865: Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2021-04-06/pathauto-derive_pathauto_migrations_per_entity_type-3179865-25.patch
    https://www.drupal.org/files/issues/2022-01-06/pathauto-migrate_multilingual_patterns-3182708--12.patch ([AMA_MIGRATE_FIX] Issue #3182708: Migrate language-specific patterns)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2022-01-06/pathauto-migrate_multilingual_patterns-3182708--12.patch
    https://www.drupal.org/files/issues/2022-01-06/pathauto-prevent_losing_custom_aliases-3079275-27--combined--do-not-test.patch ([AMA_MIGRATE_FIX] Issue #3079275: Custom aliases (which are not generated with the actual patterns) can be lost during the migration)
    https://www.drupal.org/files/issues/2021-01-05/3190980-2.patch ([AMA_MIGRATE_FIX] Issue #3190980: [PP-1] Allow source counts to be cached: implement ::doCount() instead of ::count())
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2021-01-05/3190980-2.patch

Issue #1:

  - Applying patches for drupal/pathauto
    https://www.drupal.org/files/issues/2022-01-06/pathauto-migrate_forum_pattern_if_forum_is_enabled_on_source-3179835-22--complete.patch ([AMA_MIGRATE_FIX] Issue #3179835: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2022-01-06/pathauto-migrate_forum_pattern_if_forum_is_enabled_on_source-3179835-22--complete.patch

This has a newer patch. Altering reference to https://www.drupal.org/files/issues/2023-05-08/pathauto-n3179835-25.patch → and trying again did not help the situation. I think this needs a complete re-roll. Did that here https://www.drupal.org/project/pathauto/issues/3179835#comment-15466623 ✨ Migrate forum pattern to taxonomy term forums if forum is enabled on the source site Needs review and now everything installs cleanly. Interesting...I thought I would have to re-roll a few of these but I guess the other patches must rely on the first.

www-data@2030d29ffb9a:/app/docroot/core$ ../../vendor/bin/phpunit --list-groups
PHP Fatal error:  Declaration of Drupal\Tests\pathauto\Kernel\Migrate\d7\MigratePathautoTest::setUp() must be compatible with Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase::setUp(): void in /app/docroot/modules/contrib/pathauto/tests/src/Kernel/Migrate/d7/MigratePathautoTest.php on line 91

Fatal error: Declaration of Drupal\Tests\pathauto\Kernel\Migrate\d7\MigratePathautoTest::setUp() must be compatible with Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase::setUp(): void in /app/docroot/modules/contrib/pathauto/tests/src/Kernel/Migrate/d7/MigratePathautoTest.php on line 91

I think this is something straightforward...like we are missing a void declaration in our function signatures..I updated that file locally and then hit the same issue in a number of different files:

1. PathautoPatternLabelTest
2. PathautoSourceTestBase
3. PathautoMigrateUiTest

After that was met with:

www-data@2030d29ffb9a:/app/docroot/core$ ../../vendor/bin/phpunit
PHP Fatal error:  Declaration of Drupal\Tests\pathauto\Kernel\Plugin\migrate\source\PathautoSourceTestBase::testSource(array $source_data, array $expected_data, $expected_count = null, array $configuration = [], $high_water = null) must be compatible with Drupal\Tests\migrate\Kernel\MigrateSqlSourceTestBase::testSource(array $source_data, array $expected_data, $expected_count = null, array $configuration = [], $high_water = null, $expected_cache_key = null) in /app/docroot/modules/contrib/pathauto/tests/src/Kernel/Plugin/migrate/source/PathautoSourceTestBase.php on line 162

I think this function is missing $expected_cache_key = []. I added that...then could finally run ../../vendor/bin/phpunit --group=pathauto

1. update reroll in https://www.drupal.org/project/pathauto/issues/3179835#comment-15466623 ✨ Migrate forum pattern to taxonomy term forums if forum is enabled on the source site Needs review to include new function signatures
2. update patch here - https://www.drupal.org/project/pathauto/issues/3179865 → - which needs signature updates as well
3. investigate issues with phpunit tests

Output of phpunit attached here → .

Patch is failing to apply for me on Drupal 10.2.3 + 8.x-1.12 -- rerolling

dan612 → created an issue.

Made some updates 🥹 - we also had a pipelines failure due to an issue getting resolved - https://www.drupal.org/project/monster_menus/issues/3422059 🐛 Cannot install module on any version of Drupal 10 other than 10.0.0 Fixed . So the system is working! 🎉

I think i covered all issues, but LMK if i missed something 😎

I also think that I found a bug in the crawler that was stopping crawling ~400 pages in, when there are over 650 pages of results to iterate through. Probably best to keep that in the original parent issue and break out another update once possible...assuming this one is OK.

dan612 → created an issue.

attaching a basic file to get started. i believe using this will at least stop the warnings/error during a migration and will allow this new field type to be set, but i believe we need to define how the underlying data gets mapped to the new field as well.

dan612 → created an issue.

made an MR to show i am still noodling on this 😬

I re-ran the make -B command, then my process was as follows:

1. Run sh ./scripts/test-unvetted-modules-composer-installability.sh recommendation.json /tmp/test-general-installability-unvetted 10.2.3
2. Go to new directory and run composer install --dry-run
3. Move each failure to curated.json with a note as to why it fails to install, and vetted: false
4. Run composer remove <package> then re-run composer install --dry-run

Basically just worked through all the various failures during composer installation, went to the projects to review why it failed and then updated the note with the findings. Then my pipelines jobs started to fail due to PHP 8.2 incompatibility - so I made a few more updates to address that after changing versions and testing again locally

dan612 → created an issue.

dan612 → created an issue.

dan612 → created an issue.

dan612 → created an issue.

dan612 → created an issue.

dan612 → created an issue.

😁 i did! however I am not sure we are talking about the same issue. I encountered 2 types of issues when working on this:

1. Issues where version constraints differed between curated.json and generated.json -- as you described in your previous comment, these were mostly submodules and easy to fix by manually changing the version.
2. Modules which get picked up by generated.json but are not installable via composer

The second issue above is where this is stuck, none of these modules - https://www.drupal.org/files/issues/2024-02-10/d10-composer-install-brea... → - were able to be added to a D10 project via composer, cleanly. For example drupal/field_properties:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires drupal/field_properties ^1.0@alpha -> satisfiable by drupal/field_properties[1.0.0-alpha2].
    - drupal/field_properties 1.0.0-alpha2 requires drupal/core ^8 -> found drupal/core[8.0.0-beta6, ..., 8.9.x-dev] but the package is fixed to 10.2.3 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.

or smartmenus:

[6:53:43] ~/Sites/d7-upgrades/the18-d10 [main] $ lando composer require 'drupal/smartmenus:^2.0@beta'
./composer.json has been updated
Running composer update drupal/smartmenus
Gathering patches for root package.
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires drupal/smartmenus ^2.0@beta -> satisfiable by drupal/smartmenus[2.0.1-beta1, 2.0.1-beta2].
    - drupal/smartmenus[2.0.1-beta1, ..., 2.0.1-beta2] require drmonty/smartmenus 1.1.1 -> found drmonty/smartmenus[dev-master, v0.9.6, v0.9.7, 1.0.0, 1.0.1, 1.1.0] but it does not match the constraint.

So for these...i feel like we would have to vet each on against D10 and provide a curated recommendation in order for this to be complete.

To be honest a lot of these packages look as though they are in alpha/beta and dont have a stable release to begin with...which makes me wonder why the crawler script was even picking them up because it should only get D10 packages when:

 * For the given project, try to derive a D7-D10 migration recommendation.
 *
 * Projects are ignored if:
 *
 * - Their owner has declared that it has been replaced.
 * - Their status is anything other than receiving maintenance fixes only or
 *   under active development (e.g. not obsolete).
 * - They do not have a Drupal 7 release.
 * - They do not have a Drupal 10 release.

the above snippet was taken from crawl.php

Hi - thank you for the update! 😁

I think since focal_point_focus → is now showing compatibility with Drupal 10 we should break it out into a separate issue where we can manually vet/test the path from D7 --> D10 and update our recommendation based on the results. That way we can update the recommendation & merge in without relying on completion of this issue - did that and can update status in that issue!

dan612 → created an issue.

dan612 → created an issue.

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

ooooh, ok! i ran this instead ... git diff origin/4.x -- ':!./tests/*' > ~/Desktop/3416551-change-settings-fix-only.patch and attached that 😄

In order to install this module with composer you must add the following to your project composer.json prior to requiring this module

       {
            "type": "package",
            "package": {
                 "name": "contextly/contextly-kit",
                 "version": "5.0.10",
                 "type": "drupal-library",
                 "dist": {
                      "url": "https://assets.context.ly/archives/kit/contextly-kit-5.latest.tar.gz",
                      "type": "tar"
                 },
                 "autoload": {
                      "classmap": [
                              "server/",
                              "server/includes/",
                              "server/includes/assets/",
                              "server/includes/package/"
                      ]
                 }
            }
        }

See this part of the readme.

bat has a pretty robust ecosystem that will require substantial testing. Now that auth0 recommendation has landed 📌 D10: Re-vet Auth0 (drupal/auth0) Fixed , I think my inclination is to merge what we have in here but leave this and the bat issue open as there are higher priority modules to vet. What do you think? 🥹

Pipelines is quite angry with me but im not sure if that much noise is normal or not 😬

Leave it to me to not read instructions 😆

There were a few places where "modern Drupal" didn't fit...it read awkwardly. I tried to update things within context to what seemed fit 😀

Couple screenshots of the changed ui:

This patch is failing to apply for me locally for some reason, not sure why. The part which is failing is related to the additions in the test fixture:

diff --git a/tests/fixtures/drupal7.php b/tests/fixtures/drupal7.php
index 630b6d0..9b8a86d 100644
--- a/tests/fixtures/drupal7.php
+++ b/tests/fixtures/drupal7.php
@@ -82,6 +82,18 @@ $connection->insert('variable')
     'name' => 'acquia_subscription_data',
     'value' => 'a:12:{s:9:"timestamp";i:1234567890;s:6:"active";s:1:"1";s:4:"href";s:83:"https://insight.acquia.com/node/uuid/1b2c3456-a123-456d-a789-e1234567895d/dashboard";s:4:"uuid";s:36:"1b2c3456-a123-456d-a789-e1234567895d";s:17:"subscription_name";s:4:"Test";s:15:"expiration_date";a:1:{s:5:"value";s:19:"2042-12-30T00:00:00";}s:7:"product";a:1:{s:4:"view";s:14:"Acquia Network";}s:16:"derived_key_salt";s:32:"1234e56789979a1d8ae123cd321a12c7";s:14:"update_service";s:1:"1";s:22:"search_service_enabled";i:1;s:6:"update";a:0:{}s:14:"heartbeat_data";a:4:{s:11:"acquia_lift";a:3:{s:6:"status";b:0;s:8:"decision";a:2:{s:10:"public_key";s:0:"";s:11:"private_key";s:0:"";}s:7:"profile";a:5:{s:12:"account_name";s:0:"";s:8:"hostname";s:0:"";s:10:"public_key";s:0:"";s:10:"secret_key";s:0:"";s:7:"js_path";s:0:"";}}s:22:"search_service_enabled";i:1;s:12:"search_cores";a:7:{i:0;a:2:{s:8:"balancer";s:28:"useast1-c1.acquia-search.com";s:7:"core_id";s:11:"TEST-123456";}i:1;a:2:{s:8:"balancer";s:28:"useast1-c1.acquia-search.com";s:7:"core_id";s:19:"TEST-123456.prod.v2";}i:2;a:2:{s:8:"balancer";s:28:"useast1-c1.acquia-search.com";s:7:"core_id";s:19:"TEST-123456.test.v2";}i:3;a:2:{s:8:"balancer";s:28:"useast1-c1.acquia-search.com";s:7:"core_id";s:18:"TEST-123456.dev.v2";}i:4;a:2:{s:8:"balancer";s:29:"useast1-c26.acquia-search.com";s:7:"core_id";s:24:"TEST-123456.prod.default";}i:5;a:2:{s:8:"balancer";s:29:"useast1-c26.acquia-search.com";s:7:"core_id";s:24:"TEST-123456.test.default";}i:6;a:2:{s:8:"balancer";s:29:"useast1-c26.acquia-search.com";s:7:"core_id";s:23:"TEST-123456.dev.default";}}s:21:"search_service_colony";s:28:"useast1-c1.acquia-search.com";}}',
   ])
+  ->values([
+    'name' => 'acquia_application_uuid',
+    'value' => 's:36:"297dddeb-a730-422d-bf87-f0fb4b0eaa31";',
+  ])
+  ->values([
+    'name' => 'acquia_identifier',
+    'value' => 's:10:"ABCD-12345";',
+  ])
+  ->values([
+    'name' => 'acquia_key',
+    'value' => 's:32:"0db2c95529d76edfc282e9eed0800b21";',
+  ])
   ->execute();

 $connection->insert('system')

Not sure what about this is problematic. The message from patch output is:

patch '-p1' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
Executing command (CWD): patch '-p1' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/tests/fixtures/drupal7.php b/tests/fixtures/drupal7.php
|index 630b6d0..9b8a86d 100644
|--- a/tests/fixtures/drupal7.php
|+++ b/tests/fixtures/drupal7.php
--------------------------
File to patch:
Skip this patch? [y]
Skipping patch.

1 out of 1 hunk ignored

patch '-p0' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
Executing command (CWD): patch '-p0' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/tests/fixtures/drupal7.php b/tests/fixtures/drupal7.php
|index 630b6d0..9b8a86d 100644
|--- a/tests/fixtures/drupal7.php
|+++ b/tests/fixtures/drupal7.php
--------------------------
File to patch:
Skip this patch? [y]
Skipping patch.

1 out of 1 hunk ignored

patch '-p2' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
Executing command (CWD): patch '-p2' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/tests/fixtures/drupal7.php b/tests/fixtures/drupal7.php
|index 630b6d0..9b8a86d 100644
|--- a/tests/fixtures/drupal7.php
|+++ b/tests/fixtures/drupal7.php
--------------------------
File to patch:
Skip this patch? [y]
Skipping patch.

1 out of 1 hunk ignored

patch '-p4' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
Executing command (CWD): patch '-p4' --no-backup-if-mismatch -d 'docroot/modules/contrib/acquia_connector' < '/app/patches/breaking-test.patch'
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/tests/fixtures/drupal7.php b/tests/fixtures/drupal7.php
|index 630b6d0..9b8a86d 100644
|--- a/tests/fixtures/drupal7.php
|+++ b/tests/fixtures/drupal7.php
--------------------------
File to patch:
Skip this patch? [y]
Skipping patch.

1 out of 1 hunk ignored

   Could not apply patch! Skipping. The error was: Cannot apply patch ./patches/breaking-test.patch

The only info i can really glean from that is can't find file to patch at input line 5 - but i have no idea why this would be the case.

#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.

Oh! Whoopsies, good find 😬 After making these changes I altered the tests a little bit and tried running them. I was met with:

  www-data@2fdc9bf0644d:/app/docroot/core$ ../../vendor/bin/phpunit  ../modules/contrib/acquia_connector/tests/src/Kernel/Migrate/d7/MigrateAcquiaConnectorConfigurationTest.php
  PHPUnit 9.6.16 by Sebastian Bergmann and contributors.

  Testing Drupal\Tests\acquia_connector\Kernel\Migrate\d7\MigrateAcquiaConnectorConfigurationTest
  E                                                                   1 / 1 (100%)R

  Time: 00:09.830, Memory: 10.00 MB

  There was 1 error:

  1) Drupal\Tests\acquia_connector\Kernel\Migrate\d7\MigrateAcquiaConnectorConfigurationTest::testConfigurationMigration
  PHPUnit\Framework\Exception: PHP Fatal error:  Uncaught AssertionError: The container was serialized. in /app/docroot/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php:88
  Stack trace:
  #0 /app/docroot/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php(88): assert(false, 'The container w...')
  #1 [internal function]: Drupal\Core\DependencyInjection\ContainerBuilder->__sleep()
  

I was able to get around this by implementing the same solution from here - https://www.drupal.org/project/drupal/issues/3197324 🐛 Exception trace cannot be serialized because of closure Needs work .

I made a few updates to the MR which i believe also allows it to have a passing test now 😎

  www-data@2fdc9bf0644d:/app/docroot/core$ ../../vendor/bin/phpunit  ../modules/contrib/acquia_connector/tests/src/Kernel/Migrate/d7/MigrateAcquiaConnectorConfigurationTest.php
  PHPUnit 9.6.16 by Sebastian Bergmann and contributors.

  Testing Drupal\Tests\acquia_connector\Kernel\Migrate\d7\MigrateAcquiaConnectorConfigurationTest
  .                                                                   1 / 1 (100%)

  Time: 00:08.521, Memory: 10.00 MB

  OK (1 test, 7 assertions)
  

Do you mind explaining more about how I might use the minimum_version here? My conditions are:
if >= 7004, run variables to state
if < 7004, run prior config migration
ill need to ponder more about how to accomplish this with annotation based 🫣

Currently we have 171 generated recommendations. After this there are 1423 generated recommendations.

I realized this morning that what I was doing in my previous efforts was just re-vetting all those modules which failed to install cleanly...which is what we want to do anyway but there are probably higher priorities 😄. that being said, i think i should break some of my comments in #7 📌 D10: Run crawler script to update generated.json Active into tickets as the work (or groundwork) is already done now.

New idea. What if we just omit all the d10-install-breaking-modules from generated.json? This would mean we still get an additional ~1,200 modules which install successfully on D10 and it would unblock this issue, which relies on basically vetting every module which fails to cleanly install via composer.

Locally I reset things back to recommendations-10, re-ran make -B and cofirmed the mass update in generated.json. At this point my statistics showed:

$ cat ./statistics.txt                                                                       
410 recommendations (curated)
  (of which 364 contrib recommendations)
1480 recommendations (generated)
1790 recommendations (total)
  (of which 89 vetted recommendations)
    (destination: targeting 73 composer packages)
    (destination: targeting 121 Drupal modules)
    (source: migrating from 231 Drupal modules)
    (source: obsoleting 63 Drupal modules)
  (of which 63 obsolete recommendations)
  (of which 3 patched recommendations)
  (of which ~ 310 generated recommendations overridden by curated recommendations)
41 unique patches, or 13.7 (avg) per patched recommendation for a total of 41 (36 max)
  (of which 3 patches are not migration-specific, but necessary bugfixes)

Then i added the modules i knew from previous efforts to be breaking to a new file, iterated over it and removed those from generated.json. Then re-ran the CI process locally to make sure things worked. Whenever there was a failure, i removed it from generated.json and tried again. I attached the list of packages which caused failure → . The result appears to be a passing CI pipeline now 🎉. I will leave it up for discussion what other implications this has and if it is a good idea or not 😬

I believe it is an improvement 🤞 because the majority of these modules should work on Drupal 10 (considering they have a D10 compatible release tagged). So even though we haven't tested them or vetted migration paths, i believe it makes things easier for the end user because they will come preinstalled (but not enabled) if they had them on Drupal 7.

dan612 → changed the visibility of the branch 3420498-update-generated to hidden.