Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions apps/meteor/definition/IRoomTypeConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export const RoomMemberActions = {
ARCHIVE: 'archive',
IGNORE: 'ignore',
BLOCK: 'block',
ANNOUNCEMENT: 'roomAnnouncement',
DESCRIPTION: 'roomDescription',
Comment on lines +42 to +43
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider distinct values for member actions vs. settings.

The values 'roomAnnouncement' and 'roomDescription' are already used in RoomSettingsEnum (lines 27-28). Using identical string values for both settings and member actions may cause confusion when these constants are used interchangeably, as settings and actions represent different concepts.

Consider using distinct prefixes or suffixes to differentiate between the two contexts:

 export const RoomMemberActions = {
   ARCHIVE: 'archive',
   IGNORE: 'ignore',
   BLOCK: 'block',
-  ANNOUNCEMENT: 'roomAnnouncement',
-  DESCRIPTION: 'roomDescription',
+  ANNOUNCEMENT: 'memberActionAnnouncement',
+  DESCRIPTION: 'memberActionDescription',
   MUTE: 'mute',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ANNOUNCEMENT: 'roomAnnouncement',
DESCRIPTION: 'roomDescription',
ANNOUNCEMENT: 'memberActionAnnouncement',
DESCRIPTION: 'memberActionDescription',
🤖 Prompt for AI Agents
In apps/meteor/definition/IRoomTypeConfig.ts around lines 42-43, the
ANNOUNCEMENT and DESCRIPTION entries currently reuse the same string values as
RoomSettingsEnum, which can cause confusion; change their string values to
distinct, context-specific identifiers (e.g., 'memberActionAnnouncement' and
'memberActionDescription' or add an "_ACTION" suffix) so they no longer collide
with settings, and update any references/usages across the codebase to the new
values to maintain consistency.

MUTE: 'mute',
SET_AS_OWNER: 'setAsOwner',
SET_AS_LEADER: 'setAsLeader',
Expand All @@ -57,6 +59,7 @@ export const UiTextContext = {
export interface IRoomTypeConfig {
identifier: string;
route?: IRoomTypeRouteConfig<RouteName>;
label?: string;
}

export interface IRoomTypeClientConfig extends IRoomTypeConfig {
Expand Down Expand Up @@ -84,6 +87,12 @@ export interface IRoomTypeClientDirectives {
extractOpenRoomParams?: (routeParams: Record<string, string | null | undefined>) => { type: RoomType; reference: string };
findRoom: (identifier: string) => IRoom | undefined;
showJoinLink: (roomId: string) => boolean;
showJoinLink: (roomId: string) => boolean;
Copy link

Choose a reason for hiding this comment

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

Duplicate method declaration: The showJoinLink method is declared twice in the IRoomTypeClientDirectives interface (line 89 and line 90). This will cause a TypeScript compilation error.

Confidence: 5/5

Suggested Fix

Remove the duplicate declaration on line 90. The method is already declared earlier in the interface.

isLivechatRoom: () => boolean;
isGroupChat: (room: IRoom) => boolean;
canBeDeleted: (hasPermission: (permissionId: string, rid?: string) => Promise<boolean> | boolean, room: IRoom) => Promise<boolean>;
preventRenaming: () => boolean;
getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => Promise<Ro
Copy link

Choose a reason for hiding this comment

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

Suggestion: The new getDiscussionType signature in the client directives is truncated to Promise<Ro, making the interface invalid so any implementation or caller cannot compile; it must return Promise<RoomType> just like the server interface. [type error]

Severity Level: Minor ⚠️

Suggested change
getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => Promise<Ro
getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => Promise<RoomType>;
Why it matters? ⭐

The client directive interface now declares getDiscussionType returning the incomplete Promise<Ro, so TypeScript won't compile any implementation or consumer of this interface. Restoring Promise<RoomType> matches the server signature and fixes a real type error introduced by the PR.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** apps/meteor/definition/IRoomTypeConfig.ts
**Line:** 95:95
**Comment:**
	*Type Error: The new `getDiscussionType` signature in the client directives is truncated to `Promise<Ro`, making the interface invalid so any implementation or caller cannot compile; it must return `Promise<RoomType>` just like the server interface.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

Copy link

Choose a reason for hiding this comment

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

Incomplete method signature: The getDiscussionType method signature is truncated and incomplete. The return type Promise<Ro is cut off, which will cause a TypeScript compilation error.

Confidence: 5/5

Suggested Fix

Complete the method signature with the full return type. Based on the context, it likely should be something like:

Suggested change
getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => Promise<Ro
getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => Promise<RoomType>;

Verify the correct return type from the implementation or related type definitions.

isLivechatRoom: () => boolean;
Comment on lines 87 to 96
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Remove duplicate method definitions and fix truncated type.

This interface contains multiple critical issues that will prevent compilation:

  1. Duplicate showJoinLink on lines 89 and 90 with identical signatures
  2. Duplicate isGroupChat on lines 80 and 92 with inconsistent signatures: Partial<IRoom> vs IRoom
  3. Duplicate isLivechatRoom on lines 91 and 96 with identical signatures
  4. Truncated type definition for getDiscussionType on line 95 - return type shows Promise<Ro instead of complete type

Apply this diff to fix all issues:

   getIcon?: (room: Partial<IRoom>) => IconName;
   extractOpenRoomParams?: (routeParams: Record<string, string | null | undefined>) => { type: RoomType; reference: string };
   findRoom: (identifier: string) => IRoom | undefined;
   showJoinLink: (roomId: string) => boolean;
-  showJoinLink: (roomId: string) => boolean;
-  isLivechatRoom: () => boolean;
-  isGroupChat: (room: IRoom) => boolean;
   canBeDeleted: (hasPermission: (permissionId: string, rid?: string) => Promise<boolean> | boolean, room: IRoom) => Promise<boolean>;
   preventRenaming: () => boolean;
-  getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => Promise<Ro
+  getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => Promise<RoomType>;
   isLivechatRoom: () => boolean;
   canSendMessage: (rid: string) => boolean;

Note: Verify whether isGroupChat should accept Partial<IRoom> (line 80) or IRoom. The existing definition uses Partial<IRoom>, which is less restrictive.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome (2.1.2)

[error] 96-96: expected , but instead found isLivechatRoom

Remove isLivechatRoom

(parse)


[error] 96-96: expected , but instead found :

Remove :

(parse)

🤖 Prompt for AI Agents
In apps/meteor/definition/IRoomTypeConfig.ts around lines 87–96, remove the
duplicated method declarations (extra showJoinLink, isLivechatRoom, and the
redundant isGroupChat) so each method is declared only once; reconcile
isGroupChat to use the intended parameter type (confirm and prefer
Partial<IRoom> if that matches the rest of the codebase) and update its single
signature accordingly; fix the truncated getDiscussionType return type by
completing it to the proper type (e.g., Promise<RoomType> or the actual
project-specific type) so the full signature is valid and the interface
compiles.

canSendMessage: (rid: string) => boolean;
readOnly?: (rid: string, user: AtLeast<IUser, 'username'>) => boolean;
Expand All @@ -95,6 +104,10 @@ export interface IRoomTypeServerDirectives {
allowRoomSettingChange: (room: IRoom, setting: ValueOf<typeof RoomSettingsEnum>) => boolean;
allowMemberAction: (room: IRoom, action: ValueOf<typeof RoomMemberActions>, userId?: IUser['_id']) => Promise<boolean>;
roomName: (room: IRoom, userId?: string) => Promise<string | undefined>;
extractOpenRoomParams?: (routeParams: Record<string, string | null | undefined>) => { type: RoomType; reference: string };
findRoom: (identifier: string) => IRoom | undefined;
showJoinLink: (roomId: string) => boolean;
isLivechatRoom: () => boolean;
Comment on lines +107 to +110
Copy link

Choose a reason for hiding this comment

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

Incorrect method placement: The methods extractOpenRoomParams, findRoom, showJoinLink, and isLivechatRoom appear to be client-side methods that were incorrectly added to the IRoomTypeServerDirectives interface. These methods are already present in IRoomTypeClientDirectives and don't belong in the server directives interface.

Confidence: 5/5

Suggested Fix

Remove these four method declarations from IRoomTypeServerDirectives as they are client-side methods and should only exist in IRoomTypeClientDirectives. Server directives should contain only server-specific operations.

📍 This suggestion applies to lines 107-110

isGroupChat: (room: IRoom) => boolean;
canBeDeleted: (hasPermission: (permissionId: string, rid?: string) => Promise<boolean> | boolean, room: IRoom) => Promise<boolean>;
preventRenaming: () => boolean;
Expand Down
Loading