-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: remove search federated #37031
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: remove search federated #37031
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: e2c46e6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughRemoves federation-related logic from two client hooks that build “Create New” menus, eliminating the “Explore/Search federated rooms” section. Menus now return only the permission-gated “Create new” items. Adds a changeset entry for a patch bump noting the removal of deprecated federated search. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Client UI
participant Hook as useCreateNewMenu / useCreateRoom
participant ACL as Permissions
User->>UI: Open "Create New" menu
UI->>Hook: Build sections
Hook->>ACL: Check create permissions
ACL-->>Hook: Allowed actions
Hook-->>UI: Sections: [Create_new(items)]
UI-->>User: Render only Create_new options
Note over Hook,UI: Federation/Explore logic removed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37031 +/- ##
===========================================
- Coverage 66.33% 66.32% -0.02%
===========================================
Files 3391 3381 -10
Lines 115308 115325 +17
Branches 21129 21233 +104
===========================================
- Hits 76494 76488 -6
- Misses 36198 36226 +28
+ Partials 2616 2611 -5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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
🧹 Nitpick comments (3)
.changeset/bright-boats-speak.md (1)
2-5: Changelog is fine; consider clarifying scope in the note.Suggest: “Remove ‘Search federated rooms’ entry from the Create New menu (FDR-136)” for clearer release notes context.
apps/meteor/client/NavBarV2/NavBarPagesGroup/hooks/useCreateNewMenu.tsx (1)
14-17: Minor: align variable naming with hook for readability.
useCreateNewItems()→createRoomItemsis slightly inconsistent. ConsidercreateNewItems.Apply within this block:
- const sections = [{ title: t('Create_new'), items: createRoomItems, permission: showCreate }]; + const sections = [{ title: t('Create_new'), items: createNewItems, permission: showCreate }];And also update the declaration above:
// elsewhere in this file: const createNewItems = useCreateNewItems();apps/meteor/client/sidebar/header/actions/hooks/useCreateRoomMenu.tsx (1)
6-6: Reduce duplication: extract shared CREATE_ROOM_PERMISSIONS.The same permissions array appears in multiple hooks. Extract to a shared module (e.g., a permissions constants file) and import from both to keep them in sync.
📜 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.
📒 Files selected for processing (3)
.changeset/bright-boats-speak.md(1 hunks)apps/meteor/client/NavBarV2/NavBarPagesGroup/hooks/useCreateNewMenu.tsx(1 hunks)apps/meteor/client/sidebar/header/actions/hooks/useCreateRoomMenu.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
.changeset/bright-boats-speak.mdapps/meteor/client/sidebar/header/actions/hooks/useCreateRoomMenu.tsx
🔇 Additional comments (1)
apps/meteor/client/NavBarV2/NavBarPagesGroup/hooks/useCreateNewMenu.tsx (1)
14-17: Action required — federation UI items remain; do not approveSearch found federation-related UI hooks/labels still present; remove or update these before approving:
- apps/meteor/client/sidebar/header/actions/hooks/useMatrixFederationItems.ts
- apps/meteor/client/NavBarV2/NavBarPagesGroup/hooks/useMatrixFederationItems.ts
- apps/meteor/client/hooks/useIsFederationEnabled.ts
- apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx
- apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.tsx
- apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsx
- apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.tsx
Likely an incorrect or invalid review comment.
| const sections = [{ title: t('Create_new'), items: createRoomItems, permission: showCreate }]; | ||
|
|
||
| return sections.filter((section) => section.permission); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
LGTM — menu now only shows valid “Create new” actions.
Matches the goal to remove the federated search entry.
Quick check for leftovers:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 --type=ts --type=tsx -S \
-e 'useMatrixFederationItems' \
-e 'useIsFederationEnabled' \
-e '\bisMatrixEnabled\b' \
-e 'Search[_ ]federated[_ ]rooms' \
apps/meteor || trueLength of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== show file (if exists) =="
sed -n '1,240p' apps/meteor/client/sidebar/header/actions/hooks/useCreateRoomMenu.tsx || true
echo
echo "== search for federation-related symbols =="
rg -n -C2 -S \
-e 'useMatrixFederationItems' \
-e 'useIsFederationEnabled' \
-e '\bisMatrixEnabled\b' \
-e 'Search[_ ]federated[_ ]rooms' \
apps/meteor || true
echo
echo "== broader search for 'federat' occurrences =="
rg -n -C2 -S 'federat' apps/meteor || trueLength of output: 302103
LGTM — Create menu now filters correctly; leftover federated search items remain
useCreateRoomMenu.tsx correctly filters the Create_new section by permission. The Matrix federation search item still exists in:
- apps/meteor/client/sidebar/header/actions/hooks/useMatrixFederationItems.ts
- apps/meteor/client/NavBarV2/NavBarPagesGroup/hooks/useMatrixFederationItems.ts
Remove or gate those if the intent was to remove the federated search entry globally.
🤖 Prompt for AI Agents
In apps/meteor/client/sidebar/header/actions/hooks/useCreateRoomMenu.tsx lines
14-17 you correctly filter the Create_new section by permission, but federated
search menu items still appear elsewhere; update
apps/meteor/client/sidebar/header/actions/hooks/useMatrixFederationItems.ts and
apps/meteor/client/NavBarV2/NavBarPagesGroup/hooks/useMatrixFederationItems.ts
to either remove the Matrix federation search item or gate it behind the same
permission check used for Create_new (add a permission prop/flag, return [] when
not allowed), ensuring these hooks return no items when the user lacks the
permission so the federated search entry is removed globally.
FDR-136
Proposed changes (including videos or screenshots)
Remove
Search federated roomsfrom Create new menuIssue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit