🇺🇸United States @benjifisher

I feel the same way. Thanks for all your contributions!

@kul.pratap:

Thanks for making the changes I suggested!

I see that you have several contribution credits already, but none yet for Drupal core. Maybe this will be your first. Good luck, and I hope you keep contributing!

I reviewed the changes in the MR, and they do what I requested in the issue summary. These changes are all on comment lines (the class doc block) so there is no need for testing.

Once these changes are merged into the 11.x branch, we should see them in the API docs: https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Plugi....

quietone → credited benjifisher → .

benjifisher → created an issue.

I added 📌 Improve separation of concerns in SqlBase Active , so I am removing the tag for a followup issue.

benjifisher → created an issue.

The MR looks good to me. The test coverage is great, but I have a few suggestions to polish it a little.

I ran the test locally (with my suggested changes) and it passed. When I reverted the changes to SqlBase, the test failed as expected.

@heddn, I think this issue is close enough to RTBC that you can go ahead and draft the change record.

We discussed this issue at 📌 [meeting] Migrate Meeting 2025-01-30 2100Z Active . We agreed that the changes suggested in Comment #46 should be done in a separate issue, so I am adding the issue tag for that and I am starting to review the MR here.

We (@benjifisher, @godotislate, @mikelutz, and @quietone, with some input from @larowlan) discussed this issue on Slack (https://drupal.slack.com/archives/C226VLXBP/p1736647603190789).

Despite Comment #60, we agreed to convert annotations to attributes first, and then to deal with source_module, so I am postponing this issue on 📌 Convert MigrateSource plugin discovery to attributes Active . Once that issue is done, we will have to completely rewrite the MR here, but I think that will not be hard. We have already done the work of deciding what has to be done, and all we will have to do is re-implement it.

When I am feeling really optimistic, I think that if we can get both issues done before 11.2.0 is released, then we can skip the deprecation step. If source_module never exists as an attribute in a release of Drupal core, do we have to deprecate it?

Oops: I meant to update 📌 [meeting] Migrate Meeting 2025-01-30 2100Z Active , not this issue.

I think I will swap the titles.

I reviewed the latest changes, and I think all the requested changes have been made.

Really minor point (not enough to hold up this issue, in my opinion): I would have changed (shortened) the namespace of the test instead of moving it (and creating two nested directories with no other contents).

I do not care much either way, but perhaps we should leave the trait in the content_translation module intact (but still deprecate it) instead of making it a wrapper for the new version of the trait. The only reason for doing that is so that we can add type declarations to the new code. This is the current approach in [$#3498915].

There is one remaining question, from @quietone:

Can the error be generated without using the Migrate UI?

It was hard to get a suitable test for this issue: I am still not sure why my STR do not generate an error in a test. @quietone created the first version of the current test, as a Functional test using migrate_drupal_ui. (See Comments #36, #38.)

Ideally, we should be able to write the test without using migrate_drupal_ui, but it does not look easy. There is a lot of logic in CredentialForm::validateForm() and in other methods in that form class, such as setupMigrations().

The test shows us that the man code is not well structured, but fixing that is out of scope for this issue and probably not worth the effort since migrate_drupal is going away "soon". This issue is holding up 📌 Convert MigrateSource plugin discovery to attributes Active and related issues, so I vote for leaving the test as is.

I re-reviewed the changes in the MR: back to RTBC.

In #3498915-14: Move content_entity source plugin to migrate module → , I noted several failing tests:

1. Drupal\Tests\jsonapi\Functional\JsonApiFunctionalTest
2. Drupal\Tests\file\Functional\DownloadTest
3. Drupal\Tests\package_manager\Build\PackageInstallTest
4. Drupal\Tests\package_manager\Build\PackageInstallSubmoduleTest
5. Drupal\Tests\package_manager\Build\PackageUpdateTest
6. tabbingManagerTest.js (I think: certainly one of the Nightwatch tests)

I see there is already a child issue 🐛 [random test failure] EditorSecurityTest::testEditorXssFilterOverride Active , but I do not see the others mentioned here. All these tests passed when I re-ran them.

Failing tests:

1. Drupal\Tests\jsonapi\Functional\JsonApiFunctionalTest
2. Drupal\Tests\file\Functional\DownloadTest
3. Drupal\Tests\package_manager\Build\PackageInstallTest
4. Drupal\Tests\package_manager\Build\PackageInstallSubmoduleTest
5. Drupal\Tests\package_manager\Build\PackageUpdateTest
6. tabbingManagerTest.js (I think: certainly one of the Nightwatch tests)

I am re-running the tests, and I will add a comment to 🌱 [meta] Known intermittent, random, and environment-specific test failures Active .

I resolved the new "unused parameter" error after discussing on Slack with @nicxvan and @mstrlan.

@mondrake, thanks for the comments on the MR. I replied to all three, even though I only made the changes suggested in one of them. I am adding credit to you for the review.

I updated the Proposed resolution in the issue summary: we decided not to make the old classes wrappers for the new ones (because we are adding type declarations that could break BC).

I still wonder whether I should remove core/modules/migrate_drupal/tests/src/Kernel/Plugin/migrate/source/ContentEntityConstructorTest.php, which tests the now-deprecated class. The MR copies that test class to core/modules/migrate/tests/src/Kernel/Plugin/source/ContentEntityConstructorTest.php.

@lauriii:

Thanks for the link to the user-test script.

Making a prototype will be easy. Coming up with good interface text is hard, and that is what the usability team has been working on. Organizing user tests is also hard, and it will be great if you can manage that step.

@lauriii:

Can you give a link to the script used for user testing and the results? If one of the tasks was to attach a media reference field to a content type, then it is not surprising that putting the option at the top level tested well. It is like a self-fulfilling prophecy. Did the test also ask users to attach a non-reusable image field?

The key difference between Image media and Image (file) fields is that media are reusable, and image fields are not. There are use cases for both. If someone chooses Media, reads the rather long description, and decides that they actually want an image field, then they have to back up, figure out that "File upload" is the right group, and choose that. It would be much easier if, at the time they read about the distinction, the other option were right there.

The usability team has spent a lot of effort on #3346539 recently. Any input there is very welcome. We added this issue because the suggestion came up while we were working on that issue, and we wanted to keep the discussion focused on the descriptions.

I know there was previous discussion of this decision. We should find and review that discussion before moving forward with this issue.

At the usability meeting, we also noticed that there is another path to add a media reference: on the first step of the form, choose Reference; then select Other; then choose Media from the "Type of item to reference" select list.

The media module lists the file module as a dependency, so we do not have to worry about the case where Media is available but File upload is not.

benjifisher → created an issue.

We continued the word-smithing at 📌 Drupal Usability Meeting 2025-01-17 Active : @aaronmchale, @benjifisher, @rkoller, @simohell, and @worldlinemine.

@nicxvan:

I did the following:

1. Restore the classes in the migrate_drupal module (but keep the deprecation notices).
2. Revert the changes to the phpstan baseline file.
3. In the new classes (in the migrate module), fix the errors from the phpstan baseline file.

Was I right to restore the test class in the migrate_drupal module? Maybe I should delete it again, along with the corresponding lines in the phpstan baseline file. I did not restore the test that refers to the source plugin by its plugin ID instead of by its class: that no longer tests the deprecated class in the migrate_drupal module.

Since we are giving up on keeping the new classes as close as possible to the old ones, I made some further modernization:

• Add parameter and return-type declarations.
• Use constructor promotion.

I had some trouble with phpstan. It is tricky when tests implement methods without declaring return types, for example.

If the tests pass, then I will move this issue back to NR.

benjifisher → created an issue.

@heddn:

In Comment #166, I wrote,

I resolved several threads. I left open the ones where I made a suggestion and it has not been implemented. I still think these are improvements.

and in #168,

I still think it is a bad idea to cast to string in EntityContentBase::fields(), and some of my other suggestions would be improvements. But I will not insist on those changes, so RTBC.

I think you may have resolved some threads that I intentionally left open.

benjifisher → created an issue.

I opened this issue after reviewing 🐛 Mark the XB components as experimental, not stable Active , which refers to https://git.drupalcode.org/project/experience_builder/-/blob/0.x/docs/co.... That doc says, in part,

3.1.1 Criteria for SDC components
For an SDC to be compatible/eligible for use in XB, it:

MUST always have schema, even for theme SDCs

Looking at core/lib/Drupal/Core/Theme/Component/ComponentValidator.php, I see that SDC allows an empty metadata file, but it looks as though XB does not.

Even without that, I think the reason in #5 is enough: add at least a name and description in order to avoid confusion.

Yes, I think a stub migration in migrate_sandbox/migrations/ and then adding the process pipelines in migrate_sandbox_migration_plugins_alter() is how I would try to do it.

I have never checked: does the current implementation avoid creating map and message tables?

Correction: the existing test was broken by the first commit on this issue.

The problem is that the test was asserting the specific test "From great ideas to great digital experiences". As this issue shows, that test is too brittle.

In a performance test, the only reason for an assertion like that is to make sure that we are on the page we intend to test. I replaced that assertion with two assertions on the structure:

1. the <body> tag has the CSS classes user-logged-in and path-front[age
2. there is an <article> tag with the CSS class node.

Oh, no: locally, I get this when I run the test locally:

1) Drupal\Tests\drupal_cms_starter\FunctionalJavascript\PerformanceTest::testPerformance
WebDriver\Exception\UnknownError: Unable to execute request for an existing session: Unable to find session with ID: 8e68b0652dc3dcee3580b52647a21121

Since I have to close up my laptop, I will push the commit anyway to see what happens on CI. I will check in when I get home.

A little birdie whispered in my ear that the already-committed test for this issue should be cleaned up a little bit, so I am setting the status back to NW and assigning to myself.

In order to manage scope, I added 📌 Search components need valid .component.yml files Active . I am moving this issue back to RTBC.

Those files were all added in 📌 Provide default styles for search Active . I am adding that as a related issue.

benjifisher → created an issue.

I am reviewing this issue at the Boston Drupal meetup. @phenaproxima, @pameela, and @dinarcon are here via Zoom. @mosheweitman, @charginghawk, and I are in the room, licking pizza off our fingers.

According to https://www.drupal.org/docs/develop/theming-drupal/using-single-director... → , SDC component metadata are in components/*.component.yml. I can search for that:

$ find . -type d -name components
./drupal_cms_olivero/components
./drupal_cms_olivero/css/components
$ find . -name '*.component.yml'
./drupal_cms_olivero/components/search-narrow/search-narrow.component.yml
./drupal_cms_olivero/components/paragraph/paragraph.component.yml
./drupal_cms_olivero/components/search-wide/search-wide.component.yml
./drupal_cms_olivero/components/heading/heading.component.yml

There are only CSS files in the css/components/ directory.

I looked at the four YAML files:

• search-narrow.component.yml: This file is empty (0 bytes). Can we delete it? Do I have to open a new issue to do that?
• paragraph.component.yml: This one already has status: experimental.
• search-wide.component.yml: This looks like a .libraries.yml file, not a .component.yml file.
• drupal_cms_olivero/components/heading/heading.component.yml: This is the one fixed in the MR for this issue.

For convenience, here is the full content of search-wide.component.yml:

libraryOverrides:
  dependencies:
    - core/drupal.autocomplete

I am not sure about .libraries.yml, but it is not a .component.yml file.

So the MR LGTM, and the only question is whether we need an issue for the two files that are not actually component metadatra. (Is there already an issue? Can we sneak the fixes in here or in some other issue?)

The CR was unpublished because I got confused by the base field and the regular boolean field, both of which (if you have access to them) are labeled "Published". I think you should be able to see it, now.

I suppose we do not have to make the old class a wrapper for the new one. We could just add the deprecation notice (in the class docblock and in the constructor) and remove the plugin annotations. Then we could add return-type declarations in the new class and it would have no effect on any code that extends the old class.

Good news: the test was slow, or I was impatient:

PHPUnit 10.5.40 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.14
Configuration: /var/www/html/web/core/phpunit.xml.dist

D                                                                   1 / 1 (100%)

HTML output was generated.
https://drupal-cms-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_drupal_cms_starter_FunctionalJavascript_PerformanceTest-1-52365397.html
https://drupal-cms-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_drupal_cms_starter_FunctionalJavascript_PerformanceTest-2-52365397.html
https://drupal-cms-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_drupal_cms_starter_FunctionalJavascript_PerformanceTest-3-52365397.html
https://drupal-cms-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_drupal_cms_starter_FunctionalJavascript_PerformanceTest-4-52365397.html
https://drupal-cms-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_drupal_cms_starter_FunctionalJavascript_PerformanceTest-5-52365397.html
https://drupal-cms-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_drupal_cms_starter_FunctionalJavascript_PerformanceTest-6-52365397.html
https://drupal-cms-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_drupal_cms_starter_FunctionalJavascript_PerformanceTest-7-52365397.html


Time: 03:39.369, Memory: 10.00 MB

1 test triggered 4 deprecations:

1) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:341
The "Drupal\easy_breadcrumb\EasyBreadcrumbBuilder::applies()" method will require a new "\Drupal\Core\Cache\CacheableMetadata $cacheable_metadata" argument in the next major version of its interface "Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface", not defining it is deprecated.

Triggered by:

* Drupal\Tests\drupal_cms_starter\FunctionalJavascript\PerformanceTest::testPerformance
  /var/www/html/recipes/drupal_cms_starter/tests/src/FunctionalJavaScript/PerformanceTest.php:32

2) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:341
Method "Symfony\Component\EventDispatcher\EventSubscriberInterface::getSubscribedEvents()" might add "array" as a native return type declaration in the future. Do the same in implementation "Drupal\autosave_form\EventSubscriber\ConfigSubscriber" now to avoid errors or add an explicit @return annotation to suppress this message.

Triggered by:

* Drupal\Tests\drupal_cms_starter\FunctionalJavascript\PerformanceTest::testPerformance
  /var/www/html/recipes/drupal_cms_starter/tests/src/FunctionalJavaScript/PerformanceTest.php:32

3) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:341
Method "Symfony\Component\EventDispatcher\EventSubscriberInterface::getSubscribedEvents()" might add "array" as a native return type declaration in the future. Do the same in implementation "Drupal\focal_point\EventSubscriber\MigrationSubscriber" now to avoid errors or add an explicit @return annotation to suppress this message.

