Просмотр исходного кода

Merge pull request #10835 from growilabs/feat/94790-179320-changed-to-filter-by-username-instead-of-userid

feat: implement username-based user filter
Yuki Takei 3 недель назад
Родитель
Сommit
0861c34a92

+ 7 - 5
apps/app/src/client/components/Admin/AuditLog/AuditLogExportModal.tsx

@@ -4,7 +4,7 @@ import { useAtomValue } from 'jotai';
 import { useTranslation } from 'react-i18next';
 import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap';
 
-import type { IAuditLogBulkExportFilters } from '~/features/audit-log-bulk-export/interfaces/audit-log-bulk-export';
+import type { IAuditLogBulkExportRequestFilters } from '~/features/audit-log-bulk-export/interfaces/audit-log-bulk-export';
 import type { SupportedActionType } from '~/interfaces/activity';
 import { auditLogAvailableActionsAtom } from '~/states/server-configurations';
 
@@ -32,7 +32,7 @@ const AuditLogExportModalSubstance = ({
 
   const [startDate, setStartDate] = useState<Date | null>(null);
   const [endDate, setEndDate] = useState<Date | null>(null);
-  const [_selectedUsernames, setSelectedUsernames] = useState<string[]>([]);
+  const [selectedUsernames, setSelectedUsernames] = useState<string[]>([]);
   const [actionMap, setActionMap] = useState(
     () =>
       new Map<SupportedActionType, boolean>(
@@ -78,9 +78,11 @@ const AuditLogExportModalSubstance = ({
       .filter((v) => v[1])
       .map((v) => v[0]);
 
-    const filters: IAuditLogBulkExportFilters = {};
-    // TODO: Add users filter after implementing username-to-userId conversion
+    const filters: IAuditLogBulkExportRequestFilters = {};
 
+    if (selectedUsernames.length > 0) {
+      filters.usernames = selectedUsernames;
+    }
     if (selectedActionList.length > 0) {
       filters.actions = selectedActionList;
     }
@@ -94,7 +96,7 @@ const AuditLogExportModalSubstance = ({
     }
 
     return filters;
-  }, [actionMap, startDate, endDate]);
+  }, [actionMap, selectedUsernames, startDate, endDate]);
 
   const {
     isExporting,

+ 6 - 0
apps/app/src/features/audit-log-bulk-export/interfaces/audit-log-bulk-export.ts

@@ -23,6 +23,12 @@ export const AuditLogBulkExportJobStatus = {
 export type AuditLogBulkExportJobStatus =
   (typeof AuditLogBulkExportJobStatus)[keyof typeof AuditLogBulkExportJobStatus];
 
+export interface IAuditLogBulkExportRequestFilters {
+  usernames?: string[];
+  actions?: SupportedActionType[];
+  dateFrom?: Date;
+  dateTo?: Date;
+}
 export interface IAuditLogBulkExportFilters {
   users?: Array<Ref<IUser>>;
   actions?: SupportedActionType[];

+ 2 - 2
apps/app/src/features/audit-log-bulk-export/server/routes/apiv3/audit-log-bulk-export.integ.ts

@@ -208,13 +208,13 @@ describe('POST /_api/v3/audit-log-bulk-export', () => {
       expect(res.body?.errors).toBeDefined();
     });
 
-    it('returns 400 when users contains invalid ObjectId', async () => {
+    it('returns 400 when usernames contains non-string values', async () => {
       const app = buildApp();
       const res = await request(app)
         .post('/_api/v3/audit-log-bulk-export')
         .send({
           filters: {
-            users: ['invalid-objectid'],
+            usernames: [123, 456],
           },
         });
 

+ 3 - 3
apps/app/src/features/audit-log-bulk-export/server/routes/apiv3/audit-log-bulk-export.ts

@@ -24,7 +24,7 @@ const router = Router();
 
 interface AuditLogExportReqBody {
   filters: {
-    users?: string[];
+    usernames?: string[];
     actions?: SupportedActionType[];
     dateFrom?: Date;
     dateTo?: Date;
@@ -46,8 +46,8 @@ export const factory = (crowi: Crowi): Router => {
   const validators = {
     auditLogBulkExport: [
       body('filters').exists({ checkFalsy: true }).isObject(),
-      body('filters.users').optional({ nullable: true }).isArray(),
-      body('filters.users.*').optional({ nullable: true }).isMongoId(),
+      body('filters.usernames').optional({ nullable: true }).isArray(),
+      body('filters.usernames.*').optional({ nullable: true }).isString(),
       body('filters.actions').optional({ nullable: true }).isArray(),
       body('filters.actions.*')
         .optional({ nullable: true })

+ 8 - 13
apps/app/src/features/audit-log-bulk-export/server/service/audit-log-bulk-export-job-cron/audit-log-bulk-export-job-cron-service.integ.ts

@@ -578,11 +578,11 @@ describe('AuditLogBulkExportJobCronService Integration Test', () => {
 
   describe('4. Error Handling', () => {
     describe('4-1. Nonexistent Users Filter', () => {
-      it('should throw error for nonexistent users', async () => {
+      it('should fail with no results for nonexistent usernames', async () => {
         const job = await AuditLogBulkExportJob.create({
           user: testUser._id,
           filters: {
-            users: [new mongoose.Types.ObjectId().toString()],
+            users: [new mongoose.Types.ObjectId()],
           },
           format: AuditLogBulkExportFormat.json,
           status: AuditLogBulkExportJobStatus.exporting,
@@ -591,19 +591,14 @@ describe('AuditLogBulkExportJobCronService Integration Test', () => {
           totalExportedCount: 0,
         });
 
-        try {
-          await cronService.proceedBulkExportJob(job);
-          await waitForCondition(async () => {
-            const updatedJob = await AuditLogBulkExportJob.findById(job._id);
-            return updatedJob?.status === AuditLogBulkExportJobStatus.failed;
-          });
-        } catch (_error) {}
+        await cronService.proceedBulkExportJob(job);
+        await waitForCondition(async () => {
+          const updatedJob = await AuditLogBulkExportJob.findById(job._id);
+          return updatedJob?.status === AuditLogBulkExportJobStatus.failed;
+        });
 
         const updatedJob = await AuditLogBulkExportJob.findById(job._id);
-        expect([
-          AuditLogBulkExportJobStatus.exporting,
-          AuditLogBulkExportJobStatus.failed,
-        ]).toContain(updatedJob?.status);
+        expect(updatedJob?.status).toBe(AuditLogBulkExportJobStatus.failed);
       });
     });
 

+ 2 - 12
apps/app/src/features/audit-log-bulk-export/server/service/audit-log-bulk-export-job-cron/steps/exportAuditLogsToFsAsync.ts

@@ -1,8 +1,7 @@
 import fs from 'node:fs';
 import path from 'node:path';
 import { pipeline, Writable } from 'node:stream';
-import type { IUser } from '@growi/core';
-import mongoose, { type FilterQuery } from 'mongoose';
+import type { FilterQuery } from 'mongoose';
 
 import { AuditLogBulkExportJobStatus } from '~/features/audit-log-bulk-export/interfaces/audit-log-bulk-export';
 import { SupportedAction } from '~/interfaces/activity';
@@ -101,16 +100,7 @@ export async function exportAuditLogsToFsAsync(
     }
   }
   if (filters.users && filters.users.length > 0) {
-    const User = mongoose.model<IUser>('User');
-    const userIds = await User.find({
-      _id: { $in: filters.users },
-    }).distinct('_id');
-    if (userIds.length === 0) {
-      throw new Error(
-        `No users found with userIDs: ${filters.users.join(', ')}`,
-      );
-    }
-    query.user = { $in: userIds };
+    query.user = { $in: filters.users };
   }
 
   // If the previous export was incomplete, resume from the last exported ID by adding it to the query filter

+ 18 - 18
apps/app/src/features/audit-log-bulk-export/server/service/audit-log-bulk-export.integ.ts

@@ -74,7 +74,7 @@ describe('AuditLogBulkExportService', () => {
         expect(createdJob?.status).toBe(AuditLogBulkExportJobStatus.exporting);
         expect(createdJob?.totalExportedCount).toBe(0);
         expect(createdJob?.filters).toMatchObject({
-          actions: ['PAGE_CREATE', 'PAGE_VIEW'],
+          actions: ['PAGE_VIEW', 'PAGE_CREATE'],
           dateFrom: new Date('2023-01-01T00:00:00.000Z'),
           dateTo: new Date('2023-12-31T00:00:00.000Z'),
         });
@@ -100,10 +100,11 @@ describe('AuditLogBulkExportService', () => {
       });
 
       it('should create a job with user filters', async () => {
-        const filters: { users: string[]; actions: SupportedActionType[] } = {
-          users: [user._id.toString()],
-          actions: ['PAGE_CREATE'],
-        };
+        const filters: { usernames: string[]; actions: SupportedActionType[] } =
+          {
+            usernames: [user.username],
+            actions: ['PAGE_CREATE'],
+          };
 
         const jobId = await auditLogBulkExportService.createOrResetExportJob(
           filters,
@@ -113,9 +114,9 @@ describe('AuditLogBulkExportService', () => {
 
         const createdJob = await AuditLogBulkExportJob.findById(jobId);
         expect(createdJob?.filters.actions).toEqual(['PAGE_CREATE']);
-        expect(createdJob?.filters.users?.map(String)).toEqual([
+        expect(createdJob?.filters.users?.map(String)).toContain(
           user._id.toString(),
-        ]);
+        );
       });
 
       it('should reset existing job when restartJob is true', async () => {
@@ -275,18 +276,17 @@ describe('AuditLogBulkExportService', () => {
 
   describe('filter canonicalization', () => {
     it('should generate same job for logically equivalent filters', async () => {
-      const validUserId1 = new mongoose.Types.ObjectId().toString();
-      const validUserId2 = new mongoose.Types.ObjectId().toString();
-
-      const filters1: { actions: SupportedActionType[]; users: string[] } = {
-        actions: ['PAGE_VIEW', 'PAGE_CREATE'],
-        users: [validUserId1, validUserId2],
-      };
+      const filters1: { actions: SupportedActionType[]; usernames: string[] } =
+        {
+          actions: ['PAGE_VIEW', 'PAGE_CREATE'],
+          usernames: ['alice', 'bob'],
+        };
 
-      const filters2: { actions: SupportedActionType[]; users: string[] } = {
-        actions: ['PAGE_CREATE', 'PAGE_VIEW'],
-        users: [validUserId2, validUserId1],
-      };
+      const filters2: { actions: SupportedActionType[]; usernames: string[] } =
+        {
+          actions: ['PAGE_CREATE', 'PAGE_VIEW'],
+          usernames: ['bob', 'alice'],
+        };
 
       await auditLogBulkExportService.createOrResetExportJob(
         filters1,

+ 18 - 3
apps/app/src/features/audit-log-bulk-export/server/service/audit-log-bulk-export.ts

@@ -1,8 +1,10 @@
 import { createHash } from 'node:crypto';
+import mongoose from 'mongoose';
 
 import type {
   AuditLogBulkExportFormat,
   IAuditLogBulkExportFilters,
+  IAuditLogBulkExportRequestFilters,
 } from '../../interfaces/audit-log-bulk-export';
 import {
   AuditLogBulkExportJobInProgressJobStatus,
@@ -13,7 +15,7 @@ import AuditLogBulkExportJob from '../models/audit-log-bulk-export-job';
 
 export interface IAuditLogBulkExportService {
   createOrResetExportJob: (
-    filters: IAuditLogBulkExportFilters,
+    requestFilters: IAuditLogBulkExportRequestFilters,
     format: AuditLogBulkExportFormat,
     currentUser,
     restartJob?: boolean,
@@ -72,11 +74,24 @@ class AuditLogBulkExportService implements IAuditLogBulkExportService {
    * Create a new audit-log bulk export job or reset the existing one
    */
   async createOrResetExportJob(
-    filters: IAuditLogBulkExportFilters,
+    requestFilters: IAuditLogBulkExportRequestFilters,
     format: AuditLogBulkExportFormat,
     currentUser,
     restartJob?: boolean,
   ): Promise<string> {
+    const filters: IAuditLogBulkExportFilters = {
+      actions: requestFilters.actions,
+      dateFrom: requestFilters.dateFrom,
+      dateTo: requestFilters.dateTo,
+    };
+    if (requestFilters.usernames?.length) {
+      const User = mongoose.model('User');
+      const userIds = await User.find({
+        username: { $in: requestFilters.usernames },
+      }).distinct('_id');
+      filters.users = userIds;
+    }
+
     const normalizedFilters = canonicalizeFilters(filters);
     const filterHash = sha256(JSON.stringify(normalizedFilters));
 
@@ -99,7 +114,7 @@ class AuditLogBulkExportService implements IAuditLogBulkExportService {
 
     const createdJob = await AuditLogBulkExportJob.create({
       user: currentUser,
-      filters: normalizedFilters,
+      filters,
       filterHash,
       format,
       status: AuditLogBulkExportJobStatus.exporting,