[4.x] Fix BaseAuthorze and BaseRenderless not triggered using event - #10384
Merged
Conversation
The existing test dispatches the event as an authorized user and asserts Ok, which also passes without the fix (the outcome is the same whether the authorize check runs or is skipped). This adds a test with an unauthorized user asserting Forbidden, which fails on main and passes with the instanceof fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
calebporzio
reviewed
Jul 4, 2026
calebporzio
left a comment
Collaborator
There was a problem hiding this comment.
Hey @ghabriel25 — thanks for this one, and for the clear writeup on the issue. The fix is right: the direct-call path runs these attributes for anything matching at the method level, so the event path filtering with is_subclass_of (which excludes the base class itself) was just an inconsistency. instanceof is the correct check.
Here's what I ran to verify:
- Renderless browser test: fails on
main'sSupportEvents.php, passes with your fix. Solid regression test. - Authorize unit test: I noticed your new test passes on
maintoo — it dispatches as an authorized user and asserts Ok, which is the same outcome whether the authorize check runs or gets skipped, so it wasn't actually guarding the fix. I pushed a small commit to your branch adding a variant that dispatches as an unauthorized user and asserts Forbidden — verified it fails onmainand passes here. Hope you don't mind the direct push! - Full unit suite: green on this branch.
Happy with this — good to merge from my side.
Contributor
Author
|
@calebporzio Thanks for the review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scenario
When user wants to contribute to this repo and preparing some browser / unit test related to
#[Authorize]and#[Renderless]attribute, they might be not aware that using#[BaseAuthorize]or#[BaseRenderless]wont be triggered if action called as event listener.I'm aware that all
Base*attribute should be strictly for internal use as stated here #10346 (comment).Problem
Inside
SupportEvents::call, both renderless and authorize attribute checked usingis_subclass_ofI think the intent here is a good one that prevent using
Base**attribute as public API. But, this will cause some unsual issue when running tests. Therefore, I still think this need to be fixed.Solution
Change
is_subclass_oftoinstanceofI have added regression test for both authorize unit test and renderless browser test.
Files Changed
src/Features/SupportEvents/SupportEvents.phpsrc/Features/SupportAuthorization/UnitTest.phpsrc/Features/SupportRenderless/BrowserTest.phpFix: #10383