Triggered by:

* Drupal\Tests\drupal_cms_starter\FunctionalJavascript\PerformanceTest::testPerformance
  /var/www/html/recipes/drupal_cms_starter/tests/src/FunctionalJavaScript/PerformanceTest.php:32

4) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:341
Method "IteratorAggregate::getIterator()" might add "\Traversable" as a native return type declaration in the future. Do the same in implementation "Drupal\crop\Entity\CropType" now to avoid errors or add an explicit @return annotation to suppress this message.

Triggered by:

* Drupal\Tests\drupal_cms_starter\FunctionalJavascript\PerformanceTest::testPerformance
  /var/www/html/recipes/drupal_cms_starter/tests/src/FunctionalJavaScript/PerformanceTest.php:32

OK, but there were issues!
Tests: 1, Assertions: 27, Deprecations: 4.

I tested by removing the DDEV containers and everything not tracked by Git:

ddev stop # probably redundant
ddev delete -O
git clean -dfx
rm -rf web/modules/contrib/experience_builder
git clean -dfx

I guess I should not be surprised that XB is special. Then ddev start and ddev describe. It all looks good: there is a selenium-chrome container as in my previous comment.

I am not sure whether the test is slow or if it is hanging:

drupal_cms$ ddev exec phpunit -c web/core/phpunit.xml.dist recipes/drupal_cms_starter/tests/src/FunctionalJavaScript/PerformanceTest.php
PHPUnit 10.5.40 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.14
Configuration: /var/www/html/web/core/phpunit.xml.dist

