Skip to content

Conversation

@jrauh01
Copy link
Contributor

@jrauh01 jrauh01 commented Oct 10, 2025

Add a RequestHook that hooks into different states of a request. The implementation starts with hooking into after non xhr requests are dispatched. This is meant to be extended for more more states like for example before a request gets dispatched.

@jrauh01 jrauh01 self-assigned this Oct 10, 2025
@cla-bot cla-bot bot added the cla/signed label Oct 10, 2025
@jrauh01 jrauh01 requested a review from lippserd October 10, 2025 07:30
*/
final public static function postDispatch(Request $request): void
{
array_map(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why array_map? You don't use its result.

Add a hook that hooks into different states of a request. The
implementation starts with hooking into after non xhr requests
are dispatched. This is meant to be extended for more more states
like before a request gets dispatched.
Ensuring the request is no xhr request.
Copy link
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lippserd, what do you think about the following?

if ($this->isXhr()) {
$this->postDispatchXhr();
} else {
RequestHook::postDispatch($req);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think letting the hook decide would be clever AND smart. I mean, anyone could provide a hook... and wonder why some requests are missing. If $this->isXhr() is too heavy just with $req, we can pass additional arguments like a bool.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already discussed this w/ @jrauh01. We will call postDispatch() unconditionally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrauh01 Please also add the following to our Request:

public function isAutoRefresh(): bool
{
    return $this->getHeader('X-Icinga-Autorefresh');
}

@jrauh01 jrauh01 marked this pull request as draft November 6, 2025 10:48
@jrauh01 jrauh01 marked this pull request as ready for review November 6, 2025 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants