From 74080d0902da66aa2256c79892cd5470d2894ccc Mon Sep 17 00:00:00 2001 From: VLE2FE Date: Tue, 2 Jun 2020 10:24:22 +0200 Subject: [PATCH] only allowed latest template version and allowed admin to set sample number --- api/sample.yaml | 9 ++++- api/schemas.yaml | 1 + src/index.ts | 6 +-- src/routes/measurement.spec.ts | 24 ++++++++++- src/routes/measurement.ts | 8 ++++ src/routes/sample.spec.ts | 74 +++++++++++++++++++++++++++++++++- src/routes/sample.ts | 32 +++++++++++++-- src/routes/template.spec.ts | 3 +- src/routes/validate/sample.ts | 11 +++++ src/test/db.json | 55 ++++++++++++++++++------- 10 files changed, 194 insertions(+), 29 deletions(-) diff --git a/api/sample.yaml b/api/sample.yaml index d074172..eae0ddc 100644 --- a/api/sample.yaml +++ b/api/sample.yaml @@ -170,7 +170,7 @@ /sample/new: post: summary: add sample - description: 'Auth: basic, levels: write, maintain, dev, admin' + description: 'Auth: basic, levels: write, maintain, dev, admin. Number property is only for admin when adding existing samples' x-doc: 'Adds status: 0 automatically' tags: - /sample @@ -181,7 +181,12 @@ content: application/json: schema: - $ref: 'api.yaml#/components/schemas/Sample' + allOf: + - $ref: 'api.yaml#/components/schemas/Sample' + properties: + number: + type: string + readOnly: false responses: 200: description: samples details diff --git a/api/schemas.yaml b/api/schemas.yaml index c4814b7..21ceddf 100644 --- a/api/schemas.yaml +++ b/api/schemas.yaml @@ -69,6 +69,7 @@ Sample: relation: type: string example: part to this sample + SampleDetail: allOf: - $ref: 'api.yaml#/components/schemas/_Id' diff --git a/src/index.ts b/src/index.ts index 7dda199..3776c34 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,14 +7,12 @@ import db from './db'; // TODO: changelog // TODO: check executing index.js/move everything needed into dist -// TODO: One condition per sample // TODO: validation: VZ, Humidity: min/max value, DPT: filename -// TODO: condition values not needed on initial add -// TODO: add multiple samples at once -// TODO: coverage +// TODO: add multiple samples at once (only GUI) // TODO: think about the display of deleted/new samples and validation in data and UI // TODO: improve error coverage // TODO: guess properties from material name in UI +// TODO: mongodb user // tell if server is running in debug or production environment console.info(process.env.NODE_ENV === 'production' ? '===== PRODUCTION =====' : process.env.NODE_ENV === 'test' ? '' :'===== DEVELOPMENT ====='); diff --git a/src/routes/measurement.spec.ts b/src/routes/measurement.spec.ts index 8bef49b..25cc5e9 100644 --- a/src/routes/measurement.spec.ts +++ b/src/routes/measurement.spec.ts @@ -586,14 +586,24 @@ describe('/measurement', () => { done(); }); }); + it('rejects no values', done => { + TestHelper.request(server, done, { + method: 'post', + url: '/measurement/new', + auth: {basic: 'janedoe'}, + httpStatus: 400, + req: {sample_id: '400000000000000000000001', values: {}, measurement_template: '300000000000000000000002'}, + res: {status: 'At least one value is required'} + }); + }); it('rejects a value not in the value range', done => { TestHelper.request(server, done, { method: 'post', url: '/measurement/new', auth: {basic: 'janedoe'}, httpStatus: 400, - req: {sample_id: '400000000000000000000001', values: {val1: 4}, measurement_template: '300000000000000000000003'}, - res: {status: 'Invalid body format', details: '"val1" must be one of [1, 2, 3, null]'} + req: {sample_id: '400000000000000000000001', values: {val2: 5}, measurement_template: '300000000000000000000004'}, + res: {status: 'Invalid body format', details: '"val2" must be one of [1, 2, 3, 4, null]'} }); }); it('rejects a value below minimum range', done => { @@ -664,6 +674,16 @@ describe('/measurement', () => { done(); }); }); + it('rejects an old version of a measurement template', done => { + TestHelper.request(server, done, { + method: 'post', + url: '/measurement/new', + auth: {basic: 'janedoe'}, + httpStatus: 400, + req: {sample_id: '400000000000000000000001', values: {val1: 2}, measurement_template: '300000000000000000000003'}, + res: {status: 'Old template version not allowed'} + }); + }); it('rejects an API key', done => { TestHelper.request(server, done, { method: 'post', diff --git a/src/routes/measurement.ts b/src/routes/measurement.ts index 2b94060..a1b3a94 100644 --- a/src/routes/measurement.ts +++ b/src/routes/measurement.ts @@ -130,6 +130,14 @@ async function templateCheck (measurement, param, res, next) { // validate meas // fill not given values for new measurements if (param === 'new') { + // get all template versions and check if given is latest + const templateVersions = await MeasurementTemplateModel.find({first_id: templateData.first_id}).sort({version: -1}).lean().exec().catch(err => next(err)) as any; + if (templateVersions instanceof Error) return false; + if (measurement.measurement_template !== templateVersions[0]._id.toString()) { // template not latest + res.status(400).json({status: 'Old template version not allowed'}); + return false; + } + if (Object.keys(measurement.values).length === 0) { res.status(400).json({status: 'At least one value is required'}); return false diff --git a/src/routes/sample.spec.ts b/src/routes/sample.spec.ts index f2fdd36..baa46fe 100644 --- a/src/routes/sample.spec.ts +++ b/src/routes/sample.spec.ts @@ -454,6 +454,16 @@ describe('/sample', () => { res: {status: 'Color not available for material'} }); }); + it('rejects an undefined color for the same material', done => { + TestHelper.request(server, done, { + method: 'put', + url: '/sample/400000000000000000000001', + auth: {basic: 'janedoe'}, + httpStatus: 400, + req: {type: 'part', color: 'signalviolet', batch: '114531', notes: {comment: 'Testcomment', sample_references: [{sample_id: '400000000000000000000003', relation: 'part to this sample'}]}}, + res: {status: 'Color not available for material'} + }); + }); it('rejects an unknown material id', done => { TestHelper.request(server, done, { method: 'put', @@ -573,6 +583,26 @@ describe('/sample', () => { res: {_id: '400000000000000000000006', number: 'Rng36', type: 'granulate', color: 'black', batch: '', condition: {}, material_id: '100000000000000000000004', note_id: null, user_id: '000000000000000000000002'} }); }); + it('rejects an old version of a condition template', done => { + TestHelper.request(server, done, { + method: 'put', + url: '/sample/400000000000000000000001', + auth: {basic: 'janedoe'}, + httpStatus: 400, + req: {condition: {p1: 36, condition_template: '200000000000000000000004'}}, + res: {status: 'Old template version not allowed'} + }); + }); + it('allows keeping an old version of a condition template', done => { + TestHelper.request(server, done, { + method: 'put', + url: '/sample/400000000000000000000004', + auth: {basic: 'admin'}, + httpStatus: 200, + req: {condition: {p1: 36, condition_template: '200000000000000000000004'}}, + res: {_id: '400000000000000000000004', number: '32', type: 'granulate', color: 'black', batch: '1653000308', condition: {p1: 36, condition_template: '200000000000000000000004'}, material_id: '100000000000000000000005', note_id: '500000000000000000000003', user_id: '000000000000000000000003'} + }); + }); it('rejects an changing back to an empty condition', done => { TestHelper.request(server, done, { method: 'put', @@ -1108,7 +1138,7 @@ describe('/sample', () => { res: {status: 'Material not available'} }); }); - it('rejects a sample number', done => { + it('rejects a sample number for a write user', done => { TestHelper.request(server, done, { method: 'post', url: '/sample/new', @@ -1118,6 +1148,38 @@ describe('/sample', () => { res: {status: 'Invalid body format', details: '"number" is not allowed'} }); }); + it('allows a sample number for an admin user', done => { + TestHelper.request(server, done, { + method: 'post', + url: '/sample/new', + auth: {basic: 'admin'}, + httpStatus: 200, + req: {number: 'Rng34', color: 'black', type: 'granulate', batch: '1560237365', material_id: '100000000000000000000001', notes: {comment: 'Testcomment', sample_references: [{sample_id: '400000000000000000000003', relation: 'part to this sample'}]}}, + }).end((err, res) => { + if (err) return done (err); + should(res.body).have.only.keys('_id', 'number', 'color', 'type', 'batch', 'condition', 'material_id', 'note_id', 'user_id'); + should(res.body).have.property('_id').be.type('string'); + should(res.body).have.property('number', 'Rng34'); + should(res.body).have.property('color', 'black'); + should(res.body).have.property('type', 'granulate'); + should(res.body).have.property('batch', '1560237365'); + should(res.body).have.property('condition', {}); + should(res.body).have.property('material_id', '100000000000000000000001'); + should(res.body).have.property('note_id').be.type('string'); + should(res.body).have.property('user_id', '000000000000000000000003'); + done(); + }); + }); + it('rejects an existing sample number for an admin user', done => { + TestHelper.request(server, done, { + method: 'post', + url: '/sample/new', + auth: {basic: 'admin'}, + httpStatus: 400, + req: {number: 'Rng33', color: 'black', type: 'granulate', batch: '1560237365', material_id: '100000000000000000000001', notes: {comment: 'Testcomment', sample_references: [{sample_id: '400000000000000000000003', relation: 'part to this sample'}]}}, + res: {status: 'Sample number already taken'} + }); + }); it('rejects an invalid sample reference', done => { TestHelper.request(server, done, { method: 'post', @@ -1208,6 +1270,16 @@ describe('/sample', () => { res: {status: 'Condition template not available'} }); }); + it('rejects an old version of a condition template', done => { + TestHelper.request(server, done, { + method: 'post', + url: '/sample/new', + auth: {basic: 'janedoe'}, + httpStatus: 400, + req: {color: 'black', type: 'granulate', batch: '1560237365', condition: {p1: 36, condition_template: '200000000000000000000004'}, material_id: '100000000000000000000001', notes: {comment: 'Testcomment', sample_references: [{sample_id: '400000000000000000000003', relation: 'part to this sample'}]}}, + res: {status: 'Old template version not allowed'} + }); + }); it('rejects a missing color', done => { TestHelper.request(server, done, { method: 'post', diff --git a/src/routes/sample.ts b/src/routes/sample.ts index e741c4a..15d2a62 100644 --- a/src/routes/sample.ts +++ b/src/routes/sample.ts @@ -90,7 +90,7 @@ router.put('/sample/' + IdValidate.parameter(), (req, res, next) => { } if (sample.hasOwnProperty('condition') && !(_.isEmpty(sample.condition) && _.isEmpty(sampleData.condition))) { // do not execute check if condition is and was empty - if (!await conditionCheck(sample.condition, 'change', res, next)) return; + if (!await conditionCheck(sample.condition, 'change', res, next, sampleData.condition.condition_template.toString() !== sample.condition.condition_template)) return; } if (sample.hasOwnProperty('notes')) { @@ -217,7 +217,7 @@ router.post('/sample/new', async (req, res, next) => { req.body.condition = {}; } - const {error, value: sample} = SampleValidate.input(req.body, 'new'); + const {error, value: sample} = SampleValidate.input(req.body, 'new' + (req.authDetails.level === 'admin' ? '-admin' : '')); if (error) return res400(error, res); if (!await materialCheck(sample, res, next)) return; @@ -232,7 +232,12 @@ router.post('/sample/new', async (req, res, next) => { } sample.status = globals.status.new; // set status to new - sample.number = await numberGenerate(sample, req, res, next); + if (sample.hasOwnProperty('number')) { + if (!await numberCheck(sample, res, next)) return; + } + else { + sample.number = await numberGenerate(sample, req, res, next); + } if (!sample.number) return; await new NoteModel(sample.notes).save((err, data) => { // save notes @@ -272,6 +277,15 @@ async function numberGenerate (sample, req, res, next) { // generate number in return req.authDetails.location + (sampleData ? Number(sampleData.number.replace(/[^0-9]+/g, '')) + 1 : 1); } +async function numberCheck(sample, res, next) { + const sampleData = await SampleModel.findOne({number: sample.number}).lean().exec().catch(err => {next(err); return false;}); + if (sampleData) { // found entry with sample number + res.status(400).json({status: 'Sample number already taken'}); + return false + } + return true; +} + async function materialCheck (sample, res, next, id = sample.material_id) { // validate material_id and color, returns false if invalid const materialData = await MaterialModel.findById(id).lean().exec().catch(err => next(err)) as any; if (materialData instanceof Error) return false; @@ -286,7 +300,7 @@ async function materialCheck (sample, res, next, id = sample.material_id) { // return true; } -async function conditionCheck (condition, param, res, next) { // validate treatment template, returns false if invalid, otherwise template data +async function conditionCheck (condition, param, res, next, checkVersion = true) { // validate treatment template, returns false if invalid, otherwise template data if (!condition.condition_template || !IdValidate.valid(condition.condition_template)) { // template id not found res.status(400).json({status: 'Condition template not available'}); return false; @@ -298,6 +312,16 @@ async function conditionCheck (condition, param, res, next) { // validate treat return false; } + if (checkVersion) { + // get all template versions and check if given is latest + const conditionVersions = await ConditionTemplateModel.find({first_id: conditionData.first_id}).sort({version: -1}).lean().exec().catch(err => next(err)) as any; + if (conditionVersions instanceof Error) return false; + if (condition.condition_template !== conditionVersions[0]._id.toString()) { // template not latest + res.status(400).json({status: 'Old template version not allowed'}); + return false; + } + } + // validate parameters const {error, value: ignore} = ParametersValidate.input(_.omit(condition, 'condition_template'), conditionData.parameters, param); if (error) {res400(error, res); return false;} diff --git a/src/routes/template.spec.ts b/src/routes/template.spec.ts index e69b480..cbc481a 100644 --- a/src/routes/template.spec.ts +++ b/src/routes/template.spec.ts @@ -4,7 +4,6 @@ import TemplateConditionModel from '../models/condition_template'; import TemplateMeasurementModel from '../models/measurement_template'; import TestHelper from "../test/helper"; -// TODO: do not allow usage of old templates for new samples describe('/template', () => { let server; @@ -644,7 +643,7 @@ describe('/template', () => { req: {parameters: [{name: 'weight %', range: {}}]} }).end((err, res) => { if (err) return done(err); - should(_.omit(res.body, '_id')).be.eql({name: 'kf', version: 3, parameters: [{name: 'weight %', range: {}}]}); + should(_.omit(res.body, '_id')).be.eql({name: 'kf', version: 2, parameters: [{name: 'weight %', range: {}}]}); done(); }); }); diff --git a/src/routes/validate/sample.ts b/src/routes/validate/sample.ts index 93b86b1..9cb8cbb 100644 --- a/src/routes/validate/sample.ts +++ b/src/routes/validate/sample.ts @@ -67,6 +67,17 @@ export default class SampleValidate { notes: this.sample.notes, }).validate(data); } + else if (param === 'new-admin') { + return Joi.object({ + number: this.sample.number.required(), + color: this.sample.color.required(), + type: this.sample.type.required(), + batch: this.sample.batch.required(), + condition: this.sample.condition.required(), + material_id: IdValidate.get().required(), + notes: this.sample.notes.required() + }).validate(data); + } else { return{error: 'No parameter specified!', value: {}}; } diff --git a/src/test/db.json b/src/test/db.json index 4ef811c..ea2dd10 100644 --- a/src/test/db.json +++ b/src/test/db.json @@ -59,9 +59,8 @@ "color": "black", "batch": "1653000308", "condition": { - "material": "hot air", - "weeks": 5, - "condition_template": {"$oid":"200000000000000000000001"} + "p1": 44, + "condition_template": {"$oid":"200000000000000000000004"} }, "material_id": {"$oid":"100000000000000000000005"}, "note_id": {"$oid":"500000000000000000000003"}, @@ -447,24 +446,37 @@ "__v": 0 }, { - "_id": {"$oid":"200000000000000000000002"}, - "first_id": {"$oid":"200000000000000000000001"}, - "name": "heat treatment 2", - "version": 2, + "_id": {"$oid":"200000000000000000000003"}, + "first_id": {"$oid":"200000000000000000000003"}, + "name": "raw material", + "version": 1, + "parameters": [ + ], + "__v": 0 + }, + { + "_id": {"$oid":"200000000000000000000004"}, + "first_id": {"$oid":"200000000000000000000004"}, + "name": "old condition", + "version": 1, "parameters": [ { - "name": "material", + "name": "p1", "range": {} } ], "__v": 0 }, { - "_id": {"$oid":"200000000000000000000003"}, - "first_id": {"$oid":"200000000000000000000003"}, - "name": "raw material", - "version": 1, + "_id": {"$oid":"200000000000000000000005"}, + "first_id": {"$oid":"200000000000000000000004"}, + "name": "new condition", + "version": 2, "parameters": [ + { + "name": "p11", + "range": {} + } ], "__v": 0 } @@ -487,9 +499,9 @@ }, { "_id": {"$oid":"300000000000000000000002"}, - "first_id": {"$oid":"300000000000000000000001"}, + "first_id": {"$oid":"300000000000000000000002"}, "name": "kf", - "version": 2, + "version": 1, "parameters": [ { "name": "weight %", @@ -522,6 +534,21 @@ } ], "__v": 0 + }, + { + "_id": {"$oid":"300000000000000000000004"}, + "first_id": {"$oid":"300000000000000000000003"}, + "name": "mt 31", + "version": 2, + "parameters": [ + { + "name": "val2", + "range": { + "values": [1,2,3,4] + } + } + ], + "__v": 0 } ], "users": [