Skip to content

fix: im/channels/groups.counters endpoint includes deactivated users in members count #36236

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

Merged
kodiakhq merged 5 commits into from
Jun 18, 2025
Merged

fix: im/channels/groups.counters endpoint includes deactivated users in members count #36236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5e43fd4
fix: channels/groups/im counters endpoints includes deactivated users…
abhinavkrin Jun 17, 2025
578ab81
added changeset
abhinavkrin Jun 17, 2025
0c65645
test fix
abhinavkrin Jun 17, 2025
9523872
requested changes
abhinavkrin Jun 18, 2025
4bbed28
Merge branch 'develop' into fix/im-channels-groups-counters
kodiakhq[bot] Jun 18, 2025
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
7 changes: 7 additions & 0 deletions .changeset/shaggy-moles-film.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@rocket.chat/model-typings': patch
'@rocket.chat/models': patch
'@rocket.chat/meteor': patch
---

Fixes the `channels.counters`, `groups.counters` and `im.counters` endpoint to include only active users in members count.
2 changes: 1 addition & 1 deletion apps/meteor/app/api/server/v1/channels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ API.v1.addRoute(
if (access || joined) {
msgs = room.msgs;
latest = lm;
members = room.usersCount;
members = await Users.countActiveUsersInNonDMRoom(room._id);
}

return API.v1.success({
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/api/server/v1/groups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ API.v1.addRoute(
if (access || joined) {
msgs = room.msgs;
latest = lm;
members = room.usersCount;
members = await Users.countActiveUsersInNonDMRoom(room._id);
}

return API.v1.success({
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/api/server/v1/im.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ API.v1.addRoute(
if (access || joined) {
msgs = room.msgs;
latest = lm;
members = room.usersCount;
members = await Users.countActiveUsersInDMRoom(room._id);
}

return API.v1.success({
Expand Down
100 changes: 80 additions & 20 deletions apps/meteor/tests/end-to-end/api/channels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,26 +460,86 @@ describe('[Channels]', () => {
})
.end(done);
});
it('/channels.counters', (done) => {
void request
.get(api('channels.counters'))
.set(credentials)
.query({
roomId: channel._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('joined', true);
expect(res.body).to.have.property('members');
expect(res.body).to.have.property('unreads');
expect(res.body).to.have.property('unreadsFrom');
expect(res.body).to.have.property('msgs');
expect(res.body).to.have.property('latest');
expect(res.body).to.have.property('userMentions');
})
.end(done);

describe('/channels.counters', () => {
let room: IRoom;
let user1: IUser;
let user2: IUser;
let user1Creds: { 'X-Auth-Token': string; 'X-User-Id': string };

before(async () => {
// Create two users
user1 = await createUser();
user2 = await createUser();
user1Creds = await login(user1.username, password);

// Create a new public channel with both users as members
room = (
await createRoom({
type: 'c',
name: `counters-test-${Date.now()}`,
members: [user1.username as string, user2.username as string],
})
).body.channel;
});

after(async () => {
// Delete room first
await deleteRoom({ type: 'c', roomId: room._id });
// Then delete users
await Promise.all([deleteUser(user1), deleteUser(user2)]);
});

it('should require auth', async () => {
await request
.get(api('channels.counters'))
.expect('Content-Type', 'application/json')
.expect(401)
.expect((res) => {
expect(res.body).to.have.property('status', 'error');
});
});

it('should require a roomId', async () => {
await request
.get(api('channels.counters'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
});
});

it('should return counters for a channel with correct fields', async () => {
await request
.get(api('channels.counters'))
.set(user1Creds)
.query({ roomId: room._id })
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('members').that.is.a('number').and.equals(3);
expect(res.body).to.have.property('unreads').that.is.a('number');
expect(res.body).to.have.property('unreadsFrom');
expect(res.body).to.have.property('msgs').that.is.a('number');
expect(res.body).to.have.property('latest');
expect(res.body).to.have.property('joined', true);
});
});

it('should not include deactivated users in members count', async () => {
// Deactivate the second user
await request.post(api('users.setActiveStatus')).set(credentials).send({ userId: user2._id, activeStatus: false });

const res = await request.get(api('channels.counters')).set(user1Creds).query({ roomId: room._id });

expect(res.status).to.equal(200);
expect(res.body.success).to.be.true;
// Only user1 and admin remain active
expect(res.body.members).to.equal(2);
});
});

it('/channels.rename', async () => {
Expand Down
15 changes: 15 additions & 0 deletions apps/meteor/tests/end-to-end/api/direct-message.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,21 @@ describe('[Direct Messages]', () => {
});
});
});

describe('with deactived users', async () => {
before(() => request.post(api('users.setActiveStatus')).set(credentials).send({ userId: user._id, activeStatus: false }));
after(() => request.post(api('users.setActiveStatus')).set(credentials).send({ userId: user._id, activeStatus: true }));
it('should not include deactivated users in members count', async () => {
// Deactivate the second user

const res = await request.get(api('im.counters')).set(credentials).query({ roomId: directMessage._id });

expect(res.status).to.equal(200);
expect(res.body.success).to.be.true;
// Only admin remain active
expect(res.body.members).to.equal(1);
});
});
});

describe('[/im.files]', async () => {
Expand Down
83 changes: 70 additions & 13 deletions apps/meteor/tests/end-to-end/api/groups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { CI_MAX_ROOMS_PER_GUEST as maxRoomsPerGuest } from '../../data/constants
import { createGroup, deleteGroup } from '../../data/groups.helper';
import { createIntegration, removeIntegration } from '../../data/integration.helper';
import { updatePermission, updateSetting } from '../../data/permissions.helper';
import { createRoom } from '../../data/rooms.helper';
import { createRoom, deleteRoom } from '../../data/rooms.helper';
import { deleteTeam } from '../../data/teams.helper';
import { testFileUploads } from '../../data/uploads.helper';
import { adminUsername, password } from '../../data/user';
Expand Down Expand Up @@ -1424,26 +1424,83 @@ describe('[Groups]', () => {
});

describe('/groups.counters', () => {
it('should return group counters', (done) => {
void request
let room: IRoom;
let user1: IUser;
let user2: IUser;
let user1Creds: { 'X-Auth-Token': string; 'X-User-Id': string };

before(async () => {
// Create two users
user1 = await createUser();
user2 = await createUser();
user1Creds = await login(user1.username, password);

// Create a new public channel with both users as members
room = (
await createRoom({
type: 'p',
name: `counters-test-${Date.now()}`,
members: [user1.username as string, user2.username as string],
})
).body.group;
});

after(async () => {
// Delete room first
await deleteRoom({ type: 'p', roomId: room._id });
// Then delete users
await Promise.all([deleteUser(user1), deleteUser(user2)]);
});

it('should require auth', async () => {
await request
.get(api('groups.counters'))
.expect('Content-Type', 'application/json')
.expect(401)
.expect((res) => {
expect(res.body).to.have.property('status', 'error');
});
});

it('should require a roomId', async () => {
await request
.get(api('groups.counters'))
.set(credentials)
.query({
roomId: group._id,
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
});
});

it('should return counters for a channel with correct fields', async () => {
await request
.get(api('groups.counters'))
.set(user1Creds)
.query({ roomId: room._id })
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('joined', true);
expect(res.body).to.have.property('members');
expect(res.body).to.have.property('unreads');
expect(res.body).to.have.property('members').that.is.a('number').and.equals(3);
expect(res.body).to.have.property('unreads').that.is.a('number');
expect(res.body).to.have.property('unreadsFrom');
expect(res.body).to.have.property('msgs');
expect(res.body).to.have.property('msgs').that.is.a('number');
expect(res.body).to.have.property('latest');
expect(res.body).to.have.property('userMentions');
})
.end(done);
expect(res.body).to.have.property('joined', true);
});
});

it('should not include deactivated users in members count', async () => {
// Deactivate the second user
await request.post(api('users.setActiveStatus')).set(credentials).send({ userId: user2._id, activeStatus: false });

const res = await request.get(api('groups.counters')).set(user1Creds).query({ roomId: room._id });

expect(res.status).to.equal(200);
expect(res.body.success).to.be.true;
// Only user1 and admin remain active
expect(res.body.members).to.equal(2);
});
});

Expand Down
2 changes: 2 additions & 0 deletions packages/model-typings/src/models/IUsersModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,4 +442,6 @@ export interface IUsersModel extends IBaseModel<IUser> {
countUsersInRoles(roles: IRole['_id'][]): Promise<number>;
countAllUsersWithPendingAvatar(): Promise<number>;
findOneByIdAndRole(userId: IUser['_id'], role: string, options: FindOptions<IUser>): Promise<IUser | null>;
countActiveUsersInNonDMRoom(rid: string): Promise<number>;
countActiveUsersInDMRoom(rid: string): Promise<number>;
}
14 changes: 13 additions & 1 deletion packages/models/src/models/Users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import type {
FindOneAndUpdateOptions,
} from 'mongodb';

import { Subscriptions } from '../index';
import { Rooms, Subscriptions } from '../index';
import { BaseRaw } from './BaseRaw';

const queryStatusAgentOnline = (extraFilters = {}, isLivechatEnabledWhenAgentIdle?: boolean): Filter<IUser> => ({
Expand Down Expand Up @@ -3410,4 +3410,16 @@ export class UsersRaw extends BaseRaw<IUser, DefaultFields<IUser>> implements IU
},
);
}

countActiveUsersInNonDMRoom(rid: string) {
return this.countDocuments({ active: true, __rooms: rid });
}

async countActiveUsersInDMRoom(rid: string) {
const room = await Rooms.findOneById(rid, { projection: { uids: 1 } });
if (!room?.uids?.length) {
return 0;
}
return this.countDocuments({ _id: { $in: room.uids }, active: true });
}
}
Loading