Skip to content

Commit

Permalink
fix: create and update with a scope when it is not found (#1107)
Browse files Browse the repository at this point in the history
  • Loading branch information
ShohanRahman committed May 24, 2024
1 parent 4198d59 commit 9a138c7
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 12 deletions.
24 changes: 18 additions & 6 deletions src/services/resource-creator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
}
}
}

Expand Down
20 changes: 15 additions & 5 deletions src/services/resource-updater.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}
}
}

Expand Down
52 changes: 51 additions & 1 deletion test/services/resource-creator.test.js
Original file line number Diff line number Diff line change
@@ -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 };
Expand Down Expand Up @@ -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 () => {
Expand Down
108 changes: 108 additions & 0 deletions test/services/resources-updater.test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
});

0 comments on commit 9a138c7

Please sign in to comment.