From 55190f177a3ec99119b1ca37fa6d8297f89aa3b5 Mon Sep 17 00:00:00 2001 From: Yoganandan Pandiyan Date: Mon, 13 Oct 2025 21:07:58 +0200 Subject: [PATCH 1/5] feat: upsert user by oidc --- .../0201_AddUniqueConstraintToOidcSub.sql | 12 ++ .../backend/src/datasources/UserDataSource.ts | 2 +- .../src/datasources/mockups/UserDataSource.ts | 44 ++++++- .../datasources/postgres/UserDataSource.ts | 124 ++++++++++-------- .../src/datasources/postgres/records.ts | 5 +- .../datasources/stfc/StfcUserDataSource.ts | 3 +- apps/backend/src/models/User.ts | 3 +- .../src/mutations/UserMutations.spec.ts | 53 +++++++- apps/backend/src/mutations/UserMutations.ts | 84 ++++++++++++ apps/backend/src/queries/UserQueries.ts | 6 +- .../resolvers/mutations/UpsertUserMutation.ts | 56 ++++++++ .../src/resolvers/types/BasicUserDetails.ts | 3 + .../statusActionEngine/statusActionUtils.ts | 1 + .../fragment.basicUserInformation.graphql | 1 + .../graphql/user/upsertUserByOidcSub.graphql | 33 +++++ 15 files changed, 368 insertions(+), 62 deletions(-) create mode 100644 apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql create mode 100644 apps/backend/src/resolvers/mutations/UpsertUserMutation.ts create mode 100644 apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql diff --git a/apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql b/apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql new file mode 100644 index 0000000000..ecccf939aa --- /dev/null +++ b/apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql @@ -0,0 +1,12 @@ + +DO +$$ +BEGIN + IF register_patch('AddUniqueConstraintToOidcSub.sql', 'yoganandanpandiyan', 'Unique constraint', '2025-10-13') THEN + BEGIN + ALTER TABLE users ADD CONSTRAINT unique_oidc_sub UNIQUE (oidc_sub); + END; + END IF; +END; +$$ +LANGUAGE plpgsql; diff --git a/apps/backend/src/datasources/UserDataSource.ts b/apps/backend/src/datasources/UserDataSource.ts index f6c450a566..94c89c095d 100644 --- a/apps/backend/src/datasources/UserDataSource.ts +++ b/apps/backend/src/datasources/UserDataSource.ts @@ -68,7 +68,7 @@ export interface UserDataSource { oauth_issuer: string, gender: string, birthdate: Date, - institution: number, + institution: number | undefined, department: string, position: string, email: string, diff --git a/apps/backend/src/datasources/mockups/UserDataSource.ts b/apps/backend/src/datasources/mockups/UserDataSource.ts index 71cbcf9a07..d47ecc6ed3 100644 --- a/apps/backend/src/datasources/mockups/UserDataSource.ts +++ b/apps/backend/src/datasources/mockups/UserDataSource.ts @@ -28,6 +28,7 @@ export const basicDummyUser = new BasicUserDetails( false, 'test@email.com', '', + '', '' ); @@ -43,6 +44,7 @@ export const basicDummyUserNotOnProposal = new BasicUserDetails( false, 'test@email.com', '', + '', '' ); @@ -300,6 +302,7 @@ export class UserDataSourceMock implements UserDataSource { false, 'test@email.com', '', + '', '' ); } @@ -483,8 +486,45 @@ export class UserDataSourceMock implements UserDataSource { return true; } - async create(firstname: string, lastname: string) { - return dummyUser; + async create( + user_title: string | undefined, + firstname: string, + lastname: string, + username: string, + preferredname: string | undefined, + oidc_sub: string, + oauth_refreshtoken: string, + oauth_issuer: string, + gender: string, + birthdate: Date, + institution: number | undefined, + department: string, + position: string, + email: string, + telephone: string + ) { + return new User( + 10, + user_title || 'unspecified', + firstname, + lastname, + username, + preferredname || '', + oidc_sub, + oauth_refreshtoken, + oauth_issuer, + gender || 'unspecified', + birthdate, + institution || 1, + 'Test institution', + department, + position, + email, + telephone, + false, + new Date().toISOString(), + new Date().toISOString() + ); } async ensureDummyUserExists(userId: number): Promise { diff --git a/apps/backend/src/datasources/postgres/UserDataSource.ts b/apps/backend/src/datasources/postgres/UserDataSource.ts index 96395687bf..428d4475a8 100644 --- a/apps/backend/src/datasources/postgres/UserDataSource.ts +++ b/apps/backend/src/datasources/postgres/UserDataSource.ts @@ -90,31 +90,40 @@ export default class PostgresUserDataSource implements UserDataSource { oidcSub, oauthRefreshToken, oauthIssuer, + username, } = user; - const [userRecord]: UserRecord[] = await database - .update({ - firstname, - user_title, - lastname, - preferredname, - gender, - birthdate, - institution_id: institutionId, - department, - position, - email, - telephone, - placeholder, - oidc_sub: oidcSub, - oauth_refresh_token: oauthRefreshToken, - oauth_issuer: oauthIssuer, - }) - .from('users') - .where('user_id', user.id) - .returning(['*']); - - return createUserObject(userRecord); + try { + const [userRecord]: UserRecord[] = await database + .update({ + firstname, + user_title, + lastname, + preferredname, + gender, + birthdate, + institution_id: institutionId, + department, + position, + email, + telephone, + placeholder, + oidc_sub: oidcSub, + oauth_refresh_token: oauthRefreshToken, + oauth_issuer: oauthIssuer, + username, + }) + .from('users') + .where('user_id', user.id) + .returning(['*']); + + return createUserObject(userRecord); + } catch (error) { + if (error instanceof Error && 'code' in error && error.code === '23505') { + throw new GraphQLError('User already exists'); + } + throw new GraphQLError('Could not create user. Check your Inputs.'); + } } async updateUserByOidcSub( args: UpdateUserByOidcSubArgs @@ -135,34 +144,40 @@ export default class PostgresUserDataSource implements UserDataSource { oauthRefreshToken, oauthIssuer, } = args; - - const [userRecord]: UserRecord[] = await database - .update({ - firstname, - user_title, - lastname, - preferredname, - gender, - birthdate, - institution_id: institutionId, - department, - position, - email, - telephone, - placeholder, - oauth_refresh_token: oauthRefreshToken, - oauth_issuer: oauthIssuer, - updated_at: new Date(), - }) - .from('users') - .where('oidc_sub', args.oidcSub) - .returning(['*']); - - if (!userRecord) { - return null; + try { + const [userRecord]: UserRecord[] = await database + .update({ + firstname, + user_title, + lastname, + preferredname, + gender, + birthdate, + institution_id: institutionId, + department, + position, + email, + telephone, + placeholder, + oauth_refresh_token: oauthRefreshToken, + oauth_issuer: oauthIssuer, + updated_at: new Date(), + }) + .from('users') + .where('oidc_sub', args.oidcSub) + .returning(['*']); + + if (!userRecord) { + return null; + } + + return createUserObject(userRecord); + } catch (error) { + if (error instanceof Error && 'code' in error && error.code === '23505') { + throw new GraphQLError('User already exists'); + } + throw new GraphQLError('Could not create user. Check your Inputs.'); } - - return createUserObject(userRecord); } async createInviteUser(args: CreateUserByEmailInviteArgs): Promise { @@ -363,7 +378,6 @@ export default class PostgresUserDataSource implements UserDataSource { .then((user: UserRecord) => (!user ? null : createUserObject(user))); } - // NOTE: This is used in the OAuthAuthorization only where we upsert users returned from Auth server. async create( user_title: string | undefined, firstname: string, @@ -375,7 +389,7 @@ export default class PostgresUserDataSource implements UserDataSource { oauth_issuer: string, gender: string, birthdate: Date, - institution_id: number, + institution_id: number | undefined, department: string, position: string, email: string, @@ -407,6 +421,12 @@ export default class PostgresUserDataSource implements UserDataSource { } return createUserObject(user[0]); + }) + .catch((error) => { + if (error.code === '23505') { + throw new GraphQLError('User already exists'); + } + throw new GraphQLError('Could not create user. Check your Inputs.'); }); } diff --git a/apps/backend/src/datasources/postgres/records.ts b/apps/backend/src/datasources/postgres/records.ts index 0137fdb483..9559c41fe9 100644 --- a/apps/backend/src/datasources/postgres/records.ts +++ b/apps/backend/src/datasources/postgres/records.ts @@ -249,7 +249,7 @@ export interface UserRecord { readonly lastname: string; readonly username: string; readonly preferredname: string; - readonly oidc_sub: string | null; + readonly oidc_sub: string; readonly oauth_refresh_token: string | null; readonly oauth_issuer: string | null; readonly gender: string; @@ -996,7 +996,8 @@ export const createBasicUserObject = ( user.placeholder, user.email, user.country, - user.user_title + user.user_title, + user.oidc_sub ); }; diff --git a/apps/backend/src/datasources/stfc/StfcUserDataSource.ts b/apps/backend/src/datasources/stfc/StfcUserDataSource.ts index 5fcf5c6684..aab3d850e2 100644 --- a/apps/backend/src/datasources/stfc/StfcUserDataSource.ts +++ b/apps/backend/src/datasources/stfc/StfcUserDataSource.ts @@ -107,7 +107,8 @@ export function toEssBasicUserDetails( false, stfcUser.email ?? '', stfcUser.country ?? '', - stfcUser.title ?? '' + stfcUser.title ?? '', + '' ); } diff --git a/apps/backend/src/models/User.ts b/apps/backend/src/models/User.ts index c1cb4e38b5..66d03e68f5 100644 --- a/apps/backend/src/models/User.ts +++ b/apps/backend/src/models/User.ts @@ -91,7 +91,8 @@ export class BasicUserDetails { public placeholder: boolean, public email: string, public country: string, - public title: string + public title: string, + public oidc_sub: string ) {} } diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index 24d0fd2452..309f59b589 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -12,7 +12,7 @@ import { } from '../datasources/mockups/UserDataSource'; import { EmailInviteResponse } from '../models/EmailInviteResponse'; import { isRejection, Rejection } from '../models/Rejection'; -import { AuthJwtPayload, UserRole } from '../models/User'; +import { AuthJwtPayload, User, UserRole } from '../models/User'; import { verifyToken } from '../utils/jwt'; import UserMutations from './UserMutations'; @@ -366,3 +366,54 @@ describe('updateUserByOidcSub', () => { expect((result as typeof dummyUser).department).toBe(dummyUser.department); }); }); + +describe('upsertUserByOidcSub', () => { + test('A user can be created if OIDC sub does not exist', async () => { + const newOidcSub = 'new-unique-oidc-sub'; + const result = await userMutations.upsertUserByOidcSub( + dummyUserOfficerWithRole, + { + oidcSub: newOidcSub, + firstName: 'New', + lastName: 'User', + email: 'new.user@example.com', + userTitle: null, + username: null, + preferredName: null, + gender: null, + birthDate: null, + institutionRoRId: '', + department: null, + position: '', + telephone: null, + } + ); + // Check if the result has the oidcsub + expect(isRejection(result)).toBe(false); + expect((result as User).oidcSub).toBe(dummyUser.oidcSub); + }); + test('A user will be updated if OIDC sub exists', async () => { + const result = await userMutations.upsertUserByOidcSub( + dummyUserOfficerWithRole, + { + oidcSub: dummyUser.oidcSub as string, + firstName: 'UpsertedJane', + lastName: 'UpsertedDoe', + email: 'upserted.jane.doe@example.com', + userTitle: null, + username: null, + preferredName: null, + gender: null, + birthDate: null, + institutionRoRId: '', + department: null, + position: '', + telephone: null, + } + ); + + // Check if the result has the oidcsub + expect(isRejection(result)).toBe(false); + expect((result as User).oidcSub).toBe(dummyUser.oidcSub); + }); +}); diff --git a/apps/backend/src/mutations/UserMutations.ts b/apps/backend/src/mutations/UserMutations.ts index d621a22c72..2c94e16eac 100644 --- a/apps/backend/src/mutations/UserMutations.ts +++ b/apps/backend/src/mutations/UserMutations.ts @@ -8,11 +8,13 @@ import { updateUserValidationBackendSchema, } from '@user-office-software/duo-validation'; import * as bcrypt from 'bcryptjs'; +import { DateTime } from 'luxon'; import { inject, injectable } from 'tsyringe'; import { Args } from 'type-graphql'; import { UserAuthorization } from '../auth/UserAuthorization'; import { Tokens } from '../config/Tokens'; +import { AdminDataSource } from '../datasources/AdminDataSource'; import { RedeemCodesDataSource } from '../datasources/RedeemCodesDataSource'; import { UserDataSource } from '../datasources/UserDataSource'; import { Authorized, EventBus, ValidateArgs } from '../decorators'; @@ -34,6 +36,7 @@ import { UpdateUserByOidcSubArgs, UpdateUserByIdArgs, } from '../resolvers/mutations/UpdateUserMutation'; +import { UpsertUserByOidcSubArgs } from '../resolvers/mutations/UpsertUserMutation'; import { signToken, verifyToken } from '../utils/jwt'; import { ApolloServerErrorCodeExtended } from '../utils/utilTypes'; @@ -42,6 +45,7 @@ export default class UserMutations { constructor( @inject(Tokens.UserAuthorization) private userAuth: UserAuthorization, @inject(Tokens.UserDataSource) private dataSource: UserDataSource, + @inject(Tokens.AdminDataSource) private adminDataSource: AdminDataSource, @inject(Tokens.RedeemCodesDataSource) private redeemCodeDataSource: RedeemCodesDataSource ) {} @@ -507,4 +511,84 @@ export default class UserMutations { ): Promise { return this.dataSource.setUserNotPlaceholder(id); } + + @Authorized([Roles.USER_OFFICER]) + async upsertUserByOidcSub( + agent: UserWithRole | null, + args: UpsertUserByOidcSubArgs + ) { + const { + userTitle, + firstName, + lastName, + username, + preferredName, + oidcSub, + gender, + birthDate, + institutionRoRId, + department, + position, + email, + telephone, + } = args; + + const userWithOAuthSubMatch = await this.dataSource.getByOIDCSub(oidcSub); + + let formattedBirthDate: DateTime | null = null; + formattedBirthDate = birthDate ? DateTime.fromISO(birthDate) : null; + if (formattedBirthDate && !formattedBirthDate.isValid) { + return rejection('Invalid birth date format', { birthDate, args }); + } + + // Fetch Institution ID from ROR ID + const institution = + await this.adminDataSource.getInstitutionByRorId(institutionRoRId); + + if (userWithOAuthSubMatch) { + const updatedUser = await this.dataSource.update({ + ...userWithOAuthSubMatch, + birthdate: formattedBirthDate?.toJSDate(), + department: department ?? undefined, + email, + firstname: firstName, + username: username ?? undefined, + gender: gender ?? undefined, + lastname: lastName, + oidcSub: oidcSub, + institutionId: institution?.id, + position: position, + preferredname: preferredName ?? undefined, + telephone: telephone ?? undefined, + user_title: userTitle ?? undefined, + }); + + return updatedUser; + } else { + const newUser = await this.dataSource.create( + userTitle ?? '', + firstName, + lastName, + username ?? '', + preferredName ?? '', + oidcSub, + '', + '', + gender ?? '', + formattedBirthDate?.toJSDate() ?? new Date(), + institution?.id ?? undefined, + department ?? '', + position, + email, + telephone ?? '' + ); + + await this.dataSource.addUserRole({ + userID: newUser.id, + roleID: UserRole.USER, + }); + + return newUser; + } + } } diff --git a/apps/backend/src/queries/UserQueries.ts b/apps/backend/src/queries/UserQueries.ts index e6825399c6..165d97ff42 100644 --- a/apps/backend/src/queries/UserQueries.ts +++ b/apps/backend/src/queries/UserQueries.ts @@ -73,7 +73,8 @@ export default class UserQueries { user.placeholder, user.email, user.country, - user.title + user.title, + user.oidc_sub ); } else { return null; @@ -107,7 +108,8 @@ export default class UserQueries { user.placeholder, user.email, user.country, - user.title + user.title, + user.oidc_sub ); } diff --git a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts new file mode 100644 index 0000000000..2fea916a23 --- /dev/null +++ b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts @@ -0,0 +1,56 @@ +import { Args, ArgsType, Ctx, Field, Mutation, Resolver } from 'type-graphql'; + +import { ResolverContext } from '../../context'; +import { User } from '../types/User'; +@ArgsType() +export class UpsertUserByOidcSubArgs { + @Field(() => String, { nullable: true }) + public userTitle: string | null; + + @Field(() => String) + public firstName: string; + + @Field(() => String) + public lastName: string; + + @Field(() => String, { nullable: true }) + public username: string | null; + + @Field(() => String, { nullable: true }) + public preferredName: string | null; + + @Field(() => String) + public oidcSub: string; + + @Field(() => String, { nullable: true }) + public gender: string | null; + + @Field(() => String, { nullable: true }) + public birthDate: string | null; + + @Field(() => String) + public institutionRoRId: string; + + @Field(() => String, { nullable: true }) + public department: string | null; + + @Field(() => String) + public position: string; + + @Field(() => String) + public email: string; + + @Field(() => String, { nullable: true }) + public telephone: string | null; +} + +@Resolver() +export class UpsertUserByOidcSubMutation { + @Mutation(() => User) + upsertUserByOidcSub( + @Args() input: UpsertUserByOidcSubArgs, + @Ctx() context: ResolverContext + ) { + return context.mutations.user.upsertUserByOidcSub(context.user, input); + } +} diff --git a/apps/backend/src/resolvers/types/BasicUserDetails.ts b/apps/backend/src/resolvers/types/BasicUserDetails.ts index b05aed8433..321652b203 100644 --- a/apps/backend/src/resolvers/types/BasicUserDetails.ts +++ b/apps/backend/src/resolvers/types/BasicUserDetails.ts @@ -44,6 +44,9 @@ export class BasicUserDetails implements Partial { @Field({ nullable: true }) public country: string; + + @Field(() => String, { nullable: true }) + public oidc_sub: string | undefined; } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/apps/backend/src/statusActionEngine/statusActionUtils.ts b/apps/backend/src/statusActionEngine/statusActionUtils.ts index fdf1467741..1fa7af8e38 100644 --- a/apps/backend/src/statusActionEngine/statusActionUtils.ts +++ b/apps/backend/src/statusActionEngine/statusActionUtils.ts @@ -463,6 +463,7 @@ export const getOtherAndFormatOutputForEmailSending = async ( true, otherEmail, '', + '', '' ); diff --git a/apps/frontend/src/graphql/user/fragment.basicUserInformation.graphql b/apps/frontend/src/graphql/user/fragment.basicUserInformation.graphql index 48498dbe86..d0fb90376f 100644 --- a/apps/frontend/src/graphql/user/fragment.basicUserInformation.graphql +++ b/apps/frontend/src/graphql/user/fragment.basicUserInformation.graphql @@ -10,4 +10,5 @@ fragment basicUserDetails on BasicUserDetails { placeholder email country + oidc_sub } diff --git a/apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql b/apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql new file mode 100644 index 0000000000..7efc0f09a5 --- /dev/null +++ b/apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql @@ -0,0 +1,33 @@ +mutation upsertUserByOidcSub( + $userTitle: String + $firstName: String! + $lastName: String! + $username: String + $preferredName: String + $oidcSub: String! + $gender: String + $birthDate: String + $institutionRoRId: String! + $department: String + $position: String! + $email: String! + $telephone: String +) { + upsertUserByOidcSub( + userTitle: $userTitle + firstName: $firstName + lastName: $lastName + username: $username + preferredName: $preferredName + oidcSub: $oidcSub + gender: $gender + birthDate: $birthDate + institutionRoRId: $institutionRoRId + department: $department + position: $position + email: $email + telephone: $telephone + ) { + id + } +} From 68d8a64b459d758726a964c900307531768ac69b Mon Sep 17 00:00:00 2001 From: Yoganandan Pandiyan Date: Wed, 15 Oct 2025 16:44:42 +0200 Subject: [PATCH 2/5] Changed error message and optimised error handling --- .../src/datasources/postgres/UserDataSource.ts | 12 ++++++------ apps/backend/src/datasources/postgres/database.ts | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/apps/backend/src/datasources/postgres/UserDataSource.ts b/apps/backend/src/datasources/postgres/UserDataSource.ts index 4f74bebf52..14b948fbc6 100644 --- a/apps/backend/src/datasources/postgres/UserDataSource.ts +++ b/apps/backend/src/datasources/postgres/UserDataSource.ts @@ -20,7 +20,7 @@ import { } from '../../resolvers/mutations/UpdateUserMutation'; import { UsersArgs } from '../../resolvers/queries/UsersQuery'; import { UserDataSource } from '../UserDataSource'; -import database from './database'; +import database, { isUniqueConstraintError } from './database'; import { CountryRecord, InstitutionRecord, @@ -119,10 +119,10 @@ export default class PostgresUserDataSource implements UserDataSource { return createUserObject(userRecord); } catch (error) { - if (error instanceof Error && 'code' in error && error.code === '23505') { + if (isUniqueConstraintError(error)) { throw new GraphQLError('User already exists'); } - throw new GraphQLError('Could not create user. Check your Inputs.'); + throw new GraphQLError('Could not update user. Check your Inputs.'); } } async updateUserByOidcSub( @@ -173,7 +173,7 @@ export default class PostgresUserDataSource implements UserDataSource { return createUserObject(userRecord); } catch (error) { - if (error instanceof Error && 'code' in error && error.code === '23505') { + if (isUniqueConstraintError(error)) { throw new GraphQLError('User already exists'); } throw new GraphQLError('Could not create user. Check your Inputs.'); @@ -422,10 +422,10 @@ export default class PostgresUserDataSource implements UserDataSource { return createUserObject(user[0]); }) .catch((error) => { - if (error.code === '23505') { + if (isUniqueConstraintError(error)) { throw new GraphQLError('User already exists'); } - throw new GraphQLError('Could not create user. Check your Inputs.'); + throw new GraphQLError('Could not update user. Check your Inputs.'); }); } diff --git a/apps/backend/src/datasources/postgres/database.ts b/apps/backend/src/datasources/postgres/database.ts index 1525b1bafc..52cf7d8754 100644 --- a/apps/backend/src/datasources/postgres/database.ts +++ b/apps/backend/src/datasources/postgres/database.ts @@ -38,4 +38,8 @@ if (process.env.DATABASE_LOG_QUERIES === '1') { }); } +export function isUniqueConstraintError(error: any) { + return 'code' in error && error.code === '23505'; +} + export default db; From 4b3004fe8f4af5990ff220d95d2bd6b7787b0c56 Mon Sep 17 00:00:00 2001 From: Yoganandan Pandiyan Date: Fri, 17 Oct 2025 14:57:01 +0200 Subject: [PATCH 3/5] converted empty values to null for oidc_sub and appended unique suffix to the non empty duplicate values to facilitate smooth process of enforing uniqueness to the oidc_sub --- .../0201_AddUniqueConstraintToOidcSub.sql | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql b/apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql index ecccf939aa..bc12b5d48d 100644 --- a/apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql +++ b/apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql @@ -2,11 +2,34 @@ DO $$ BEGIN - IF register_patch('AddUniqueConstraintToOidcSub.sql', 'yoganandanpandiyan', 'Unique constraint', '2025-10-13') THEN - BEGIN - ALTER TABLE users ADD CONSTRAINT unique_oidc_sub UNIQUE (oidc_sub); - END; - END IF; + IF register_patch('AddUniqueConstraintToOidcSub.sql', 'yoganandanpandiyan', 'Unique constraint', '2025-10-13') THEN + BEGIN + -- Clean up empty string values in oidc_sub column by setting them to NULL + -- This prevents empty strings from conflicting with the unique constraint + UPDATE users + SET oidc_sub = NULL + WHERE oidc_sub = ''; + + -- Handle duplicate oidc_sub values by appending a unique suffix. ex., '_1', '_2', etc. + WITH duplicates AS ( + SELECT + user_id, + oidc_sub, + ROW_NUMBER() OVER (PARTITION BY oidc_sub ORDER BY user_id) AS rn + FROM users + WHERE oidc_sub IS NOT NULL + ) + UPDATE users u + SET oidc_sub = u.oidc_sub || '_' || d.rn + FROM duplicates d + WHERE u.user_id = d.user_id + AND d.rn > 1; + + -- Add unique constraint to prevent duplicate oidc_sub values in the future + ALTER TABLE users + ADD CONSTRAINT unique_oidc_sub UNIQUE (oidc_sub); + END; + END IF; END; $$ LANGUAGE plpgsql; From 267b79e80ee0c22e9d9afe8f1c710ec63afe7909 Mon Sep 17 00:00:00 2001 From: Yoganandan Pandiyan Date: Sun, 19 Oct 2025 20:28:25 +0200 Subject: [PATCH 4/5] fix: aligning the type of users based on the original db schema --- apps/backend/src/auth/OAuthAuthorization.ts | 26 ++++++---- .../src/datasources/postgres/records.ts | 16 +++---- .../eventHandlers/email/eliEmailHandler.ts | 39 ++++++++------- .../eventHandlers/email/essEmailHandler.ts | 48 +++++++++++++------ apps/backend/src/models/User.ts | 28 +++++------ apps/backend/src/queries/UserQueries.ts | 4 +- .../resolvers/mutations/UpdateUserMutation.ts | 12 ++--- .../src/resolvers/types/BasicUserDetails.ts | 20 ++++---- apps/backend/src/resolvers/types/User.ts | 22 ++++----- apps/backend/src/resolvers/types/UserJWT.ts | 16 +++---- .../statusActionEngine/statusActionUtils.ts | 30 ++++++------ 11 files changed, 148 insertions(+), 113 deletions(-) diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index 888c7a9712..fe77342cd0 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -179,16 +179,26 @@ export class OAuthAuthorization extends UserAuthorization { '' ); - const roleID = this.getUserRole(newUser); + if (newUser.email) { + const roleID = this.getUserRole({ + id: newUser.id, + email: newUser.email, + }); - await this.userDataSource.addUserRole({ - userID: newUser.id, - roleID, - }); + await this.userDataSource.addUserRole({ + userID: newUser.id, + roleID, + }); - if (roleID === UserRole.USER_OFFICER) { - logger.logInfo('Initial User Officer created', { - email: newUser.email, + if (roleID === UserRole.USER_OFFICER) { + logger.logInfo('Initial User Officer created', { + email: newUser.email, + userID: newUser.id, + }); + } + } else { + logger.logInfo('User created without email, cannot assign role', { + userID: newUser.id, }); } diff --git a/apps/backend/src/datasources/postgres/records.ts b/apps/backend/src/datasources/postgres/records.ts index caa9095701..613e798731 100644 --- a/apps/backend/src/datasources/postgres/records.ts +++ b/apps/backend/src/datasources/postgres/records.ts @@ -245,26 +245,26 @@ export interface TemplateRecord { export interface UserRecord { readonly user_id: number; - readonly user_title: string; + readonly user_title: string | null; readonly firstname: string; readonly lastname: string; - readonly username: string; - readonly preferredname: string; - readonly oidc_sub: string; + readonly username: string | null; + readonly preferredname: string | null; + readonly oidc_sub: string | null; readonly oauth_refresh_token: string | null; readonly oauth_issuer: string | null; readonly gender: string; readonly birthdate: Date; readonly department: string; readonly position: string; - readonly email: string; + readonly email: string | null; readonly telephone: string; readonly created_at: Date; readonly updated_at: Date; readonly full_count: number; - readonly institution_id: number; - readonly institution: string; - readonly placeholder: boolean; + readonly institution_id: number | null; + readonly institution: string | null; + readonly placeholder: boolean | null; } export interface VisitRegistrationRecord { diff --git a/apps/backend/src/eventHandlers/email/eliEmailHandler.ts b/apps/backend/src/eventHandlers/email/eliEmailHandler.ts index f5f93dd1a8..99749adfe4 100644 --- a/apps/backend/src/eventHandlers/email/eliEmailHandler.ts +++ b/apps/backend/src/eventHandlers/email/eliEmailHandler.ts @@ -56,7 +56,7 @@ export async function eliEmailHandler(event: ApplicationEvent) { event.emailinviteresponse.inviterId ); - if (!user || !inviter) { + if (!user || !user.email || !inviter) { logger.logError('Failed email invite', { user, inviter, event }); return; @@ -146,7 +146,7 @@ export async function eliEmailHandler(event: ApplicationEvent) { const call = await callDataSource.getCall(event.proposal.callId); - if (!principalInvestigator || !call) { + if (!principalInvestigator || !principalInvestigator.email || !call) { return; } @@ -189,10 +189,25 @@ export async function eliEmailHandler(event: ApplicationEvent) { const participants = await userDataSource.getProposalUsersFull( event.proposal.primaryKey ); - if (!principalInvestigator) { + if (!principalInvestigator || !principalInvestigator.email) { return; } + const recipients: ( + | { address: string } + | { address: { email: string; header_to: string } } + )[] = [{ address: principalInvestigator.email }]; + for (const partipant of participants) { + if (partipant.email) { + recipients.push({ + address: { + email: partipant.email, + header_to: principalInvestigator.email, + }, + }); + } + } + const options: EmailSettings = { content: { template_id: 'proposal-submitted', @@ -207,17 +222,7 @@ export async function eliEmailHandler(event: ApplicationEvent) { ), call: '', }, - recipients: [ - { address: principalInvestigator.email }, - ...participants.map((partipant) => { - return { - address: { - email: partipant.email, - header_to: principalInvestigator.email, - }, - }; - }), - ], + recipients, }; mailService @@ -242,7 +247,7 @@ export async function eliEmailHandler(event: ApplicationEvent) { const principalInvestigator = await userDataSource.getUser( event.proposal.proposerId ); - if (!principalInvestigator) { + if (!principalInvestigator || !principalInvestigator.email) { return; } const { finalStatus } = event.proposal; @@ -293,7 +298,7 @@ export async function eliEmailHandler(event: ApplicationEvent) { const fapReviewer = await userDataSource.getUser(userID); const proposal = await proposalDataSource.get(proposalPk); - if (!fapReviewer || !proposal) { + if (!fapReviewer || !fapReviewer.email || !proposal) { return; } @@ -348,7 +353,7 @@ export async function eliEmailHandler(event: ApplicationEvent) { event.internalreview.technicalReviewId ); - if (!assignedBy || !reviewer || !technicalReview) { + if (!assignedBy || !reviewer || !reviewer.email || !technicalReview) { logger.logError('Failed email invite', { event }); return; diff --git a/apps/backend/src/eventHandlers/email/essEmailHandler.ts b/apps/backend/src/eventHandlers/email/essEmailHandler.ts index 9d1b64cdbe..fb2c871785 100644 --- a/apps/backend/src/eventHandlers/email/essEmailHandler.ts +++ b/apps/backend/src/eventHandlers/email/essEmailHandler.ts @@ -93,6 +93,7 @@ export async function essEmailHandler(event: ApplicationEvent) { const principalInvestigator = await userDataSource.getUser( proposal.proposerId ); + if (!principalInvestigator) { logger.logError( 'No principal investigator found when trying to send invite accepted email', @@ -105,6 +106,18 @@ export async function essEmailHandler(event: ApplicationEvent) { return; } + if (!principalInvestigator.email) { + logger.logError( + 'No principal investigator email found when trying to send invite accepted email', + { + claim, + event, + } + ); + + return; + } + const claimer = await userDataSource.getUser( invite.claimedByUserId as number ); @@ -155,7 +168,7 @@ export async function essEmailHandler(event: ApplicationEvent) { event.proposal.proposerId ); - if (!principalInvestigator) { + if (!principalInvestigator || !principalInvestigator.email) { return; } @@ -165,6 +178,21 @@ export async function essEmailHandler(event: ApplicationEvent) { const call = await callDataSource.getCall(event.proposal.callId); + const recipients: ( + | { address: string } + | { address: { email: string; header_to: string } } + )[] = [{ address: principalInvestigator.email }]; + for (const partipant of participants) { + if (partipant.email) { + recipients.push({ + address: { + email: partipant.email, + header_to: principalInvestigator.email, + }, + }); + } + } + const options: EmailSettings = { content: { template_id: EmailTemplateId.PROPOSAL_SUBMITTED, @@ -179,17 +207,7 @@ export async function essEmailHandler(event: ApplicationEvent) { ), callShortCode: call?.shortCode, }, - recipients: [ - { address: principalInvestigator.email }, - ...participants.map((partipant) => { - return { - address: { - email: partipant.email, - header_to: principalInvestigator.email, - }, - }; - }), - ], + recipients, }; mailService @@ -215,7 +233,7 @@ export async function essEmailHandler(event: ApplicationEvent) { event.proposal.proposerId ); const call = await callDataSource.getCall(event.proposal.callId); - if (!principalInvestigator) { + if (!principalInvestigator || !principalInvestigator.email) { return; } const { finalStatus } = event.proposal; @@ -347,7 +365,7 @@ export async function essEmailHandler(event: ApplicationEvent) { const fapReviewer = await userDataSource.getUser(userID); const proposal = await proposalDataSource.get(proposalPk); - if (!fapReviewer || !proposal) { + if (!fapReviewer || !fapReviewer.email || !proposal) { return; } @@ -408,7 +426,7 @@ export async function essEmailHandler(event: ApplicationEvent) { } const user = await userDataSource.getUser(visitRegistration.userId); - if (!user) { + if (!user || !user.email) { return; } diff --git a/apps/backend/src/models/User.ts b/apps/backend/src/models/User.ts index 66d03e68f5..17f2176144 100644 --- a/apps/backend/src/models/User.ts +++ b/apps/backend/src/models/User.ts @@ -24,23 +24,23 @@ export type PasswordResetJwtPayload = SpecialActionJwtPayload & { export class User { constructor( public id: number, - public user_title: string, + public user_title: string | null, public firstname: string, public lastname: string, - public username: string, - public preferredname: string | undefined, + public username: string | null, + public preferredname: string | null, public oidcSub: string | null, public oauthRefreshToken: string | null, public oauthIssuer: string | null, public gender: string, public birthdate: Date, - public institutionId: number, - public institution: string, + public institutionId: number | null, + public institution: string | null, public department: string, public position: string, - public email: string, + public email: string | null, public telephone: string, - public placeholder: boolean, + public placeholder: boolean | null, public created: string, public updated: string ) {} @@ -83,16 +83,16 @@ export class BasicUserDetails { public id: number, public firstname: string, public lastname: string, - public preferredname: string, - public institution: string, - public institutionId: number, + public preferredname: string | null, + public institution: string | null, + public institutionId: number | null, public position: string, public created: Date, - public placeholder: boolean, - public email: string, + public placeholder: boolean | null, + public email: string | null, public country: string, - public title: string, - public oidc_sub: string + public user_title: string | null, + public oidc_sub: string | null ) {} } diff --git a/apps/backend/src/queries/UserQueries.ts b/apps/backend/src/queries/UserQueries.ts index 165d97ff42..f35b9c3fea 100644 --- a/apps/backend/src/queries/UserQueries.ts +++ b/apps/backend/src/queries/UserQueries.ts @@ -73,7 +73,7 @@ export default class UserQueries { user.placeholder, user.email, user.country, - user.title, + user.user_title, user.oidc_sub ); } else { @@ -108,7 +108,7 @@ export default class UserQueries { user.placeholder, user.email, user.country, - user.title, + user.user_title, user.oidc_sub ); } diff --git a/apps/backend/src/resolvers/mutations/UpdateUserMutation.ts b/apps/backend/src/resolvers/mutations/UpdateUserMutation.ts index 3091fc96a5..5443859859 100644 --- a/apps/backend/src/resolvers/mutations/UpdateUserMutation.ts +++ b/apps/backend/src/resolvers/mutations/UpdateUserMutation.ts @@ -15,7 +15,7 @@ import { User } from '../types/User'; @ArgsType() class UpdateUserArgs { @Field(() => String, { nullable: true }) - public user_title?: string; + public user_title?: string | null; @Field(() => String, { nullable: true }) public firstname?: string; @@ -24,10 +24,10 @@ class UpdateUserArgs { public lastname?: string; @Field(() => String, { nullable: true }) - public username?: string; + public username?: string | null; @Field(() => String, { nullable: true }) - public preferredname?: string; + public preferredname?: string | null; @Field(() => String, { nullable: true }) public gender?: string; @@ -36,7 +36,7 @@ class UpdateUserArgs { public birthdate?: Date; @Field(() => Int, { nullable: true }) - public institutionId?: number; + public institutionId?: number | null; @Field(() => String, { nullable: true }) public department?: string; @@ -45,13 +45,13 @@ class UpdateUserArgs { public position?: string; @Field(() => String, { nullable: true }) - public email?: string; + public email?: string | null; @Field(() => String, { nullable: true }) public telephone?: string; @Field(() => String, { nullable: true }) - public placeholder?: boolean; + public placeholder?: boolean | null; @Field(() => [Int], { nullable: true }) public roles?: number[]; diff --git a/apps/backend/src/resolvers/types/BasicUserDetails.ts b/apps/backend/src/resolvers/types/BasicUserDetails.ts index 321652b203..9a7d93a99c 100644 --- a/apps/backend/src/resolvers/types/BasicUserDetails.ts +++ b/apps/backend/src/resolvers/types/BasicUserDetails.ts @@ -17,13 +17,13 @@ export class BasicUserDetails implements Partial { public lastname: string; @Field(() => String, { nullable: true }) - public preferredname: string | undefined; + public preferredname?: string | null; - @Field() - public institution: string; + @Field(() => String, { nullable: true }) + public institution?: string | null; - @Field(() => Int) - public institutionId: number; + @Field(() => Int, { nullable: true }) + public institutionId?: number | null; @Field() public position: string; @@ -34,19 +34,19 @@ export class BasicUserDetails implements Partial { Roles.INTERNAL_REVIEWER, ]) @Field({ nullable: true }) - public email: string; + public email?: string | null; @Field(() => Boolean, { nullable: true }) - public placeholder?: boolean; + public placeholder?: boolean | null; - @Field(() => Date, { nullable: true }) - public created?: Date; + @Field(() => Date) + public created: Date; @Field({ nullable: true }) public country: string; @Field(() => String, { nullable: true }) - public oidc_sub: string | undefined; + public oidc_sub: string | null; } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/apps/backend/src/resolvers/types/User.ts b/apps/backend/src/resolvers/types/User.ts index 8b69c0ef6f..2c93dd9e39 100644 --- a/apps/backend/src/resolvers/types/User.ts +++ b/apps/backend/src/resolvers/types/User.ts @@ -49,8 +49,8 @@ export class User implements Partial { @Field(() => Int) public id: number; - @Field(() => String) - public user_title: string; + @Field(() => String, { nullable: true }) + public user_title?: string | null; @Field() public firstname: string; @@ -58,11 +58,11 @@ export class User implements Partial { @Field() public lastname: string; - @Field() - public username: string; + @Field(() => String, { nullable: true }) + public username?: string | null; @Field(() => String, { nullable: true }) - public preferredname: string | undefined; + public preferredname?: string | null; @Field(() => String, { nullable: true }) public oidcSub: string | null; @@ -76,8 +76,8 @@ export class User implements Partial { @Field() public birthdate: Date; - @Field(() => Int) - public institutionId: number; + @Field(() => Int, { nullable: true }) + public institutionId?: number | null; @Field() public department: string; @@ -85,14 +85,14 @@ export class User implements Partial { @Field() public position: string; - @Field() - public email: string; + @Field(() => String, { nullable: true }) + public email?: string | null; @Field() public telephone: string; - @Field() - public placeholder: boolean; + @Field(() => Boolean, { nullable: true }) + public placeholder?: boolean | null; @Field() public created: string; diff --git a/apps/backend/src/resolvers/types/UserJWT.ts b/apps/backend/src/resolvers/types/UserJWT.ts index 2af650a870..0fee4c1321 100644 --- a/apps/backend/src/resolvers/types/UserJWT.ts +++ b/apps/backend/src/resolvers/types/UserJWT.ts @@ -14,16 +14,16 @@ export class UserJWT implements Partial { public lastname: string; @Field(() => String, { nullable: true }) - public preferredname: string | undefined; + public preferredname?: string | null; - @Field() - public email: string; + @Field(() => String, { nullable: true }) + public email?: string | null; @Field(() => String, { nullable: true }) - public oidcSub: string | null; + public oidcSub?: string | null; - @Field() - public placeholder: boolean; + @Field(() => Boolean, { nullable: true }) + public placeholder?: boolean | null; @Field() public created: string; @@ -31,6 +31,6 @@ export class UserJWT implements Partial { @Field() public position: string; - @Field() - public institutionId: number; + @Field(() => Int, { nullable: true }) + public institutionId?: number | null; } diff --git a/apps/backend/src/statusActionEngine/statusActionUtils.ts b/apps/backend/src/statusActionEngine/statusActionUtils.ts index 1fa7af8e38..7dabac654c 100644 --- a/apps/backend/src/statusActionEngine/statusActionUtils.ts +++ b/apps/backend/src/statusActionEngine/statusActionUtils.ts @@ -211,20 +211,22 @@ export const getEmailReadyArrayOfUsersAndProposals = async ( } } } - emailReadyUsersWithProposals.push({ - id: recipientsWithEmailTemplate.recipient.name, - proposals: [proposal], - template: recipientsWithEmailTemplate.emailTemplate.id, - email: recipient.email, - firstName: recipient.firstname, - lastName: recipient.lastname, - preferredName: recipient.preferredname, - pi: pi, - coProposers: coProposers, - techniques: techniques, - samples: sampleAnswers, - hazards: hazardAnswers, - }); + if (recipient.email) { + emailReadyUsersWithProposals.push({ + id: recipientsWithEmailTemplate.recipient.name, + proposals: [proposal], + template: recipientsWithEmailTemplate.emailTemplate.id, + email: recipient.email, + firstName: recipient.firstname, + lastName: recipient.lastname, + preferredName: recipient.preferredname ?? undefined, + pi: pi, + coProposers: coProposers, + techniques: techniques, + samples: sampleAnswers, + hazards: hazardAnswers, + }); + } } }) ); From 5dfaa0a9f3b9158dae44a62fdb9ad768641471a8 Mon Sep 17 00:00:00 2001 From: Yoganandan Pandiyan Date: Sun, 19 Oct 2025 20:57:33 +0200 Subject: [PATCH 5/5] fix: ellipsis added for the overflowing question id and the clash with the icon is avoided --- .../components/template/TemplateQuestionEditor.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/apps/frontend/src/components/template/TemplateQuestionEditor.tsx b/apps/frontend/src/components/template/TemplateQuestionEditor.tsx index dd9bfd8b6a..3d36001820 100644 --- a/apps/frontend/src/components/template/TemplateQuestionEditor.tsx +++ b/apps/frontend/src/components/template/TemplateQuestionEditor.tsx @@ -59,7 +59,7 @@ export default function TemplateQuestionEditor(props: { ? '&&' : '||'; const dependencyJsx = dependencies.length ? ( -
+ <>
    {dependencies.map((dependency, i) => { @@ -99,7 +99,7 @@ export default function TemplateQuestionEditor(props: { ); })}
-
+ ) : null; const questionDefinition = getQuestionaryComponentDefinition( props.data.dataType @@ -138,6 +138,9 @@ export default function TemplateQuestionEditor(props: { fontSize: '12px', fontWeight: 'bold', color: theme.palette.grey[800], + overflow: 'hidden', + textOverflow: 'ellipsis', + whiteSpace: 'nowrap', }} data-cy="proposal-question-id" > @@ -148,9 +151,9 @@ export default function TemplateQuestionEditor(props: { xs={2} sx={{ color: theme.palette.grey[400], - justifyItems: 'flex-end', - justifyContent: 'flex-end', display: 'flex', + justifyContent: 'flex-end', + alignItems: 'center', }} > {getTemplateFieldIcon(props.data.dataType)} @@ -159,7 +162,7 @@ export default function TemplateQuestionEditor(props: { {questionDefinition.renderers ? questionDefinition.renderers.questionRenderer(props.data)