🇧🇷Brazil @brandonlira

Hi @aaron, I tested this and it's working as expected.

The export file correctly shows the spelling correction in the expected format:
marmalade,mermelad,marmellade => marmelade

Thanks for the fix, everything looks good!

Hi @joachim,

I’ve made the changes you suggested, removed the docblock from __construct() and added the inline comment at the top of the method.

About the wording: I’m not sure if any of it was AI-generated, as the work had already been started by someone else. I just continued from what was already there.

Let me know if anything else needs changing.

Hi @joachim,

Fixed all issues flagged by @needs-review-queue-bot:

• Removed unused imports
• Fixed spacing before `array_merge(...)`

Let me know if a rebase to 11.x is needed, happy to do it if required.

Thanks!

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

Hi @joachim,

I've removed MR !11745 the method-level docblocks for getWeight() and setWeight() as suggested. The class-level and constructor documentation were kept to clarify usage requirements. Let me know if anything else should be adjusted.

Thank you!

Hi @smustgrave,

I've updated the issue summary to reflect the purpose and outcome of this task, including steps to reproduce the issue from a documentation perspective, and clarifying that this was a documentation-only change.

Please let me know if any further adjustments are needed.

Thanks!

Hi @aaron.ferris

Manually tested MR !25.

• Confirmed the issue initially: anonymous users received `The 'administer search api synonyms' permission is required` when accessing `/jsonapi/search_api_synonym/search_api_synonym`.
• Checked out the MR !25 branch locally.
• Created two synonym entries (one regular synonym and one spelling error) to test.
• Granted the new permission `View search api synonyms` to the Anonymous role.
• Retested the endpoint as an anonymous user.
• Synonym entities were successfully returned in the JSON:API response.
• Removing the permission again blocks access as expected.

Hi @aaron.ferris,

I manually tested MR !22.

• Selected "Every Cron run" in the configuration form.
• Triggered cron multiple times manually.
• Confirmed that the synonym export was executed on each cron run.

Verified that the following messages appeared in the logs:

• Executing export
• Export for English complete

Confirmed expected behaviour without requiring a time-based interval.
Looks good to me.
Thank you!

Hi @mradcliffe
I've made the requested adjustments:

1. Replaced the _custom_access on the migrate_drupal_ui.log route with _permission: 'access site reports'.
2. Updated MigrateControllerTest to create and log in a user with the proper permissions (access site reports and administer views), instead of relying on $this->rootUser.
3. Removed the outdated @todo and the usesSuperUserAccessPolicy flag.

All tests are now passing successfully.
I'll ask someone to test these changes tomorrow during our Contrib Day.

Thank you all for the helpful guidance throughout this issue!

Thanks so much @mradcliffe and @kristiaanvandeneynde — that clears things up perfectly!

I’ll make the adjustments now based on the updated direction: using access site reports, removing the UID 1 check, and updating the test.

Really appreciate your help — I’ll update the MR soon!

Hi @mradcliffe, thanks for highlighting that.

To clarify why this test still requires super user access:

Although the test doesn't perform migration actions itself, it accesses the /admin/reports/upgrade route. That route — as defined in migrate_drupal_ui.routing.yml — uses a _custom_access requirement pointing to \Drupal\migrate_drupal_ui\MigrateAccessCheck::checkAccess().

This access check explicitly allows only UID 1:

return AccessResultAllowed::allowedIf((int) $account->id() === 1);

o even just viewing the upgrade report (MigrateController::showLog) fails unless the test runs as the root user. Removing usesSuperUserAccessPolicy = TRUE causes an access denied error.

That’s why in the MR, we restored the flag and removed only the outdated @todo. A broader solution (e.g., updating that route to rely on a permission like access site reports) would be ideal, but it's out of scope for this issue.

Happy to help push that discussion further if needed.

Thank you!

Hi,
I've rebased the branch onto the latest 11.x and fixed the coding standard issues.

All tests are now passing.

Please review when you have a moment, and feel free to let me know if anything else need.

Thank you!

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

Hi, could you please review this MR !11645?

I've restored the usesSuperUserAccessPolicy = TRUE property and removed only the outdated @todo, as discussed.
This change reflects the current behaviour where Migrate UI still requires user 1.

Please feel free to review, and if anything needs to be adjusted or clarified, please let me know!
Thank you!

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

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

Hi, I've created a new clean branch to address this issue.

