Skip to content

Reaction class should use Emoji Part rather than stdClass#1422

Open
valzargaming wants to merge 4 commits intomasterfrom
reaction-emoji-fix
Open

Reaction class should use Emoji Part rather than stdClass#1422
valzargaming wants to merge 4 commits intomasterfrom
reaction-emoji-fix

Conversation

@valzargaming
Copy link
Copy Markdown
Member

This pull request makes a minor improvement to the Reaction class in the Discord package by initializing the emoji attribute with an Emoji object instead of a generic stdClass. This change enhances type safety and consistency when handling emoji data.

  • Improved emoji initialization:
    • In the setIdAttribute method of Reaction.php, replaced the initialization of the emoji attribute from a stdClass to an Emoji object for better type safety and clarity.
    • Removed the unused stdClass import from Reaction.php.

@valzargaming valzargaming requested a review from a team October 15, 2025 19:47
@valzargaming valzargaming changed the title Update Reaction.php Reaction class should use Emoji Part rather than stdClass Oct 20, 2025
@valzargaming valzargaming added the stale Information contained within may be old or outdated. label Apr 4, 2026
@valzargaming
Copy link
Copy Markdown
Member Author

It's unclear if this would introduce a BC in existing bots

Copilot AI review requested due to automatic review settings April 27, 2026 14:51
Copy link
Copy Markdown
Contributor

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 PR updates the Reaction part to initialize its emoji attribute as a proper Emoji Part rather than a generic stdClass, improving type safety and consistency when manipulating reaction emoji data.

Changes:

  • Removed the unused stdClass import from Reaction.php.
  • Updated Reaction::setIdAttribute() to initialize the emoji attribute via Factory::part(Emoji::class, ...) instead of new stdClass().

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

Comment thread src/Discord/Parts/Channel/Reaction.php Outdated
if (! isset($this->attributes['emoji'])) {
$this->attributes['emoji'] = new stdClass();
if ($this->emoji === null) {
$this->attributes['emoji'] = $this->factory->part(Emoji::class, [], true);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

When the reaction has no existing emoji attribute, this initializes an Emoji Part with an empty payload. Because getEmojiAttribute() only injects guild_id via attributePartHelper() when converting non-Emoji data, an already-instantiated Emoji will not get guild_id populated, which can break Emoji behavior that relies on guild context (e.g., Emoji::guild, roles resolution). Consider initializing the Emoji Part with ['guild_id' => $this->guild_id] (or otherwise ensuring guild_id is set) when creating it here.

Suggested change
$this->attributes['emoji'] = $this->factory->part(Emoji::class, [], true);
$this->attributes['emoji'] = $this->factory->part(Emoji::class, ['guild_id' => $this->guild_id], true);

Copilot uses AI. Check for mistakes.
@valzargaming
Copy link
Copy Markdown
Member Author

This PR might be pointless thankss to the getEmojiAttribute's attributePartHelper, but should probably be merged anyway just to satisfy the expectation of an Emoji being the value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Information contained within may be old or outdated.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants