Merge pull request #43 in ~VLE2FE/definma-api from develop to master

* commit 'd28e9a1cc92c3c4dc1f00bfd762ec077b6c34472':
  changelog delete
  fixed deleting materials
  small material changes for easier UI implementation
  model now get deleted from user

api/material.yaml

@@ -20,7 +20,13 @@
             schema:
               type: array
               items:
-                $ref: 'api.yaml#/components/schemas/Material'
+                allOf:
+                  - $ref: 'api.yaml#/components/schemas/Material'
+                properties:
+                  status:
+                    type: string
+                    description: can be deleted/new/validated
+                    example: new
       401:
         $ref: 'api.yaml#/components/responses/401'
       500:

package-lock.json

@@ -385,14 +385,6 @@
       "integrity": "sha1-n7OjzzEyMoFR81PeRjLgHlIQK+o=",
       "dev": true
     },
-    "@szmarczak/http-timer": {
-      "version": "4.0.5",
-      "resolved": "https://r.cnpmjs.org/@szmarczak/http-timer/download/@szmarczak/http-timer-4.0.5.tgz",
-      "integrity": "sha1-v71QIR6d+lG6B9pYoUzf0zMgUVI=",
-      "requires": {
-        "defer-to-connect": "^2.0.0"
-      }
-    },
     "@types/bcrypt": {
       "version": "3.0.0",
       "resolved": "https://r.cnpmjs.org/@types/bcrypt/download/@types/bcrypt-3.0.0.tgz",
@@ -1297,11 +1289,6 @@
         "strip-bom": "^4.0.0"
       }
     },
-    "defer-to-connect": {
-      "version": "2.0.0",
-      "resolved": "https://r.cnpmjs.org/defer-to-connect/download/defer-to-connect-2.0.0.tgz",
-      "integrity": "sha1-g9axmdsEFZOshNeBtSIjCMz0wsE="
-    },
     "define-properties": {
       "version": "1.1.3",
       "resolved": "https://r.cnpmjs.org/define-properties/download/define-properties-1.1.3.tgz",
@@ -2603,6 +2590,15 @@
       "resolved": "https://r.cnpmjs.org/nocache/download/nocache-2.1.0.tgz",
       "integrity": "sha1-Egyf/sQ7Vymx1d6IzXGqdaC6SR8="
     },
+    "node-cron": {
+      "version": "2.0.3",
+      "resolved": "https://registry.npmjs.org/node-cron/-/node-cron-2.0.3.tgz",
+      "integrity": "sha512-eJI+QitXlwcgiZwNNSRbqsjeZMp5shyajMR81RZCqeW0ZDEj4zU9tpd4nTh/1JsBiKbF8d08FCewiipDmVIYjg==",
+      "requires": {
+        "opencollective-postinstall": "^2.0.0",
+        "tz-offset": "0.0.1"
+      }
+    },
     "node-environment-flags": {
       "version": "1.0.6",
       "resolved": "https://r.cnpmjs.org/node-environment-flags/download/node-environment-flags-1.0.6.tgz",
@@ -2958,6 +2954,11 @@
       "resolved": "https://r.cnpmjs.org/openapi-types/download/openapi-types-1.3.5.tgz",
       "integrity": "sha1-ZxjPvIV/5sbxRx9lsyveu5wQzkA="
     },
+    "opencollective-postinstall": {
+      "version": "2.0.3",
+      "resolved": "https://registry.npmjs.org/opencollective-postinstall/-/opencollective-postinstall-2.0.3.tgz",
+      "integrity": "sha512-8AV/sCtuzUeTo8gQK5qDZzARrulB3egtLzFgteqB2tcT4Mw7B8Kt7JcDHmltjz6FOAHsvTevk70gZEbhM4ZS9Q=="
+    },
     "os-tmpdir": {
       "version": "1.0.2",
       "resolved": "https://r.cnpmjs.org/os-tmpdir/download/os-tmpdir-1.0.2.tgz",
@@ -3936,6 +3937,11 @@
       "integrity": "sha1-mNYApevcOPQMsndSLxLcgA6eJfo=",
       "dev": true
     },
