Skip to content

Conversation

@rodrigok
Copy link
Member

@rodrigok rodrigok commented Oct 2, 2025

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Bug Fixes
    • Fixed handling of reaction events during redactions, ensuring reactions are correctly recognized and processed when event data is nested.
    • Improved reliability of reaction content extraction, reducing missed or misclassified reactions in timelines and notifications.
    • Users should see more accurate reaction visibility after message edits or deletions, with fewer inconsistencies.

Copilot AI review requested due to automatic review settings October 2, 2025 20:30
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 2, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Oct 2, 2025

⚠️ No Changeset found

Latest commit: fa1db94

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The change updates reaction redaction handling to read event properties from a nested event object. It now uses reactionEvent.event.type and reactionEvent.event.content['m.relates_to'] instead of accessing type and content at the top level.

Changes

Cohort / File(s) Summary
Matrix federation reaction redaction
ee/packages/federation-matrix/src/events/reaction.ts
Shifted accesses from top-level to nested: validate via reactionEvent.event.type; extract relates_to via reactionEvent.event.content?.['m.relates_to']; logic for non-reaction/missing content preserved.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor MatrixServer as Matrix Server
  participant FederationHandler as Federation Handler
  participant ReactionEvent as Reaction Event

  MatrixServer->>FederationHandler: Redaction event received
  FederationHandler->>ReactionEvent: Inspect event
  Note right of ReactionEvent: New: use event.type and event.content['m.relates_to']
  alt Is reaction
    FederationHandler->>FederationHandler: Extract m.relates_to from event.content
    FederationHandler->>FederationHandler: Apply redaction logic
    FederationHandler-->>MatrixServer: Ack redaction
  else Not a reaction or missing content
    FederationHandler->>FederationHandler: Skip or default handling
    FederationHandler-->>MatrixServer: Ack without reaction-specific changes
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

A nibble of code, a hop through the nest,
I burrowed to event.content, found it best.
From top to deep, I follow the trace—
type in the burrow, a cleaner place.
With twitchy nose and tidy delight,
Reactions redact just right. 🐇✨

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/federation-unset-reactions

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9db5917 and fa1db94.

📒 Files selected for processing (1)
  • ee/packages/federation-matrix/src/events/reaction.ts (1 hunks)

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 and usage tips.

Copy link
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 fixes incorrect property access in the reaction event handling code for the Matrix federation integration. The changes correct the path to access event type and content properties from a reaction event object.

  • Fixed property access from reactionEvent.type to reactionEvent.event.type
  • Fixed property access from reactionEvent.content to reactionEvent.event.content

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@rodrigok rodrigok changed the title fix reactionEvent property element to get event fix(federation): unset reactions not working from remote Oct 2, 2025
@rodrigok rodrigok merged commit bda2fa4 into release-7.11.0 Oct 2, 2025
9 of 13 checks passed
@rodrigok rodrigok deleted the fix/federation-unset-reactions branch October 2, 2025 20:32
@codecov
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.45%. Comparing base (9db5917) to head (fa1db94).
⚠️ Report is 2 commits behind head on release-7.11.0.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff               @@
##           release-7.11.0   #37129   +/-   ##
===============================================
  Coverage           67.45%   67.45%           
===============================================
  Files                3329     3329           
  Lines              113387   113387           
  Branches            20579    20584    +5     
===============================================
+ Hits                76488    76490    +2     
+ Misses              34299    34294    -5     
- Partials             2600     2603    +3     
Flag Coverage Δ
e2e 57.25% <ø> (-0.05%) ⬇️
unit 71.26% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

5 participants