Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access request actor in error handler #2410

Merged
merged 6 commits into from
Apr 12, 2021
Merged

Conversation

SychO9
Copy link
Member

@SychO9 SychO9 commented Oct 24, 2020

Fixes #1847

Changes proposed in this pull request:
Very well explained here: #1847 (comment)

Reviewers should focus on:
With this, the actor can be accessed through $request->getAttribute('actorReference')->getActor()
Do we want to deprecate $request->getAttribute('actor') ? If so do we want to replace it with the above everywhere as well or leave that for #869 ?

Confirmed

  • Backend changes: tests are green (run composer test).

@askvortsov1
Copy link
Sponsor Member

askvortsov1 commented Oct 25, 2020

To be honest, I would prefer to see #869 happen first; that way we can start by abstracting away access, then switch up the underlying implementation. That being said, I'd lean towards not switching anything up yet, and only deprecating the old way after we abstract it away through RequestUtil.

@SychO9
Copy link
Member Author

SychO9 commented Oct 25, 2020

Sounds good, we can come back to this after #869 is implemented.

@stale
Copy link

stale bot commented Jan 23, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale Issues that have had over 90 days of inactivity label Jan 23, 2021
@askvortsov1 askvortsov1 added the org/keep Issues we want to keep open label Jan 24, 2021
@stale stale bot removed the stale Issues that have had over 90 days of inactivity label Jan 24, 2021
@askvortsov1
Copy link
Sponsor Member

This is from half a year ago???? How time flies... Anyways, this can be updated now that #2449 has been merged.

@SychO9 SychO9 marked this pull request as draft April 10, 2021 17:00
@SychO9 SychO9 marked this pull request as ready for review April 10, 2021 19:09
@askvortsov1
Copy link
Sponsor Member

Do we want to deprecate $request->getAttribute('actor') ?

I would be in favor of deprecating it, and leaving it for now.

@SychO9 SychO9 requested a review from askvortsov1 April 11, 2021 22:11
@SychO9 SychO9 merged commit 9e3699e into flarum:master Apr 12, 2021
@SychO9 SychO9 deleted the sm/actor-reference branch April 12, 2021 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
org/keep Issues we want to keep open
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to access request actor in error handler
3 participants