Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: create and update with a scope when it is not found #1107

Merged
merged 7 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
});
});
});
Loading