diff --git a/src/controller/request/user-request.ts b/src/controller/request/user-request.ts index e99b5d103..b2d40cf3d 100644 --- a/src/controller/request/user-request.ts +++ b/src/controller/request/user-request.ts @@ -36,7 +36,7 @@ export default interface BaseUserRequest { * @property {boolean} canGoIntoDebt.required * @property {boolean} ofAge.required * @property {string} email.required - * @property {number} type.required + * @property {string} type.required */ export interface CreateUserRequest extends BaseUserRequest { type: UserType; diff --git a/src/controller/response/rbac/role-response.ts b/src/controller/response/rbac/role-response.ts index 7bed2f809..0ad850d23 100644 --- a/src/controller/response/rbac/role-response.ts +++ b/src/controller/response/rbac/role-response.ts @@ -24,7 +24,7 @@ import PermissionResponse from './permission-response'; * @property {integer} id.required - The ID of the role. * @property {string} name.required - The name of the role. * @property {boolean} systemDefault.required - Whether the role is a system default role - * @property {Array.} userTypes - The user types this role is default for + * @property {Array.} userTypes - The user types this role is default for */ /** @@ -35,6 +35,6 @@ export default interface RoleResponse { id: number; name: string; systemDefault: boolean; - userTypes?: number[]; + userTypes?: string[]; permissions?: PermissionResponse[]; } diff --git a/src/database/database.ts b/src/database/database.ts index 980b0335e..e200c1971 100644 --- a/src/database/database.ts +++ b/src/database/database.ts @@ -93,6 +93,7 @@ import SellerPayout from '../entity/transactions/payout/seller-payout'; import { InvoiceAsTopups1724506999318 } from '../migrations/1724506999318-invoice-as-topups'; import { SellerPayouts1724855153990 } from '../migrations/1724855153990-seller-payouts'; import SellerPayoutPdf from '../entity/file/seller-payout-pdf'; +import { UserTypeEnums1725196803203 } from '../migrations/1725196803203-user-type-enums'; // We need to load the dotenv to prevent the env from being undefined. dotenv.config(); @@ -125,6 +126,7 @@ const options: DataSourceOptions = { NestedProductCategories1722517212441, InvoiceAsTopups1724506999318, SellerPayouts1724855153990, + UserTypeEnums1725196803203, ], extra: { authPlugins: { diff --git a/src/database/migrate.ts b/src/database/migrate.ts index 645afd8c0..bfd58a6c5 100644 --- a/src/database/migrate.ts +++ b/src/database/migrate.ts @@ -36,11 +36,15 @@ export default async function migrate() { try { application.logger.log('Starting synchronize + migrations.'); + application.logger.debug('Synchronize...'); await application.connection.synchronize(); + application.logger.debug('Fake migrations...'); await application.connection.runMigrations({ transaction: 'all', fake: true }); + application.logger.debug('Revert last migration...'); await application.connection.undoLastMigration({ transaction: 'all' }); + application.logger.debug('Run last migration...'); await application.connection.runMigrations({ transaction: 'all' }); - await application.connection.close(); + await application.connection.destroy(); application.logger.log('Finished synchronize + migrations.'); } catch (e) { application.logger.error('Error migrating db', e); diff --git a/src/entity/user/user.ts b/src/entity/user/user.ts index 406010fc8..c5af79ce1 100644 --- a/src/entity/user/user.ts +++ b/src/entity/user/user.ts @@ -30,13 +30,13 @@ export enum TermsOfServiceStatus { } export enum UserType { - MEMBER = 1, - ORGAN = 2, - VOUCHER = 3, - LOCAL_USER = 4, - LOCAL_ADMIN = 5, - INVOICE = 6, - POINT_OF_SALE = 7, + MEMBER = 'MEMBER', + ORGAN = 'ORGAN', + VOUCHER = 'VOUCHER', + LOCAL_USER = 'LOCAL_USER', + LOCAL_ADMIN = 'LOCAL_ADMIN', + INVOICE = 'INVOICE', + POINT_OF_SALE = 'POINT_OF_SALE', } /** diff --git a/src/helpers/revision-to-response.ts b/src/helpers/revision-to-response.ts index 88e616bd9..065c79e48 100644 --- a/src/helpers/revision-to-response.ts +++ b/src/helpers/revision-to-response.ts @@ -128,7 +128,7 @@ export interface RawUser { ofAge: number, email: string, deleted: number, - type: number, + type: string, acceptedToS: TermsOfServiceStatus, extensiveDataProcessing: number, canGoIntoDebt: number, @@ -149,7 +149,7 @@ export function parseRawUserToResponse(user: RawUser, timestamps = false): UserR updatedAt: timestamps ? user.updatedAt : undefined, active: user.active === 1, deleted: user.deleted === 1, - type: UserType[user.type], + type: user.type, email: user.type === UserType.LOCAL_USER ? user.email : undefined, acceptedToS: user.acceptedToS, extensiveDataProcessing: user.extensiveDataProcessing === 1, diff --git a/src/helpers/validators.ts b/src/helpers/validators.ts index 98b941097..e4924d216 100644 --- a/src/helpers/validators.ts +++ b/src/helpers/validators.ts @@ -141,22 +141,16 @@ export function asUserType(input: any): UserType | undefined { if (input === undefined || input === null) return undefined; // Convert input to a number if it's a string representation of a number - if (typeof input === 'string' && !isNaN(Number(input))) { - input = Number(input); + if (typeof input !== 'string') { + input = input.toString(); } - // Check if input is now a number and a valid enum value - if (typeof input === 'number' && UserType[input] !== undefined) { - return input; - } - - // Check if input is a string and a valid enum key - const state: UserType = UserType[input as keyof typeof UserType]; - if (state === undefined) { + // Check if input is a valid userType + if (!Object.values(UserType).includes(input)) { throw new TypeError(`Input '${input}' is not a valid UserType.`); } - return state; + return input; } /** diff --git a/src/migrations/1724506999318-invoice-as-topups.ts b/src/migrations/1724506999318-invoice-as-topups.ts index 1fc24ff1a..8ccd725af 100644 --- a/src/migrations/1724506999318-invoice-as-topups.ts +++ b/src/migrations/1724506999318-invoice-as-topups.ts @@ -21,7 +21,7 @@ import Transfer from '../entity/transactions/transfer'; import InvoiceStatus from '../entity/invoices/invoice-status'; import assert from 'assert'; import BalanceService from '../service/balance-service'; -import User from '../entity/user/user'; +import User, { UserType } from '../entity/user/user'; import fs from 'fs'; export class InvoiceAsTopups1724506999318 implements MigrationInterface { @@ -54,7 +54,7 @@ export class InvoiceAsTopups1724506999318 implements MigrationInterface { console.error(balanceBefore._pagination.count, ids.length); assert(balanceBefore._pagination.count === ids.length); - const organs = await queryRunner.manager.find(User, { where: { type: 2 } }); + const organs = await queryRunner.manager.find(User, { where: { type: UserType.ORGAN } }); let organBalances = await new BalanceService().getBalances({ ids: organs.map((o) => o.id), allowDeleted: true }, { take: organs.length }); fs.writeFileSync('./organBalances-before.json', JSON.stringify(organBalances, null, 2)); diff --git a/src/migrations/1724855153990-seller-payouts.ts b/src/migrations/1724855153990-seller-payouts.ts index 1761591f4..1d384e7f6 100644 --- a/src/migrations/1724855153990-seller-payouts.ts +++ b/src/migrations/1724855153990-seller-payouts.ts @@ -27,7 +27,7 @@ import assert from 'assert'; import BalanceService from '../service/balance-service'; import { SalesReportService } from '../service/report-service'; import SubTransactionRow from '../entity/transactions/sub-transaction-row'; -import User from '../entity/user/user'; +import User, { UserType } from '../entity/user/user'; import fs from 'fs'; export class SellerPayouts1724855153990 implements MigrationInterface { @@ -64,7 +64,7 @@ export class SellerPayouts1724855153990 implements MigrationInterface { console.error(balanceBefore._pagination.count, ids.length); assert(balanceBefore._pagination.count === ids.length); - const organs = await queryRunner.manager.find(User, { where: { type: 2 } }); + const organs = await queryRunner.manager.find(User, { where: { type: UserType.ORGAN } }); let organBalances = await new BalanceService().getBalances({ ids: organs.map((o) => o.id), allowDeleted: true }, { take: organs.length }); fs.writeFileSync('./organBalances-before-payouts.json', JSON.stringify(organBalances, null, 2)); diff --git a/src/migrations/1725196803203-user-type-enums.ts b/src/migrations/1725196803203-user-type-enums.ts new file mode 100644 index 000000000..e7dc10fdf --- /dev/null +++ b/src/migrations/1725196803203-user-type-enums.ts @@ -0,0 +1,77 @@ +/** + * SudoSOS back-end API service. + * Copyright (C) 2024 Study association GEWIS + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +import { MigrationInterface, QueryRunner, TableColumn } from 'typeorm'; + +const UserTypeMapping: Record = { + MEMBER: 1, + ORGAN: 2, + VOUCHER: 3, + LOCAL_USER: 4, + LOCAL_ADMIN: 5, + INVOICE: 6, + POINT_OF_SALE: 7, +}; + +export class UserTypeEnums1725196803203 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + if (process.env.TYPEORM_CONNECTION === 'sqlite') { + await queryRunner.changeColumn('user', 'type', new TableColumn({ + name: 'type', + type: 'varchar', + length: '64', + isNullable: false, + })); + await queryRunner.changeColumn('role_user_type', 'userType', new TableColumn({ + name: 'userType', + type: 'varchar', + length: '64', + isNullable: false, + isPrimary: true, + })); + } else { + // TypeORM completely deletes the column and adds it anew, which deletes all values + // So a manual query it is! SQLite somehow does work with TypeORM + await queryRunner.query('ALTER TABLE user MODIFY type varchar(64) NOT NULL'); + await queryRunner.query('ALTER TABLE role_user_type MODIFY userType varchar(64) NOT NULL'); + } + + const promises: Promise[] = []; + Object.entries(UserTypeMapping).forEach(([key, value]) => { + promises.push(queryRunner.query('UPDATE user SET type = ? WHERE type = ?', [key, value])); + promises.push(queryRunner.query('UPDATE role_user_type SET userType = ? WHERE userType = ?', [key, value])); + }); + + await Promise.all(promises); + } + + public async down(queryRunner: QueryRunner): Promise { + const promises: Promise[] = []; + Object.entries(UserTypeMapping).forEach(([key, value]) => { + promises.push(queryRunner.query('UPDATE user SET type = ? WHERE type = ?', [value, key])); + promises.push(queryRunner.query('UPDATE role_user_type SET userType = ? WHERE userType = ?', [value, key])); + }); + + await Promise.all(promises); + + // TypeORM completely deletes the column and adds it anew, which makes all values 0... + // So a manual query it is! The migration:revert will now no longer work on SQLite though. + await queryRunner.query('ALTER TABLE user MODIFY type integer NOT NULL'); + await queryRunner.query('ALTER TABLE role_user_type MODIFY userType integer NOT NULL'); + } + +} diff --git a/src/service/balance-service.ts b/src/service/balance-service.ts index a7070e589..6bdc10e48 100644 --- a/src/service/balance-service.ts +++ b/src/service/balance-service.ts @@ -295,7 +295,7 @@ export default class BalanceService { + 'where user.currentFinesId = user_fine_group.id ' + 'group by user.id ' + ') as f on f.id = moneys2.id ' - + `where u.type not in (${UserType.POINT_OF_SALE}) `; + + `where u.type not in ("${UserType.POINT_OF_SALE}") `; if (minBalance !== undefined) query += `and moneys2.totalvalue + Coalesce(b5.amount, 0) >= ${minBalance.getAmount()} `; if (maxBalance !== undefined) query += `and moneys2.totalvalue + Coalesce(b5.amount, 0) <= ${maxBalance.getAmount()} `; @@ -303,7 +303,7 @@ export default class BalanceService { if (hasFine === true) query += 'and f.fine is not null '; if (minFine !== undefined) query += `and f.fine >= ${minFine.getAmount()} `; if (maxFine !== undefined) query += `and f.fine <= ${maxFine.getAmount()} `; - if (userTypes !== undefined) query += `and u.type in (${userTypes.join(',')}) `; + if (userTypes !== undefined) query += `and u.type in (${userTypes.map((t) => `"${t}"`).join(',')}) `; if (!allowDeleted) query += 'and u.deleted = 0 '; if (orderBy !== undefined) query += `order by ${orderBy} ${orderDirection ?? ''} `; diff --git a/src/service/user-service.ts b/src/service/user-service.ts index 6c111c825..ceb3981f2 100644 --- a/src/service/user-service.ts +++ b/src/service/user-service.ts @@ -131,7 +131,7 @@ export default class UserService { } const builder = Bindings.Users.getBuilder(); - builder.where(`user.type NOT IN (${UserType.POINT_OF_SALE})`); + builder.where(`user.type NOT IN ("${UserType.POINT_OF_SALE}")`); QueryFilter.applyFilter(builder, filterMapping, f); // Note this is only for MySQL diff --git a/src/subscriber/transaction-subscriber.ts b/src/subscriber/transaction-subscriber.ts index a86163efb..2280f1092 100644 --- a/src/subscriber/transaction-subscriber.ts +++ b/src/subscriber/transaction-subscriber.ts @@ -69,7 +69,7 @@ export default class TransactionSubscriber implements EntitySubscriberInterface if (balanceBefore.amount.amount < 0) return; // User was not in debt before this new transaction - if (!(user.type in NotifyDebtUserTypes)) return; + if (!NotifyDebtUserTypes.includes(user.type)) return; // User should be notified of debt Mailer.getInstance().send(user, new UserDebtNotification({ diff --git a/test/unit/controller/invoice-controller.ts b/test/unit/controller/invoice-controller.ts index 3f40c97cc..e71296db5 100644 --- a/test/unit/controller/invoice-controller.ts +++ b/test/unit/controller/invoice-controller.ts @@ -179,7 +179,7 @@ describe('InvoiceController', async () => { addressee: 'InvoiceRequest', byId: adminUser.id, description: 'InvoiceRequest test', - forId: localUser.type, + forId: localUser.id, reference: 'BAC-41', city: 'city', country: 'country', diff --git a/test/unit/controller/transaction-controller.ts b/test/unit/controller/transaction-controller.ts index 29afcfc65..cf7185a19 100644 --- a/test/unit/controller/transaction-controller.ts +++ b/test/unit/controller/transaction-controller.ts @@ -648,13 +648,13 @@ describe('TransactionController', (): void => { lastName: 'voucher', active: true, deleted: false, - type: 3, + type: UserType.VOUCHER, acceptedToS: TermsOfServiceStatus.NOT_REQUIRED, canGoIntoDebt: false, } as User); const voucherUser = await User.findOne({ - where: { active: true, deleted: false, type: 3 }, + where: { active: true, deleted: false, type: UserType.VOUCHER }, }); const badReq = { ...ctx.validTransReq, diff --git a/test/unit/helpers/validators.ts b/test/unit/helpers/validators.ts index 0a99d9ba5..8ac8d2f61 100644 --- a/test/unit/helpers/validators.ts +++ b/test/unit/helpers/validators.ts @@ -18,17 +18,17 @@ import { asArrayOfUserTypes, asUserType } from '../../../src/helpers/validators'; import { expect } from 'chai'; +import { UserType } from '../../../src/entity/user/user'; describe('Validators', (): void => { describe('asUserType', (): void => { - it('should accept number inputs', () => { - const valid = asUserType(1); - expect(valid).to.not.be.undefined; - }); it('should accept string inputs', async () => { const valid = asUserType('MEMBER'); expect(valid).to.not.be.undefined; - expect(valid).to.be.a('number'); + expect(valid).to.equal(UserType.MEMBER); + }); + it('should throw erorr if number', () => { + expect(() => asUserType(1)).to.throw(); }); it('should throw error for bad input', async () => { expect(() => asUserType('TEST')).to.throw(); @@ -36,43 +36,30 @@ describe('Validators', (): void => { }); }); describe('asArrayOfUserTypes', (): void => { - it('should accept number input', () => { - const valid = asArrayOfUserTypes(1); - expect(valid).to.not.be.undefined; - }); - it('should accept number inputs', () => { - const valid = asArrayOfUserTypes([1, 2]); - expect(valid).to.not.be.undefined; - }); it('should accept string input', () => { const valid = asArrayOfUserTypes('MEMBER'); expect(valid).to.not.be.undefined; + expect(valid).to.deep.equalInAnyOrder([UserType.MEMBER]); }); it('should accept string inputs', () => { const valid = asArrayOfUserTypes(['MEMBER', 'LOCAL_ADMIN']); expect(valid).to.not.be.undefined; + expect(valid).to.deep.equalInAnyOrder([UserType.MEMBER, UserType.LOCAL_ADMIN]); }); - it('should convert string to int', async () => { - const valid = asArrayOfUserTypes(['MEMBER', 'LOCAL_ADMIN']); - expect(valid).to.not.be.undefined; - valid.forEach(item => { - expect(item).to.be.a('number'); - }); + it('should throw if number input', () => { + expect(() => asArrayOfUserTypes(1)).to.throw(); }); - it('should accept string number input', () => { - const valid = asArrayOfUserTypes('1'); - expect(valid).to.not.be.undefined; + it('should throw if multiple number inputs', () => { + expect(() => asArrayOfUserTypes([1, 2])).to.throw(); }); - it('should accept string number inputs', () => { - const valid = asArrayOfUserTypes(['1', '2']); - expect(valid).to.not.be.undefined; + it('should throw if combination of string and number inputs', () => { + expect(() => asArrayOfUserTypes([1, UserType.MEMBER])).to.throw(); }); - it('should convert string number to int', async () => { - const valid = asArrayOfUserTypes(['1', '2']); - expect(valid).to.not.be.undefined; - valid.forEach(item => { - expect(item).to.be.a('number'); - }); + it('should throw if invalid userType', () => { + expect(() => asArrayOfUserTypes('TEST')).to.throw(); + }); + it('should throw if one invalid userType', () => { + expect(() => asArrayOfUserTypes([UserType.MEMBER, 'TEST'])).to.throw(); }); }); }); diff --git a/test/unit/subscribe/transaction-subscriber.ts b/test/unit/subscribe/transaction-subscriber.ts index 0c0d7ec1c..f4540a5ea 100644 --- a/test/unit/subscribe/transaction-subscriber.ts +++ b/test/unit/subscribe/transaction-subscriber.ts @@ -91,8 +91,8 @@ describe('TransactionSubscriber', () => { connection, adminUser, users, - usersNotInDebt: users.filter((u) => calculateBalance(u, transactions, subTransactions, transfers).amount.getAmount() >= 0 && u.type in NotifyDebtUserTypes), - usersInDebt: users.filter((u) => calculateBalance(u, transactions, subTransactions, transfers).amount.getAmount() < 0 && u.type in NotifyDebtUserTypes), + usersNotInDebt: users.filter((u) => calculateBalance(u, transactions, subTransactions, transfers).amount.getAmount() >= 0 && NotifyDebtUserTypes.includes(u.type)), + usersInDebt: users.filter((u) => calculateBalance(u, transactions, subTransactions, transfers).amount.getAmount() < 0 && NotifyDebtUserTypes.includes(u.type)), products: productRevisions, containers: containerRevisions, pointOfSales: pointOfSaleRevisions, @@ -111,6 +111,9 @@ describe('TransactionSubscriber', () => { env = process.env.NODE_ENV; process.env.NODE_ENV = 'test-transactions'; + + // Sanity check + expect(ctx.usersInDebt.length).to.be.at.least(3); }); after(async () => {