(no further output yet).

It seems that ddev get is deprecated, but it still works:

drupal_cms$ ddev get ddev/ddev-selenium-standalone-chrome
Command "get" is deprecated, use 'ddev add-on get' instead
Installing ddev/ddev-selenium-standalone-chrome:1.1.0
1.1.0_1694058897.tar.gz 4.53 KiB / ? [--------------------------------------------------------------------------------------------------------------------=----------------------]  46.43% 0s 

Installing project-level components:
👍 docker-compose.selenium-chrome.yaml
👍 config.selenium-standalone-chrome.yaml

Installed DDEV add-on ddev/ddev-selenium-standalone-chrome, use `ddev restart` to enable.
Please read instructions for this add-on at the source repo at
https://github.com/ddev/ddev-selenium-standalone-chrome
Please file issues and create pull requests there to improve it.
Installed ddev-selenium-standalone-chrome:1.1.0 from ddev/ddev-selenium-standalone-chrome

After that, I did git add .ddev and git commit in order to create the MR here.

After doing that, and after switching to the feature branch, you need ddev restart in order to see the additional containers:

drupal_cms$ ddev restart
Restarting project drupal-cms-dev... 
...
Restarted drupal-cms-dev 
Your project can be reached at https://drupal-cms-dev.ddev.site
See 'ddev describe' for alternate URLs. 

