Skip to content

Conversation

@ciastektk
Copy link
Contributor

@ciastektk ciastektk commented Nov 13, 2025

🎫 Issue IBX-9663

Description:

For QA:

Documentation:

@sonarqubecloud
Copy link

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds a Request property to the ContentEditEvent class, allowing event subscribers to access HTTP request context when handling content edit events.

Key Changes:

  • Added Request as a required constructor parameter and property to ContentEditEvent
  • Updated ContentController::editAction() to pass the request instance when dispatching the event

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/contracts/Event/ContentEditEvent.php Added Request property, constructor parameter, and getter method
src/bundle/Controller/ContentController.php Updated event instantiation to include request parameter and modified closure signature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 32 to 37
public function __construct(
Content $content,
VersionInfo $versionInfo,
string $languageCode
string $languageCode,
Request $request
) {
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Adding a required parameter to an existing event constructor is a breaking change that will break all existing event instantiations. Consider making the Request parameter optional with a default value of null to maintain backward compatibility.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

The event object in question is not supposed to be instantiated outside of our code, so it is 👌

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.

5 participants