feat(channel-settings): add per-channel link preview setting #38360
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: db2d8e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 |
|
|
WalkthroughAdds a per-room "Enable Link Previews" setting: new room field, UI toggle in EditRoomInfo, persistence in room settings saver, and a check in the OEmbed post-save hook to skip previews when the room disables them. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as EditRoomInfo UI
participant Server as Settings API
participant DB as Rooms DB
participant Hook as AfterSaveOEmbed Hook
User->>UI: Toggle "Enable Link Previews"
UI->>Server: Save settings (linksEmbed)
Server->>DB: updateOne Rooms { _id:rid } $set linksEmbed
DB-->>Server: update OK
Server-->>UI: saved
User->>UI: Send message containing URL
UI->>Hook: message saved event
Hook->>DB: fetch Room by message.rid
DB-->>Hook: return room (linksEmbed)
alt linksEmbed is false
Hook-->>Hook: skip embedding, leave message without preview
else linksEmbed true/undefined
Hook->>Hook: process embedding, attach preview
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx`:
- Around line 495-513: Replace the hardcoded label text and remove the inline
JSX comments: change FieldLabel htmlFor={linksEmbedField} content from "Enable
Link Previews" to use the i18n helper t('Enable_Link_Previews') (or the
appropriate key used in the codebase) so the string goes through translation,
and delete the surrounding {/* START OF NEW LINK PREVIEW TOGGLE */} and {/* END
OF NEW LINK PREVIEW TOGGLE */} comment nodes; leave the Controller
(name='linksEmbed') and ToggleSwitch usage (id={linksEmbedField},
checked={value}, disabled={isFederated}) unchanged.
🧹 Nitpick comments (1)
apps/meteor/server/services/messages/hooks/AfterSaveOEmbed.ts (1)
11-11: Avoid a room lookup when the message has no URLs.
This reduces DB traffic in the hot path and keeps the hook cheap; also remove the new inline comments per guidelines.♻️ Suggested refactor
- // 1. Keep the Global Check if (!settings.get('API_Embed')) { return message; } - // 2. NEW: Check the Per-Channel Setting - // We fetch the room to see if the user disabled previews for this specific channel. - const room = await Rooms.findOneById(message.rid, { projection: { linksEmbed: 1 } }); - if (room?.linksEmbed === false) { - return message; // STOP here if the room says "No Previews" - } - - // 3. Keep the rest of the existing checks... if (!Array.isArray(message.urls)) { return message; } + + const room = await Rooms.findOneById(message.rid, { projection: { linksEmbed: 1 } }); + if (room?.linksEmbed === false) { + return message; + }Also applies to: 334-347
apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx`:
- Around line 496-512: The linksEmbed toggle (Controller for name='linksEmbed',
id linksEmbedField, rendering ToggleSwitch) needs a FieldHint added to match
other toggles and should be rendered conditionally using the same
permission-guard pattern as the other fields; wrap the existing <Field> block in
the same permission check used for the other toggles (e.g., the canView... guard
pattern) and add a <FieldHint> with the appropriate translation key (e.g.,
t('Links_Previews_Hint') or similar) to explain linksEmbed behavior so it
matches the UI/permission consistency of readOnly/archived/reactWhenReadOnly.
🧹 Nitpick comments (2)
apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx (1)
231-233: Minor: Remove extra blank lines for consistency.The other
useId()declarations are consecutive without blank lines between them.🔧 Suggested cleanup
const retentionIgnoreThreads = useId(); - - const linksEmbedField = useId();apps/meteor/server/services/messages/hooks/AfterSaveOEmbed.ts (1)
335-351: Remove inline comments per coding guidelines.The added comments violate the coding guideline: "Avoid code comments in the implementation" for TypeScript files. The code is self-explanatory and doesn't require these comments.
♻️ Suggested refactor
const rocketUrlParser = async function (message: IMessage): Promise<IMessage> { log.debug({ msg: 'Parsing message URLs' }); - // 1. Global Check stays at the top if (!settings.get('API_Embed')) { return message; } - // 2. CHECK URLs FIRST (Optimization) - // If the message has no URLs, we stop here. We don't bother the database. if (!Array.isArray(message.urls)) { return message; } - // 3. NOW check the Room Setting - // We only fetch the room data if we know we actually need to process URLs. const room = await Rooms.findOneById(message.rid, { projection: { linksEmbed: 1 } }); if (room?.linksEmbed === false) { return message; }As per coding guidelines, avoid code comments in TypeScript implementations.
| <Field> | ||
| <FieldRow> | ||
| <FieldLabel htmlFor={linksEmbedField}>{t('Enable_Link_Previews')}</FieldLabel> | ||
| <Controller | ||
| control={control} | ||
| name='linksEmbed' | ||
| render={({ field: { value, ...field } }) => ( | ||
| <ToggleSwitch | ||
| id={linksEmbedField} | ||
| {...field} | ||
| checked={value} | ||
| disabled={isFederated} | ||
| /> | ||
| )} | ||
| /> | ||
| </FieldRow> | ||
| </Field> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file structure and find the relevant context
cat -n apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx | sed -n '480,530p'Repository: RocketChat/Rocket.Chat
Length of output: 2007
🏁 Script executed:
# Check for permission checks on other toggles in the same file
rg -n 'canView|Permission' apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx -B2 -A2Repository: RocketChat/Rocket.Chat
Length of output: 2560
🏁 Script executed:
# Search for linksEmbed references to understand permission handling
rg -n 'linksEmbed' apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/ -r . --type ts -C3Repository: RocketChat/Rocket.Chat
Length of output: 4133
🏁 Script executed:
# Check the useEditRoomPermissions hook or similar to understand the permission pattern
rg -n 'useEditRoomPermissions\|const.*Permission.*=' apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx -A3Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Find and read the useEditRoomPermissions file
fd -n 'useEditRoomPermissions.ts' apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/Repository: RocketChat/Rocket.Chat
Length of output: 296
🏁 Script executed:
# Read the useEditRoomPermissions file to see all available permissions
cat -n apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/useEditRoomPermissions.tsRepository: RocketChat/Rocket.Chat
Length of output: 3580
🏁 Script executed:
# Check if LINKS_EMBED or similar setting exists in RoomSettingsEnum
rg -n 'LINKS_EMBED|RoomSettingsEnum' apps/meteor --type ts | grep -i 'links\|embed'Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Search for RoomSettingsEnum definition to see all available settings
fd 'RoomSettingsEnum' --type tsRepository: RocketChat/Rocket.Chat
Length of output: 235
🏁 Script executed:
# Search for RoomSettingsEnum definition
find . -name '*RoomSettingsEnum*' -type fRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Search for LINKS_EMBED in definition directory
rg 'LINKS_EMBED|linksEmbed' definition/ --max-count 20Repository: RocketChat/Rocket.Chat
Length of output: 117
🏁 Script executed:
# Search for IRoomTypeConfig file mentioned in imports
find . -name 'IRoomTypeConfig*' -type fRepository: RocketChat/Rocket.Chat
Length of output: 109
🏁 Script executed:
# Read the IRoomTypeConfig.ts file to find RoomSettingsEnum
cat -n ./apps/meteor/definition/IRoomTypeConfig.tsRepository: RocketChat/Rocket.Chat
Length of output: 4798
🏁 Script executed:
# Also check if linksEmbed is mentioned in any permission or settings context
rg 'linksEmbed' apps/meteor --max-count 30Repository: RocketChat/Rocket.Chat
Length of output: 1480
🏁 Script executed:
# Search for global "Embed Link Previews" or similar admin setting
rg 'Embed.*Link|linksEmbed.*admin|Link.*Previews.*setting' apps/meteor --max-count 20Repository: RocketChat/Rocket.Chat
Length of output: 774
🏁 Script executed:
# Check the PR or recent commits mentioning linksEmbed to understand PR objectives
git log --oneline --all --grep='linksEmbed\|link.*preview' | head -20Repository: RocketChat/Rocket.Chat
Length of output: 711
🏁 Script executed:
# Look for FieldHint usage patterns in the same file
rg -A3 '<FieldHint' apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsxRepository: RocketChat/Rocket.Chat
Length of output: 1148
Add a FieldHint to explain the linksEmbed behavior and consider adding a permission check for consistency.
The linksEmbed toggle lacks a FieldHint, while all other toggles in this section (readOnly, archived, reactWhenReadOnly) have explanatory hints. Additionally, linksEmbed is rendered unconditionally, unlike the other toggles which all check permission guards (canViewReadOnly, canViewArchived, canViewJoinCode, canViewHideSysMes). Consider whether this field should follow the same permission pattern established by the rest of the section.
🔧 Suggested improvement with hint
<Field>
<FieldRow>
<FieldLabel htmlFor={linksEmbedField}>{t('Enable_Link_Previews')}</FieldLabel>
<Controller
control={control}
name='linksEmbed'
render={({ field: { value, ...field } }) => (
<ToggleSwitch
id={linksEmbedField}
{...field}
checked={value}
disabled={isFederated}
+ aria-describedby={`${linksEmbedField}-hint`}
/>
)}
/>
</FieldRow>
+ <FieldHint id={`${linksEmbedField}-hint`}>
+ {t('Enable_Link_Previews_Description')}
+ </FieldHint>
</Field>🤖 Prompt for AI Agents
In
`@apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx`
around lines 496 - 512, The linksEmbed toggle (Controller for name='linksEmbed',
id linksEmbedField, rendering ToggleSwitch) needs a FieldHint added to match
other toggles and should be rendered conditionally using the same
permission-guard pattern as the other fields; wrap the existing <Field> block in
the same permission check used for the other toggles (e.g., the canView... guard
pattern) and add a <FieldHint> with the appropriate translation key (e.g.,
t('Links_Previews_Hint') or similar) to explain linksEmbed behavior so it
matches the UI/permission consistency of readOnly/archived/reactWhenReadOnly.
dougfabris
changed the title
dougfabris
added
area: ui
3 participants
Proposed changes (including videos or screenshots)
This PR implements the requested feature to enable or disable Link Previews (OEmbed) on a per-channel basis. Previously, this setting was only available globally via the Administration settings.
Summary of Changes:
linksEmbedboolean property to theIRoominterface inpackages/core-typings.EditRoomInfo.tsx).saveRoomSettingsserver method to whitelist and validate thelinksEmbedfield so it can be saved to the database.rocketUrlParser(OEmbed service) to check the room'slinksEmbedsetting. If set tofalse, it skips fetching metadata for that specific message.Proof of Fix:
toggle button added in fronted
screenshot
video
Screencast.From.2026-01-27.14-39-19.mp4
Issue(s)
Closes #7734
Steps to test or reproduce
#general).https://google.com) in the chat.https://github.com) in the chat.Further comments
This implementation defaults to
true(enabled) if the setting is undefined in the database, ensuring backward compatibility with existing rooms. It strictly respects the Global Setting first (if Global is OFF, this switch does nothing).Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.