drupal_cms$ ddev describe
 Project: drupal-cms-dev ~/repos/drupal_cms https://drupal-cms-dev.ddev.site                                          
 Docker platform: linux-docker                                                                                        
 Router: traefik                                                                                                      
 SERVICE       STAT  URL/PORT                                                                      INFO               
 web           OK    https://drupal-cms-dev.ddev.site                                              drupal11 PHP8.3    
                     InDocker -> Host:                                                             nginx-fpm          
                      - web:80 -> 127.0.0.1:32828                                                  docroot:'web'      
                      - web:443 -> 127.0.0.1:32827                                                 Perf mode: none    
                      - web:8025                                                                   NodeJS:22          
 db            OK    InDocker -> Host:                                                             mariadb:10.11      
                      - db:3306 -> 127.0.0.1:32826                                                 User/Pass: 'db/db' 
                                                                                                   or 'root/root'     
 selenium-chr  OK    https://drupal-cms-dev.ddev.site:7900                                                            
 ome                 InDocker:                                                                                        
                      - selenium-chrome:4444                                                                          
                      - selenium-chrome:5900                                                                          
                      - selenium-chrome:7900                                                                          
 Mailpit             Mailpit: https://drupal-cms-dev.ddev.site:8026                                                   
                     Launch: ddev mailpit                                                                             
 Project URLs        https://drupal-cms-dev.ddev.site, https://127.0.0.1:32827,                                       
                     http://drupal-cms-dev.ddev.site, http://127.0.0.1:32828                                          
 

