Skip to content

Commit

Permalink
[BUGFIX] Ne plus envoyer de 500 lors d'une recherche par ID invalide …
Browse files Browse the repository at this point in the history
…sur 2 pages PixAdmin (PIX-12576)

 #8962
  • Loading branch information
pix-service-auto-merge authored May 21, 2024
2 parents 73c5a5a + 36560f5 commit eeedc37
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 79 deletions.
34 changes: 20 additions & 14 deletions admin/app/routes/authenticated/organizations/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,26 @@ export default class ListRoute extends Route {
hideArchived: { refreshModel: true },
};

model(params) {
return this.store.query('organization', {
filter: {
id: params.id ? params.id.trim() : '',
name: params.name ? params.name.trim() : '',
type: params.type ? params.type.trim() : '',
externalId: params.externalId ? params.externalId.trim() : '',
hideArchived: params.hideArchived,
},
page: {
number: params.pageNumber,
size: params.pageSize,
},
});
async model(params) {
let model;
try {
model = await this.store.query('organization', {
filter: {
id: params.id ? params.id.trim() : '',
name: params.name ? params.name.trim() : '',
type: params.type ? params.type.trim() : '',
externalId: params.externalId ? params.externalId.trim() : '',
hideArchived: params.hideArchived,
},
page: {
number: params.pageNumber,
size: params.pageSize,
},
});
} catch (error) {
model = [];
}
return model;
}

resetController(controller, isExiting) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default class TargetProfileOrganizationsRoute extends Route {
}

async model(params) {
let organizations;
const targetProfile = this.modelFor('authenticated.target-profiles.target-profile');
const queryParams = {
targetProfileId: targetProfile.id,
Expand All @@ -33,7 +34,12 @@ export default class TargetProfileOrganizationsRoute extends Route {
externalId: params.externalId,
},
};
const organizations = await this.store.query('organization', queryParams);
try {
organizations = await this.store.query('organization', queryParams);
} catch (e) {
organizations = [];
}

return {
organizations,
targetProfile,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ function _findByListItemText(screen, text) {
return (
screen.getAllByRole('listitem').find((listitem) => {
const cleanListItemText = listitem.textContent.replace(/(\r\n|\n|\r)/gm, '').trim();
console.log(cleanListItemText);
return cleanListItemText === text;
}) || null
);
Expand Down
15 changes: 15 additions & 0 deletions admin/tests/unit/routes/authenticated/organizations/list_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ module('Unit | Route | authenticated/organizations/list', function (hooks) {
assert.ok(true);
});
});

module('when an error occurs', function () {
test('returns an empty array', async function (assert) {
// given
const params = {};

route.store.query = sinon.stub().rejects();

// when
const organizations = await route.model(params);

// then
assert.deepEqual(organizations, []);
});
});
});

module('#resetController', function (hooks) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { setupTest } from 'ember-qunit';
import { module, test } from 'qunit';
import sinon from 'sinon';

module('Unit | Route | authenticated/target-profiles/target-profile/organizations', function (hooks) {
setupTest(hooks);
Expand Down Expand Up @@ -50,4 +51,21 @@ module('Unit | Route | authenticated/target-profiles/target-profile/organization
});
});
});

module('#model', function () {
module('when an error occurs', function () {
test('returns an empty array', async function (assert) {
// given
const params = {};
route.modelFor = sinon.stub().returns({ id: '123' });
route.store.query = sinon.stub().rejects();

// when
const model = await route.model(params);

// then
assert.deepEqual(model.organizations, []);
});
});
});
});
4 changes: 2 additions & 2 deletions api/lib/application/target-profiles/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Joi from 'joi';

import { securityPreHandlers } from '../../../src/shared/application/security-pre-handlers.js';
import { identifiersType } from '../../../src/shared/domain/types/identifiers-type.js';
import { identifiersType, optionalIdentifiersType } from '../../../src/shared/domain/types/identifiers-type.js';
import { BadRequestError, sendJsonApiError } from '../http-errors.js';
import { targetProfileController } from './target-profile-controller.js';