The previous branch had too many unrelated commits and was outdated, causing multiple conflicts and making it hard to review.
This new branch includes only the intended change: replacing all usages of the deprecated assertWaitOnAjaxRequest() with assertExpectedAjaxRequest() across the core/ directory.

A new merge request has been opened from this clean branch.

Could someone please review and test this updated MR !12108?
Thank you!

brandonlira → created an issue.

Hi @quietone
I've addressed all the feedback:

• Clarified the wording regarding the anonymous user to explain why it's used during cron execution.
• Wrapped all PHPDoc lines at 80 characters to comply with Drupal coding standards.
• Removed the out-of-scope change in Cron.php as suggested.

Please let me know if anything else is needed — happy to make further adjustments if required.

Thanks again for the guidance!

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

chadhester → credited brandonlira → .

brandonlira → changed the visibility of the branch 3515604-use-drupal-behaviours to hidden.

Hi everyone,
I Fixed the PHPCS issues and updated service usage with dependency injection. The code is ready for review.

Thank you!

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

Hi @aaron,
I adjusted the MR as suggested, could you please review it again.
Thanks.

aaron.ferris → credited brandonlira → .

aaron.ferris → credited brandonlira → .

Fixed broken link Reroll a patch

I'll work on this!

Hi @aaron, I am assigning this ticket to myself to work on our contrib day!

Updated the outdated/broken link to the current rerolling patches documentation page.

Sorry in my previous comment I uploaded the wrong image.

Thanks!

Hi everyone,
The "Timer Completed" message can now be translated through the User Interface Translation screen.

This MR! 17 resolves the feature request.
Screenshot attached showing the string translated to Portuguese (Brazil).

Thanks!

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

This patch replaces the preg_replace() used to locate .webp derivative files with the getWebpDestination() method to ensure consistent file naming and allow proper deletion when the image is updated (e.g., after focal point change).

Cheers!

Hi @jwilson3,

The original merge request seemed to be abandoned and the title was still incorrect.
Since there was no update, I created a new branch using cherry-pick and opened a new MR with the correct title as suggested.

Some minor adjustments were also made to fix issues that were causing test failures.

Let me know if there's anything that needs to be changed or improved.

brandonlira → changed the visibility of the branch 3436435-blockcreationtraitplaceblock-should-explain to hidden.

I’ve rebased this issue branch with the latest changes from 11.x (via merge), resolved all merge conflicts, and ensured the code is up to date. All automated tests are passing successfully.

The issue is now ready for review.

Thanks!

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

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

Hi @joachim,

I’ve updated the comment to reflect that setRebuild(TRUE) is what triggers the behavior, as you suggested, while preserving the original intention of explaining how to get this behavior from a DX perspective.

I also rebased the branch onto the latest 11.x since it was many commits behind, to ensure the pipeline runs accurately.

Let me know if anything else needs adjusting!

Hi @smustgrave

I've now added a clear Proposed resolution section to the issue summary, describing the adjustments made to the PHPDoc and how it addresses the original problem.
Let me know if anything else needs to be clarified.

Hi @quietone

The previous MR (!8124) was quite outdated and significantly behind the 11.x branch. I initially attempted a rebase, but due to a large number of conflicts, it became difficult to manage cleanly.

To ensure a clean and conflict-free update, I’ve created a new branch and opened this fresh MR (!11606) with the necessary changes, fully rebased with the latest from 11.x.

All tests are now passing. Let me know if anything else is needed!

Thanks!

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

Hi @smustgrave,

Apologies if I might be misunderstanding something here, but I just want to clarify the intent behind this change to ensure I'm following the best approach. I'm still getting started with contributing to Drupal, and I really appreciate your guidance.

The issue summary suggests that handleEntityDelete() should be updated to use isLayoutCompatibleEntity(), but after reviewing the implementation, I noticed that removeByLayoutEntity($entity) does not fail if the entity is not layout-compatible. Instead, it simply attempts to clean up inline_block_usage, setting layout_entity_id and layout_entity_type to NULL if applicable. If no matching records exist, nothing happens, which makes me wonder if the extra check is truly necessary.

I just want to make sure I'm not missing something here:

• Was there a specific issue that required adding isLayoutCompatibleEntity()?
• Is there any scenario where calling removeByLayoutEntity() directly could cause unintended side effects?