I think that is all we need, but I am not sure what the next steps are. Where are the performance tests?

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

Tests are passing. This issue is ready for review.

In my opinion, the menu_link_parent process plugin should not be doing a migration lookup at all. That should be left to the migration plugin. Of course, this process plugin was added a long time ago, before the null_coalesce process plugin was added, so at the time it would have been awkward for the migration to handle it. For BC, we want to keep doing the same thing (by default).

I just added ✨ Add lookup_migrations configuration to menu_link_parent process plugin Active . If that issue gets fixed, then it will provide a work-around for the particular example I gave in this issue. But there might be other process plugins (core, contrib, custom) that try to instantiate the migrate_sandbox migration, so I hope that the issue can be fixed in this module.

benjifisher → created an issue.

Since some people can reproduce this problem and some cannot, there is something missing. Have a look at 🐛 Running the installer leads into a wsod Active , where the bug only showed up when using a particular version of Safari on macOS. For example:

1. What OS are you using?
2. What browser are you using?
3. What version of DDEV are you using?
4. How do you configure DDEV?

For (4), can you give options for ddev config so that we can run it non-interactively? That is easier and more reliable for the tester.

I would also like to know whether it makes a difference how you download the code:

• composer create-project drupal/recommended-project
• ddev start && ddev create drupal/recommended-project
• git clone https://git.drupalcode.org/project/drupal.git && cd drupal && git checkout 11.1.0
• other options?