Expand Down Expand Up @@ -96,7 +96,7 @@ const register = async function (server) {
id: identifiersType.targetProfileId,
}),
query: Joi.object({
'filter[id]': Joi.number().integer().empty('').allow(null).optional(),
'filter[id]': optionalIdentifiersType.organizationId,
'filter[name]': Joi.string().empty('').allow(null).optional(),
'filter[type]': Joi.string().empty('').allow(null).optional(),
'filter[external-id]': Joi.string().empty('').allow(null).optional(),
Expand Down
20 changes: 9 additions & 11 deletions api/src/shared/domain/types/identifiers-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,18 @@ const postgreSQLSequenceDefaultStart = 1;
const postgreSQLSequenceEnd = 2 ** 31 - 1;

const implementationType = {
positiveInteger32bits: Joi.number()
.integer()
.min(postgreSQLSequenceDefaultStart)
.max(postgreSQLSequenceEnd)
.required(),
alphanumeric255: Joi.string().max(255).required(),
alphanumeric: Joi.string().required(),
positiveInteger32bits: Joi.number().integer().min(postgreSQLSequenceDefaultStart).max(postgreSQLSequenceEnd),
alphanumeric255: Joi.string().max(255),
alphanumeric: Joi.string(),
};

const valuesToExport = {};
const paramsToExport = {};
const queryToExport = {};

function _assignValueToExport(array, implementationType) {
_.each(array, function (value) {
valuesToExport[value] = implementationType;
paramsToExport[value] = implementationType.required();
queryToExport[value] = implementationType.empty('').allow(null).optional();
});
}

Expand Down Expand Up @@ -70,9 +68,9 @@ _assignValueToExport(typesPositiveInteger32bits, implementationType.positiveInte
_assignValueToExport(typesAlphanumeric, implementationType.alphanumeric);
_assignValueToExport(typesAlphanumeric255, implementationType.alphanumeric255);

valuesToExport.positiveInteger32bits = {
paramsToExport.positiveInteger32bits = {
min: postgreSQLSequenceDefaultStart,
max: postgreSQLSequenceEnd,
};

export { valuesToExport as identifiersType };
export { paramsToExport as identifiersType, queryToExport as optionalIdentifiersType };
168 changes: 118 additions & 50 deletions api/tests/shared/unit/domain/types/identifiers-type_test.js
Original file line number Diff line number Diff line change
@@ -1,82 +1,150 @@
import { identifiersType } from '../../../../../src/shared/domain/types/identifiers-type.js';
import { identifiersType, optionalIdentifiersType } from '../../../../../src/shared/domain/types/identifiers-type.js';
import { expect } from '../../../../test-helper.js';
const { userId, competenceId } = identifiersType;
const { organizationId } = optionalIdentifiersType;

describe('Unit | Domain | Type | identifier-types', function () {
describe('#userId', function () {
context('when id is valid', function () {
it('should not reject', function () {
// given
const validId = 1;
context('identifiersType', function () {
describe('#userId', function () {
context('when id is valid', function () {
it('should not reject', function () {
// given
const validId = 1;

// when
const { error } = userId.validate(validId);

// then
expect(error).to.be.undefined;
});
});

context('when id is invalid', function () {
it('should reject outside of lower bound', async function () {
// given
const lowerBoundOutOfRangeId = 0;

// when
const { error } = userId.validate(lowerBoundOutOfRangeId);

// then
expect(error.message).to.equal('"value" must be greater than or equal to 1');
});

// when
const { error } = userId.validate(validId);
it('should reject outside of upper bound', async function () {
// given
const upperBoundOutOfRangeId = 2147483648;

// then
expect(error).to.be.undefined;
// when
const { error } = userId.validate(upperBoundOutOfRangeId);

// then
expect(error.message).to.equal('"value" must be less than or equal to 2147483647');
});
});
});

context('when id is invalid', function () {
it('should reject outside of lower bound', async function () {
// given
const lowerBoundOutOfRangeId = 0;
describe('#competenceId', function () {
context('when id is valid', function () {
it('should not reject', function () {
// given
const validId = '1234567890123456';

// when
const { error } = userId.validate(lowerBoundOutOfRangeId);
// when then
const { error } = competenceId.validate(validId);

// then
expect(error.message).to.equal('"value" must be greater than or equal to 1');
// then
expect(error).to.be.undefined;
});
});

it('should reject outside of upper bound', async function () {
// given
const upperBoundOutOfRangeId = 2147483648;
context('when id is invalid', function () {
it('should reject when empty', async function () {
// given
const emptyString = '';

// when
const { error } = competenceId.validate(emptyString);

// then
expect(error.message).to.equal('"value" is not allowed to be empty');
});

// when
const { error } = userId.validate(upperBoundOutOfRangeId);
it('should reject when too large', async function () {
// given
const tooLargeString = 'A'.repeat(256);

// then
expect(error.message).to.equal('"value" must be less than or equal to 2147483647');
// when
const { error } = competenceId.validate(tooLargeString);

// then
expect(error.message).to.equal('"value" length must be less than or equal to 255 characters long');
});
});
});
});

describe('#competenceId', function () {
context('when id is valid', function () {
it('should not reject', function () {
// given
const validId = '1234567890123456';
context('queryIdentifiersType', function () {
describe('#organizationId', function () {
context('when organizationId is null', function () {
it('should not throw', function () {
// given
const nullId = null;

// when then
const { error } = competenceId.validate(validId);
// when
const { error } = organizationId.validate(nullId);

// then
expect(error).to.be.undefined;
// then
expect(error).to.be.undefined;
});
});
});
context('when organizationId is empty', function () {
it('should not throw', function () {
// given
const emptyId = '';

// when
const { error } = organizationId.validate(emptyId);

context('when id is invalid', function () {
it('should reject when empty', async function () {
// given
const emptyString = '';
// then
expect(error).to.be.undefined;
});
});
context('when organizationId is undefined', function () {
it('should not throw', function () {
// given
const undefinedId = '';

// when
const { error } = competenceId.validate(emptyString);
// when
const { error } = organizationId.validate(undefinedId);

// then
expect(error.message).to.equal('"value" is not allowed to be empty');
// then
expect(error).to.be.undefined;
});
});
context('when organizationId is lower of range', function () {
it('should reject outside of lower bound', async function () {
// given
const lowerBoundOutOfRangeId = 0;

// when
const { error } = organizationId.validate(lowerBoundOutOfRangeId);

it('should reject when too large', async function () {
// given
const tooLargeString = 'A'.repeat(256);
// then
expect(error.message).to.equal('"value" must be greater than or equal to 1');
});
});
context('when organizationId is higher of range', function () {
it('should reject outside of upper bound', async function () {
// given
const upperBoundOutOfRangeId = 2147483648;

// when
const { error } = competenceId.validate(tooLargeString);
// when
const { error } = organizationId.validate(upperBoundOutOfRangeId);

// then
expect(error.message).to.equal('"value" length must be less than or equal to 255 characters long');
// then
expect(error.message).to.equal('"value" must be less than or equal to 2147483647');
});
});
});
});
Expand Down

0 comments on commit eeedc37

Please sign in to comment.