From 91993dbb07e1bddd8556935f2d71bc2f308149b1 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Tue, 15 Oct 2024 14:22:38 +0200 Subject: [PATCH] refactor: AppConfig model and corresponding tests --- packages/backend/src/models/app-config.js | 47 +++++---- .../backend/src/models/app-config.test.js | 99 +++++++++++++++++-- 2 files changed, 111 insertions(+), 35 deletions(-) diff --git a/packages/backend/src/models/app-config.js b/packages/backend/src/models/app-config.js index 1fb0ee46..465600be 100644 --- a/packages/backend/src/models/app-config.js +++ b/packages/backend/src/models/app-config.js @@ -38,48 +38,45 @@ class AppConfig extends Base { return await App.findOneByKey(this.key); } - async computeConnectionAllowedProperty(oldAppConfig) { - const appAuthClients = await oldAppConfig.$relatedQuery('appAuthClients'); - const hasSomeActiveAppAuthClients = !!appAuthClients?.some( - (appAuthClient) => appAuthClient.active - ); - const shared = this.shared ?? oldAppConfig.shared; - const disabled = this.disabled ?? oldAppConfig.disabled; - const active = disabled === false; - - const conditions = [hasSomeActiveAppAuthClients, shared, active]; - - const connectionAllowed = conditions.every(Boolean); - - return connectionAllowed; - } - async updateConnectionAllowedProperty() { - const connectionAllowed = await this.computeConnectionAllowedProperty(this); + const connectionAllowed = await this.computeConnectionAllowedProperty(); return await this.$query().patch({ connectionAllowed, }); } - async computeAndAssignConnectionAllowedProperty(oldAppConfig) { - this.connectionAllowed = await this.computeConnectionAllowedProperty( - oldAppConfig - ); + async computeAndAssignConnectionAllowedProperty() { + this.connectionAllowed = await this.computeConnectionAllowedProperty(); + } + + async computeConnectionAllowedProperty() { + const appAuthClients = await this.$relatedQuery('appAuthClients'); + + const hasSomeActiveAppAuthClients = + appAuthClients?.some((appAuthClient) => appAuthClient.active) || false; + + const conditions = [ + hasSomeActiveAppAuthClients, + this.shared, + !this.disabled, + ]; + + const connectionAllowed = conditions.every(Boolean); + + return connectionAllowed; } async $beforeInsert(queryContext) { await super.$beforeInsert(queryContext); - await this.computeAndAssignConnectionAllowedProperty(this); + await this.computeAndAssignConnectionAllowedProperty(); } async $beforeUpdate(opt, queryContext) { await super.$beforeUpdate(opt, queryContext); - const oldAppConfig = opt.old; - - await this.computeAndAssignConnectionAllowedProperty(oldAppConfig); + await this.computeAndAssignConnectionAllowedProperty(); } } diff --git a/packages/backend/src/models/app-config.test.js b/packages/backend/src/models/app-config.test.js index 786924fc..03bf6b71 100644 --- a/packages/backend/src/models/app-config.test.js +++ b/packages/backend/src/models/app-config.test.js @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { vi, describe, it, expect } from 'vitest'; import Base from './base.js'; import AppConfig from './app-config.js'; @@ -52,8 +52,48 @@ describe('AppConfig model', () => { }); }); - describe('connectionAllowed', () => { - it('should return true when app is enabled, shared and allows custom connection with an active app auth client at least', async () => { + describe('updateConnectionAllowedProperty', () => { + it('should call computeConnectionAllowedProperty and patch the result', async () => { + const appConfig = await createAppConfig(); + + const computeConnectionAllowedPropertySpy = vi + .spyOn(appConfig, 'computeConnectionAllowedProperty') + .mockResolvedValue(true); + + const patchSpy = vi + .fn() + .mockImplementation((newAppConfig) => newAppConfig); + + vi.spyOn(appConfig, '$query').mockImplementation(() => ({ + patch: patchSpy, + })); + + await appConfig.updateConnectionAllowedProperty(); + + expect(computeConnectionAllowedPropertySpy).toHaveBeenCalled(); + expect(patchSpy).toHaveBeenCalledWith({ + connectionAllowed: true, + }); + }); + }); + + describe('computeAndAssignConnectionAllowedProperty', () => { + it('should call computeConnectionAllowedProperty and assign the result', async () => { + const appConfig = await createAppConfig(); + + const computeConnectionAllowedPropertySpy = vi + .spyOn(appConfig, 'computeConnectionAllowedProperty') + .mockResolvedValue(true); + + await appConfig.computeAndAssignConnectionAllowedProperty(); + + expect(computeConnectionAllowedPropertySpy).toHaveBeenCalled(); + expect(appConfig.connectionAllowed).toBe(true); + }); + }); + + describe('computeConnectionAllowedProperty', () => { + it('should return true when app is enabled, shared and allows custom connection with an active app auth client', async () => { await createAppAuthClient({ appKey: 'deepl', active: true, @@ -71,10 +111,13 @@ describe('AppConfig model', () => { key: 'deepl', }); - expect(appConfig.connectionAllowed).toBe(true); + const connectionAllowed = + await appConfig.computeConnectionAllowedProperty(); + + expect(connectionAllowed).toBe(true); }); - it('should return true when app is enabled, shared and allows custom connection with no active app auth client', async () => { + it('should return false if there is no active app auth client', async () => { await createAppAuthClient({ appKey: 'deepl', active: false, @@ -87,10 +130,13 @@ describe('AppConfig model', () => { key: 'deepl', }); - expect(appConfig.connectionAllowed).toBe(false); + const connectionAllowed = + await appConfig.computeConnectionAllowedProperty(); + + expect(connectionAllowed).toBe(false); }); - it('should return false when app is enabled, shared and allows custom connection without any app auth clients', async () => { + it('should return false if there is no app auth clients', async () => { const appConfig = await createAppConfig({ disabled: false, customConnectionAllowed: true, @@ -98,7 +144,10 @@ describe('AppConfig model', () => { key: 'deepl', }); - expect(appConfig.connectionAllowed).toBe(false); + const connectionAllowed = + await appConfig.computeConnectionAllowedProperty(); + + expect(connectionAllowed).toBe(false); }); it('should return false when app is disabled', async () => { @@ -107,7 +156,10 @@ describe('AppConfig model', () => { customConnectionAllowed: true, }); - expect(appConfig.connectionAllowed).toBe(false); + const connectionAllowed = + await appConfig.computeConnectionAllowedProperty(); + + expect(connectionAllowed).toBe(false); }); it(`should return false when app doesn't allow custom connection`, async () => { @@ -116,7 +168,34 @@ describe('AppConfig model', () => { customConnectionAllowed: false, }); - expect(appConfig.connectionAllowed).toBe(false); + const connectionAllowed = + await appConfig.computeConnectionAllowedProperty(); + + expect(connectionAllowed).toBe(false); }); }); + + it('$beforeInsert should call computeAndAssignConnectionAllowedProperty', async () => { + const computeAndAssignConnectionAllowedPropertySpy = vi + .spyOn(AppConfig.prototype, 'computeAndAssignConnectionAllowedProperty') + .mockResolvedValue(true); + + await createAppConfig(); + + expect(computeAndAssignConnectionAllowedPropertySpy).toHaveBeenCalledOnce(); + }); + + it('$beforeUpdate should call computeAndAssignConnectionAllowedProperty', async () => { + const appConfig = await createAppConfig(); + + const computeAndAssignConnectionAllowedPropertySpy = vi + .spyOn(AppConfig.prototype, 'computeAndAssignConnectionAllowedProperty') + .mockResolvedValue(true); + + await appConfig.$query().patch({ + key: 'deepl', + }); + + expect(computeAndAssignConnectionAllowedPropertySpy).toHaveBeenCalledOnce(); + }); });