I moved four classes: the source plugin, its deriver, and two test classes. Maybe that is enough for this issue.

I updated the phpstan baseline file. Maybe, as part of this issue, I should fix those errors and remove the entries from the baseline, instead of updating those entries. I might also add return-type declarations and use constructor promotion ... or maybe that is scope creep.

Let's see what the testbot thinks.

There is a kernel test. We can just move that, right? Or do we need to lave behind a wrapper class and deprecate it?

I added a draft change record so that the deprecation messages can refer to it: https://www.drupal.org/node/3498916 →

benjifisher → created an issue.

At first, I missed that the return values were strings 'true' and 'false', not booleans true and false. I also forgot to create the MR after pushing my changes, which is just as well.

The list of attendees agrees with what I remember from yesterday, and I confirmed by checking the private notes kept by the security team.

@alexpott:

In #37, you wrote,

FWIW it would great to file a follow up to add a TRUE to the in_array in this code.

I just added 📌 Clean up code in UserRolesCacheContext::getContext() Active .

I have never before used three commits for a net +2/-3 change, but I did see three different problems with that code.

As long as I am editing this file, should I add return type declarations?

benjifisher → created an issue.

benjifisher → created an issue.

At 📌 Drupal Usability Meeting 2025-01-10 Active , we continued work on the spreadsheet (see Comment #13). For the record, the attendees at the usability meeting were @benjifisher, @rkoller, and @simohell.

benjifisher → created an issue.

benjifisher → created an issue.

I still cannot reproduce the error.

The status should not be NW, since there are no changes in the issue fork. I will set the status to Active (not Postponed, maintainer needs more information). Let's see if anyone else can reproduce the error.

Sorry, I just noticed that 11.1.1 was released today. I tried that, with the same result: no errors.

Thar release does contain the fix for 🐛 When Batch ID doesn't exist, Drupal should emit a 404 Needs work , so the 404 response makes more sense.

I tried to reproduce the problem with 11.1.0, the current 11.1.x, and the current 11.x. In all of my attempts, the installation completed without the error described here.

Can you still reproduce the problem? If so, then please give more detailed steps to reproduce.

Create a new application with 11.1 drupal release.<

At first, I assumed that "release" meant 11.1.0. But that version does not include the fix for 🐛 When Batch ID doesn't exist, Drupal should emit a 404 Needs work , so you would be redirected to the home page instead of getting a 404 response. I guess you meant 11.1.x. Can you say exactly which commit? Can you reproduce the problem with 11.x?

How did you "Create a new application"? Did you git clone https://git.drupalcode.org/project/drupal.git? Did you composer create-project ...?

I used my existing git repository, and dropped the database with drush sql-drop. Can you reproduce the problem that way, or do you only see the problem in a freshly created project?

@godotislate: If I understand Comment #69, then we do not need to move those four source plugins. We just need to move the trait and update the source plugins, which is done in 🐛 Move content_translation I18nQueryTrait to migrate module Active .

Let's mark this issue as postponed until #3258581 is Fixed, at which point the MR here will have to be updated. I just marked that issue RTBC.

Are we still using the convention of updating the title of postponed issues? I hope I got it right, adding "[PP-1]".

In Comment #34, I added the "Needs issue summary update" tag. I asked for STR that do not involve a contrib module. I have rewritten that part of the issue summary, so I am removing the tag.

Looking again at my previous comment, I think we do not need the custom message " Fatal error during migrate" at all. The detail I requested is in the default message:

 Failed asserting that '[06-Jan-2025 16:00:29 Australia/Sydney] PHP Fatal error: Trait "Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait" not found in /builds/issue/drupal-3258581/core/modules/block_content/src/Plugin/migrate/source/d7/BlockCustomTranslation.php on line 22\n
' is false.

So I will add two more suggestions to the MR (one for each assertion) but I will still mark this issue RTBC. (The only reason it was NW instead of NR was for the issue summary update.)

@nikolay shapovalov:

Thanks for those updates. +1 for adding the helper method instead of suppressing the warning with @file(...).

GitLab CI has a test-only job. (It is not run automatically. You have to trigger it when you want it.) We do not need a test-only branch, so I am hiding the one you added.

I ran the test-only job: https://git.drupalcode.org/issue/drupal-3258581/-/jobs/3880198

There was 1 failure:
1) Drupal\Tests\block_content\Functional\migrate\d7\I18nQueryTraitTest::testUpgradeStart
Fatal error during migrate.
Failed asserting that '[06-Jan-2025 16:00:29 Australia/Sydney] PHP Fatal error: Trait "Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait" not found in /builds/issue/drupal-3258581/core/modules/block_content/src/Plugin/migrate/source/d7/BlockCustomTranslation.php on line 22\n
' is false.
/builds/issue/drupal-3258581/core/modules/block_content/tests/src/Functional/migrate/d7/I18nQueryTraitTest.php:79
FAILURES!
Tests: 1, Assertions: 6, Failures: 1.

