refactor: change UserType enum from integers to strings (#296)

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;

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.<integer>} userTypes - The user types this role is default for
+ * @property {Array.<string>} 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[];
 }

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: {

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);

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',
 }
 
 /**

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,

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;
 }
 
 /**

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));
 

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));
 

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 <https://www.gnu.org/licenses/>.
+ */
+import { MigrationInterface, QueryRunner, TableColumn } from 'typeorm';
+
+const UserTypeMapping: Record<string, number> = {
+  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<void> {
+    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<void>[] = [];
+    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<void> {
+    const promises: Promise<void>[] = [];
+    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');
+  }
+
+}

src/service/balance-service.ts

@@ -295,15 +295,15 @@ 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()} `;
     if (hasFine === false) query += 'and f.fine is null ';
     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 ?? ''} `;

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

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({

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',

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,

test/unit/helpers/validators.ts

@@ -18,61 +18,48 @@
 
 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();
       expect(() => asUserType(-1)).to.throw();
     });
   });
   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();
     });
   });
 });