From 9a138c77e857d2ad756e32145f89b4d769b2b3cc Mon Sep 17 00:00:00 2001 From: Shohan Rahman Date: Fri, 24 May 2024 16:00:08 +0200 Subject: [PATCH] fix: create and update with a scope when it is not found (#1107) --- src/services/resource-creator.js | 24 ++++-- src/services/resource-updater.js | 20 +++-- test/services/resource-creator.test.js | 52 +++++++++++- test/services/resources-updater.test.js | 108 ++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 12 deletions(-) create mode 100644 test/services/resources-updater.test.js diff --git a/src/services/resource-creator.js b/src/services/resource-creator.js index 4d9c99f9a..1f76d8914 100644 --- a/src/services/resource-creator.js +++ b/src/services/resource-creator.js @@ -68,6 +68,12 @@ class ResourceCreator { // handleAssociationsBeforeSave await this._handleSave(recordCreated, this._makePromisesBeforeSave); + const scopeFilters = await Interface.scopeManager.getScopeForUser( + this.user, + this.model.name, + true, + ); + // saveInstance (validate then save) try { await recordCreated.validate(); @@ -84,12 +90,18 @@ class ResourceCreator { // appendCompositePrimary new PrimaryKeysManager(this.model).annotateRecords([record]); - // return makeResourceGetter() - return new ResourceGetter( - this.model, - { ...this.params, recordId: record[this.schema.idField] }, - this.user, - ).perform(); + try { + return await new ResourceGetter( + this.model, + { ...this.params, recordId: record[this.schema.idField] }, + this.user, + ).perform(); + } catch (error) { + if (error.statusCode === 404 && scopeFilters) { + return record; + } + throw error; + } } } diff --git a/src/services/resource-updater.js b/src/services/resource-updater.js index 5c8790dc9..5dfb75461 100644 --- a/src/services/resource-updater.js +++ b/src/services/resource-updater.js @@ -1,4 +1,5 @@ import { scopeManager } from 'forest-express'; +import createError from 'http-errors'; import { ErrorHTTP422 } from './errors'; import QueryOptions from './query-options'; import ResourceGetter from './resource-getter'; @@ -29,13 +30,22 @@ class ResourceUpdater { } catch (error) { throw new ErrorHTTP422(error.message); } + } else { + throw createError(404, `The ${this._model.name} #${this._params.recordId} does not exist.`); } - return new ResourceGetter( - this._model, - { ...this._params, recordId: this._params.recordId }, - this._user, - ).perform(); + try { + return await new ResourceGetter( + this._model, + { ...this._params, recordId: this._params.recordId }, + this._user, + ).perform(); + } catch (error) { + if (error.statusCode === 404 && scopeFilters) { + return record; + } + throw error; + } } } diff --git a/test/services/resource-creator.test.js b/test/services/resource-creator.test.js index f0c5d6976..987a2e4fa 100644 --- a/test/services/resource-creator.test.js +++ b/test/services/resource-creator.test.js @@ -1,7 +1,8 @@ -import Interface from 'forest-express'; +import Interface, { scopeManager } from 'forest-express'; import Sequelize from 'sequelize'; import associationRecord from '../../src/utils/association-record'; import ResourceCreator from '../../src/services/resource-creator'; +import ResourceGetter from '../../src/services/resource-getter'; describe('services > resource-creator', () => { const user = { renderingId: 1 }; @@ -40,6 +41,55 @@ describe('services > resource-creator', () => { return { Actor, Author, Film }; }; + describe('perform', () => { + describe('when the getter does not found the record', () => { + it('should catch the 404 error and return the record', async () => { + expect.assertions(1); + + const { Film } = buildModelMock(); + const record = { dataValues: { id: 1, title: 'The Godfather' } }; + + const error = new Error('Record not found'); + error.statusCode = 404; + jest + .spyOn(ResourceGetter.prototype, 'perform') + .mockRejectedValue(error); + + jest.spyOn(Film.prototype, 'save').mockReturnValue(record); + jest.spyOn(scopeManager, 'getScopeForUser').mockReturnValue({}); + + const body = { actor: 2 }; + + const resourceCreator = new ResourceCreator(Film, params, body, user); + const result = await resourceCreator.perform(); + expect(result).toStrictEqual(record); + }); + }); + + describe('when there is a scope and ResourcesGetter throw an error', () => { + it('should throw the error', async () => { + expect.assertions(1); + + const { Film } = buildModelMock(); + const record = { dataValues: { id: 1, title: 'The Godfather' } }; + + const error = new Error('Unauthorized'); + error.statusCode = 401; + jest + .spyOn(ResourceGetter.prototype, 'perform') + .mockRejectedValue(error); + + jest.spyOn(Film.prototype, 'save').mockReturnValue(record); + jest.spyOn(scopeManager, 'getScopeForUser').mockReturnValue({}); + + const body = { actor: 2 }; + + const resourceCreator = new ResourceCreator(Film, params, body, user); + await expect(resourceCreator.perform()).rejects.toThrow(error); + }); + }); + }); + describe('_getTargetKey', () => { describe('when association does not have entry in body', () => { it('should return null', async () => { diff --git a/test/services/resources-updater.test.js b/test/services/resources-updater.test.js new file mode 100644 index 000000000..06bcc9dd5 --- /dev/null +++ b/test/services/resources-updater.test.js @@ -0,0 +1,108 @@ +import { scopeManager } from 'forest-express'; +import Sequelize from 'sequelize'; +import createError from 'http-errors'; +import ResourceUpdater from '../../src/services/resource-updater'; +import ResourceGetter from '../../src/services/resource-getter'; +import QueryOptions from '../../src/services/query-options'; + +describe('services > resources-updater', () => { + const user = { renderingId: 1 }; + const params = { timezone: 'Europe/Paris' }; + + const buildModelMock = () => { + // Sequelize is created here without connection to a database + const sequelize = new Sequelize({ dialect: 'postgres' }); + + const Actor = sequelize.define('actor', {}); + const Film = sequelize.define('film', {}); + const ActorFilm = sequelize.define('ActorFilem', { + actorId: { + type: Sequelize.DataTypes.INTEGER, + primaryKey: true, + }, + filmId: { + type: Sequelize.DataTypes.INTEGER, + primaryKey: true, + }, + }); + + ActorFilm.belongsTo(Actor); + ActorFilm.belongsTo(Film); + + return { Actor, Film, ActorFilm }; + }; + + describe('when it update with a scope and it is not in scope anymore', () => { + it('should still return the record', async () => { + expect.assertions(1); + + const { Film } = buildModelMock(); + jest.spyOn(scopeManager, 'getScopeForUser').mockReturnValue({ aggregator: 'and', conditions: [{ field: 'name', operator: 'contains', value: 'Scope value' }] }); + + const record = { dataValues: { id: 1, title: 'The Godfather' }, validate: () => {}, save: () => {} }; + + jest.spyOn(record, 'validate'); + jest.spyOn(record, 'save'); + + const error = new Error('Record not found'); + error.statusCode = 404; + jest + .spyOn(ResourceGetter.prototype, 'perform') + .mockRejectedValue(error); + + jest.spyOn(QueryOptions.prototype, 'filterByConditionTree').mockResolvedValue(); + + const resourceUpdater = new ResourceUpdater(Film, params, { name: 'new name' }, user); + jest.spyOn(resourceUpdater._model, 'findOne').mockReturnValue(record); + + const result = await resourceUpdater.perform(); + + expect(result).toStrictEqual(record); + }); + }); + + describe('when it update with a scope but the record does not exist', () => { + it('should throw 404', async () => { + expect.assertions(1); + + const { Film } = buildModelMock(); + jest.spyOn(scopeManager, 'getScopeForUser').mockReturnValue({ aggregator: 'and', conditions: [{ field: 'name', operator: 'contains', value: 'Scope value' }] }); + + const record = null; + + jest.spyOn(QueryOptions.prototype, 'filterByConditionTree').mockResolvedValue(); + + const resourceUpdater = new ResourceUpdater(Film, { ...params, recordId: 2 }, { name: 'new name' }, user); + jest.spyOn(resourceUpdater._model, 'findOne').mockReturnValue(record); + + await expect(resourceUpdater.perform()).rejects.toThrow(createError(404, 'The film #2 does not exist.')); + }); + }); + + describe('when there is a scope and ResourcesGetter throw an error', () => { + it('should throw the error', async () => { + expect.assertions(1); + + const { Film } = buildModelMock(); + jest.spyOn(scopeManager, 'getScopeForUser').mockReturnValue({ aggregator: 'and', conditions: [{ field: 'name', operator: 'contains', value: 'Scope value' }] }); + + const record = { dataValues: { id: 1, title: 'The Godfather' }, validate: () => {}, save: () => {} }; + + jest.spyOn(record, 'validate'); + jest.spyOn(record, 'save'); + + const error = new Error('Unauthorized'); + error.statusCode = 401; + jest + .spyOn(ResourceGetter.prototype, 'perform') + .mockRejectedValue(error); + + jest.spyOn(QueryOptions.prototype, 'filterByConditionTree').mockResolvedValue(); + + const resourceUpdater = new ResourceUpdater(Film, { ...params, recordId: 2 }, { name: 'new name' }, user); + jest.spyOn(resourceUpdater._model, 'findOne').mockReturnValue(record); + + await expect(resourceUpdater.perform()).rejects.toThrow(error); + }); + }); +});