Problem/Motivation

Follow up from #2982949: Introduce CallableResolver to help standardise the DX and error handling for callbacks across various subsystems We should use CallableResolver for \Drupal\user\PermissionHandler::buildPermissionsYaml instead of the controller resolver

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 use_callable_permission_handler-3363424-2.patch8.74 KBvbouchet

Issue fork drupal-3363424

Command icon Show commands

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

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

About issue forks

Comments

kim.pepper created an issue. See original summary.

vbouchet’s picture

vbouchet
Location Toulouse
commented
Status: Active » Needs review
StatusFileSize
new use_callable_permission_handler-3363424-2.patch8.74 KB
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

@vbouchet thank you for the recent work on the drupal.org issues. If you could start using gitlab vs uploading patches though. Eventually everything will be on gitlab and patches are no longer auto ran, manually had to trigger those tests. Also gitlab tests take around 10 minutes while drupalCi could be an 1-2 hours. Did nothing wrong but patches are slowly being phased out. Let me know if you have any questions!

But reviewing your patch and deprecation seems correct. Message seems good and all tests updated.

LGTM

vbouchet’s picture

vbouchet
Location Toulouse
commented

Thanks for the headsup @smustgrave. Will try to setup my local for easier work with issue fork. 😉

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

_shY made their first commit to this issue’s fork.

_shY opened merge request !5345

_shy’s picture

_shy
Primary language Ukrainian
Location Ukraine, Lutsk 🇺🇦
commented
Status: Needs work » Needs review

Created MR from the patch.

_shy’s picture

_shy
Primary language Ukrainian
Location Ukraine, Lutsk 🇺🇦
commented
smustgrave’s picture

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

Reroll to MR seems fine.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Use CallableResolver for \Drupal\user\PermissionHandler::buildPermissionsYaml instead of the controller resolver » Use CallableResolver for \Drupal\user\PermissionHandler::buildPermissionsYaml() instead of the controller resolver
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 15ac0b4 and pushed to 11.x. Thanks!

  • catch committed 15ac0b4e on 11.x 
    Issue #3363424 by _shY, vbouchet, kim.pepper, smustgrave: Use...
catch’s picture

catch
he/him
Primary language English
commented

Status: Fixed » Closed (fixed)

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

quietone’s picture

quietone commented

The trigger_error here is for deprecating in 10.2.0 and should be 10.3.0. I have made a followup to fix that. #3417668: Correct version in deprecation for #3363424