Skip to content

Conversation

@rodrigok
Copy link
Member

@rodrigok rodrigok commented Oct 7, 2025

https://rocketchat.atlassian.net/browse/FDR-200

Summary by CodeRabbit

  • New Features

    • Added support for recognizing room topic events in authorization, improving interoperability with more homeservers.
  • Bug Fixes

    • Refined authorization logic for create, redaction, power levels, and name/message events to reduce erroneous rejections.
    • Improved consistency when updating room metadata and handling message-related actions during federation.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

Walkthrough

Adds m.room.topic as a recognized auth event type and updates event-to-query mappings in the repository. Adjusts switch-case logic for several Matrix event types, including create, redaction, name, message, power_levels, and topic. Service layer now includes m.room.topic in getAuthEventIds.

Changes

Cohort / File(s) Summary
Auth-event mapping update
packages/federation-sdk/src/repositories/event.repository.ts
Revised switch mapping for findAuthEvents: m.room.create -> baseQueries.create; m.room.redaction -> create + powerLevels; m.room.name/message collapsed; added m.room.topic treated like power_levels (create + powerLevels + membership); removed explicit member-related groupings.
Auth-event type list update
packages/federation-sdk/src/services/event.service.ts
getAuthEventIds now includes m.room.topic in recognized auth event types; ordering of existing entries adjusted without semantic change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant S as EventService
  participant R as EventRepository

  C->>S: getAuthEventIds(event)
  S->>S: Determine auth types (includes "m.room.topic")
  S->>R: findAuthEvents(event.type)
  alt type == m.room.create
    R->>R: Use baseQueries.create
  else type == m.room.redaction
    R->>R: Use create + powerLevels
  else type == m.room.power_levels or m.room.topic
    R->>R: Use create + powerLevels + membership
  else type == m.room.name or m.room.message
    R->>R: Use default/collapsed handling
  end
  R-->>S: Auth events
  S-->>C: Auth event IDs
  note over S,R: New handling path for "m.room.topic"
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • ggazzo
  • ricardogarim

Poem

A carrot of topics, a hop through the log,
No more “unsupported” croaks in the fog.
Events align, queries in sync,
Power, topic—click, click, clink.
I thump with joy, ears up to the sky—
Federation hums; the packets fly! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The changes include refactoring of other event type mappings in findAuthEvents—such as collapsing name and message cases and altering redaction handling—that fall outside the scope of adding m.room.topic support specified in FDR-200. Please limit changes to the addition of m.room.topic support and revert unrelated modifications to other event type mappings to keep the PR focused on the linked issue.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely states the primary change of adding support for the “m.room.topic” event type, aligning directly with the PR’s main intent without extraneous details.
Linked Issues Check ✅ Passed The PR updates both the repository and service layers to recognize and handle the m.room.topic event type, directly resolving the unsupported event error described in FDR-200 without leaving any aspect of the issue unaddressed.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/support-for-m-topic

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.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.39%. Comparing base (770431b) to head (f7b7467).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ederation-sdk/src/repositories/event.repository.ts 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #266      +/-   ##
==========================================
+ Coverage   62.12%   62.39%   +0.26%     
==========================================
  Files          67       67              
  Lines        6390     6363      -27     
==========================================
  Hits         3970     3970              
+ Misses       2420     2393      -27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@rodrigok rodrigok marked this pull request as ready for review October 7, 2025 22:47
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 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 770431b and f7b7467.

📒 Files selected for processing (2)
  • packages/federation-sdk/src/repositories/event.repository.ts (1 hunks)
  • packages/federation-sdk/src/services/event.service.ts (1 hunks)
🔇 Additional comments (3)
packages/federation-sdk/src/services/event.service.ts (1)

559-571: LGTM! Service layer now recognizes m.room.topic.

The addition of m.room.topic to the allowed auth event types array ensures consistency with the repository changes and prevents the unrecognized event type warning at lines 574-576.

This change correctly completes the support for m.room.topic events across both the repository and service layers, addressing the issue described in FDR-200.

packages/federation-sdk/src/repositories/event.repository.ts (2)

58-64: Confirm intent and add tests for m.room.create and m.room.redaction auth events
Ensure the updated auth event mappings align with the Matrix specification and add dedicated tests for findAuthEvents covering these event types.


71-71: LGTM – m.room.topic support added correctly

Verified auth_events (m.room.create, m.room.power_levels, m.room.member) align with Matrix spec.

Comment on lines 57 to 81
switch (eventType) {
case 'm.room.name':
queries = [
baseQueries.create,
baseQueries.powerLevels,
baseQueries.membership,
];
case 'm.room.create':
queries = [baseQueries.create];
break;

case 'm.room.message':
queries = [
baseQueries.create,
baseQueries.powerLevels,
baseQueries.membership,
];
case 'm.room.redaction':
queries = [baseQueries.create, baseQueries.powerLevels];
break;

case 'm.reaction':
queries = [
baseQueries.create,
baseQueries.powerLevels,
baseQueries.membership,
];
break;

case 'm.room.name':
case 'm.room.message':
case 'm.room.member':
queries = [
baseQueries.create,
baseQueries.powerLevels,
baseQueries.membership,
];
break;

case 'm.room.create':
queries = [baseQueries.create];
break;

case 'm.room.power_levels':
case 'm.room.topic':
queries = [
baseQueries.create,
baseQueries.powerLevels,
baseQueries.membership,
];
break;

case 'm.room.redaction':
queries = [baseQueries.create, baseQueries.powerLevels];
break;

default:
throw new Error(`Unsupported event type: ${eventType}`);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add test coverage for the new m.room.topic support.

The Codecov report indicates 0% patch coverage with 7 lines missing coverage in this file. Since this change adds support for a new event type and modifies auth event behavior for existing types, comprehensive test coverage is essential to prevent regressions.

Consider adding tests that:

  1. Verify m.room.topic returns the correct auth events (create, powerLevels, membership)
  2. Validate the refactored auth event queries for m.room.create and m.room.redaction
  3. Ensure the default case throws an error for truly unsupported event types

Would you like me to generate test cases for the findAuthEvents method covering the new m.room.topic support?

🤖 Prompt for AI Agents
In packages/federation-sdk/src/repositories/event.repository.ts around lines 57
to 81, tests are missing for the newly-added m.room.topic branch and modified
auth-event behavior; add unit tests for findAuthEvents that (1) assert
m.room.topic returns [create, powerLevels, membership] auth event queries, (2)
verify m.room.create and m.room.redaction return their expected query sets
(create only and create+powerLevels respectively), and (3) confirm the default
branch throws for unsupported event types; mock any dependencies or baseQueries
inputs and assert that the method selects the correct query array for each
eventType.

@ggazzo ggazzo merged commit 520c356 into main Oct 7, 2025
3 checks passed
@ggazzo ggazzo deleted the chore/support-for-m-topic branch October 7, 2025 23:43
@coderabbitai coderabbitai bot mentioned this pull request Nov 3, 2025
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