If the check is needed for a reason I’m not seeing, I’ll be happy to update it accordingly. Otherwise, removing it might help simplify the logic while still ensuring the cleanup happens.

Again, thanks for your patience and feedback—I really appreciate learning from this process!

Hi @smustgrave

I have successfully rebased the branch with the latest changes from 11.x. The MR !7106 is now up to date, and the previous changes remain intact.

Additionally, I noticed that one test failed, but it does not appear to be related to the changes made in this MR.

Please let me know if you need any additional adjustments.

Thanks!

Hi @smustgrave,

I have successfully rebased the branch with the latest changes from 11.x. The MR is now up to date, and the documentation improvements remain intact.

Additionally, I have fixed the PHPCS violations by adjusting the line lengths to conform with Drupal coding standards.

I also noticed that some tests have failed (`Drupal\Tests\package_manager\Build\PackageInstallTest` and `Drupal\Tests\ckeditor5\FunctionalJavascript\ImageUrlProviderTest`), but they do not seem to be related to the changes made in this MR.

Let me know if any further adjustments are needed.

Thanks!

Hi all,
I have resolved the conflicts and reverted the method's argument definition as suggested MR! 5052. Let me know if any further adjustments are needed.

Thanks!

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

Hi all,
I removed the isLayoutCompatibleEntity($entity) check before calling removeByLayoutEntity($entity), as suggested.

Tests performed:

PHPUnit: All tests passed successfully, with no errors found. Only pre-existing skipped tests remained unchanged.

The change has been committed and submitted in the Merge Request. Awaiting feedback for any necessary adjustments!

Thank you!

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

I have created a new MR !11528 with the necessary documentation update.

This is now ready for review. Let me know if any further adjustments are needed.

Thanks!

Hi @smustgrave ,

I can confirm that my changes are included in the latest pipeline (#449876), even though the commit history might not explicitly show my name due to the rebase process.

The updates focus solely on documentation improvements for EntityDisplayRepositoryInterface.php MR !9637 , and all unrelated changes have been removed.

Please review and let me know if any further adjustments are needed.

Thanks!

Hi everyone,

I have updated the merge request to remove unrelated changes in the composer/ directory and keep only the documentation improvements for EntityDisplayRepositoryInterface.php.

Now, the MR focuses solely on clarifying the return values for the getAllViewModes(), getViewModes(), getAllFormModes(), and getFormModes() methods, following the previous feedback.

Please review the changes and let me know if any further adjustments are needed.

Thanks!

Hi @adamps,

I have moved the fix to ContactMailer.php (line 100) for 2.x MR (!144) and added a check to ensure $sender is not NULL before calling setReplyTo().

Please review and let me know if any adjustments are needed.

Thanks!

Hi everyone,

I checked the "Add this event to your calendar" link and it redirects to Google Calendar, but I also get the error:
"Could not find the requested event."

Suggested solutions:

If the event link is incorrect but the event still exists, we should update the documentation with the correct Google Calendar link.

If the event is not publicly accessible, we can update the text to guide users to request access via the Drupal Accessibility Slack channel, as previously suggested.
Let me know the best approach, and I’d be happy to submit an update!

Thanks!

Hi @mondrake,

I have updated the code according to the deprecation guidelines for method parameters in Drupal:

The $root parameter is marked as deprecated, but the method itself remains functional.
A @trigger_error() has been added following the documentation.
The @see annotation references the correct change record.
However, I noticed that some tests fail due to deprecation warnings related to passing $root. These warnings appear in multiple functional JavaScript tests, such as:

ToolbarActiveTrailTest
RegistrationWithUserFieldsTest
ClickSortingAJAXTest
FilterTest
FieldDialogsTest
ViewsWizardTest

Would you recommend updating these tests to avoid passing $root, or should we add an explicit handling for this in the code? Let me know the best approach so I can make the necessary adjustments.

Thank you!

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

Hi everyone,

I have updated the fix to target the correct version (1.4.0) by creating a new branch based on 1.x. The previous merge request was for 2.x, but after reviewing the issue and its comments, I realized that the correct fix should be applied to 1.4.0.

I have now submitted a new Merge Request (!143) for this version. Please review the changes and let me know if anything needs to be adjusted.

Additionally, I noticed that some PHPStan checks have failed. From my understanding, these errors seem to be pre-existing and unrelated to this MR. Could someone confirm if I should address them, or if they can be handled separately?

Thanks!

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