Skip to content
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.

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

[Fix] Activity Log Event use generic method #8449

Merged
merged 9 commits into from
Oct 17, 2024
13 changes: 10 additions & 3 deletions packages/core/src/activity-log/activity-log.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,25 @@ export function generateActivityLogDescription(
});
}

export function activityLogUpdatedFieldsAndValues<T>(original: T, updated: Partial<T>) {
/**
* @description Log updated field names, old and new values for Activity Log Updated Actions
* @template T
* @param {T} originalValues - Old values before update
* @param {Partial<T>} updated - Updated values
* @returns An object with updated fields, their old and new values
*/
export function activityLogUpdatedFieldsAndValues<T>(originalValues: T, updated: Partial<T>) {
const updatedFields: string[] = [];
const previousValues: IActivityLogUpdatedValues[] = [];
const updatedValues: IActivityLogUpdatedValues[] = [];

for (const key of Object.keys(updated)) {
if (original[key] !== updated[key]) {
if (originalValues[key] !== updated[key]) {
// Add updated field
updatedFields.push(key);

// Add old and new values
previousValues.push({ [key]: original[key] });
previousValues.push({ [key]: originalValues[key] });
Comment on lines +46 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Deep Comparison for Accurate Change Detection

In the activityLogUpdatedFieldsAndValues function, the equality check originalValues[key] !== updated[key] uses shallow comparison. This may not accurately detect changes in nested objects or arrays because it compares references rather than their contents. Consider using a deep equality check to ensure all changes are captured.

Apply this diff to use deep equality comparison:

+import { isEqual } from 'lodash';

 // ...

 for (const key of Object.keys(updated)) {
-	if (originalValues[key] !== updated[key]) {
+	if (!isEqual(originalValues[key], updated[key])) {
 		// Add updated field
 		updatedFields.push(key);

 		// Add old and new values
 		previousValues.push({ [key]: originalValues[key] });
 		updatedValues.push({ [key]: updated[key] });
 	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function activityLogUpdatedFieldsAndValues<T>(originalValues: T, updated: Partial<T>) {
const updatedFields: string[] = [];
const previousValues: IActivityLogUpdatedValues[] = [];
const updatedValues: IActivityLogUpdatedValues[] = [];
for (const key of Object.keys(updated)) {
if (original[key] !== updated[key]) {
if (originalValues[key] !== updated[key]) {
// Add updated field
updatedFields.push(key);
// Add old and new values
previousValues.push({ [key]: original[key] });
previousValues.push({ [key]: originalValues[key] });
import { isEqual } from 'lodash';
export function activityLogUpdatedFieldsAndValues<T>(originalValues: T, updated: Partial<T>) {
const updatedFields: string[] = [];
const previousValues: IActivityLogUpdatedValues[] = [];
const updatedValues: IActivityLogUpdatedValues[] = [];
for (const key of Object.keys(updated)) {
if (!isEqual(originalValues[key], updated[key])) {
// Add updated field
updatedFields.push(key);
// Add old and new values
previousValues.push({ [key]: originalValues[key] });
updatedValues.push({ [key]: updated[key] });
}
}

updatedValues.push({ [key]: updated[key] });
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/activity-log/activity-log.module.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CqrsModule } from '@nestjs/cqrs';
import { Module } from '@nestjs/common';
import { Global, Module } from '@nestjs/common';
import { MikroOrmModule } from '@mikro-orm/nestjs';
import { TypeOrmModule } from '@nestjs/typeorm';
import { RolePermissionModule } from '../role-permission/role-permission.module';
Expand All @@ -9,6 +9,7 @@ import { ActivityLogService } from './activity-log.service';
import { EventHandlers } from './events/handlers';
import { TypeOrmActivityLogRepository } from './repository/type-orm-activity-log.repository';

@Global()
@Module({
imports: [
TypeOrmModule.forFeature([ActivityLog]),
Expand Down
103 changes: 87 additions & 16 deletions packages/core/src/activity-log/activity-log.service.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
import { BadRequestException, Injectable } from '@nestjs/common';
import { EventBus } from '@nestjs/cqrs';
import { FindManyOptions, FindOptionsOrder, FindOptionsWhere } from 'typeorm';
import { IActivityLog, IActivityLogInput, IPagination } from '@gauzy/contracts';
import {
ActionTypeEnum,
ActorTypeEnum,
BaseEntityEnum,
IActivityLog,
IActivityLogInput,
ID,
IPagination
} from '@gauzy/contracts';
import { isNotNullOrUndefined } from '@gauzy/common';
import { TenantAwareCrudService } from './../core/crud';
import { RequestContext } from '../core/context';
import { activityLogUpdatedFieldsAndValues, generateActivityLogDescription } from './activity-log.helper';
import { ActivityLogEvent } from './events/activity-log.event';
import { GetActivityLogsDTO, allowedOrderDirections, allowedOrderFields } from './dto/get-activity-logs.dto';
import { ActivityLog } from './activity-log.entity';
import { MikroOrmActivityLogRepository, TypeOrmActivityLogRepository } from './repository';
Expand All @@ -12,11 +23,35 @@ import { MikroOrmActivityLogRepository, TypeOrmActivityLogRepository } from './r
export class ActivityLogService extends TenantAwareCrudService<ActivityLog> {
constructor(
readonly typeOrmActivityLogRepository: TypeOrmActivityLogRepository,
readonly mikroOrmActivityLogRepository: MikroOrmActivityLogRepository
readonly mikroOrmActivityLogRepository: MikroOrmActivityLogRepository,
private readonly _eventBus: EventBus
) {
super(typeOrmActivityLogRepository, mikroOrmActivityLogRepository);
}

/**
* Creates a new activity log entry with the provided input, while associating it with the current user and tenant.
*
* @param input - The data required to create an activity log entry.
* @returns The created activity log entry.
* @throws BadRequestException when the log creation fails.
*/
async create(input: IActivityLogInput): Promise<IActivityLog> {
try {
// Retrieve the current user's ID from the request context
const creatorId = RequestContext.currentUserId();

// Retrieve the current tenant ID from the request context or use the provided tenantId
const tenantId = RequestContext.currentTenantId() || input.tenantId;

// Create the activity log entry using the provided input along with the tenantId and creatorId
return await super.create({ ...input, tenantId, creatorId });
} catch (error) {
console.log('Error while creating activity log:', error);
throw new BadRequestException('Error while creating activity log', error);
}
}

/**
* Finds and retrieves activity logs based on the given filters criteria.
*
Expand Down Expand Up @@ -67,7 +102,7 @@ export class ActivityLogService extends TenantAwareCrudService<ActivityLog> {

const take = filters.take ? filters.take : 100; // Default take value if not provided
// Pagination: ensure `filters.skip` is a positive integer starting from 1
const skip = (filters.skip && Number.isInteger(filters.skip) && filters.skip > 0) ? filters.skip : 1;
const skip = filters.skip && Number.isInteger(filters.skip) && filters.skip > 0 ? filters.skip : 1;

// Ensure that filters are properly defined
const queryOptions: FindManyOptions<ActivityLog> = {
Expand All @@ -85,20 +120,56 @@ export class ActivityLogService extends TenantAwareCrudService<ActivityLog> {
}

/**
* Creates a new activity log entry with the provided input, while associating it with the current user and tenant.
*
* @param input - The data required to create an activity log entry.
* @returns The created activity log entry.
* @throws BadRequestException when the log creation fails.
* @description Create or Update Activity Log
* @template T
* @param {BaseEntityEnum} entityType - Entity type for whom creating activity log (E.g : Task, OrganizationProject, etc.)
* @param {string} entityName - Name or Title of the entity
* @param {ActorTypeEnum} actor - The actor type performing the action (User or System)
* @param {ID} organizationId
* @param {ID} tenantId
* @param {ActionTypeEnum} actionType - Action performed (Created or Updated)
* @param {T} data - Entity data (for Created action) or Updated entity data (for Updated action)
* @param {Partial<T>} [originalValues] - Entity data before update (optional for Update action)
* @param {Partial<T>} [newValues] - Entity updated data per field (optional for Update action)
*/
async logActivity(input: IActivityLogInput): Promise<IActivityLog> {
try {
const creatorId = RequestContext.currentUserId(); // Retrieve the current user's ID from the request context
// Create the activity log entry using the provided input along with the tenantId and creatorId
return await super.create({ ...input, creatorId });
} catch (error) {
console.log('Error while creating activity log:', error);
throw new BadRequestException('Error while creating activity log', error);
logActivity<T>(
entity: BaseEntityEnum,
actionType: ActionTypeEnum,
actor: ActorTypeEnum,
entityId: ID,
entityName: string,
data: T,
organizationId: ID,
tenantId: ID,
originalValues?: Partial<T>,
newValues?: Partial<T>
) {
let jsonFields: Record<string, any> = new Object();

// If it's an update action, add updated fields and values
if (actionType === ActionTypeEnum.Updated && originalValues && newValues) {
const { updatedFields, previousValues, updatedValues } = activityLogUpdatedFieldsAndValues(
originalValues,
newValues
);

// Add updated fields and values to the log
jsonFields = Object.assign({}, { updatedFields, previousValues, updatedValues });
}

// Emit the event to log the activity
this._eventBus.publish(
new ActivityLogEvent({
entity,
entityId,
action: actionType,
actorType: actor,
description: generateActivityLogDescription(actionType, entity, entityName),
data,
organizationId,
tenantId,
...jsonFields
})
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ export class ActivityLogEventHandler implements IEventHandler<ActivityLogEvent>
*/
async handle(event: ActivityLogEvent) {
// Extract the input from the event and create a new activity log entry
return await this.activityLogService.logActivity(event.input);
return await this.activityLogService.create(event.input);
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { BadRequestException, HttpException, HttpStatus, Injectable } from '@nestjs/common';
import { EventBus } from '@nestjs/cqrs';
import { Brackets, FindManyOptions, SelectQueryBuilder, UpdateResult, WhereExpressionBuilder } from 'typeorm';
import {
ActionTypeEnum,
BaseEntityEnum,
ActorTypeEnum,
ID,
Expand All @@ -12,16 +10,16 @@ import {
IOrganizationProjectModuleUpdateInput,
IPagination,
PermissionsEnum,
ProjectModuleStatusEnum
ProjectModuleStatusEnum,
ActionTypeEnum
} from '@gauzy/contracts';
import { isEmpty, isNotEmpty } from '@gauzy/common';
import { isPostgres } from '@gauzy/config';
import { PaginationParams, TenantAwareCrudService } from './../core/crud';
import { RequestContext } from '../core/context';
import { ActivityLogEvent } from '../activity-log/events';
import { activityLogUpdatedFieldsAndValues, generateActivityLogDescription } from '../activity-log/activity-log.helper';
import { OrganizationProjectModule } from './organization-project-module.entity';
import { prepareSQLQuery as p } from './../database/database.helper';
import { ActivityLogService } from '../activity-log/activity-log.service';
import { TypeOrmOrganizationProjectModuleRepository } from './repository/type-orm-organization-project-module.repository';
import { MikroOrmOrganizationProjectModuleRepository } from './repository/mikro-orm-organization-project-module.repository';

Expand All @@ -30,7 +28,7 @@ export class OrganizationProjectModuleService extends TenantAwareCrudService<Org
constructor(
readonly typeOrmProjectModuleRepository: TypeOrmOrganizationProjectModuleRepository,
readonly mikroOrmProjectModuleRepository: MikroOrmOrganizationProjectModuleRepository,
private readonly _eventBus: EventBus
private readonly activityLogService: ActivityLogService
) {
super(typeOrmProjectModuleRepository, mikroOrmProjectModuleRepository);
}
Expand All @@ -46,33 +44,24 @@ export class OrganizationProjectModuleService extends TenantAwareCrudService<Org
const { organizationId } = entity;

try {
const module = await super.create({
const projectModule = await super.create({
...entity,
creatorId
});

// Generate the activity log description
const description = generateActivityLogDescription(
ActionTypeEnum.Created,
// Generate the activity log
this.activityLogService.logActivity<OrganizationProjectModule>(
BaseEntityEnum.OrganizationProjectModule,
module.name
);

// Emit an event to log the activity
this._eventBus.publish(
new ActivityLogEvent({
entity: BaseEntityEnum.OrganizationProjectModule,
entityId: module.id,
action: ActionTypeEnum.Created,
actorType: ActorTypeEnum.User,
description,
data: module,
organizationId,
tenantId
})
ActionTypeEnum.Created,
ActorTypeEnum.User,
projectModule.id,
projectModule.name,
projectModule,
organizationId,
tenantId
);

return module;
return projectModule;
} catch (error) {
throw new BadRequestException(error);
}
Expand All @@ -93,54 +82,41 @@ export class OrganizationProjectModuleService extends TenantAwareCrudService<Org

try {
// Retrieve existing module.
const existingModule = await this.findOneByIdString(id, {
const existingProjectModule = await this.findOneByIdString(id, {
relations: {
members: true,
manager: true
}
});

if (!existingModule) {
if (!existingProjectModule) {
throw new BadRequestException('Module not found');
}

// Update module with new values
const updatedModule = await super.create({
const updatedProjectModule = await super.create({
...entity,
id
});

// Generate the activity log description
const description = generateActivityLogDescription(
ActionTypeEnum.Updated,
BaseEntityEnum.OrganizationProjectModule,
updatedModule.name
);
// Generate the activity log
const { organizationId } = updatedProjectModule;

const { updatedFields, previousValues, updatedValues } = activityLogUpdatedFieldsAndValues(
updatedModule,
this.activityLogService.logActivity<OrganizationProjectModule>(
BaseEntityEnum.OrganizationProjectModule,
ActionTypeEnum.Updated,
ActorTypeEnum.User,
updatedProjectModule.id,
updatedProjectModule.name,
updatedProjectModule,
organizationId,
tenantId,
existingProjectModule,
entity
);

// Emit an event to log the activity
this._eventBus.publish(
new ActivityLogEvent({
entity: BaseEntityEnum.OrganizationProjectModule,
entityId: updatedModule.id,
action: ActionTypeEnum.Updated,
actorType: ActorTypeEnum.User,
description,
updatedFields,
updatedValues,
previousValues,
data: updatedModule,
organizationId: updatedModule.organizationId,
tenantId
})
);

// return updated Module
return updatedModule;
return updatedProjectModule;
} catch (error) {
throw new BadRequestException(error);
}
Expand Down
Loading
Loading