From df92c608c75e85aa3bdc7ae15cce13b02bd87911 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Mon, 2 Sep 2024 13:37:50 +0200 Subject: [PATCH 01/14] refacor: separate search ui logic from frontend --- src/config/default-filters.config.json | 11 ---- src/config/frontend.config.json | 17 ++++++- .../create-user-settings.interceptor.ts | 50 ------------------- .../default-user-settings.interceptor.ts | 33 ------------ src/users/schemas/user-settings.schema.ts | 7 +-- src/users/users.controller.ts | 27 +--------- 6 files changed, 19 insertions(+), 126 deletions(-) delete mode 100644 src/config/default-filters.config.json delete mode 100644 src/users/interceptors/create-user-settings.interceptor.ts delete mode 100644 src/users/interceptors/default-user-settings.interceptor.ts diff --git a/src/config/default-filters.config.json b/src/config/default-filters.config.json deleted file mode 100644 index 848bb1970..000000000 --- a/src/config/default-filters.config.json +++ /dev/null @@ -1,11 +0,0 @@ -[ - { "type": "LocationFilterComponent", "visible": true }, - { "type": "PidFilterComponent", "visible": true }, - { "type": "PidFilterContainsComponent", "visible": false }, - { "type": "PidFilterStartsWithComponent", "visible": false }, - { "type": "GroupFilterComponent", "visible": true }, - { "type": "TypeFilterComponent", "visible": true }, - { "type": "KeywordFilterComponent", "visible": true }, - { "type": "DateRangeFilterComponent", "visible": true }, - { "type": "TextFilterComponent", "visible": true } -] \ No newline at end of file diff --git a/src/config/frontend.config.json b/src/config/frontend.config.json index 39ec77d8b..5dc093125 100644 --- a/src/config/frontend.config.json +++ b/src/config/frontend.config.json @@ -180,5 +180,20 @@ "enabled": "#Selected", "authorization": ["#datasetAccess", "#datasetPublic"] } - ] + ], + "defaultSearchInterfaceSettings": { + "columns": [], + "filters": [ + { "type": "LocationFilterComponent", "visible": true }, + { "type": "PidFilterComponent", "visible": true }, + { "type": "PidFilterContainsComponent", "visible": false }, + { "type": "PidFilterStartsWithComponent", "visible": false }, + { "type": "GroupFilterComponent", "visible": true }, + { "type": "TypeFilterComponent", "visible": true }, + { "type": "KeywordFilterComponent", "visible": true }, + { "type": "DateRangeFilterComponent", "visible": true }, + { "type": "TextFilterComponent", "visible": true } + ], + "conditions": [] + } } diff --git a/src/users/interceptors/create-user-settings.interceptor.ts b/src/users/interceptors/create-user-settings.interceptor.ts deleted file mode 100644 index e22d4f881..000000000 --- a/src/users/interceptors/create-user-settings.interceptor.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { - CallHandler, - ExecutionContext, - Injectable, - Logger, - NestInterceptor, -} from "@nestjs/common"; -import { Observable, tap } from "rxjs"; -import { CreateUserSettingsDto } from "../dto/create-user-settings.dto"; -import { UsersService } from "../users.service"; -import { FILTER_CONFIGS } from "../schemas/user-settings.schema"; - -@Injectable() -export class CreateUserSettingsInterceptor implements NestInterceptor { - constructor(private usersService: UsersService) {} - async intercept( - context: ExecutionContext, - next: CallHandler, - ): Promise> { - return next.handle().pipe( - tap(async () => { - const res = context.switchToHttp().getResponse(); - const user = res.req.user; - if (!user) { - return; - } - const userId = user._id; - const userSettings = - await this.usersService.findByIdUserSettings(userId); - if (!userSettings) { - Logger.log( - `Adding default settings to user ${user.username}`, - "CreateUserSettingsInterceptor", - ); - const createUserSettingsDto: CreateUserSettingsDto = { - userId, - columns: [], - filters: FILTER_CONFIGS, - conditions: [], - }; - return this.usersService.createUserSettings( - userId, - createUserSettingsDto, - ); - } - return; - }), - ); - } -} diff --git a/src/users/interceptors/default-user-settings.interceptor.ts b/src/users/interceptors/default-user-settings.interceptor.ts deleted file mode 100644 index 61b8049e9..000000000 --- a/src/users/interceptors/default-user-settings.interceptor.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { - CallHandler, - ExecutionContext, - Injectable, - Logger, - NestInterceptor, -} from "@nestjs/common"; -import { map, Observable } from "rxjs"; -import { UsersService } from "../users.service"; -import { FILTER_CONFIGS } from "../schemas/user-settings.schema"; -import { UpdateUserSettingsDto } from "../dto/update-user-settings.dto"; - -@Injectable() -export class DefaultUserSettingsInterceptor implements NestInterceptor { - constructor(private usersService: UsersService) {} - async intercept( - context: ExecutionContext, - next: CallHandler, - ): Promise> { - return next.handle().pipe( - map(async () => { - Logger.log("DefaultUserSettingsInterceptor"); - const defaultUserSettings: UpdateUserSettingsDto = { - columns: [], - filters: FILTER_CONFIGS, - conditions: [], - }; - console.log(defaultUserSettings); - return defaultUserSettings; - }), - ); - } -} diff --git a/src/users/schemas/user-settings.schema.ts b/src/users/schemas/user-settings.schema.ts index 9cfe2cae2..59f4aea5a 100644 --- a/src/users/schemas/user-settings.schema.ts +++ b/src/users/schemas/user-settings.schema.ts @@ -2,7 +2,6 @@ import * as mongoose from "mongoose"; import { Prop, Schema, SchemaFactory } from "@nestjs/mongoose"; import { ApiProperty } from "@nestjs/swagger"; import { Document } from "mongoose"; -import filterConfigs from "../../config/default-filters.config.json"; export type UserSettingsDocument = UserSettings & Document; @@ -31,8 +30,6 @@ export interface ScientificCondition { operator: string; } -export const FILTER_CONFIGS: FilterConfig[] = filterConfigs as FilterConfig[]; - @Schema({ collection: "UserSetting", toJSON: { @@ -74,12 +71,12 @@ export class UserSettings { @ApiProperty({ type: [Object], - default: FILTER_CONFIGS, + default: [], description: "Array of filters the user has set", }) @Prop({ type: [{ type: Object }], - default: FILTER_CONFIGS, + default: [], }) filters: FilterConfig[]; diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 1e1dcceed..d894ef955 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -7,12 +7,10 @@ import { Req, Patch, Delete, - UseInterceptors, Put, Body, ForbiddenException, HttpCode, - CanActivate, } from "@nestjs/common"; import { ApiBearerAuth, @@ -32,12 +30,8 @@ import { Request } from "express"; import { JWTUser } from "../auth/interfaces/jwt-user.interface"; import { UserSettings } from "./schemas/user-settings.schema"; import { CreateUserSettingsDto } from "./dto/create-user-settings.dto"; -import { - PartialUpdateUserSettingsDto, - UpdateUserSettingsDto, -} from "./dto/update-user-settings.dto"; +import { PartialUpdateUserSettingsDto } from "./dto/update-user-settings.dto"; import { User } from "./schemas/user.schema"; -import { CreateUserSettingsInterceptor } from "./interceptors/create-user-settings.interceptor"; import { AuthService } from "src/auth/auth.service"; import { CredentialsDto } from "src/auth/dto/credentials.dto"; import { LocalAuthGuard } from "src/auth/guards/local-auth.guard"; @@ -49,7 +43,6 @@ import { AuthenticatedPoliciesGuard } from "../casl/guards/auth-check.guard"; import { ReturnedUserDto } from "./dto/returned-user.dto"; import { ReturnedAuthLoginDto } from "src/auth/dto/returnedLogin.dto"; import { PoliciesGuard } from "src/casl/guards/policies.guard"; -import { DefaultUserSettingsInterceptor } from "./interceptors/default-user-settings.interceptor"; @ApiBearerAuth() @ApiTags("users") @@ -122,7 +115,6 @@ export class UsersController { @CheckPolicies("users", (ability: AppAbility) => ability.can(Action.UserReadOwn, User), ) - @UseInterceptors(CreateUserSettingsInterceptor) @Get("/my/self") @ApiOperation({ summary: "Returns the information of the user currently logged in.", @@ -184,7 +176,6 @@ export class UsersController { ability.can(Action.UserReadOwn, User) || ability.can(Action.UserReadAny, User), ) - @UseInterceptors(CreateUserSettingsInterceptor) @Get("/:id") async findById( @Req() request: Request, @@ -327,22 +318,6 @@ export class UsersController { return this.usersService.findOneAndDeleteUserSettings(id); } - @UseInterceptors(DefaultUserSettingsInterceptor) - @UseGuards( - class ByPassAuthenticatedPoliciesGuard - extends PoliciesGuard - implements CanActivate - { - async canActivate(): Promise { - return Promise.resolve(true); - } - }, - ) - @Get("/settings/default") - async getDefaultSettings(): Promise { - return Promise.resolve(new UserSettings()); - } - @UseGuards(AuthenticatedPoliciesGuard) @CheckPolicies("users", (ability: AppAbility) => { return ( From 8e1903a7ab897189d92908b7c43440a04d9975a1 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Thu, 5 Sep 2024 09:42:25 +0200 Subject: [PATCH 02/14] refactor: separate search UI logic from frontend --- src/config/frontend.config.json | 190 +++++++++++----------- src/users/schemas/user-settings.schema.ts | 32 ++-- src/users/users.controller.ts | 1 - src/users/users.service.ts | 40 ++++- 4 files changed, 152 insertions(+), 111 deletions(-) diff --git a/src/config/frontend.config.json b/src/config/frontend.config.json index 5dc093125..437e0a8cd 100644 --- a/src/config/frontend.config.json +++ b/src/config/frontend.config.json @@ -27,86 +27,6 @@ "jupyterHubUrl": "", "landingPage": "doi.ess.eu/detail/", "lbBaseURL": "http://127.0.0.1:3000", - "localColumns": [ - { - "name": "select", - "order": 0, - "type": "standard", - "enabled": true - }, - { - "name": "pid", - "order": 1, - "type": "standard", - "enabled": true - }, - { - "name": "datasetName", - "order": 2, - "type": "standard", - "enabled": true - }, - { - "name": "runNumber", - "order": 3, - "type": "standard", - "enabled": true - }, - { - "name": "sourceFolder", - "order": 4, - "type": "standard", - "enabled": true - }, - { - "name": "size", - "order": 5, - "type": "standard", - "enabled": true - }, - { - "name": "creationTime", - "order": 6, - "type": "standard", - "enabled": true - }, - { - "name": "type", - "order": 7, - "type": "standard", - "enabled": true - }, - { - "name": "image", - "order": 8, - "type": "standard", - "enabled": true - }, - { - "name": "metadata", - "order": 9, - "type": "standard", - "enabled": false - }, - { - "name": "proposalId", - "order": 10, - "type": "standard", - "enabled": true - }, - { - "name": "ownerGroup", - "order": 11, - "type": "standard", - "enabled": false - }, - { - "name": "dataStatus", - "order": 12, - "type": "standard", - "enabled": false - } - ], "logbookEnabled": true, "loginFormEnabled": true, "maxDirectDownloadSize": 5000000000, @@ -181,18 +101,106 @@ "authorization": ["#datasetAccess", "#datasetPublic"] } ], - "defaultSearchInterfaceSettings": { - "columns": [], + "labelMaps": { + "filters": { + "LocationFilter": "asdasd", + "PidFilter": "asdasdsad identifiler", + "GroupFilter": "fff", + "TypeFilter": "Typfggge", + "KeywordFilter": "ggg", + "DateRangeFilter": "Start Date - End Date", + "TextFilter": "Text" + } + }, + "defaultDatasetsListSettings": { + "columns": [ + { + "name": "select", + "order": 0, + "type": "standard", + "enabled": true + }, + { + "name": "pid", + "order": 1, + "type": "standard", + "enabled": true + }, + { + "name": "datasetName", + "order": 2, + "type": "standard", + "enabled": true + }, + { + "name": "runNumber", + "order": 3, + "type": "standard", + "enabled": true + }, + { + "name": "sourceFolder", + "order": 4, + "type": "standard", + "enabled": true + }, + { + "name": "size", + "order": 5, + "type": "standard", + "enabled": true + }, + { + "name": "creationTime", + "order": 6, + "type": "standard", + "enabled": true + }, + { + "name": "type", + "order": 7, + "type": "standard", + "enabled": true + }, + { + "name": "image", + "order": 8, + "type": "standard", + "enabled": true + }, + { + "name": "metadata", + "order": 9, + "type": "standard", + "enabled": false + }, + { + "name": "proposalId", + "order": 10, + "type": "standard", + "enabled": true + }, + { + "name": "ownerGroup", + "order": 11, + "type": "standard", + "enabled": false + }, + { + "name": "dataStatus", + "order": 12, + "type": "standard", + "enabled": false + } + ], "filters": [ - { "type": "LocationFilterComponent", "visible": true }, - { "type": "PidFilterComponent", "visible": true }, - { "type": "PidFilterContainsComponent", "visible": false }, - { "type": "PidFilterStartsWithComponent", "visible": false }, - { "type": "GroupFilterComponent", "visible": true }, - { "type": "TypeFilterComponent", "visible": true }, - { "type": "KeywordFilterComponent", "visible": true }, - { "type": "DateRangeFilterComponent", "visible": true }, - { "type": "TextFilterComponent", "visible": true } + { "LocationFilter": true }, + { "PidFilter": true }, + { "GroupFilter": true }, + { "TypeFilter": true }, + { "KeywordFilter": true }, + { "DateRangeFilter": true }, + { "TextFilter": true } ], "conditions": [] } diff --git a/src/users/schemas/user-settings.schema.ts b/src/users/schemas/user-settings.schema.ts index 59f4aea5a..ad9d9b564 100644 --- a/src/users/schemas/user-settings.schema.ts +++ b/src/users/schemas/user-settings.schema.ts @@ -5,24 +5,24 @@ import { Document } from "mongoose"; export type UserSettingsDocument = UserSettings & Document; -// Define possible filter component types as a union of string literals -export type FilterComponentType = - | "LocationFilterComponent" - | "PidFilterComponent" - | "PidFilterContainsComponent" - | "PidFilterStartsWithComponent" - | "GroupFilterComponent" - | "TypeFilterComponent" - | "KeywordFilterComponent" - | "DateRangeFilterComponent" - | "TextFilterComponent"; - -// Define the Filter interface -export interface FilterConfig { - type: FilterComponentType; - visible: boolean; +// NOTE: PidFilterContains and PidFilterStartsWith filters are not implemented +export enum FilterComponentType { + LocationFilter = "LocationFilter", + PidFilter = "PidFilter", + PidFilterContains = "PidFilterContains", + PidFilterStartsWith = "PidFilterStartsWith", + GroupFilter = "GroupFilter", + TypeFilter = "TypeFilter", + KeywordFilter = "KeywordFilter", + DateRangeFilter = "DateRangeFilter", + TextFilter = "TextFilter", } +// NOTE: The key is one of FilterComponentType, and the value is a string +export type FilterConfig = Partial<{ + [K in FilterComponentType]: boolean; +}>; + // Define the Condition interface export interface ScientificCondition { field: string; diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index d894ef955..687ae6544 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -42,7 +42,6 @@ import { CreateCustomJwt } from "./dto/create-custom-jwt.dto"; import { AuthenticatedPoliciesGuard } from "../casl/guards/auth-check.guard"; import { ReturnedUserDto } from "./dto/returned-user.dto"; import { ReturnedAuthLoginDto } from "src/auth/dto/returnedLogin.dto"; -import { PoliciesGuard } from "src/casl/guards/policies.guard"; @ApiBearerAuth() @ApiTags("users") diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 7369730d7..09c5fab21 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -18,6 +18,7 @@ import { JwtService, JwtSignOptions } from "@nestjs/jwt"; import { JWTUser } from "../auth/interfaces/jwt-user.interface"; import * as fs from "fs"; import { + FilterComponentType, UserSettings, UserSettingsDocument, } from "./schemas/user-settings.schema"; @@ -273,16 +274,39 @@ export class UsersService implements OnModuleInit { } async findByIdUserSettings(userId: string): Promise { - return this.userSettingsModel.findOne({ userId }).exec(); + const result = await this.userSettingsModel.findOne({ userId }).exec(); + + if (!result) { + return null; + } + + // NOTE: The extra functions ensure filters in user setting record match the FilterComponentType format. + // If not, reset the user settings to maintain consistency. + const validFilters = result.filters.some((filter) => { + const [key, value] = Object.entries(filter)[0]; + return this.isValidFilterComponentType(key, value); + }); + + if (!validFilters) { + return this.findOneAndUpdateUserSettings(userId, { filters: [] }); + } + + return result; } async findOneAndUpdateUserSettings( userId: string, updateUserSettingsDto: UpdateUserSettingsDto | PartialUpdateUserSettingsDto, ): Promise { - return this.userSettingsModel - .findOneAndUpdate({ userId }, updateUserSettingsDto, { new: true }) + const result = await this.userSettingsModel + .findOneAndUpdate({ userId }, updateUserSettingsDto, { + new: true, + upsert: true, + setDefaultsOnInsert: true, + }) .exec(); + + return result; } async findOneAndDeleteUserSettings(userId: string): Promise { @@ -339,4 +363,14 @@ export class UsersService implements OnModuleInit { const jwtString = this.jwtService.sign(user, signAndVerifyOptions); return { jwt: jwtString }; } + + private isValidFilterComponentType( + key: string, + value: unknown, + ): key is FilterComponentType { + return ( + Object.keys(FilterComponentType).includes(key) && + typeof value === "boolean" + ); + } } From e0ec9f273f06b51ad3f77c74b6b700647804fc3a Mon Sep 17 00:00:00 2001 From: junjiequan Date: Thu, 5 Sep 2024 10:34:50 +0200 Subject: [PATCH 03/14] fix for failing test --- src/users/users.service.ts | 14 ++++++++------ test/UserAuthorization.js | 12 ++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 09c5fab21..fdc7cf9f4 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -276,19 +276,21 @@ export class UsersService implements OnModuleInit { async findByIdUserSettings(userId: string): Promise { const result = await this.userSettingsModel.findOne({ userId }).exec(); - if (!result) { - return null; - } - // NOTE: The extra functions ensure filters in user setting record match the FilterComponentType format. // If not, reset the user settings to maintain consistency. - const validFilters = result.filters.some((filter) => { + const validFilters = result?.filters.some((filter) => { const [key, value] = Object.entries(filter)[0]; return this.isValidFilterComponentType(key, value); }); if (!validFilters) { - return this.findOneAndUpdateUserSettings(userId, { filters: [] }); + const updatedUsersFilter = await this.findOneAndUpdateUserSettings( + userId, + { + filters: [], + }, + ); + return updatedUsersFilter; } return result; diff --git a/test/UserAuthorization.js b/test/UserAuthorization.js index a8fb227f0..012b09d1f 100644 --- a/test/UserAuthorization.js +++ b/test/UserAuthorization.js @@ -21,7 +21,7 @@ let accessTokenAdminIngestor = null, userIdArchiveManager = null; describe("2300: User Authorization: test that user authorization are correct", () => { - beforeEach(async() => { + beforeEach(async () => { const loginResponseIngestor = await utils.getIdAndToken(appUrl, { username: "adminIngestor", password: TestData.Accounts["adminIngestor"]["password"], @@ -34,7 +34,7 @@ describe("2300: User Authorization: test that user authorization are correct", ( password: TestData.Accounts["user1"]["password"], }); userIdUser1 = loginResponseUser1.userId; - accessTokenUser1 = loginResponseUser1.token; + accessTokenUser1 = loginResponseUser1.token; const loginResponseUser2 = await utils.getIdAndToken(appUrl, { username: "user2", @@ -47,8 +47,8 @@ describe("2300: User Authorization: test that user authorization are correct", ( username: "user3", password: TestData.Accounts["user3"]["password"], }); - userIdUser3 = loginResponseUser3.userId - accessTokenUser3 = loginResponseUser3.token + userIdUser3 = loginResponseUser3.userId; + accessTokenUser3 = loginResponseUser3.token; const loginResponseUser4 = await utils.getIdAndToken(appUrl, { username: "user4", @@ -56,7 +56,7 @@ describe("2300: User Authorization: test that user authorization are correct", ( }); userIdUser4 = loginResponseUser4.userId; accessTokenUser4 = loginResponseUser4.token; - + const loginResponseAdmin = await utils.getIdAndToken(appUrl, { username: "admin", password: TestData.Accounts["admin"]["password"], @@ -71,7 +71,7 @@ describe("2300: User Authorization: test that user authorization are correct", ( userIdArchiveManager = loginResponseArchiveManager.userId; accessTokenArchiveManager = loginResponseArchiveManager.token; }); - + afterEach((done) => { sandbox.restore(); done(); From d6a1d71c71ad674efa077f751cf463e128df39be Mon Sep 17 00:00:00 2001 From: junjiequan Date: Thu, 5 Sep 2024 10:44:59 +0200 Subject: [PATCH 04/14] fix default filter labelmaps --- src/config/frontend.config.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/config/frontend.config.json b/src/config/frontend.config.json index 437e0a8cd..cb9c5394f 100644 --- a/src/config/frontend.config.json +++ b/src/config/frontend.config.json @@ -103,11 +103,11 @@ ], "labelMaps": { "filters": { - "LocationFilter": "asdasd", - "PidFilter": "asdasdsad identifiler", - "GroupFilter": "fff", - "TypeFilter": "Typfggge", - "KeywordFilter": "ggg", + "LocationFilter": "Location", + "PidFilter": "Pid", + "GroupFilter": "Group", + "TypeFilter": "Type", + "KeywordFilter": "Keyword", "DateRangeFilter": "Start Date - End Date", "TextFilter": "Text" } From 4ea9ff3dbadf6509a27d70c48a1efa8481fe0125 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Thu, 5 Sep 2024 11:12:55 +0200 Subject: [PATCH 05/14] minor fixes --- src/users/schemas/user-settings.schema.ts | 2 +- src/users/users.service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/users/schemas/user-settings.schema.ts b/src/users/schemas/user-settings.schema.ts index ad9d9b564..25703460c 100644 --- a/src/users/schemas/user-settings.schema.ts +++ b/src/users/schemas/user-settings.schema.ts @@ -18,7 +18,7 @@ export enum FilterComponentType { TextFilter = "TextFilter", } -// NOTE: The key is one of FilterComponentType, and the value is a string +// NOTE: The key is one of FilterComponentType export type FilterConfig = Partial<{ [K in FilterComponentType]: boolean; }>; diff --git a/src/users/users.service.ts b/src/users/users.service.ts index fdc7cf9f4..9b60d7faf 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -283,7 +283,7 @@ export class UsersService implements OnModuleInit { return this.isValidFilterComponentType(key, value); }); - if (!validFilters) { + if (result && !validFilters) { const updatedUsersFilter = await this.findOneAndUpdateUserSettings( userId, { From c32b2165c41ccb5745876a582b00f0368278bfd5 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Thu, 5 Sep 2024 14:23:41 +0200 Subject: [PATCH 06/14] revert user settings interceptor and added reset user setting function --- .../interceptors/user-settings.interceptor.ts | 93 +++++++++++++++++++ src/users/users.controller.ts | 3 + src/users/users.service.ts | 32 +------ 3 files changed, 100 insertions(+), 28 deletions(-) create mode 100644 src/users/interceptors/user-settings.interceptor.ts diff --git a/src/users/interceptors/user-settings.interceptor.ts b/src/users/interceptors/user-settings.interceptor.ts new file mode 100644 index 000000000..fbdba49bc --- /dev/null +++ b/src/users/interceptors/user-settings.interceptor.ts @@ -0,0 +1,93 @@ +import { + CallHandler, + ExecutionContext, + Injectable, + Logger, + NestInterceptor, +} from "@nestjs/common"; +import { Observable, tap } from "rxjs"; +import { CreateUserSettingsDto } from "../dto/create-user-settings.dto"; +import { UsersService } from "../users.service"; +import { + FilterComponentType, + UserSettings, +} from "../schemas/user-settings.schema"; +import { UpdateUserSettingsDto } from "../dto/update-user-settings.dto"; + +@Injectable() +export class UserSettingsInterceptor implements NestInterceptor { + constructor(private usersService: UsersService) {} + async intercept( + context: ExecutionContext, + next: CallHandler, + ): Promise> { + return next.handle().pipe( + tap(async () => { + const res = context.switchToHttp().getResponse(); + const user = res.req.user; + if (!user) { + return; + } + const userId = user._id; + const userSettings = + await this.usersService.findByIdUserSettings(userId); + if (!userSettings) { + Logger.log( + `Adding default settings to user ${user.username}`, + "UserSettingsInterceptor", + ); + const createUserSettingsDto: CreateUserSettingsDto = { + userId, + filters: [], + conditions: [], + columns: [], + }; + return this.usersService.createUserSettings( + userId, + createUserSettingsDto, + ); + } else { + const isValidFilters = this.isValidFilterComponentType(userSettings); + + if (!isValidFilters) { + Logger.log( + `Reset default settings to user ${user.username}`, + "UserSettingsInterceptor", + ); + return await this.resetUserSettings(userId, userSettings); + } + } + + return; + }), + ); + } + + private async resetUserSettings(userId: string, userSettings: UserSettings) { + // NOTE: The extra functions ensure filters in user setting record match the FilterComponentType format. + // If not, reset the user settings to maintain consistency. + const updateUserSettingsDto: UpdateUserSettingsDto = { + ...userSettings, + filters: [], + }; + const updatedUsersFilter = + await this.usersService.findOneAndUpdateUserSettings( + userId, + updateUserSettingsDto, + ); + return updatedUsersFilter; + } + + private isValidFilterComponentType(userSettings: UserSettings): boolean { + if (userSettings.filters.length === 0) { + return true; + } + return userSettings.filters.every((filter) => { + const [key, value] = Object.entries(filter)[0]; + return ( + Object.keys(FilterComponentType).includes(key) && + typeof value === "boolean" + ); + }); + } +} diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 687ae6544..170fe20d3 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -11,6 +11,7 @@ import { Body, ForbiddenException, HttpCode, + UseInterceptors, } from "@nestjs/common"; import { ApiBearerAuth, @@ -42,6 +43,7 @@ import { CreateCustomJwt } from "./dto/create-custom-jwt.dto"; import { AuthenticatedPoliciesGuard } from "../casl/guards/auth-check.guard"; import { ReturnedUserDto } from "./dto/returned-user.dto"; import { ReturnedAuthLoginDto } from "src/auth/dto/returnedLogin.dto"; +import { UserSettingsInterceptor } from "./interceptors/user-settings.interceptor"; @ApiBearerAuth() @ApiTags("users") @@ -230,6 +232,7 @@ export class UsersController { } @UseGuards(AuthenticatedPoliciesGuard) + @UseInterceptors(UserSettingsInterceptor) @CheckPolicies( "users", (ability: AppAbility) => diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 9b60d7faf..639385517 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -274,24 +274,10 @@ export class UsersService implements OnModuleInit { } async findByIdUserSettings(userId: string): Promise { - const result = await this.userSettingsModel.findOne({ userId }).exec(); - - // NOTE: The extra functions ensure filters in user setting record match the FilterComponentType format. - // If not, reset the user settings to maintain consistency. - const validFilters = result?.filters.some((filter) => { - const [key, value] = Object.entries(filter)[0]; - return this.isValidFilterComponentType(key, value); - }); - - if (result && !validFilters) { - const updatedUsersFilter = await this.findOneAndUpdateUserSettings( - userId, - { - filters: [], - }, - ); - return updatedUsersFilter; - } + const result = await this.userSettingsModel + .findOne({ userId }) + .lean() + .exec(); return result; } @@ -365,14 +351,4 @@ export class UsersService implements OnModuleInit { const jwtString = this.jwtService.sign(user, signAndVerifyOptions); return { jwt: jwtString }; } - - private isValidFilterComponentType( - key: string, - value: unknown, - ): key is FilterComponentType { - return ( - Object.keys(FilterComponentType).includes(key) && - typeof value === "boolean" - ); - } } From e476c409acfe4b1a97420f9b8ab724528930f9c2 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Thu, 5 Sep 2024 15:18:01 +0200 Subject: [PATCH 07/14] changed tap to swtichMap for asynchronous events for user-settings interceptor. moved user-settings interceptor to login endpoint --- .../interceptors/user-settings.interceptor.ts | 34 +++++++++---------- src/users/users.controller.ts | 2 +- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/users/interceptors/user-settings.interceptor.ts b/src/users/interceptors/user-settings.interceptor.ts index fbdba49bc..1f5426e14 100644 --- a/src/users/interceptors/user-settings.interceptor.ts +++ b/src/users/interceptors/user-settings.interceptor.ts @@ -5,7 +5,7 @@ import { Logger, NestInterceptor, } from "@nestjs/common"; -import { Observable, tap } from "rxjs"; +import { Observable, switchMap } from "rxjs"; import { CreateUserSettingsDto } from "../dto/create-user-settings.dto"; import { UsersService } from "../users.service"; import { @@ -17,20 +17,22 @@ import { UpdateUserSettingsDto } from "../dto/update-user-settings.dto"; @Injectable() export class UserSettingsInterceptor implements NestInterceptor { constructor(private usersService: UsersService) {} - async intercept( - context: ExecutionContext, - next: CallHandler, - ): Promise> { + + intercept(context: ExecutionContext, next: CallHandler): Observable { + const res = context.switchToHttp().getResponse(); + const user = res.req.user; + + if (!user) { + return next.handle(); + } + + const userId = user._id; + return next.handle().pipe( - tap(async () => { - const res = context.switchToHttp().getResponse(); - const user = res.req.user; - if (!user) { - return; - } - const userId = user._id; + switchMap(async (payload) => { const userSettings = await this.usersService.findByIdUserSettings(userId); + if (!userSettings) { Logger.log( `Adding default settings to user ${user.username}`, @@ -42,7 +44,7 @@ export class UserSettingsInterceptor implements NestInterceptor { conditions: [], columns: [], }; - return this.usersService.createUserSettings( + await this.usersService.createUserSettings( userId, createUserSettingsDto, ); @@ -54,15 +56,13 @@ export class UserSettingsInterceptor implements NestInterceptor { `Reset default settings to user ${user.username}`, "UserSettingsInterceptor", ); - return await this.resetUserSettings(userId, userSettings); + await this.resetUserSettings(userId, userSettings); } } - - return; + return payload; }), ); } - private async resetUserSettings(userId: string, userSettings: UserSettings) { // NOTE: The extra functions ensure filters in user setting record match the FilterComponentType format. // If not, reset the user settings to maintain consistency. diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 170fe20d3..142dff0f4 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -94,6 +94,7 @@ export class UsersController { @ApiBody({ type: CredentialsDto }) @AllowAny() + @UseInterceptors(UserSettingsInterceptor) @UseGuards(LocalAuthGuard) @Post("login") @ApiOperation({ @@ -232,7 +233,6 @@ export class UsersController { } @UseGuards(AuthenticatedPoliciesGuard) - @UseInterceptors(UserSettingsInterceptor) @CheckPolicies( "users", (ability: AppAbility) => From f0afcaa409c9a740a59eb5a4e134b2a4b0c24b50 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Mon, 16 Sep 2024 13:15:36 +0200 Subject: [PATCH 08/14] refactor(auth): implement post-login tasks for user settings management and remove UserSettingsInterceptor --- src/auth/auth.service.ts | 65 +++++++++++++ src/config/frontend.config.json | 2 +- .../interceptors/user-settings.interceptor.ts | 93 ------------------- src/users/users.controller.spec.ts | 58 +++++++----- src/users/users.controller.ts | 17 ++-- 5 files changed, 106 insertions(+), 129 deletions(-) delete mode 100644 src/users/interceptors/user-settings.interceptor.ts diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 3ee2933ae..c36dde925 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -10,6 +10,12 @@ import { flattenObject, parseBoolean } from "src/common/utils"; import { Issuer } from "openid-client"; import { ReturnedAuthLoginDto } from "./dto/returnedLogin.dto"; import { ReturnedUserDto } from "src/users/dto/returned-user.dto"; +import { CreateUserSettingsDto } from "src/users/dto/create-user-settings.dto"; +import { + FilterComponentType, + UserSettings, +} from "src/users/schemas/user-settings.schema"; +import { UpdateUserSettingsDto } from "src/users/dto/update-user-settings.dto"; @Injectable() export class AuthService { @@ -43,6 +49,7 @@ export class AuthService { async login(user: Omit): Promise { const expiresIn = this.configService.get("jwt.expiresIn"); const accessToken = this.jwtService.sign(user, { expiresIn }); + await this.postLoginTasks(user); return { access_token: accessToken, id: accessToken, @@ -122,4 +129,62 @@ export class AuthService { return { logout: "successful" }; } + /** + * postLoginTasks: Executes additional tasks after user login. + * + * - Checks if the user has userSettings record. + * - If user has no userSetting, it creates default userSetting for the user. + * - If userSetting exist but are invalid (filters does not belong to FilterComponentType), it resets the filters to default - empty array. + * + * @param user - The logged-in user (without password). + */ + async postLoginTasks(user: Omit) { + if (!user) return; + + const userId = user._id; + + const userSettings = await this.usersService.findByIdUserSettings(userId); + + if (!userSettings) { + Logger.log( + `Adding default settings to user ${user.username}`, + "UserSettingsInterceptor", + ); + const createUserSettingsDto: CreateUserSettingsDto = { + userId, + filters: [], + conditions: [], + columns: [], + }; + await this.usersService.createUserSettings(userId, createUserSettingsDto); + } else { + const isValidFilters = (userSettings: UserSettings): boolean => { + if (userSettings.filters.length === 0) { + return true; + } + return userSettings.filters.every((filter) => { + const [key, value] = Object.entries(filter)[0]; + return ( + Object.keys(FilterComponentType).includes(key) && + typeof value === "boolean" + ); + }); + }; + + if (!isValidFilters) { + Logger.log( + `Reset default settings to user ${user.username}`, + "UserSettingsInterceptor", + ); + const updateUserSettingsDto: UpdateUserSettingsDto = { + ...userSettings, + filters: [], + }; + await this.usersService.findOneAndUpdateUserSettings( + userId, + updateUserSettingsDto, + ); + } + } + } } diff --git a/src/config/frontend.config.json b/src/config/frontend.config.json index cb9c5394f..213c69a5e 100644 --- a/src/config/frontend.config.json +++ b/src/config/frontend.config.json @@ -26,7 +26,7 @@ "jsonMetadataEnabled": true, "jupyterHubUrl": "", "landingPage": "doi.ess.eu/detail/", - "lbBaseURL": "http://127.0.0.1:3000", + "lbBaseURL": "http://localhost:3000", "logbookEnabled": true, "loginFormEnabled": true, "maxDirectDownloadSize": 5000000000, diff --git a/src/users/interceptors/user-settings.interceptor.ts b/src/users/interceptors/user-settings.interceptor.ts deleted file mode 100644 index 1f5426e14..000000000 --- a/src/users/interceptors/user-settings.interceptor.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { - CallHandler, - ExecutionContext, - Injectable, - Logger, - NestInterceptor, -} from "@nestjs/common"; -import { Observable, switchMap } from "rxjs"; -import { CreateUserSettingsDto } from "../dto/create-user-settings.dto"; -import { UsersService } from "../users.service"; -import { - FilterComponentType, - UserSettings, -} from "../schemas/user-settings.schema"; -import { UpdateUserSettingsDto } from "../dto/update-user-settings.dto"; - -@Injectable() -export class UserSettingsInterceptor implements NestInterceptor { - constructor(private usersService: UsersService) {} - - intercept(context: ExecutionContext, next: CallHandler): Observable { - const res = context.switchToHttp().getResponse(); - const user = res.req.user; - - if (!user) { - return next.handle(); - } - - const userId = user._id; - - return next.handle().pipe( - switchMap(async (payload) => { - const userSettings = - await this.usersService.findByIdUserSettings(userId); - - if (!userSettings) { - Logger.log( - `Adding default settings to user ${user.username}`, - "UserSettingsInterceptor", - ); - const createUserSettingsDto: CreateUserSettingsDto = { - userId, - filters: [], - conditions: [], - columns: [], - }; - await this.usersService.createUserSettings( - userId, - createUserSettingsDto, - ); - } else { - const isValidFilters = this.isValidFilterComponentType(userSettings); - - if (!isValidFilters) { - Logger.log( - `Reset default settings to user ${user.username}`, - "UserSettingsInterceptor", - ); - await this.resetUserSettings(userId, userSettings); - } - } - return payload; - }), - ); - } - private async resetUserSettings(userId: string, userSettings: UserSettings) { - // NOTE: The extra functions ensure filters in user setting record match the FilterComponentType format. - // If not, reset the user settings to maintain consistency. - const updateUserSettingsDto: UpdateUserSettingsDto = { - ...userSettings, - filters: [], - }; - const updatedUsersFilter = - await this.usersService.findOneAndUpdateUserSettings( - userId, - updateUserSettingsDto, - ); - return updatedUsersFilter; - } - - private isValidFilterComponentType(userSettings: UserSettings): boolean { - if (userSettings.filters.length === 0) { - return true; - } - return userSettings.filters.every((filter) => { - const [key, value] = Object.entries(filter)[0]; - return ( - Object.keys(FilterComponentType).includes(key) && - typeof value === "boolean" - ); - }); - } -} diff --git a/src/users/users.controller.spec.ts b/src/users/users.controller.spec.ts index 6bedb6203..1d8f16b33 100644 --- a/src/users/users.controller.spec.ts +++ b/src/users/users.controller.spec.ts @@ -4,6 +4,7 @@ import { CaslModule } from "src/casl/casl.module"; import { UsersController } from "./users.controller"; import { UsersService } from "./users.service"; import { UpdateUserSettingsDto } from "./dto/update-user-settings.dto"; +import { Request } from "express"; class UsersServiceMock { findByIdUserIdentity(id: string) { @@ -28,10 +29,7 @@ const mockUserSettings = { columns: [], datasetCount: 25, jobCount: 25, - filters: [ - { type: "LocationFilterComponent", visible: true }, - { type: "PidFilterComponent", visible: true }, - ], + filters: [{ LocationFilter: true }, { PidFilter: true }], conditions: [{ field: "status", value: "active", operator: "equals" }], }; @@ -54,7 +52,6 @@ describe("UsersController", () => { controller = module.get(UsersController); usersService = module.get(UsersService); - // bypass authorization jest .spyOn(controller as UsersController, "checkUserAuthorization") .mockImplementation(() => Promise.resolve()); @@ -65,45 +62,56 @@ describe("UsersController", () => { }); it("should return user settings with filters and conditions", async () => { - jest - .spyOn(usersService, "findByIdUserSettings") - .mockResolvedValue(mockUserSettings); - const userId = "user1"; - const result = await controller.getSettings( - { user: { _id: userId } }, - userId, - ); + mockUserSettings._id = userId; + + const mockRequest: Partial = { + user: { _id: userId }, + }; + + const result = await controller.getSettings(mockRequest as Request, userId); + // Assert expect(result).toEqual(mockUserSettings); - expect(result.filters).toBeDefined(); - expect(result.filters.length).toBeGreaterThan(0); - expect(result.conditions).toBeDefined(); - expect(result.conditions.length).toBeGreaterThan(0); + expect(result?.filters).toBeDefined(); + expect(result?.filters.length).toBeGreaterThan(0); + expect(result?.conditions).toBeDefined(); + expect(result?.conditions.length).toBeGreaterThan(0); }); it("should update user settings with filters and conditions", async () => { + const userId = "user-id"; + mockUserSettings._id = userId; + const updatedSettings = { ...mockUserSettings, - filters: [{ type: "PidFilterContainsComponent", visible: false }], + filters: [{ PidFilter: true }], conditions: [{ field: "status", value: "inactive", operator: "equals" }], }; + const mockRequest: Partial = { + user: { _id: userId }, + }; + + const expectedResponse = { + ...updatedSettings, + _id: userId, + }; + jest .spyOn(usersService, "findOneAndUpdateUserSettings") - .mockResolvedValue(updatedSettings); + .mockResolvedValue(expectedResponse); - const userId = "user-id"; const result = await controller.updateSettings( - { user: { _id: userId } }, + mockRequest as Request, userId, updatedSettings, ); expect(result).toEqual(updatedSettings); - expect(result.filters).toBeDefined(); - expect(result.filters.length).toBe(1); - expect(result.conditions).toBeDefined(); - expect(result.conditions.length).toBe(1); + expect(result?.filters).toBeDefined(); + expect(result?.filters.length).toBe(1); + expect(result?.conditions).toBeDefined(); + expect(result?.conditions.length).toBe(1); }); }); diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 142dff0f4..2822c28c7 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -11,7 +11,6 @@ import { Body, ForbiddenException, HttpCode, - UseInterceptors, } from "@nestjs/common"; import { ApiBearerAuth, @@ -43,7 +42,6 @@ import { CreateCustomJwt } from "./dto/create-custom-jwt.dto"; import { AuthenticatedPoliciesGuard } from "../casl/guards/auth-check.guard"; import { ReturnedUserDto } from "./dto/returned-user.dto"; import { ReturnedAuthLoginDto } from "src/auth/dto/returnedLogin.dto"; -import { UserSettingsInterceptor } from "./interceptors/user-settings.interceptor"; @ApiBearerAuth() @ApiTags("users") @@ -94,23 +92,22 @@ export class UsersController { @ApiBody({ type: CredentialsDto }) @AllowAny() - @UseInterceptors(UserSettingsInterceptor) @UseGuards(LocalAuthGuard) @Post("login") @ApiOperation({ - summary: "Functional accounts login.", - description: "It allows to login with functional (local) accounts.", + summary: + "This endpoint is deprecated and will be removed soon. Use /auth/login instead", + description: + "This endpoint is deprecated and will be removed soon. Use /auth/login instead", }) @ApiResponse({ status: 201, type: ReturnedAuthLoginDto, description: - "Create a new JWT token for anonymous or the user that is currently logged in", + "This endpoint is deprecated and will be removed soon. Use /auth/login instead", }) - async login( - @Req() req: Record, - ): Promise { - return await this.authService.login(req.user as Omit); + async login(@Req() req: Record): Promise { + return null; } @UseGuards(AuthenticatedPoliciesGuard) From b1d47bd61fa6b15a395d5f6a6855055992b5b02c Mon Sep 17 00:00:00 2001 From: junjiequan Date: Mon, 16 Sep 2024 15:04:46 +0200 Subject: [PATCH 09/14] removed all user filter settings related logic --- src/auth/auth.service.ts | 35 ----------------------- src/users/dto/update-user-settings.dto.ts | 7 ++--- src/users/schemas/user-settings.schema.ts | 20 +------------ src/users/users.service.ts | 1 - 4 files changed, 3 insertions(+), 60 deletions(-) diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index c36dde925..2acdbccc8 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -11,11 +11,6 @@ import { Issuer } from "openid-client"; import { ReturnedAuthLoginDto } from "./dto/returnedLogin.dto"; import { ReturnedUserDto } from "src/users/dto/returned-user.dto"; import { CreateUserSettingsDto } from "src/users/dto/create-user-settings.dto"; -import { - FilterComponentType, - UserSettings, -} from "src/users/schemas/user-settings.schema"; -import { UpdateUserSettingsDto } from "src/users/dto/update-user-settings.dto"; @Injectable() export class AuthService { @@ -134,8 +129,6 @@ export class AuthService { * * - Checks if the user has userSettings record. * - If user has no userSetting, it creates default userSetting for the user. - * - If userSetting exist but are invalid (filters does not belong to FilterComponentType), it resets the filters to default - empty array. - * * @param user - The logged-in user (without password). */ async postLoginTasks(user: Omit) { @@ -157,34 +150,6 @@ export class AuthService { columns: [], }; await this.usersService.createUserSettings(userId, createUserSettingsDto); - } else { - const isValidFilters = (userSettings: UserSettings): boolean => { - if (userSettings.filters.length === 0) { - return true; - } - return userSettings.filters.every((filter) => { - const [key, value] = Object.entries(filter)[0]; - return ( - Object.keys(FilterComponentType).includes(key) && - typeof value === "boolean" - ); - }); - }; - - if (!isValidFilters) { - Logger.log( - `Reset default settings to user ${user.username}`, - "UserSettingsInterceptor", - ); - const updateUserSettingsDto: UpdateUserSettingsDto = { - ...userSettings, - filters: [], - }; - await this.usersService.findOneAndUpdateUserSettings( - userId, - updateUserSettingsDto, - ); - } } } } diff --git a/src/users/dto/update-user-settings.dto.ts b/src/users/dto/update-user-settings.dto.ts index 8458e4e84..d0159c39b 100644 --- a/src/users/dto/update-user-settings.dto.ts +++ b/src/users/dto/update-user-settings.dto.ts @@ -1,8 +1,5 @@ import { ApiProperty, PartialType } from "@nestjs/swagger"; -import { - FilterConfig, - ScientificCondition, -} from "../schemas/user-settings.schema"; +import { ScientificCondition } from "../schemas/user-settings.schema"; import { IsArray, IsNumber } from "class-validator"; export class UpdateUserSettingsDto { @@ -20,7 +17,7 @@ export class UpdateUserSettingsDto { @ApiProperty() @IsArray() - readonly filters: FilterConfig[]; + readonly filters: Record[]; @ApiProperty() @IsArray() diff --git a/src/users/schemas/user-settings.schema.ts b/src/users/schemas/user-settings.schema.ts index 25703460c..b6ebf6a7d 100644 --- a/src/users/schemas/user-settings.schema.ts +++ b/src/users/schemas/user-settings.schema.ts @@ -5,24 +5,6 @@ import { Document } from "mongoose"; export type UserSettingsDocument = UserSettings & Document; -// NOTE: PidFilterContains and PidFilterStartsWith filters are not implemented -export enum FilterComponentType { - LocationFilter = "LocationFilter", - PidFilter = "PidFilter", - PidFilterContains = "PidFilterContains", - PidFilterStartsWith = "PidFilterStartsWith", - GroupFilter = "GroupFilter", - TypeFilter = "TypeFilter", - KeywordFilter = "KeywordFilter", - DateRangeFilter = "DateRangeFilter", - TextFilter = "TextFilter", -} - -// NOTE: The key is one of FilterComponentType -export type FilterConfig = Partial<{ - [K in FilterComponentType]: boolean; -}>; - // Define the Condition interface export interface ScientificCondition { field: string; @@ -78,7 +60,7 @@ export class UserSettings { type: [{ type: Object }], default: [], }) - filters: FilterConfig[]; + filters: Record[]; @ApiProperty({ type: [Object], diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 639385517..4c479c1b3 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -18,7 +18,6 @@ import { JwtService, JwtSignOptions } from "@nestjs/jwt"; import { JWTUser } from "../auth/interfaces/jwt-user.interface"; import * as fs from "fs"; import { - FilterComponentType, UserSettings, UserSettingsDocument, } from "./schemas/user-settings.schema"; From 53daa02b2c247a3b58da54dc0c28e6593ab83435 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Tue, 17 Sep 2024 12:31:56 +0200 Subject: [PATCH 10/14] refactor(user-settings): consolidate user settings structure and remove deprecated fields --- src/auth/auth.service.ts | 8 ++--- src/users/dto/update-user-settings.dto.ts | 22 +++++------- src/users/schemas/user-settings.schema.ts | 30 ++++------------ src/users/users.controller.spec.ts | 44 ++++++++++++++--------- 4 files changed, 46 insertions(+), 58 deletions(-) diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 2acdbccc8..fa201b7b5 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -140,14 +140,12 @@ export class AuthService { if (!userSettings) { Logger.log( - `Adding default settings to user ${user.username}`, - "UserSettingsInterceptor", + `Adding default settings to user ${user.username} with userId: ${user._id}`, + "postLoginTasks", ); const createUserSettingsDto: CreateUserSettingsDto = { userId, - filters: [], - conditions: [], - columns: [], + frontendSettings: {}, }; await this.usersService.createUserSettings(userId, createUserSettingsDto); } diff --git a/src/users/dto/update-user-settings.dto.ts b/src/users/dto/update-user-settings.dto.ts index d0159c39b..a9cca316e 100644 --- a/src/users/dto/update-user-settings.dto.ts +++ b/src/users/dto/update-user-settings.dto.ts @@ -1,12 +1,7 @@ import { ApiProperty, PartialType } from "@nestjs/swagger"; -import { ScientificCondition } from "../schemas/user-settings.schema"; -import { IsArray, IsNumber } from "class-validator"; +import { IsNumber, IsObject, IsOptional } from "class-validator"; export class UpdateUserSettingsDto { - @ApiProperty() - @IsArray() - readonly columns: Record[]; - @ApiProperty({ type: Number, required: false, default: 25 }) @IsNumber() readonly datasetCount?: number; @@ -15,13 +10,14 @@ export class UpdateUserSettingsDto { @IsNumber() readonly jobCount?: number; - @ApiProperty() - @IsArray() - readonly filters: Record[]; - - @ApiProperty() - @IsArray() - readonly conditions: ScientificCondition[]; + @ApiProperty({ + type: "object", + additionalProperties: { type: "array", items: {} }, + required: false, + }) + @IsOptional() + @IsObject() + readonly frontendSettings?: Record; } export class PartialUpdateUserSettingsDto extends PartialType( diff --git a/src/users/schemas/user-settings.schema.ts b/src/users/schemas/user-settings.schema.ts index b6ebf6a7d..9a629abe7 100644 --- a/src/users/schemas/user-settings.schema.ts +++ b/src/users/schemas/user-settings.schema.ts @@ -23,14 +23,6 @@ export class UserSettings { id?: string; - @ApiProperty({ - type: [Object], - default: [], - description: "Array of the users preferred columns in dataset table", - }) - @Prop({ type: [Object], default: [] }) - columns: Record[]; - @ApiProperty({ type: Number, default: 25, @@ -52,23 +44,13 @@ export class UserSettings { userId: string; @ApiProperty({ - type: [Object], - default: [], - description: "Array of filters the user has set", - }) - @Prop({ - type: [{ type: Object }], - default: [], - }) - filters: Record[]; - - @ApiProperty({ - type: [Object], - default: [], - description: "Array of conditions the user has set", + type: "object", + additionalProperties: { type: "array", items: {} }, + default: {}, + description: "users preferred ui settings in dataset table", }) - @Prop({ type: [{ type: Object }], default: [] }) - conditions: ScientificCondition[]; + @Prop({ type: Object, default: {}, required: false }) + frontendSettings?: Record; } export const UserSettingsSchema = SchemaFactory.createForClass(UserSettings); diff --git a/src/users/users.controller.spec.ts b/src/users/users.controller.spec.ts index 1d8f16b33..580e154cb 100644 --- a/src/users/users.controller.spec.ts +++ b/src/users/users.controller.spec.ts @@ -5,6 +5,7 @@ import { UsersController } from "./users.controller"; import { UsersService } from "./users.service"; import { UpdateUserSettingsDto } from "./dto/update-user-settings.dto"; import { Request } from "express"; +import { UserSettings } from "./schemas/user-settings.schema"; class UsersServiceMock { findByIdUserIdentity(id: string) { @@ -26,11 +27,13 @@ class UsersServiceMock { const mockUserSettings = { _id: "user1", userId: "user1", - columns: [], datasetCount: 25, jobCount: 25, - filters: [{ LocationFilter: true }, { PidFilter: true }], - conditions: [{ field: "status", value: "active", operator: "equals" }], + frontendSettings: { + filters: [{ LocationFilter: true }, { PidFilter: true }], + conditions: [{ field: "status", value: "active", operator: "equals" }], + columns: [], + }, }; class AuthServiceMock {} @@ -72,11 +75,11 @@ describe("UsersController", () => { const result = await controller.getSettings(mockRequest as Request, userId); // Assert - expect(result).toEqual(mockUserSettings); - expect(result?.filters).toBeDefined(); - expect(result?.filters.length).toBeGreaterThan(0); - expect(result?.conditions).toBeDefined(); - expect(result?.conditions.length).toBeGreaterThan(0); + expect(result?.frontendSettings).toEqual(mockUserSettings); + expect(result?.frontendSettings?.filters).toBeDefined(); + expect(result?.frontendSettings?.filters.length).toBeGreaterThan(0); + expect(result?.frontendSettings?.conditions).toBeDefined(); + expect(result?.frontendSettings?.conditions.length).toBeGreaterThan(0); }); it("should update user settings with filters and conditions", async () => { @@ -85,17 +88,26 @@ describe("UsersController", () => { const updatedSettings = { ...mockUserSettings, - filters: [{ PidFilter: true }], - conditions: [{ field: "status", value: "inactive", operator: "equals" }], + frontendSettings: { + filters: [{ PidFilter: true }], + conditions: [ + { field: "status", value: "inactive", operator: "equals" }, + ], + columns: [], + }, }; const mockRequest: Partial = { user: { _id: userId }, }; - const expectedResponse = { + const expectedResponse: UserSettings = { ...updatedSettings, _id: userId, + userId: userId, // Ensure all required properties are included + datasetCount: updatedSettings.datasetCount, + jobCount: updatedSettings.jobCount, + frontendSettings: updatedSettings.frontendSettings, }; jest @@ -108,10 +120,10 @@ describe("UsersController", () => { updatedSettings, ); - expect(result).toEqual(updatedSettings); - expect(result?.filters).toBeDefined(); - expect(result?.filters.length).toBe(1); - expect(result?.conditions).toBeDefined(); - expect(result?.conditions.length).toBe(1); + expect(result?.frontendSettings).toEqual(updatedSettings); + expect(result?.frontendSettings?.filters).toBeDefined(); + expect(result?.frontendSettings?.filters.length).toBe(1); + expect(result?.frontendSettings?.conditions).toBeDefined(); + expect(result?.frontendSettings?.conditions.length).toBe(1); }); }); From 0e73c70f29bd36b7c558b2705d7b1f8a0b074be5 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Wed, 18 Sep 2024 14:48:44 +0200 Subject: [PATCH 11/14] refactor(user-settings): update user settings structure and remove deprecated fields This commit refactors the user settings structure by updating the field names and removing deprecated fields. The `frontendSettings` field has been renamed to `externalSettings` to better reflect its purpose. Additionally, the deprecated fields have been removed from the user settings schema. --- src/auth/auth.service.ts | 2 +- src/users/dto/update-user-settings.dto.ts | 6 ++-- src/users/schemas/user-settings.schema.ts | 6 ++-- src/users/users.controller.spec.ts | 35 ++++++++++++++--------- src/users/users.controller.ts | 28 ++++++++++++++++-- src/users/users.service.ts | 28 ++++++++++++++++++ 6 files changed, 83 insertions(+), 22 deletions(-) diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index fa201b7b5..4d77c2461 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -145,7 +145,7 @@ export class AuthService { ); const createUserSettingsDto: CreateUserSettingsDto = { userId, - frontendSettings: {}, + externalSettings: {}, }; await this.usersService.createUserSettings(userId, createUserSettingsDto); } diff --git a/src/users/dto/update-user-settings.dto.ts b/src/users/dto/update-user-settings.dto.ts index a9cca316e..63794c6ea 100644 --- a/src/users/dto/update-user-settings.dto.ts +++ b/src/users/dto/update-user-settings.dto.ts @@ -11,13 +11,13 @@ export class UpdateUserSettingsDto { readonly jobCount?: number; @ApiProperty({ - type: "object", - additionalProperties: { type: "array", items: {} }, + type: Object, required: false, + default: {}, }) @IsOptional() @IsObject() - readonly frontendSettings?: Record; + readonly externalSettings?: Record; } export class PartialUpdateUserSettingsDto extends PartialType( diff --git a/src/users/schemas/user-settings.schema.ts b/src/users/schemas/user-settings.schema.ts index 9a629abe7..9aa9a9f08 100644 --- a/src/users/schemas/user-settings.schema.ts +++ b/src/users/schemas/user-settings.schema.ts @@ -45,12 +45,12 @@ export class UserSettings { @ApiProperty({ type: "object", - additionalProperties: { type: "array", items: {} }, default: {}, - description: "users preferred ui settings in dataset table", + description: + "A customizable object for storing the user's external settings, which can contain various nested properties and configurations.", }) @Prop({ type: Object, default: {}, required: false }) - frontendSettings?: Record; + externalSettings?: Record; } export const UserSettingsSchema = SchemaFactory.createForClass(UserSettings); diff --git a/src/users/users.controller.spec.ts b/src/users/users.controller.spec.ts index 580e154cb..40ac85311 100644 --- a/src/users/users.controller.spec.ts +++ b/src/users/users.controller.spec.ts @@ -29,7 +29,7 @@ const mockUserSettings = { userId: "user1", datasetCount: 25, jobCount: 25, - frontendSettings: { + externalSettings: { filters: [{ LocationFilter: true }, { PidFilter: true }], conditions: [{ field: "status", value: "active", operator: "equals" }], columns: [], @@ -75,11 +75,15 @@ describe("UsersController", () => { const result = await controller.getSettings(mockRequest as Request, userId); // Assert - expect(result?.frontendSettings).toEqual(mockUserSettings); - expect(result?.frontendSettings?.filters).toBeDefined(); - expect(result?.frontendSettings?.filters.length).toBeGreaterThan(0); - expect(result?.frontendSettings?.conditions).toBeDefined(); - expect(result?.frontendSettings?.conditions.length).toBeGreaterThan(0); + expect(result?.externalSettings).toEqual(mockUserSettings); + expect(result?.externalSettings?.filters).toBeDefined(); + expect( + (result?.externalSettings?.filters as Record).length, + ).toBeGreaterThan(0); + expect(result?.externalSettings?.conditions).toBeDefined(); + expect( + (result?.externalSettings?.conditions as Record).length, + ).toBeGreaterThan(0); }); it("should update user settings with filters and conditions", async () => { @@ -88,7 +92,7 @@ describe("UsersController", () => { const updatedSettings = { ...mockUserSettings, - frontendSettings: { + externalSettings: { filters: [{ PidFilter: true }], conditions: [ { field: "status", value: "inactive", operator: "equals" }, @@ -107,7 +111,7 @@ describe("UsersController", () => { userId: userId, // Ensure all required properties are included datasetCount: updatedSettings.datasetCount, jobCount: updatedSettings.jobCount, - frontendSettings: updatedSettings.frontendSettings, + externalSettings: updatedSettings.externalSettings, }; jest @@ -120,10 +124,15 @@ describe("UsersController", () => { updatedSettings, ); - expect(result?.frontendSettings).toEqual(updatedSettings); - expect(result?.frontendSettings?.filters).toBeDefined(); - expect(result?.frontendSettings?.filters.length).toBe(1); - expect(result?.frontendSettings?.conditions).toBeDefined(); - expect(result?.frontendSettings?.conditions.length).toBe(1); + expect(result?.externalSettings).toEqual(updatedSettings); + expect(result?.externalSettings?.filters).toBeDefined(); + expect( + (result?.externalSettings?.filters as Record).length, + ).toBe(1); + expect(result?.externalSettings?.conditions).toBeDefined(); + expect( + (result?.externalSettings?.conditions as Record) + .length, + ).toBe(1); }); }); diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 2822c28c7..0747b450c 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -284,19 +284,43 @@ export class UsersController { async patchSettings( @Req() request: Request, @Param("id") id: string, - updateUserSettingsDto: PartialUpdateUserSettingsDto, + @Body() updateUserSettingsDto: PartialUpdateUserSettingsDto, ): Promise { await this.checkUserAuthorization( request, [Action.UserUpdateAny, Action.UserUpdateOwn], id, ); - return this.usersService.findOneAndUpdateUserSettings( + return this.usersService.findOneAndPatchUserSettings( id, updateUserSettingsDto, ); } + @UseGuards(AuthenticatedPoliciesGuard) + @CheckPolicies( + "users", + (ability: AppAbility) => + ability.can(Action.UserUpdateOwn, User) || + ability.can(Action.UserUpdateAny, User), + ) + @Patch("/:id/settings/external") + async patchExternalSettings( + @Req() request: Request, + @Param("id") id: string, + @Body() externalSettings: Record, + ): Promise { + await this.checkUserAuthorization( + request, + [Action.UserUpdateAny, Action.UserUpdateOwn], + id, + ); + return this.usersService.findOneAndPatchUserExternalSettings( + id, + externalSettings, + ); + } + @UseGuards(AuthenticatedPoliciesGuard) @CheckPolicies( "users", diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 4c479c1b3..c94394aa0 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -296,6 +296,34 @@ export class UsersService implements OnModuleInit { return result; } + async findOneAndPatchUserSettings( + userId: string, + updateUserSettingsDto: UpdateUserSettingsDto | PartialUpdateUserSettingsDto, + ): Promise { + const result = await this.userSettingsModel + .findOneAndUpdate( + { userId }, + { $set: updateUserSettingsDto }, + { new: true }, + ) + .exec(); + return result; + } + + async findOneAndPatchUserExternalSettings( + userId: string, + externalSettings: Record, + ): Promise { + const updateQuery: Record = {}; + + for (const [key, value] of Object.entries(externalSettings)) { + updateQuery[`externalSettings.${key}`] = value; + } + const result = await this.userSettingsModel + .findOneAndUpdate({ userId }, { $set: updateQuery }, { new: true }) + .exec(); + return result; + } async findOneAndDeleteUserSettings(userId: string): Promise { return this.userSettingsModel.findOneAndDelete({ userId }).exec(); } From f891727644a62db1677900eb279e63e66b03ade5 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Wed, 18 Sep 2024 14:59:35 +0200 Subject: [PATCH 12/14] fix unit test for users controller --- src/users/users.controller.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/users/users.controller.spec.ts b/src/users/users.controller.spec.ts index 40ac85311..5c111e716 100644 --- a/src/users/users.controller.spec.ts +++ b/src/users/users.controller.spec.ts @@ -75,7 +75,7 @@ describe("UsersController", () => { const result = await controller.getSettings(mockRequest as Request, userId); // Assert - expect(result?.externalSettings).toEqual(mockUserSettings); + expect(result).toEqual(mockUserSettings); expect(result?.externalSettings?.filters).toBeDefined(); expect( (result?.externalSettings?.filters as Record).length, @@ -108,7 +108,7 @@ describe("UsersController", () => { const expectedResponse: UserSettings = { ...updatedSettings, _id: userId, - userId: userId, // Ensure all required properties are included + userId: userId, datasetCount: updatedSettings.datasetCount, jobCount: updatedSettings.jobCount, externalSettings: updatedSettings.externalSettings, @@ -124,7 +124,7 @@ describe("UsersController", () => { updatedSettings, ); - expect(result?.externalSettings).toEqual(updatedSettings); + expect(result).toEqual(expectedResponse); expect(result?.externalSettings?.filters).toBeDefined(); expect( (result?.externalSettings?.filters as Record).length, From 678c890864858b22d0391b40e5fbfaae0d0b7683 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Wed, 18 Sep 2024 15:13:46 +0200 Subject: [PATCH 13/14] changed login endpoint from userService to authService --- test/LoginUtils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/LoginUtils.js b/test/LoginUtils.js index 0a1cb950e..d5694d7ff 100644 --- a/test/LoginUtils.js +++ b/test/LoginUtils.js @@ -4,7 +4,7 @@ var request = require("supertest"); exports.getToken = function (appUrl, user) { return new Promise((resolve, reject) => { request(appUrl) - .post("/api/v3/Users/Login?include=user") + .post("/api/v3/auth/Login?include=user") .send(user) .set("Accept", "application/json") .end((err, res) => { @@ -20,17 +20,17 @@ exports.getToken = function (appUrl, user) { exports.getIdAndToken = function (appUrl, user) { return new Promise((resolve, reject) => { request(appUrl) - .post("/api/v3/Users/Login?include=user") + .post("/api/v3/auth/Login?include=user") .send(user) .set("Accept", "application/json") .end((err, res) => { if (err) { reject(err); } else { - resolve({userId:res.body.userId, token:res.body.id}); + resolve({ userId: res.body.userId, token: res.body.id }); } }); - }); + }); }; exports.getTokenAD = function (appUrl, user, cb) { From e97ec50641b73110019883084973cf62ab138537 Mon Sep 17 00:00:00 2001 From: junjiequan Date: Wed, 18 Sep 2024 16:12:11 +0200 Subject: [PATCH 14/14] added test for user settings patch --- src/users/schemas/user-settings.schema.ts | 2 +- test/TestData.js | 31 +++++++++++++++ test/Users.js | 46 +++++++++++++++++++---- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/users/schemas/user-settings.schema.ts b/src/users/schemas/user-settings.schema.ts index 9aa9a9f08..e5d524936 100644 --- a/src/users/schemas/user-settings.schema.ts +++ b/src/users/schemas/user-settings.schema.ts @@ -50,7 +50,7 @@ export class UserSettings { "A customizable object for storing the user's external settings, which can contain various nested properties and configurations.", }) @Prop({ type: Object, default: {}, required: false }) - externalSettings?: Record; + externalSettings: Record; } export const UserSettingsSchema = SchemaFactory.createForClass(UserSettings); diff --git a/test/TestData.js b/test/TestData.js index 538f657bf..2f5f0a22e 100644 --- a/test/TestData.js +++ b/test/TestData.js @@ -43,6 +43,37 @@ const TestData = { accessGroups: [], }, + userSettingsCorrect: { + datasetCount: 10, + jobCount: 25, + externalSettings: { + columns: [ + { + name: "select", + order: 0, + type: "standard", + enabled: true, + }, + ], + filters: [ + { + LocationFilter: true, + }, + ], + conditions: [ + { + condition: { + lhs: "test", + relation: "GREATER_THAN", + rhs: 1, + unit: "", + }, + enabled: true, + }, + ], + }, + }, + ProposalCorrectComplete: { proposalId: "20170267", pi_email: "pi@uni.edu", diff --git a/test/Users.js b/test/Users.js index c4f8d7f66..a6e81ec91 100644 --- a/test/Users.js +++ b/test/Users.js @@ -10,7 +10,7 @@ let userIdUser1 = null, describe("2350: Users: Login with functional accounts", () => { it("0010: Admin ingestor login fails with incorrect credentials", async () => { return request(appUrl) - .post("/api/v3/Users/Login?include=user") + .post("/api/v3/auth/Login?include=user") .send({ username: "adminIngestor", password: TestData.Accounts["user1"]["password"], @@ -23,7 +23,7 @@ describe("2350: Users: Login with functional accounts", () => { it("0020: Login should succeed with correct credentials", async () => { return request(appUrl) - .post("/api/v3/Users/Login?include=user") + .post("/api/v3/auth/Login?include=user") .send({ username: "adminIngestor", password: TestData.Accounts["adminIngestor"]["password"], @@ -38,18 +38,19 @@ describe("2350: Users: Login with functional accounts", () => { }); describe("2360: Users settings", () => { - beforeEach(async() => { + beforeEach(async () => { const loginResponseUser1 = await utils.getIdAndToken(appUrl, { username: "user1", password: TestData.Accounts["user1"]["password"], }); userIdUser1 = loginResponseUser1.userId; - accessTokenUser1 = loginResponseUser1.token; + accessTokenUser1 = loginResponseUser1.token; }); - it("0010: Update users settings with valid value should success ", async () => { + it("0020: Update users settings with valid value should success ", async () => { return request(appUrl) .put(`/api/v3/Users/${userIdUser1}/settings`) + .send(TestData.userSettingsCorrect) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser1}` }) .expect(TestData.SuccessfulPatchStatusCode) @@ -58,8 +59,39 @@ describe("2360: Users settings", () => { res.body.should.have.property("userId", userIdUser1); res.body.should.have.property("datasetCount"); res.body.should.have.property("jobCount"); - res.body.should.have.property("filters"); - res.body.should.have.property("conditions"); + res.body.should.have.property("externalSettings"); + }); + }); + + it("0030: Patch users settings with valid value should success ", async () => { + return request(appUrl) + .patch(`/api/v3/Users/${userIdUser1}/settings`) + .send(TestData.userSettingsCorrect) + .set("Accept", "application/json") + .set({ Authorization: `Bearer ${accessTokenUser1}` }) + .expect(TestData.SuccessfulPatchStatusCode) + .expect("Content-Type", /json/) + .then((res) => { + res.body.should.have.property("userId", userIdUser1); + res.body.should.have.property("datasetCount"); + res.body.should.have.property("jobCount"); + res.body.should.have.property("externalSettings"); + }); + }); + + it("0040: Patch users external settings with valid value should success ", async () => { + return request(appUrl) + .patch(`/api/v3/Users/${userIdUser1}/settings/external`) + .send(TestData.userSettingsCorrect.externalSettings) + .set("Accept", "application/json") + .set({ Authorization: `Bearer ${accessTokenUser1}` }) + .expect(TestData.SuccessfulPatchStatusCode) + .expect("Content-Type", /json/) + .then((res) => { + res.body.should.have.property("userId", userIdUser1); + res.body.should.have.property("datasetCount"); + res.body.should.have.property("jobCount"); + res.body.should.have.property("externalSettings"); }); }); });