+    "tz-offset": {
+      "version": "0.0.1",
+      "resolved": "https://registry.npmjs.org/tz-offset/-/tz-offset-0.0.1.tgz",
+      "integrity": "sha512-kMBmblijHJXyOpKzgDhKx9INYU4u4E1RPMB0HqmKSgWG8vEcf3exEfLh4FFfzd3xdQOw9EuIy/cP0akY6rHopQ=="
+    },
     "undefsafe": {
       "version": "2.0.3",
       "resolved": "https://r.cnpmjs.org/undefsafe/download/undefsafe-2.0.3.tgz",

package.json

@@ -38,6 +38,7 @@
     "json2csv": "^5.0.1",
     "lodash": "^4.17.15",
     "mongoose": "^5.8.7",
+    "node-cron": "^2.0.3",
     "swagger-ui-dist": "^3.30.2"
   },
   "devDependencies": {

src/db.ts

@@ -2,12 +2,14 @@ import mongoose from 'mongoose';
 import cfenv from 'cfenv';
 import _ from 'lodash';
 import ChangelogModel from './models/changelog';
+import cron from 'node-cron';
 
 
 // database urls, prod db url is retrieved automatically
 const TESTING_URL = 'mongodb://localhost/dfopdb_test';
 const DEV_URL = 'mongodb://localhost/dfopdb';
 const debugging = true;
+const changelogKeepDays = 30;  // days to keep the changelog
 
 if (process.env.NODE_ENV !== 'production' && debugging) {
   mongoose.set('debug', true);  // enable mongoose debug
@@ -78,6 +80,16 @@ export default class db {
       this.state.db = mongoose.connection;
       done();
     });
+
+    if (mode !== 'test') {  // clear old changelog regularly
+      cron.schedule('0 0 * * *', () => {
+        ChangelogModel.deleteMany({_id: {$lt:  // id from time
+          Math.floor(new Date().getTime() / 1000 - changelogKeepDays * 24 * 60 * 60).toString(16) + '0000000000000000'
+        }}).log({method: 'scheduled changelog delete', url: '', authDetails: {}}).lean().exec(err => {
+          if (err) console.error(err);
+        });
+      });
+    }
   }
 
   static disconnect (done) {
@@ -137,31 +149,34 @@ export default class db {
   // changelog entry, expects (req, this (from query helper)) or (req, collection, conditions, data)
   static log(req, thisOrCollection, conditions = null, data = null) {
     if (! (conditions || data)) {  // (req, this)
+      console.log(11);
       data = thisOrCollection._update ? _.cloneDeep(thisOrCollection._update) : {};  // replace undefined with {}
+      // replace keys with a leading $
       Object.keys(data).forEach(key => {
         if (key[0] === '$') {
           data[key.substr(1)] = data[key];
           delete data[key];
         }
       });
-      new ChangelogModel({
+      console.log(thisOrCollection._conditions);
+      new ChangelogModel(this.logEscape(_.cloneDeep({
         action: req.method + ' ' + req.url,
         collection_name: thisOrCollection._collection.collectionName,
         conditions: thisOrCollection._conditions,
         data: data,
         user_id: req.authDetails.id ? req.authDetails.id : null
-      }).save(err => {
+      }))).save({validateBeforeSave: false}, err => {
         if (err) console.error(err);
       });
     }
     else {  // (req, collection, conditions, data)
-      new ChangelogModel({
+      new ChangelogModel(this.logEscape(_.cloneDeep({
         action: req.method + ' ' + req.url,
         collection_name: thisOrCollection,
         conditions: conditions,
         data: data,
         user_id: req.authDetails.id ? req.authDetails.id : null
-      }).save(err => {
+      }))).save(err => {
         if (err) console.error(err);
       });
     }
@@ -178,4 +193,17 @@ export default class db {
     });
     return object;
   }
+
+  private static logEscape(obj) {  // replace MongoDB control characters in keys
+    if (Object(obj) === obj && Object.keys(obj).length > 0) {
+      Object.keys(obj).forEach(key => {
+        const safeKey = key.replace(/[$.]/g, '');
+        obj[safeKey] = this.logEscape(obj[key]);
+        if (key !== safeKey) {
+          delete obj[key];
+        }
+      });
+    }
+    return obj;
+  }
 };

src/models/changelog.ts

@@ -3,9 +3,9 @@ import mongoose from 'mongoose';
 const ChangelogSchema = new mongoose.Schema({
   action: String,
   collection_name: String,
-  conditions: Object,
+  conditions: mongoose.Schema.Types.Mixed,
   data: Object,
   user_id: mongoose.Schema.Types.ObjectId
-}, {minimize: false});
+}, {minimize: false, strict: false});
 
 export default mongoose.model<any, mongoose.Model<any, any>>('changelog', ChangelogSchema);

src/routes/material.spec.ts

@@ -24,11 +24,12 @@ describe('/material', () => {
         const json = require('../test/db.json');
         should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'validated').length);
         should(res.body).matchEach(material => {
-          should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers');
+          should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status');
           should(material).have.property('_id').be.type('string');
           should(material).have.property('name').be.type('string');
           should(material).have.property('supplier').be.type('string');
           should(material).have.property('group').be.type('string');
+          should(material).have.property('status', 'validated');
           should(material.properties).have.property('material_template').be.type('string');
           should(material.numbers).be.instanceof(Array);
         });
@@ -46,11 +47,12 @@ describe('/material', () => {
         const json = require('../test/db.json');
         should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'validated').length);
         should(res.body).matchEach(material => {
-          should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers');
+          should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status');
           should(material).have.property('_id').be.type('string');
           should(material).have.property('name').be.type('string');
           should(material).have.property('supplier').be.type('string');
           should(material).have.property('group').be.type('string');
+          should(material).have.property('status', 'validated');
           should(material.properties).have.property('material_template').be.type('string');
           should(material.numbers).be.instanceof(Array);
         });
@@ -60,7 +62,7 @@ describe('/material', () => {
     it('allows filtering by state', done => {
       TestHelper.request(server, done, {
         method: 'get',
-        url: '/materials?status=new',
+        url: '/materials?status[]=new',
         auth: {basic: 'janedoe'},
         httpStatus: 200
       }).end((err, res) => {
@@ -68,24 +70,57 @@ describe('/material', () => {
         const json = require('../test/db.json');
         should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'new').length);
         should(res.body).matchEach(material => {
-          should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers');
+          should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status');
           should(material).have.property('_id').be.type('string');
           should(material).have.property('name').be.type('string');
           should(material).have.property('supplier').be.type('string');
           should(material).have.property('group').be.type('string');
+          should(material).have.property('status', 'new');
           should(material.properties).have.property('material_template').be.type('string');
           should(material.numbers).be.instanceof(Array);
         });
         done();
       });
     });
+    it('allows filtering by deleted state for admins', done => {
+      TestHelper.request(server, done, {
+        method: 'get',
+        url: '/materials?status[]=deleted',
+        auth: {basic: 'admin'},
+        httpStatus: 200
+      }).end((err, res) => {
+        if (err) return done(err);
+        const json = require('../test/db.json');
+        should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'deleted').length);
+        should(res.body).matchEach(material => {
+          should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status');
+          should(material).have.property('_id').be.type('string');
+          should(material).have.property('name').be.type('string');
+          should(material).have.property('supplier').be.type('string');
+          should(material).have.property('group').be.type('string');
+          should(material).have.property('status', 'deleted');
+          should(material.properties).have.property('material_template').be.type('string');
+          should(material.numbers).be.instanceof(Array);
+        });
+        done();
+      });
+    });
+    it('rejects filtering by deleted state for a write user', done => {
+      TestHelper.request(server, done, {
+        method: 'get',
+        url: '/materials?status[]=deleted',
+        auth: {basic: 'janedoe'},
+        httpStatus: 400,
+        res: {status: 'Invalid body format', details: '"status[0]" must be one of [validated, new]'}
+      });
+    });
     it('rejects an invalid state name', done => {
       TestHelper.request(server, done, {
         method: 'get',
-        url: '/materials?status=xxx',
+        url: '/materials?status[]=xxx',
         auth: {basic: 'janedoe'},
         httpStatus: 400,
-        res: {status: 'Invalid body format', details: '"status" must be one of [validated, new, all]'}
+        res: {status: 'Invalid body format', details: '"status[0]" must be one of [validated, new]'}
       });
     });
     it('rejects unauthorized requests', done => {

src/routes/material.ts

@@ -21,7 +21,8 @@ const router = express.Router();
 router.get('/materials', (req, res, next) => {
   if (!req.auth(res, ['read', 'write', 'dev', 'admin'], 'all')) return;
 
-  const {error, value: filters} = MaterialValidate.query(req.query);
+  const {error, value: filters} =
+    MaterialValidate.query(req.query, ['dev', 'admin'].indexOf(req.authDetails.level) >= 0);
   if (error) return res400(error, res);
 
   let conditions;
@@ -38,11 +39,12 @@ router.get('/materials', (req, res, next) => {
     conditions = {status: globals.status.val};
   }
 
-  MaterialModel.find(conditions).populate('group_id').populate('supplier_id').lean().exec((err, data) => {
+  MaterialModel.find(conditions).sort({name: 1}).populate('group_id').populate('supplier_id')
+    .lean().exec((err, data) => {
     if (err) return next(err);
 
     // validate all and filter null values from validation errors
-    res.json(_.compact(data.map(e => MaterialValidate.output(e))));
+    res.json(_.compact(data.map(e => MaterialValidate.output(e, true))));
   });
 });
 
@@ -120,7 +122,8 @@ router.delete('/material/' + IdValidate.parameter(), (req, res, next) => {
   if (!req.auth(res, ['write', 'dev', 'admin'], 'basic')) return;
 
   // check if there are still samples referencing this material
-  SampleModel.find({'material_id': new mongoose.Types.ObjectId(req.params.id)}).lean().exec((err, data) => {
+  SampleModel.find({'material_id': new mongoose.Types.ObjectId(req.params.id), status: {$ne: globals.status.del}})
+    .lean().exec((err, data) => {
     if (err) return next(err);
     if (data.length) {
       return res.status(400).json({status: 'Material still in use'});

src/routes/model.spec.ts

@@ -2,6 +2,8 @@ import should from 'should/as-function';
 import ModelFileModel from '../models/model_file';
 import TestHelper from "../test/helper";
 import ModelModel from '../models/model';
+import UserModel from '../models/user';
+import mongoose from 'mongoose';
 
 
 describe('/model', () => {
@@ -232,6 +234,22 @@ describe('/model', () => {
         });
       });
     });
+    it ('removes the model_id from all user.models', done => {
+      TestHelper.request(server, done, {
+        method: 'delete',
+        url: '/model/VN/Model%20A',
+        auth: {basic: 'admin'},
+        httpStatus: 200
+      }).end((err, res) => {
+        if (err) return done(err);
+        should(res.body).be.eql({status: 'OK'});
+        UserModel.find({models: mongoose.Types.ObjectId("120000000000000000000001")}).lean().exec((err, res) => {
+          if (err) return done(err);
+          should(res).have.lengthOf(0);
+          done();
+        });
+      });
+    });
     it('returns 404 for an unknown group', done => {
       TestHelper.request(server, done, {
         method: 'delete',

src/routes/model.ts

@@ -3,6 +3,7 @@ import bodyParser from 'body-parser';
 
 import ModelFileModel from '../models/model_file';
 import ModelModel from '../models/model';
+import UserModel from '../models/user';
 import _ from 'lodash';
 import ModelValidate from './validate/model';
 import res400 from './validate/res400';
@@ -54,7 +55,7 @@ router.post('/model/:group', (req, res, next) => {
           {$push: {models: model as never}}
         ).log(req).lean().exec(err => {
           if (err) return next(err);
-          res.json({status: 'OK'})
+          res.json({status: 'OK'});
         });
       }
     }
@@ -62,7 +63,7 @@ router.post('/model/:group', (req, res, next) => {
       new ModelModel({group: req.params.group, models: [model]}).save((err, data) => {
         if (err) return next(err);
         db.log(req, 'models', {_id: data._id}, data.toObject());
-        res.json({status: 'OK'})
+        res.json({status: 'OK'});
       });
     }
   });
@@ -77,21 +78,26 @@ router.delete('/model/:group(((?!file)[^\\/]+?))/:name', (req, res, next) => {
     if (!data || !data.models.find(e => e.name === req.params.name)) {
       return res.status(404).json({status: 'Not found'});
     }
-    if (data.models.length > 1) {  // only remove model
+    // delete all references in user.models
-      ModelModel.findOneAndUpdate(
+    UserModel.updateMany({}, {$pull: {models: data.models.find(e => e.name === req.params.name)._id as never}},
-        {group: req.params.group},
+      { multi: true }).log(req).lean().exec(err => {
-        {$pull: {models: data.models.find(e => e.name === req.params.name) as never}}
+      if (err) return next(err);
-      ).log(req).lean().exec(err => {
+      if (data.models.length > 1) {  // only remove model
-        if (err) return next(err);
+        ModelModel.findOneAndUpdate(
-        res.json({status: 'OK'})
+          {group: req.params.group},
-      });
+          {$pull: {models: data.models.find(e => e.name === req.params.name) as never}}
-    }
+        ).log(req).lean().exec(err => {
-    else {  // remove document
+          if (err) return next(err);
-      ModelModel.findOneAndDelete({group: req.params.group}).log(req).lean().exec(err => {
+          res.json({status: 'OK'})
-        if (err) return next(err);
+        });
-        res.json({status: 'OK'})
+      }
-      });
+      else {  // remove document
-    }
+        ModelModel.findOneAndDelete({group: req.params.group}).log(req).lean().exec(err => {
+          if (err) return next(err);
+          res.json({status: 'OK'})
+        });
+      }
+    });
   });
 });
 

src/routes/validate/material.ts

@@ -20,7 +20,10 @@ export default class MaterialValidate {  // validate input for material
       .items(
         Joi.string()
           .max(64)
-      )
+      ),
+
+    status: Joi.string()
+      .valid(...Object.values(globals.status))
   };
 
   static input (data, param) {  // validate input, set param to 'new' to make all attributes required
@@ -47,18 +50,22 @@ export default class MaterialValidate {  // validate input for material
     }
   }
 
-  static output (data) {  // validate output and strip unwanted properties, returns null if not valid
+  static output (data, status = false) {  // validate output and strip unwanted properties, returns null if not valid
     data = IdValidate.stringify(data);
     data.group = data.group_id.name;
     data.supplier = data.supplier_id.name;
-    const {value, error} = Joi.object({
+    const validate: any = {
       _id: IdValidate.get(),
       name: this.material.name,
       supplier: this.material.supplier,
       group: this.material.group,
       properties: this.material.properties,
       numbers: this.material.numbers
-    }).validate(data, {stripUnknown: true});
+    };
+    if (status) {
+      validate.status = this.material.status;
+    }
+    const {value, error} = Joi.object(validate).validate(data, {stripUnknown: true});
     return error !== undefined? null : value;
   }
 
@@ -83,9 +90,13 @@ export default class MaterialValidate {  // validate input for material
     });
   }
 
-  static query (data) {
+  static query (data, dev = false) {
+    const acceptedStatuses = [globals.status.val, globals.status.new];
+    if (dev) {  // dev and admin can also access deleted samples
+      acceptedStatuses.push(globals.status.del)
+    }
     return Joi.object({
-      status: Joi.string().valid(globals.status.val, globals.status.new, 'all')
+      status: Joi.array().items(Joi.string().valid(...acceptedStatuses)).default([globals.status.val])
     }).validate(data);
   }
 }