I was not sure whether the optional message in an assertion method is supposed to be the expected result or the error condition, but that output looks right, so I think you made the right choice: "Fatal error during migrate."

I am satisfied with the code review, and the tests are green. If I can get the issue summary updated, then I will call this issue RTBC.

benjifisher → changed the visibility of the branch 3258581-tests-only to hidden.

benjifisher → created an issue.

Update links to /docs/8/ so that following the link does not cause a redirect.

benjifisher → created an issue.

One problem I noticed in the current README:

#### Exports an entity and all its referenced entities

##### default-content-export-references

The command is actually drush default-content:export-references.

It seems that the README was updated in other issues, so the MR for this issue no longer applies.

The points mentioned in the issue summary are no longer true: they have been fixed. But at least some text in the MR is probably worth saving. (I do not see anything in the current README about access control.) So I am marking this issue as NW, not Closed (outdated).

The issue summary is a little out of date: I now see

## Requirements
* Drupal 9 or 10

but the .info.yml file has

core_version_requirement: ^9.1 || ^10 || ^11

@smustgrave:

The "T" in RTBC is for "testing", and that usually means manual testing (not just confirming that the automated tests pass). You did not mention how you tested this issue.

@plopesc:

Can you explain why you want this change?

In the issue summary, you wrote,

As an example of this feature, Text Formats opt-in to provide the Manage permissions tab-

I tested the MR with the Umami demo profile. As expected, I got a permissions form at /en/admin/config/content/formats/manage/basic_html/permissions:

Do we really need a whole page just to manage one permission? Maybe we do, if people look for a local task (tab) to manage that permission. But we already have a page for managing all permissions from the filter module: /en/admin/people/permissions/module/filter:

