Skip to content

refactor: migrate rooms.favorite endpoint to new OpenAPI pattern with AJV validation #38929

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.

Sign up for GitHub

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

Closed
Verifieddanny wants to merge 4 commits into from
Closed

refactor: migrate rooms.favorite endpoint to new OpenAPI pattern with AJV validation #38929

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
18b6d0f
refactor: migrate rooms.favorite endpoint to new OpenAPI pattern with…
Verifieddanny Feb 23, 2026
d4cb375
Merge branch 'develop' into refactor/migrate-rooms-favorite-endpoint
Verifieddanny Feb 23, 2026
9381755
Fix: Lint error
Verifieddanny Feb 23, 2026
528a5d6
Merge branch 'develop' into refactor/migrate-rooms-favorite-endpoint
Verifieddanny Feb 23, 2026
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
5 changes: 5 additions & 0 deletions .changeset/migrate-rooms-favorite-endpoint.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Migrated rooms.favorite endpoint to new OpenAPI pattern with AJV validation
52 changes: 37 additions & 15 deletions apps/meteor/app/api/server/v1/rooms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
import { getPaginationItems } from '../helpers/getPaginationItems';
import { getUserFromParams } from '../helpers/getUserFromParams';
import { MultipartUploadHandler } from '../lib/MultipartUploadHandler';
import {

Check failure on line 59 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

There should be at least one empty line between import groups
findAdminRoom,
findAdminRooms,
findAdminRoomsAutocomplete,
Expand All @@ -64,6 +64,7 @@
findChannelAndPrivateAutocompleteWithPagination,
findRoomsAvailableForTeams,
} from '../lib/rooms';
import { required } from 'zod/mini';

Check failure on line 67 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

'required' is defined but never used

Check failure on line 67 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

`zod/mini` import should occur before import of `../../../../lib/rooms/adminFields`

export async function findRoomByIdOrName({
params,
Expand Down Expand Up @@ -311,25 +312,46 @@
},
);

API.v1.addRoute(
'rooms.favorite',
{ authRequired: true },
{
async post() {
const { favorite } = this.bodyParams;
export const roomsFavoriteEndpoint = API.v1.post('rooms.favorite', {

Check failure on line 315 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

Replace `'rooms.favorite',·` with `⏎↹'rooms.favorite',⏎↹`
authRequired: true,

Check failure on line 316 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

Insert `↹`
body: ajv.compile<{ favorite: boolean} & ({ roomId: string} | { roomName: string })>({

Check failure on line 317 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

Replace `↹body:·ajv.compile<{·favorite:·boolean}·&·({·roomId:·string}·|·{·roomName:··` with `↹↹body:·ajv.compile<{·favorite:·boolean·}·&·({·roomId:·string·}·|·{·roomName:·`
type: 'object',

Check failure on line 318 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

Insert `↹`
properties: {

Check failure on line 319 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

Insert `↹`
roomId: { type: 'string', minLength: 1 },

Check failure on line 320 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

Insert `↹`
roomName: { type: 'string', minLength: 1 },

Check failure on line 321 in apps/meteor/app/api/server/v1/rooms.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint 

Insert `↹`
favorite: { type: 'boolean' },
},
oneOf: [
{ required: ['roomId', 'favorite'] },
{ required: ['roomName', 'favorite'] },
],
additionalProperties: false
}),
response: {
200: ajv.compile({
type: 'object',
properties: { success: { type: 'boolean', enum: [true] } },
required: ['success'],
additionalProperties: false,
}),
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
}
},
async function action() {
const { favorite } = this.bodyParams;
const room = await findRoomByIdOrName({ params: this.bodyParams });

if (!this.bodyParams.hasOwnProperty('favorite')) {
return API.v1.failure("The 'favorite' param is required");
}
await toggleFavoriteMethod(this.userId, room._id, favorite);
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Feb 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

findRoomByIdOrName with the default checkedArchived: true may silently block favoriting archived rooms.

The old endpoint did not go through findRoomByIdOrName (per the AI summary, room resolution "now" happens here), so users who had archived rooms in their favorites could previously still toggle them. With checkedArchived: true the handler now throws error-room-archived before reaching toggleFavoriteMethod, making the behavior change inconsistent with the PR claim that "business logic is unchanged."

If archiving a room should not prevent it from being un-favorited, pass checkedArchived: false:

🐛 Proposed fix 
-	const room = await findRoomByIdOrName({ params: this.bodyParams });
+	const room = await findRoomByIdOrName({ params: this.bodyParams, checkedArchived: false });
#!/bin/bash
# Check what the old rooms.favorite addRoute handler did before this PR (git blame / old code search)
rg -n 'rooms\.favorite' apps/meteor/app/api/server/v1/rooms.ts
rg -n 'toggleFavorite' apps/meteor/app/api/server/v1/rooms.ts
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/api/server/v1/rooms.ts` around lines 341 - 345, The handler
now calls findRoomByIdOrName with its default checkedArchived:true which causes
archived rooms to throw error-room-archived and prevents toggleFavoriteMethod
from running; update the action function to call findRoomByIdOrName with
checkedArchived:false so archived rooms can still be unfavorited (i.e., pass {
params: this.bodyParams, checkedArchived: false } to findRoomByIdOrName) while
leaving toggleFavoriteMethod(this.userId, room._id, favorite) unchanged.


const room = await findRoomByIdOrName({ params: this.bodyParams });
return API.v1.success();
});

await toggleFavoriteMethod(this.userId, room._id, favorite);
type RoomsFavoriteEndpoint = ExtractRoutesFromAPI<typeof roomsFavoriteEndpoint>;

return API.v1.success();
},
},
);
declare module '@rocket.chat/rest-typings' {
interface Endpoints extends RoomsFavoriteEndpoint {}
}

API.v1.addRoute(
'rooms.cleanHistory',
Expand Down
14 changes: 0 additions & 14 deletions packages/rest-typings/src/v1/rooms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,20 +805,6 @@ export type RoomsEndpoints = {
POST: (params: { roomId: string; notifications: Notifications }) => void;
};

'/v1/rooms.favorite': {
POST: (
params:
| {
roomId: string;
favorite: boolean;
}
| {
roomName: string;
favorite: boolean;
},
) => void;
};

'/v1/rooms.nameExists': {
GET: (params: { roomName: string }) => {
exists: boolean;
Expand Down
Loading
⚡