Skip to content

Commit 2609433

Browse files
ricardogarimricardogarim
committed
hold declared status for connectionless bots/apps
1 parent 531d35f commit 2609433

7 files changed

Lines changed: 145 additions & 36 deletions

File tree

‎apps/meteor/tests/end-to-end/api/calendar.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -668,10 +668,10 @@ describe('[Calendar Events]', () => {
668668

669669
(IS_EE ? describe : describe.skip)('[Calendar Events Status Sync]', () => {
670670
before(async () => {
671-
await request.post('/api/v1/users.setStatus').set(userCredentials).send({ status: 'away' }).expect(200);
671+
await request.post('/api/v1/users.setStatus').set(userCredentials).send({ status: 'online' }).expect(200);
672672
});
673673

674-
it('should set user status to busy during event and restore manual status after event ends', async () => {
674+
it('should apply a calendar (external) claim during an event and clear it after', async () => {
675675
const now = new Date();
676676
const startTime = new Date(now.getTime() + 1000);
677677
// Event cannot be less than 5 secs in duration, otherwise `processStatusChangesAtTime` would trigger both start/end status changes at the same time, due to the 5s offset
@@ -691,16 +691,15 @@ describe('[Calendar Events]', () => {
691691

692692
await sleep(3000);
693693

694-
// The display `status` remains offline because the test user has no DDP session.
695-
// The presence engine persists the claim in statusDefault but the display status
696-
// respects connection reality. We verify the claim via statusSource instead.
694+
// During the event the calendar claim is active. (Display `status` stays offline because this
695+
// REST-only user has no DDP session, so we assert the active claim via statusSource.)
697696
const statusResponseDuring = await request.get('/api/v1/users.getStatus').set(userCredentials).expect(200);
698697
expect(statusResponseDuring.body.statusSource).to.equal('external');
699698

700699
await sleep(5000);
701700

702701
const statusResponseAfter = await request.get('/api/v1/users.getStatus').set(userCredentials).expect(200);
703-
expect(statusResponseAfter.body.statusSource).to.equal('manual');
702+
expect(statusResponseAfter.body.statusSource).to.be.undefined;
704703

705704
await request.post('/api/v1/calendar-events.delete').set(userCredentials).send({ eventId }).expect(200);
706705
});

‎apps/meteor/tests/end-to-end/apps/presence-state.ts‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { App } from '@rocket.chat/core-typings';
22
import { expect } from 'chai';
3-
import { after, before, describe, it } from 'mocha';
3+
import { after, afterEach, before, describe, it } from 'mocha';
44

55
import { getCredentials, request, credentials } from '../../data/api-data';
66
import { appPresenceStateTest } from '../../data/apps/app-packages';
@@ -22,6 +22,15 @@ import { IS_EE } from '../../e2e/config/constants';
2222

2323
after(() => cleanupApps());
2424

25+
afterEach(async () => {
26+
const user = await getUserByUsername(adminUsername);
27+
await request
28+
.post(apps(`/public/${app.id}/end-active-state`))
29+
.set(credentials)
30+
.send({ userId: user._id })
31+
.expect(200);
32+
});
33+
2534
describe('[setActiveState]', () => {
2635
it('should set the user presence with status text and source', async () => {
2736
const user = await getUserByUsername(adminUsername);
@@ -44,10 +53,20 @@ import { IS_EE } from '../../e2e/config/constants';
4453
});
4554

4655
describe('[endActiveState]', () => {
47-
it('should restore the user presence to previous state', async () => {
56+
it('should restore the displaced same-priority state and reset once the queue is exhausted', async () => {
4857
const user = await getUserByUsername(adminUsername);
4958

50-
// First set an active state
59+
await request
60+
.post(apps(`/public/${app.id}/set-active-state`))
61+
.set(credentials)
62+
.send({
63+
userId: user._id,
64+
statusDefault: 'busy',
65+
statusText: 'In a meeting',
66+
statusSource: 'internal',
67+
})
68+
.expect(200);
69+
5170
await request
5271
.post(apps(`/public/${app.id}/set-active-state`))
5372
.set(credentials)
@@ -59,16 +78,25 @@ import { IS_EE } from '../../e2e/config/constants';
5978
})
6079
.expect(200);
6180

62-
// Then end it
6381
await request
6482
.post(apps(`/public/${app.id}/end-active-state`))
6583
.set(credentials)
6684
.send({ userId: user._id })
6785
.expect(200);
6886

69-
const updatedUser = await getUserByUsername(adminUsername);
70-
expect(updatedUser.statusText).to.not.equal('On a call');
71-
expect(updatedUser.statusSource).to.not.equal('internal');
87+
const restoredUser = await getUserByUsername(adminUsername);
88+
expect(restoredUser.statusText).to.equal('In a meeting');
89+
expect(restoredUser.statusSource).to.equal('internal');
90+
91+
await request
92+
.post(apps(`/public/${app.id}/end-active-state`))
93+
.set(credentials)
94+
.send({ userId: user._id })
95+
.expect(200);
96+
97+
const clearedUser = await getUserByUsername(adminUsername);
98+
expect(clearedUser.statusText).to.equal('');
99+
expect(clearedUser.statusSource).to.be.undefined;
72100
});
73101
});
74102
});

‎ee/packages/presence/src/Presence.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ type PresenceUser = Pick<
1717
IUser,
1818
| '_id'
1919
| 'username'
20+
| 'type'
2021
| 'roles'
2122
| 'status'
2223
| 'statusDefault'
@@ -358,6 +359,7 @@ export class Presence extends ServiceClass implements IPresence {
358359
? await Users.findOneById<PresenceUser>(uidOrUser, {
359360
projection: {
360361
username: 1,
362+
type: 1,
361363
roles: 1,
362364
status: 1,
363365
statusDefault: 1,

‎ee/packages/presence/src/lib/presenceEngine.spec.ts‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,16 @@ import { processPresence } from './presenceEngine';
55

66
const ONE_HOUR = 3600_000;
77

8-
type PresenceUser = Pick<IUser, 'statusDefault' | 'statusSource' | 'statusText' | 'statusExpiresAt' | 'previousState'>;
8+
type PresenceUser = Pick<IUser, 'type' | 'roles' | 'statusDefault' | 'statusSource' | 'statusText' | 'statusExpiresAt' | 'previousState'>;
99

1010
const user = (data: Partial<PresenceUser> = {}): PresenceUser => ({
1111
statusDefault: UserStatus.ONLINE,
1212
statusText: '',
1313
...data,
1414
});
1515

16+
const bot = (data: Partial<PresenceUser> = {}): PresenceUser => user({ type: 'bot', ...data });
17+
1618
const session = (status: UserStatus = UserStatus.ONLINE): IUserSessionConnection => ({
1719
id: 'random',
1820
instanceId: 'random',
@@ -61,6 +63,40 @@ describe('processPresence', () => {
6163
});
6264
});
6365

66+
describe('auto-away over an existing claim (claimless recompute)', () => {
67+
test('a manual online claim still goes AWAY on idle, without touching the claim or statusText', () => {
68+
const result = processPresence(user({ statusSource: 'manual', statusDefault: UserStatus.ONLINE, statusText: 'Working' }), [
69+
session(UserStatus.AWAY),
70+
]);
71+
72+
expect(result.values).toMatchObject({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY });
73+
expect(result.values).not.toHaveProperty('statusText');
74+
expect(result.values).not.toHaveProperty('statusSource');
75+
expect(result.values).not.toHaveProperty('statusDefault');
76+
expect(result.clear).toBeUndefined();
77+
});
78+
79+
test('a manual busy claim stays BUSY on idle (auto-away suppressed), claim preserved', () => {
80+
const result = processPresence(user({ statusSource: 'manual', statusDefault: UserStatus.BUSY, statusText: 'Focusing' }), [
81+
session(UserStatus.AWAY),
82+
]);
83+
84+
expect(result.values).toMatchObject({ status: UserStatus.BUSY, statusConnection: UserStatus.AWAY });
85+
expect(result.values).not.toHaveProperty('statusText');
86+
expect(result.values).not.toHaveProperty('statusSource');
87+
expect(result.clear).toBeUndefined();
88+
});
89+
90+
test('when the user becomes active again the manual online claim resolves back to ONLINE', () => {
91+
const result = processPresence(user({ statusSource: 'manual', statusDefault: UserStatus.ONLINE, statusText: 'Working' }), [
92+
session(UserStatus.ONLINE),
93+
]);
94+
95+
expect(result.values).toMatchObject({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE });
96+
expect(result.values).not.toHaveProperty('statusText');
97+
});
98+
});
99+
64100
describe('setActive', () => {
65101
test('should apply manual claim when user is online', () => {
66102
const result = processPresence(user(), [session()], {
@@ -103,14 +139,15 @@ describe('processPresence', () => {
103139
expect(result.values.previousState).toBeUndefined();
104140
});
105141

106-
test('should overwrite when same priority claim arrives', () => {
142+
test('should display the new claim and save the displaced one as previousState on same priority', () => {
107143
const result = processPresence(
108144
user({ statusSource: 'internal', statusDefault: UserStatus.BUSY, statusText: 'On a call' }),
109145
[session()],
110146
{ type: 'setActive', newState: { statusDefault: UserStatus.BUSY, statusText: 'In a meeting', statusSource: 'internal' } },
111147
);
112148
expect(result.values.statusText).toBe('In a meeting');
113149
expect(result.values.statusSource).toBe('internal');
150+
expect(result.values.previousState).toMatchObject({ statusSource: 'internal', statusText: 'On a call' });
114151
});
115152

116153
test('should queue lower priority claim as previousState', () => {
@@ -331,4 +368,44 @@ describe('processPresence', () => {
331368
expect(result.values.status).toBe(UserStatus.OFFLINE);
332369
});
333370
});
371+
372+
describe('no connection - humans (connection-bound)', () => {
373+
test('clearActive: explicit "set online" is honored (stays online)', () => {
374+
const result = processPresence(user({ statusSource: 'manual', statusDefault: UserStatus.BUSY }), [], { type: 'clearActive' });
375+
expect(result.values.status).toBe(UserStatus.ONLINE);
376+
});
377+
378+
test('setActive: busy claim is persisted but display is OFFLINE', () => {
379+
const result = processPresence(user(), [], {
380+
type: 'setActive',
381+
newState: { statusDefault: UserStatus.BUSY, statusSource: 'manual', statusText: 'Focus' },
382+
});
383+
expect(result.values.status).toBe(UserStatus.OFFLINE);
384+
expect(result.values.statusDefault).toBe(UserStatus.BUSY); // claim persisted for reconnect
385+
});
386+
387+
test('endActive: ending a claim reverts to connection reality (OFFLINE), not statusDefault', () => {
388+
const result = processPresence(user({ statusSource: 'manual', statusDefault: UserStatus.BUSY }), [], { type: 'endActive' });
389+
expect(result.values.status).toBe(UserStatus.OFFLINE);
390+
});
391+
});
392+
393+
describe('no connection - service users (bots/apps) hold their declared status', () => {
394+
test.each([
395+
['type "bot"', bot(), UserStatus.BUSY, 'manual' as const],
396+
['type "app"', user({ type: 'app' }), UserStatus.AWAY, 'external' as const],
397+
['role "bot" (type "user")', user({ type: 'user', roles: ['bot'] }), UserStatus.BUSY, 'manual' as const],
398+
])('setActive: a service user (%s) displays its declared status, not OFFLINE', (_label, serviceUser, statusDefault, statusSource) => {
399+
const result = processPresence(serviceUser, [], {
400+
type: 'setActive',
401+
newState: { statusDefault, statusSource },
402+
});
403+
expect(result.values.status).toBe(statusDefault);
404+
});
405+
406+
test('endActive: a bot whose claim is released reverts to OFFLINE (re-assert to restore)', () => {
407+
const result = processPresence(bot({ statusSource: 'manual', statusDefault: UserStatus.BUSY }), [], { type: 'endActive' });
408+
expect(result.values.status).toBe(UserStatus.OFFLINE);
409+
});
410+
});
334411
});

‎ee/packages/presence/src/lib/presenceEngine.ts‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@ function computeStatus(statusConnection: UserStatus, statusDefault: UserStatus):
5454
return statusDefault;
5555
}
5656

57+
function disconnectedStatus(claimType: ClaimUpdate['type'], isServiceUser: boolean, statusDefault: UserStatus): UserStatus {
58+
switch (claimType) {
59+
case 'clearActive':
60+
return statusDefault;
61+
case 'setActive':
62+
return isServiceUser ? statusDefault : UserStatus.OFFLINE;
63+
case 'endActive':
64+
return UserStatus.OFFLINE;
65+
}
66+
}
67+
5768
/**
5869
* Resolves a claim update against the user's current state using the priority system.
5970
* Returns the DB fields to set/unset, or null if the claim is rejected.
@@ -99,8 +110,10 @@ function resolveIntent(
99110
// displaced claim and clears any queued one, so it never gets auto-reverted.
100111
const isManual = newState.statusSource === 'manual';
101112

102-
// higher priority -> apply new; stash displaced claim unless the new claim is manual
103-
if (newPriority < currentPriority) {
113+
// higher or equal priority -> apply new; stash displaced claim unless the new claim is manual.
114+
// Equal priority (e.g. two 'internal' sources: a voice call + a video conference) stashes the
115+
// displaced claim too, so ending the newer event restores the still-active earlier one (UC-20).
116+
if (newPriority <= currentPriority) {
104117
const previousState =
105118
!isManual && user.statusSource
106119
? {
@@ -115,27 +128,14 @@ function resolveIntent(
115128
set: {
116129
statusDefault: newState.statusDefault,
117130
statusSource: newState.statusSource,
118-
...(newState.statusText != null && { statusText: newState.statusText }),
131+
...(newState.statusText !== undefined && { statusText: newState.statusText }),
119132
...(newState.statusExpiresAt && { statusExpiresAt: newState.statusExpiresAt }),
120133
...(previousState && { previousState }),
121134
},
122135
unset: fieldsToUnset(newState, isManual ? ['previousState'] : []),
123136
};
124137
}
125138

126-
// same priority -> overwrite; manual also drops any queued previous
127-
if (newPriority === currentPriority) {
128-
return {
129-
set: {
130-
statusDefault: newState.statusDefault,
131-
statusSource: newState.statusSource,
132-
...(newState.statusText != null && { statusText: newState.statusText }),
133-
...(newState.statusExpiresAt && { statusExpiresAt: newState.statusExpiresAt }),
134-
},
135-
unset: fieldsToUnset(newState, isManual ? ['previousState'] : []),
136-
};
137-
}
138-
139139
const previousState = {
140140
statusDefault: newState.statusDefault,
141141
statusSource: newState.statusSource,
@@ -171,7 +171,7 @@ function resolveIntent(
171171
* Returns the DB fields to $set and optionally $unset.
172172
*/
173173
export function processPresence(
174-
user: Pick<IUser, 'statusDefault' | 'statusSource' | 'statusText' | 'statusExpiresAt' | 'previousState'>,
174+
user: Pick<IUser, 'type' | 'roles' | 'statusDefault' | 'statusSource' | 'statusText' | 'statusExpiresAt' | 'previousState'>,
175175
sessions: IUserSessionConnection[],
176176
claimUpdate?: ClaimUpdate,
177177
): { values: Record<string, unknown>; clear?: string[] } {
@@ -194,11 +194,11 @@ export function processPresence(
194194
const statusDefault = set.statusDefault ?? user.statusDefault ?? UserStatus.ONLINE;
195195
const clear = unset.length ? unset : undefined;
196196

197-
// setActive with no DDP sessions: user is disconnected but holding a claim — persist
198-
// it for reconnect but display OFFLINE. Other types (clearActive/endActive) use
199-
// statusDefault so REST-only callers and bots can appear online.
197+
// No live connection, so the displayed status comes from the claim instead of the connection.
198+
// The helper applies the rule (humans fall back to offline; bots/apps can still show a status).
200199
if (!sessions.length) {
201-
const status = claimUpdate.type === 'setActive' ? UserStatus.OFFLINE : statusDefault;
200+
const isServiceUser = user.type === 'bot' || user.type === 'app' || (user.roles?.includes('bot') ?? false);
201+
const status = disconnectedStatus(claimUpdate.type, isServiceUser, statusDefault);
202202
return { values: { ...set, status, statusConnection: UserStatus.OFFLINE }, clear };
203203
}
204204

‎packages/model-typings/src/models/IUsersModel.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ export interface IUsersModel extends IBaseModel<IUser> {
177177
IUser,
178178
| '_id'
179179
| 'username'
180+
| 'type'
180181
| 'roles'
181182
| 'status'
182183
| 'statusDefault'

‎packages/models/src/models/Users.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,7 @@ export class UsersRaw extends BaseRaw<IUser, DefaultFields<IUser>> implements IU
11031103
IUser,
11041104
| '_id'
11051105
| 'username'
1106+
| 'type'
11061107
| 'roles'
11071108
| 'status'
11081109
| 'statusDefault'
@@ -1117,6 +1118,7 @@ export class UsersRaw extends BaseRaw<IUser, DefaultFields<IUser>> implements IU
11171118
{
11181119
projection: {
11191120
username: 1,
1121+
type: 1,
11201122
roles: 1,
11211123
status: 1,
11221124
statusDefault: 1,

0 commit comments

Comments
 (0)