Skip to content

fix(web): delete reaction map entries by id - #9578

Open
Khizarshah01 wants to merge 1 commit into
makeplane:previewfrom
Khizarshah01:fix/reaction-map-delete-by-id
Open

fix(web): delete reaction map entries by id#9578
Khizarshah01 wants to merge 1 commit into
makeplane:previewfrom
Khizarshah01:fix/reaction-map-delete-by-id

Conversation

@Khizarshah01

@Khizarshah01 Khizarshah01 commented Aug 10, 2026

Copy link
Copy Markdown

Description

When removing a work item or comment reaction, the store deleted from the reaction map using the emoji string instead of the reaction id. Entries are stored by id on create, so the old entry was left behind.

This PR deletes by currentReaction.id in both issue and comment reaction stores.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Test Scenarios

NA

References

NA

Summary by CodeRabbit

  • Bug Fixes
    • Fixed removal of comment reactions so the correct reaction is deleted.
    • Fixed removal of issue reactions to keep reaction displays accurate after deletion.

Copilot AI lite review requested due to automatic review settings August 10, 2026 10:37
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Both issue reaction removal methods now delete reaction-map entries by the matched reaction ID instead of the reaction type.

Changes

Reaction removal

Layer / File(s) Summary
Reaction map key correction
apps/web/core/store/issue/issue-details/reaction.store.ts, apps/web/core/store/issue/issue-details/comment_reaction.store.ts
removeReaction and removeCommentReaction now delete entries using currentReaction.id.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: sriramveeraghanta

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main fix: deleting reaction-map entries by reaction ID.
Description check ✅ Passed The description explains the bug, identifies both affected stores, marks the change as a bug fix, and notes unavailable tests and references.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@CLAassistant

CLAassistant commented Aug 10, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a bug in the web issue/comment reaction MobX stores where reaction map entries were being deleted using the emoji string key instead of the reaction id, leaving stale entries behind after removals.

Changes:

  • Delete reactionMap entries by currentReaction.id when removing an issue reaction.
  • Delete commentReactionMap entries by currentReaction.id when removing a comment reaction.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
apps/web/core/store/issue/issue-details/reaction.store.ts Corrects reaction map deletion to use reaction id (consistent with how entries are stored).
apps/web/core/store/issue/issue-details/comment_reaction.store.ts Corrects comment reaction map deletion to use reaction id (consistent with how entries are stored).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants