Merge pull request #444 from Hexastack/fix/settings-saved-db-string

Fix : Number settings gets saved as strings in MongoDB

api/src/setting/repositories/setting.repository.ts

@@ -7,12 +7,16 @@
  */
 
 import { Injectable } from '@nestjs/common';
-import {
-  EventEmitter2,
-  IHookSettingsGroupLabelOperationMap,
-} from '@nestjs/event-emitter';
+import { EventEmitter2 } from '@nestjs/event-emitter';
 import { InjectModel } from '@nestjs/mongoose';
-import { Document, Model, Query, Types } from 'mongoose';
+import {
+  Document,
+  FilterQuery,
+  Model,
+  Types,
+  UpdateQuery,
+  UpdateWithAggregationPipeline,
+} from 'mongoose';
 
 import { I18nService } from '@/i18n/services/i18n.service';
 import { BaseRepository } from '@/utils/generics/base-repository';
@@ -30,6 +34,27 @@ export class SettingRepository extends BaseRepository<Setting> {
     super(eventEmitter, model, Setting);
   }
 
+  async preCreateValidate(
+    doc: Document<unknown, unknown, Setting> &
+      Setting & { _id: Types.ObjectId },
+  ) {
+    this.validateSettingValue(doc.type, doc.value);
+  }
+
+  async preUpdateValidate(
+    criteria: FilterQuery<Setting>,
+    updates: UpdateWithAggregationPipeline | UpdateQuery<Setting>,
+  ): Promise<void> {
+    if (!Array.isArray(updates)) {
+      const payload = updates.$set;
+      if (typeof payload.value !== 'undefined') {
+        const { type } =
+          'type' in payload ? payload : await this.findOne(criteria);
+        this.validateSettingValue(type, payload.value);
+      }
+    }
+  }
+
   /**
    * Validates the `Setting` document after it has been retrieved.
    *
@@ -40,64 +65,51 @@ export class SettingRepository extends BaseRepository<Setting> {
    *
    * @param setting The `Setting` document to be validated.
    */
-  async postValidate(
-    setting: Document<unknown, unknown, Setting> &
-      Setting & { _id: Types.ObjectId },
-  ) {
+  private validateSettingValue(type: SettingType, value: any) {
     if (
-      (setting.type === SettingType.text ||
-        setting.type === SettingType.textarea) &&
-      typeof setting.value !== 'string' &&
-      setting.value !== null
+      (type === SettingType.text || type === SettingType.textarea) &&
+      typeof value !== 'string' &&
+      value !== null
     ) {
       throw new Error('Setting Model : Value must be a string!');
-    } else if (setting.type === SettingType.multiple_text) {
-      const isStringArray =
-        Array.isArray(setting.value) &&
-        setting.value.every((v) => {
-          return typeof v === 'string';
-        });
-      if (!isStringArray) {
-        throw new Error('Setting Model : Value must be a string array!');
+    } else if (type === SettingType.multiple_text) {
+      if (!this.isArrayOfString(value)) {
+        throw new Error(
+          'Setting Model (Multiple Text) : Value must be a string array!',
+        );
       }
     } else if (
-      setting.type === SettingType.checkbox &&
-      typeof setting.value !== 'boolean' &&
-      setting.value !== null
+      type === SettingType.checkbox &&
+      typeof value !== 'boolean' &&
+      value !== null
     ) {
       throw new Error('Setting Model : Value must be a boolean!');
     } else if (
-      setting.type === SettingType.number &&
-      typeof setting.value !== 'number' &&
-      setting.value !== null
+      type === SettingType.number &&
+      typeof value !== 'number' &&
+      value !== null
     ) {
       throw new Error('Setting Model : Value must be a number!');
+    } else if (type === SettingType.multiple_attachment) {
+      if (!this.isArrayOfString(value)) {
+        throw new Error(
+          'Setting Model (Multiple Attachement): Value must be a string array!',
+        );
+      }
+    } else if (type === SettingType.attachment) {
+      if (typeof value !== 'string' && typeof value !== null) {
+        throw new Error(
+          'Setting Model (attachement): Value must be a string or null !',
+        );
+      }
+    } else if (type === SettingType.secret && typeof value !== 'string') {
+      throw new Error('Setting Model (secret) : Value must be a string');
+    } else if (type === SettingType.select && typeof value !== 'string') {
+      throw new Error('Setting Model (select): Value must be a string!');
     }
   }
 
-  /**
-   * Emits an event after a `Setting` has been updated.
-   *
-   * This method is used to synchronize global settings by emitting an event
-   * based on the `group` and `label` of the `Setting`.
-   *
-   * @param _query The Mongoose query object used to find and update the document.
-   * @param setting The updated `Setting` object.
-   */
-  async postUpdate(
-    _query: Query<
-      Document<Setting, any, any>,
-      Document<Setting, any, any>,
-      unknown,
-      Setting,
-      'findOneAndUpdate'
-    >,
-    setting: Setting,
-  ) {
-    const group = setting.group as keyof IHookSettingsGroupLabelOperationMap;
-    const label = setting.label as '*';
-
-    // Sync global settings var
-    this.eventEmitter.emit(`hook:${group}:${label}`, setting);
+  private isArrayOfString(value: any): boolean {
+    return Array.isArray(value) && value.every((v) => typeof v === 'string');
   }
 }

api/src/utils/generics/base-repository.spec.ts

@@ -162,6 +162,105 @@ describe('BaseRepository', () => {
         expect.objectContaining({ dummy: 'updated dummy text' }),
       );
     });
+
+    it('should updateOne by id and trigger preUpdateValidate and postUpdateValidate methods', async () => {
+      const created = await dummyRepository.create({ dummy: 'initial text' });
+      const mockGetFilterValue = { _id: created.id };
+      const mockedGetFilter = jest.fn().mockReturnValue(mockGetFilterValue);
+
+      const mockGetUpdateValue = {
+        $set: {
+          value: 'updated dummy text',
+        },
+      };
+
+      const mockedGetUpdate = jest.fn().mockReturnValue(mockGetUpdateValue);
+      const mockQueryValue = {
+        getFilter: mockedGetFilter,
+        getUpdate: mockedGetUpdate,
+        lean: jest.fn(() => {
+          return {
+            exec: jest.fn(),
+          };
+        }),
+      };
+
+      jest
+        .spyOn(dummyModel, 'findOneAndUpdate')
+        .mockReturnValue(mockQueryValue as any);
+
+      const mockUpdate = { dummy: 'updated dummy text' };
+      const spyPreUpdateValidate = jest
+        .spyOn(dummyRepository, 'preUpdateValidate')
+        .mockResolvedValue();
+      const spyPostUpdateValidate = jest
+        .spyOn(dummyRepository, 'postUpdateValidate')
+        .mockResolvedValue();
+
+      const spyExecutoneOne = jest
+        .spyOn(
+          dummyRepository as DummyRepository & {
+            executeOne: () => Promise<{ dummy: string }>;
+          },
+          'executeOne',
+        )
+        .mockResolvedValue({ dummy: 'updated dummy text' });
+
+      await dummyRepository.updateOne(created.id, mockUpdate);
+
+      expect(spyPreUpdateValidate).toHaveBeenCalledWith(
+        mockGetFilterValue,
+        mockGetUpdateValue,
+      );
+      expect(spyPostUpdateValidate).toHaveBeenCalledWith(
+        mockGetFilterValue,
+        mockGetUpdateValue,
+      );
+      expect(spyExecutoneOne).toHaveBeenCalledWith(mockQueryValue, Dummy);
+    });
+
+    it('should throw an error while trying to updateOne when calling preUpdateValidate', async () => {
+      const created = await dummyRepository.create({ dummy: 'initial text' });
+      const mockGetFilterValue = { _id: created.id };
+      const mockedGetFilter = jest.fn().mockReturnValue(mockGetFilterValue);
+
+      const mockGetUpdateValue = {
+        $set: {
+          value: 10,
+        },
+      };
+
+      const mockedGetUpdate = jest.fn().mockReturnValue(mockGetUpdateValue);
+      const mockQueryValue = {
+        getFilter: mockedGetFilter,
+        getUpdate: mockedGetUpdate,
+        lean: jest.fn(() => {
+          return {
+            exec: jest.fn(),
+          };
+        }),
+      };
+
+      jest
+        .spyOn(dummyModel, 'findOneAndUpdate')
+        .mockReturnValue(mockQueryValue as any);
+
+      const mockUpdate = { dummy: 10 };
+      const spyPreUpdateValidate = jest
+        .spyOn(dummyRepository, 'preUpdateValidate')
+        .mockImplementation(() => {
+          throw new Error('Mocked error while validating dummy');
+        });
+
+      await expect(
+        dummyRepository.updateOne(created.id, mockUpdate),
+      ).rejects.toThrow('Mocked error while validating dummy');
+
+      expect(spyPreUpdateValidate).toHaveBeenCalledWith(
+        mockGetFilterValue,
+        mockGetUpdateValue,
+      );
+    });
   });
 
   describe('deleteOne', () => {

api/src/utils/generics/base-repository.ts

@@ -14,6 +14,7 @@ import {
 import { ClassTransformOptions, plainToClass } from 'class-transformer';
 import {
   Document,
+  FilterQuery,
   FlattenMaps,
   HydratedDocument,
   Model,
@@ -38,18 +39,32 @@ export type DeleteResult = {
 };
 
 export enum EHook {
+  preCreateValidate = 'preCreateValidate',
   preCreate = 'preCreate',
+  preUpdateValidate = 'preUpdateValidate',
   preUpdate = 'preUpdate',
   preUpdateMany = 'preUpdateMany',
   preDelete = 'preDelete',
-  preValidate = 'preValidate',
+  postCreateValidate = 'postCreateValidate',
   postCreate = 'postCreate',
+  postUpdateValidate = 'postUpdateValidate',
   postUpdate = 'postUpdate',
   postUpdateMany = 'postUpdateMany',
   postDelete = 'postDelete',
-  postValidate = 'postValidate',
 }
 
+// ! ------------------------------------ Note --------------------------------------------
+// Methods like `update()`, `updateOne()`, `updateMany()`, `findOneAndUpdate()`,
+// `findByIdAndUpdate()`, `findOneAndReplace()`, `findOneAndDelete()`, and `findByIdAndDelete()`
+// do not trigger Mongoose validation hooks by default. This is because these methods do not
+// return Mongoose Documents but plain JavaScript objects (POJOs), which do not have Mongoose
+// instance methods like `validate()` attached.
+//
+// Be cautious when using the `.lean()` function as well. It returns POJOs instead of Mongoose
+// Documents, so methods and hooks like `validate()` will not be available when working with
+// the returned data. If you need validation, ensure that you're working with a Mongoose Document
+// or explicitly use `runValidators: true` in the options for update operations.
+
 export abstract class BaseRepository<
   T extends FlattenMaps<unknown>,
   P extends string = never,
@@ -86,14 +101,17 @@ export abstract class BaseRepository<
 
     hooks?.validate.pre.execute(async function () {
       const doc = this as HydratedDocument<T>;
-      await repository.preValidate(doc);
-      repository.emitter.emit(repository.getEventName(EHook.preValidate), doc);
+      await repository.preCreateValidate(doc);
+      repository.emitter.emit(
+        repository.getEventName(EHook.preCreateValidate),
+        doc,
+      );
     });
 
     hooks?.validate.post.execute(async function (created: HydratedDocument<T>) {
-      await repository.postValidate(created);
+      await repository.postCreateValidate(created);
       repository.emitter.emit(
-        repository.getEventName(EHook.postValidate),
+        repository.getEventName(EHook.postCreateValidate),
         created,
       );
     });
@@ -457,6 +475,23 @@ export abstract class BaseRepository<
         new: true,
       },
     );
+    const filterCriteria = query.getFilter();
+    const queryUpdates = query.getUpdate();
+
+    await this.preUpdateValidate(filterCriteria, queryUpdates);
+    this.emitter.emit(
+      this.getEventName(EHook.preUpdateValidate),
+      filterCriteria,
+      queryUpdates,
+    );
+
+    await this.postUpdateValidate(filterCriteria, queryUpdates);
+    this.emitter.emit(
+      this.getEventName(EHook.postUpdateValidate),
+      filterCriteria,
+      queryUpdates,
+    );
+
     return await this.executeOne(query, this.cls);
   }
 
@@ -479,11 +514,29 @@ export abstract class BaseRepository<
     return await this.model.deleteMany(criteria);
   }
 
-  async preValidate(_doc: HydratedDocument<T>): Promise<void> {
+  async preCreateValidate(
+    _doc: HydratedDocument<T>,
+    _filterCriteria?: FilterQuery<T>,
+    _updates?: UpdateWithAggregationPipeline | UpdateQuery<T>,
+  ): Promise<void> {
     // Nothing ...
   }
 
-  async postValidate(_validated: HydratedDocument<T>): Promise<void> {
+  async postCreateValidate(_validated: HydratedDocument<T>): Promise<void> {
+    // Nothing ...
+  }
+
+  async preUpdateValidate(
+    _filterCriteria: FilterQuery<T>,
+    _updates: UpdateWithAggregationPipeline | UpdateQuery<T>,
+  ): Promise<void> {
+    // Nothing ...
+  }
+
+  async postUpdateValidate(
+    _filterCriteria: FilterQuery<T>,
+    _updates: UpdateWithAggregationPipeline | UpdateQuery<T>,
+  ): Promise<void> {
     // Nothing ...
   }
 

api/types/event-emitter.d.ts

@@ -179,11 +179,13 @@ declare module '@nestjs/event-emitter' {
   type EventNamespaces = keyof IHookEntityOperationMap;
 
   /* pre hooks */
-  type TPreValidate<T> = THydratedDocument<T>;
+  type TPreCreateValidate<T> = THydratedDocument<T>;
 
   type TPreCreate<T> = THydratedDocument<T>;
 
-  type TPreUpdate<T> = TFilterQuery<T> & object;
+  type TPreUpdateValidate<T> = FilterQuery<T>;
+
+  type TPreUpdate<T> = TFilterQuery<T>;
 
   type TPreDelete<T> = Query<
     DeleteResult,
@@ -195,23 +197,27 @@ declare module '@nestjs/event-emitter' {
   >;
 
   type TPreUnion<T> =
-    | TPreValidate<T>
+    | TPreCreateValidate<T>
     | TPreCreate<T>
+    | TPreUpdateValidate<T>
     | TPreUpdate<T>
     | TPreDelete<T>;
 
   /* post hooks */
-  type TPostValidate<T> = THydratedDocument<T>;
+  type TPostCreateValidate<T> = THydratedDocument<T>;
 
   type TPostCreate<T> = THydratedDocument<T>;
 
+  type TPostUpdateValidate<T> = FilterQuery<T>;
+
   type TPostUpdate<T> = THydratedDocument<T>;
 
   type TPostDelete = DeleteResult;
 
   type TPostUnion<T> =
-    | TPostValidate<T>
+    | TPostCreateValidate<T>
     | TPostCreate<T>
+    | TPostUpdateValidate<T>
     | TPostUpdate<T>
     | TPostDelete;
 
@@ -247,11 +253,14 @@ declare module '@nestjs/event-emitter' {
     T = IHookEntityOperationMap[E]['schema'],
   > =
     | {
-        [EHook.preValidate]: TPreValidate<T>;
+        [EHook.preCreateValidate]: TPreCreateValidate<T>;
       }
     | {
         [EHook.preCreate]: TPreCreate<T>;
       }
+    | {
+        [EHook.preUpdateValidate]: TPreUpdateValidate<T>;
+      }
     | {
         [EHook.preUpdate]: TPreUpdate<T>;
       }
@@ -259,11 +268,14 @@ declare module '@nestjs/event-emitter' {
         [EHook.preDelete]: TPreDelete<T>;
       }
     | {
-        [EHook.postValidate]: TPostValidate<T>;
+        [EHook.postCreateValidate]: TPostCreateValidate<T>;
       }
     | {
         [EHook.postCreate]: TPostCreate<T>;
       }
+    | {
+        [EHook.postUpdateValidate]: TPostUpdateValidate<T>;
+      }
     | {
         [EHook.postUpdate]: TPostUpdate<T>;
       }

frontend/src/components/settings/SettingInput.tsx

@@ -95,6 +95,7 @@ const SettingInput: React.FC<RenderSettingInputProps> = ({
           label={label}
           helperText={helperText}
           {...field}
+          onChange={(e) => field.onChange(Number(e.target.value))}
           disabled={isDisabled(setting)}
         />
       );