I see that you added this issue after working on ✨ provide a Manage Permissions tab for Dashboard entity Active for the Dashboard module. I have not tried that module, so I am not sure of the use case there. Are there typically several dashboards on a site (perhaps most roles get a dedicated dashboard)? Are there several permissions for each one? If either answer is "no", then maybe /en/admin/people/permissions/module/dashboard is good enough.

While working on #2934995: Add a "Manage permissions" tab for each bundle that has associated permissions → , we considered adding permissions forms for more generic config entities. In Comment #65 on that issue, I wrote,

I wonder if we want to make this work for entity types which don't define bundles, say config entities or content entities without bundles?

I am not sure what you mean. If there are no bundles, then there are no per-bundle permissions. For example, the user entity has no bundles.

There are permissions for each text format: see /admin/people/permissions/module/filter. There could be other examples, or a conrib [sic] module could add additional permissions for each text format. Even if we want to implement a permissions form for that use case, I think it is out of scope for this issue, since the implementation here relies heavily on the information we get from the Entity API.

This suggestion looks to me like a solution in search of a problem.

So we explicitly considered the filter module. I am not saying that the decision we made then was final, just that we need some justification for overriding that decision.

@niharika.s:

Thanks for working on this issue.

I have a question on the MR, so back to NW.

I no longer have a client using this module, so I cannot test this change myself. If another user can test it and add a comment, that will help.

@shalini_jha: Thanks for adding the test coverage.

I reviewed the test. I think a single assertion is enough to avoid a regression.

The test passes on the feature branch, and the new assertion fails if I revert the non-test changes.

Back to RTBC.

Now that ✨ Process plugin to build an array Active is fixed (and part of the 6.0.5 release) I think we can close this issue.

@mikelutz:

(BTW, I'm starting to come around to just documenting the 'hack' with the get plugin above and officially supporting it. I've found enough cases where it's useful)

Your timing is interesting. Now that we have the array_template plugin, we can get the same functionality (a little more verbose, but perhaps easier to read/clearer intention). So now you decide to change your mind? ;)

@bhanu951:

Thanks for those final changes. I have no more suggestions.

I updated the change record. It still had the message from before the usability review in Comment #187.

I also tested manually and got something that looks a lot like the screenshot in the issue summary. (That is already up to date.)

I crossed off the last of the "Remaining tasks" in the issue summary.

@sagarmohite0031:

Thanks for the additional testing. But this issue already has up-to-date screenshots in the issue summary.

If you search for issues with the Needs screenshots → tag, you will find plenty of issues that need new screenshots. (You will probably find some that already have good screenshots, but someone forgot to remove the tag.)

Instead of uploading screenshots, it helps more to embed them. This issue is a good example: see the "before" and "after" screenshots under "User interface changes" in the issue summary. See Add screenshots to an issue → for more detailed instructions.

I ran across this old issue while looking for something else.

I am closing this issue as outdated. In 📌 Provide a relationship from the revision table to the main table Fixed , along with replacing the node-specific relationship with a more generic one, the labels were updated. Here is a screenshot:

The description does not explain exactly what the difference is, but it is clear that these are two different options. With a little experimentation, anyone building a view should be able to figure out which one to use.

MR 10750 is not ready for review, but it is ready for discussion.

The default edit form for the menu item has text like this:

This link is provided by the Standard module. The title and path cannot be edited.

That example is from the Home link in the Main navigation menu. It means the Standard profile: there is no Standard module. The menu_link_content module provides a different form that does not have this text.

As a first step, I added the Provider column to the table of menu links, making that same information available without opening the edit form.

I also added the Expanded column. (The MR needs work: the choices made in this column are ignored when the form is saved.) That is out of scope for this issue, so I should probably make a new issue for it. Or we could expand the scope of this issue: add more information to the table of menu links from the edit forms for those links.

Screenshot using 11.x:

Screenshot using the branch from the MR:

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

@bhanu951:

Thanks for the updates. Now the test passes (locally and in GitLab CI). Yay!

I just have some suggestions for code cleanup on the MR. Back to NW for that.