diff --git a/.changeset/lazy-pianos-care.md b/.changeset/lazy-pianos-care.md new file mode 100644 index 0000000000000..d5afd4cb714af --- /dev/null +++ b/.changeset/lazy-pianos-care.md @@ -0,0 +1,9 @@ +--- +'@rocket.chat/model-typings': minor +'@rocket.chat/core-typings': minor +'@rocket.chat/rest-typings': minor +'@rocket.chat/models': minor +'@rocket.chat/meteor': minor +--- + +Enhance user's deactivated state handling to correctly distinguish between pending and deactivated users. diff --git a/apps/meteor/app/api/server/lib/users.ts b/apps/meteor/app/api/server/lib/users.ts index e1451f4dd6698..9bb27e09f02ec 100644 --- a/apps/meteor/app/api/server/lib/users.ts +++ b/apps/meteor/app/api/server/lib/users.ts @@ -131,6 +131,7 @@ type FindPaginatedUsersByStatusProps = { searchTerm: string; hasLoggedIn: boolean; type: string; + inactiveReason?: ('deactivated' | 'pending_approval' | 'idle_too_long')[]; }; export async function findPaginatedUsersByStatus({ @@ -143,6 +144,7 @@ export async function findPaginatedUsersByStatus({ searchTerm, hasLoggedIn, type, + inactiveReason, }: FindPaginatedUsersByStatusProps) { const actualSort: Record = sort || { username: 1 }; if (sort?.status) { @@ -199,6 +201,26 @@ export async function findPaginatedUsersByStatus({ match.roles = { $in: roles }; } + if (inactiveReason) { + const inactiveReasonCondition = { + $or: [ + { inactiveReason: { $in: inactiveReason } }, + // This condition is to make it backward compatible with the old behavior + // The deactivated users not having the inactiveReason field should be returned as well + ...(inactiveReason.includes('deactivated') || inactiveReason.includes('idle_too_long') + ? [{ inactiveReason: { $exists: false } }] + : []), + ], + }; + + if (match.$or) { + match.$and = [{ $or: match.$or }, inactiveReasonCondition]; + delete match.$or; + } else { + Object.assign(match, inactiveReasonCondition); + } + } + const { cursor, totalCount } = Users.findPaginated( { ...match, diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index d4f0012cd5784..e949df339aad0 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -611,7 +611,7 @@ API.v1.addRoute( const { offset, count } = await getPaginationItems(this.queryParams); const { sort } = await this.parseJsonQuery(); - const { status, hasLoggedIn, type, roles, searchTerm } = this.queryParams; + const { status, hasLoggedIn, type, roles, searchTerm, inactiveReason } = this.queryParams; return API.v1.success( await findPaginatedUsersByStatus({ @@ -624,6 +624,7 @@ API.v1.addRoute( searchTerm, hasLoggedIn, type, + inactiveReason, }), ); }, diff --git a/apps/meteor/app/authentication/server/startup/index.js b/apps/meteor/app/authentication/server/startup/index.js index a56f237c6e527..b7704cbe57357 100644 --- a/apps/meteor/app/authentication/server/startup/index.js +++ b/apps/meteor/app/authentication/server/startup/index.js @@ -204,7 +204,9 @@ const onCreateUserAsync = async function (options, user = {}) { } user.status = 'offline'; + user.active = user.active !== undefined ? user.active : !settings.get('Accounts_ManuallyApproveNewUsers'); + user.inactiveReason = settings.get('Accounts_ManuallyApproveNewUsers') && !user.active ? 'pending_approval' : undefined; if (!user.name) { if (options.profile) { diff --git a/apps/meteor/client/views/admin/users/hooks/useFilteredUsers.ts b/apps/meteor/client/views/admin/users/hooks/useFilteredUsers.ts index 435c033ed529f..7fd57f34dbdad 100644 --- a/apps/meteor/client/views/admin/users/hooks/useFilteredUsers.ts +++ b/apps/meteor/client/views/admin/users/hooks/useFilteredUsers.ts @@ -29,16 +29,16 @@ const useFilteredUsers = ({ searchTerm, prevSearchTerm, sortData, paginationData const listUsersPayload: Partial> = { all: {}, pending: { - hasLoggedIn: false, type: 'user', - status: 'active', + status: 'deactivated', + inactiveReason: ['pending_approval'], }, active: { - hasLoggedIn: true, status: 'active', }, deactivated: { status: 'deactivated', + inactiveReason: ['deactivated', 'idle_too_long'], }, }; diff --git a/apps/meteor/client/views/admin/users/hooks/usePendingUsersCount.ts b/apps/meteor/client/views/admin/users/hooks/usePendingUsersCount.ts index 7010a3a50ff18..2aa1cb8f1f0d4 100644 --- a/apps/meteor/client/views/admin/users/hooks/usePendingUsersCount.ts +++ b/apps/meteor/client/views/admin/users/hooks/usePendingUsersCount.ts @@ -11,8 +11,8 @@ const usePendingUsersCount = (users: Serialized | undefined) queryFn: async () => { const payload: UsersListStatusParamsGET = { - hasLoggedIn: false, status: 'deactivated', + inactiveReason: ['pending_approval'], type: 'user', count: 1, }; diff --git a/apps/meteor/tests/e2e/admin-users.spec.ts b/apps/meteor/tests/e2e/admin-users.spec.ts index 6ac8b35a82400..6b9a697eaa9c3 100644 --- a/apps/meteor/tests/e2e/admin-users.spec.ts +++ b/apps/meteor/tests/e2e/admin-users.spec.ts @@ -1,5 +1,6 @@ import { Users } from './fixtures/userStates'; import { AdminUsers } from './page-objects'; +import { setSettingValueById } from './utils/setSettingValueById'; import { test, expect } from './utils/test'; import type { ITestUser } from './utils/user-helpers'; import { createTestUser } from './utils/user-helpers'; @@ -11,11 +12,13 @@ test.use({ storageState: Users.admin.state }); test.describe('Admin > Users', () => { test.beforeAll('Create a new user', async ({ api }) => { + await setSettingValueById(api, 'Accounts_ManuallyApproveNewUsers', true); user = await createTestUser(api); }); - test.afterAll('Delete the new user', async () => { + test.afterAll('Delete the new user', async ({ api }) => { await user.delete(); + await setSettingValueById(api, 'Accounts_ManuallyApproveNewUsers', false); }); test.beforeEach('Go to /admin/users', async ({ page }) => { @@ -44,19 +47,19 @@ test.describe('Admin > Users', () => { await expect(admin.getUserRowByUsername(user.data.username)).not.toBeVisible(); }); - await test.step('should move from Pending to Deactivated tab', async () => { + await test.step('should move from Pending to Active tab', async () => { await admin.getTabByName('Pending').click(); - await admin.dispatchUserAction(user.data.username, 'Deactivate'); + await admin.dispatchUserAction(user.data.username, 'Activate'); await expect(admin.getUserRowByUsername(user.data.username)).not.toBeVisible(); - await admin.getTabByName('Deactivated').click(); + await admin.getTabByName('Active').click(); await expect(admin.getUserRowByUsername(user.data.username)).toBeVisible(); }); - await test.step('should move from Deactivated to Pending tab', async () => { - await admin.getTabByName('Deactivated').click(); - await admin.dispatchUserAction(user.data.username, 'Activate'); + await test.step('should move from Active to Deactivated tab', async () => { + await admin.getTabByName('Active').click(); + await admin.dispatchUserAction(user.data.username, 'Deactivate'); await expect(admin.getUserRowByUsername(user.data.username)).not.toBeVisible(); - await admin.getTabByName('Pending').click(); + await admin.getTabByName('Deactivated').click(); await expect(admin.getUserRowByUsername(user.data.username)).toBeVisible(); }); }); diff --git a/packages/core-typings/src/IUser.ts b/packages/core-typings/src/IUser.ts index f725b2738cf92..6961781a88dbb 100644 --- a/packages/core-typings/src/IUser.ts +++ b/packages/core-typings/src/IUser.ts @@ -252,6 +252,7 @@ export interface IUser extends IRocketChatRecord { roomRolePriorities?: Record; isOAuthUser?: boolean; // client only field __rooms?: string[]; + inactiveReason?: 'deactivated' | 'pending_approval' | 'idle_too_long'; } export interface IRegisterUser extends IUser { diff --git a/packages/model-typings/src/models/IUsersModel.ts b/packages/model-typings/src/models/IUsersModel.ts index 2d50bfd377cb9..66cc22607f929 100644 --- a/packages/model-typings/src/models/IUsersModel.ts +++ b/packages/model-typings/src/models/IUsersModel.ts @@ -387,7 +387,6 @@ export interface IUsersModel extends IBaseModel { setAvatarData(userId: string, origin: string, etag?: Date | null | string, options?: UpdateOptions): Promise; unsetAvatarData(userId: string): Promise; setUserActive(userId: string, active: boolean): Promise; - setAllUsersActive(active: boolean): Promise; setActiveNotLoggedInAfterWithRole(latestLastLoginDate: Date, role?: string, active?: boolean): Promise; unsetRequirePasswordChange(userId: string): Promise; resetPasswordAndSetRequirePasswordChange( diff --git a/packages/models/src/models/Users.ts b/packages/models/src/models/Users.ts index f84411ff50953..49aa37037b4c6 100644 --- a/packages/models/src/models/Users.ts +++ b/packages/models/src/models/Users.ts @@ -2954,22 +2954,14 @@ export class UsersRaw extends BaseRaw> implements IU const update = { $set: { active, + ...(!active && { inactiveReason: 'deactivated' as const }), }, + ...(active && { $unset: { inactiveReason: 1 as const } }), }; return this.updateOne({ _id }, update); } - setAllUsersActive(active: boolean) { - const update = { - $set: { - active, - }, - }; - - return this.updateMany({}, update); - } - /** * @param latestLastLoginDate * @param {IRole['_id']} role the role id @@ -2988,7 +2980,9 @@ export class UsersRaw extends BaseRaw> implements IU const update = { $set: { active, + ...(!active && { inactiveReason: 'idle_too_long' as const }), }, + ...(active && { $unset: { inactiveReason: 1 as const } }), }; return this.updateMany(query, update); diff --git a/packages/rest-typings/src/v1/users/UsersListStatusParamsGET.ts b/packages/rest-typings/src/v1/users/UsersListStatusParamsGET.ts index 25024aee12cb1..1221e00985c06 100644 --- a/packages/rest-typings/src/v1/users/UsersListStatusParamsGET.ts +++ b/packages/rest-typings/src/v1/users/UsersListStatusParamsGET.ts @@ -12,6 +12,7 @@ export type UsersListStatusParamsGET = PaginatedRequest<{ type?: string; roles?: string[]; searchTerm?: string; + inactiveReason?: ('deactivated' | 'pending_approval' | 'idle_too_long')[]; }>; const UsersListStatusParamsGetSchema = { type: 'object', @@ -51,6 +52,13 @@ const UsersListStatusParamsGetSchema = { type: 'number', nullable: true, }, + inactiveReason: { + type: 'array', + items: { + type: 'string', + enum: ['deactivated', 'pending_approval', 'idle_too_long'], + }, + }, }, additionalProperties: false, };