From 2aef24a97b41c7aa078a9e607ae6b5ac4f417db8 Mon Sep 17 00:00:00 2001
From: Jure Triglav <juretriglav@gmail.com>
Date: Mon, 10 Dec 2018 17:24:19 +0100
Subject: [PATCH] fix: various migration related fixes

---
 packages/base-model/src/index.js              |  7 ++++
 .../test/extended_manuscript_graphql_test.js  | 10 +++++-
 .../test/extended_manuscript_test.js          |  1 +
 .../test/manuscript_graphql_test.js           | 10 ++++--
 .../model-fragment/src/api_fragments.js       |  4 +--
 .../components/model-fragment/src/fragment.js |  1 +
 packages/db-manager/config/test.js            |  1 +
 .../db-manager/src/commands/add-collection.js |  3 +-
 packages/db-manager/src/validations.js        | 32 +++++--------------
 .../test/commands/add-collection.test.js      |  2 +-
 packages/server/test/helpers/authsome_mode.js |  6 ++--
 .../src/extended_fragment.js                  | 31 ++++++------------
 .../1543166818-add-columns-to-fragment.sql    |  3 +-
 packages/server/test/model_test.js            | 13 --------
 yarn.lock                                     |  2 +-
 15 files changed, 54 insertions(+), 72 deletions(-)

diff --git a/packages/base-model/src/index.js b/packages/base-model/src/index.js
index f839ea2d8..4d228c463 100644
--- a/packages/base-model/src/index.js
+++ b/packages/base-model/src/index.js
@@ -20,6 +20,11 @@ class BaseModel extends Model {
   }
 
   static get jsonSchema() {
+    // JSON schema validation is getting proper support for inheritance in
+    // its draft 8: https://github.com/json-schema-org/json-schema-spec/issues/556
+    // Until then, we're not using additionalProperties: false, and letting the
+    // database handle this bit of the integrity checks.
+
     let schema
 
     const mergeSchema = additionalSchema => {
@@ -58,11 +63,13 @@ class BaseModel extends Model {
           ],
         },
       },
+      additionalProperties: false,
     }
 
     if (schema) {
       return merge(baseSchema, schema)
     }
+
     return baseSchema
   }
 
diff --git a/packages/base-model/test/extended_manuscript_graphql_test.js b/packages/base-model/test/extended_manuscript_graphql_test.js
index cba6bd2bd..02cd9a282 100644
--- a/packages/base-model/test/extended_manuscript_graphql_test.js
+++ b/packages/base-model/test/extended_manuscript_graphql_test.js
@@ -1,7 +1,15 @@
 const path = require('path')
 
 const pathToComponent = path.resolve(__dirname, 'extended-data-model-component')
-process.env.NODE_CONFIG = `{"pubsweet":{"components":["@pubsweet/model-user", "@pubsweet/model-team", "${pathToComponent}"]}}`
+process.env.NODE_CONFIG = `{"pubsweet":{
+  "components":[
+    "@pubsweet/model-user",
+    "@pubsweet/model-team",
+    "@pubsweet/model-fragment",
+    "@pubsweet/model-collection",
+    "${pathToComponent}"
+  ]
+}}`
 global.NODE_CONFIG = null
 delete require.cache[require.resolve('config')]
 const { model: Manuscript } = require('./extended-data-model-component')
diff --git a/packages/base-model/test/extended_manuscript_test.js b/packages/base-model/test/extended_manuscript_test.js
index 1f0d3cea9..f27968767 100644
--- a/packages/base-model/test/extended_manuscript_test.js
+++ b/packages/base-model/test/extended_manuscript_test.js
@@ -6,6 +6,7 @@ process.env.NODE_CONFIG = `{"pubsweet":{
     "@pubsweet/model-user",
     "@pubsweet/model-team",
     "@pubsweet/model-fragment",
+    "@pubsweet/model-collection",
     "${pathToComponent}"
   ]
 }}`
diff --git a/packages/base-model/test/manuscript_graphql_test.js b/packages/base-model/test/manuscript_graphql_test.js
index eb232b81d..acf9930b4 100644
--- a/packages/base-model/test/manuscript_graphql_test.js
+++ b/packages/base-model/test/manuscript_graphql_test.js
@@ -1,8 +1,14 @@
 const path = require('path')
 
 const pathToComponent = path.resolve(__dirname, 'data-model-component')
-process.env.NODE_CONFIG = `{"pubsweet":{"components":["@pubsweet/model-user", "@pubsweet/model-team", "${pathToComponent}"]}}`
-
+process.env.NODE_CONFIG = `{"pubsweet":{
+  "components":[
+    "@pubsweet/model-user",
+    "@pubsweet/model-team",
+    "@pubsweet/model-fragment",
+    "${pathToComponent}"
+  ]
+}}`
 const { model: User } = require('@pubsweet/model-user')
 const { dbCleaner, api } = require('pubsweet-server/test')
 
diff --git a/packages/components/model-fragment/src/api_fragments.js b/packages/components/model-fragment/src/api_fragments.js
index 274ef1192..86e2fd9c5 100644
--- a/packages/components/model-fragment/src/api_fragments.js
+++ b/packages/components/model-fragment/src/api_fragments.js
@@ -23,11 +23,11 @@ const authBearerAndPublic = passport.authenticate(['bearer', 'anonymous'], {
 const FragmentsAPI = app => {
   const authsome = require('pubsweet-server/src/helpers/authsome')
 
-  const { Collection, AuthorizationError } = require('pubsweet-server')
+  const { AuthorizationError } = require('pubsweet-server')
 
   const { model: Team } = require('@pubsweet/model-team')
   const { model: User } = require('@pubsweet/model-user')
-  const { Fragment } = require('pubsweet-server/src/models')
+  const { Fragment, Collection } = require('pubsweet-server/src/models')
 
   // Create a fragment and update the collection with the fragment
   app.post(
diff --git a/packages/components/model-fragment/src/fragment.js b/packages/components/model-fragment/src/fragment.js
index 209f1fb01..d48f806c3 100644
--- a/packages/components/model-fragment/src/fragment.js
+++ b/packages/components/model-fragment/src/fragment.js
@@ -14,6 +14,7 @@ class Fragment extends BaseModel {
   static get schema() {
     return {
       required: ['fragmentType'],
+      type: 'object',
       properties: {
         fragmentType: { type: 'string' },
         fragments: {
diff --git a/packages/db-manager/config/test.js b/packages/db-manager/config/test.js
index bf988bd3e..d7f26fcdb 100644
--- a/packages/db-manager/config/test.js
+++ b/packages/db-manager/config/test.js
@@ -25,6 +25,7 @@ module.exports = {
       '@pubsweet/model-user',
       '@pubsweet/model-team',
       '@pubsweet/model-fragment',
+      '@pubsweet/model-collection',
     ],
   },
 }
diff --git a/packages/db-manager/src/commands/add-collection.js b/packages/db-manager/src/commands/add-collection.js
index 9cbbc7c6b..47ad2c7a7 100644
--- a/packages/db-manager/src/commands/add-collection.js
+++ b/packages/db-manager/src/commands/add-collection.js
@@ -1,8 +1,9 @@
 const logger = require('@pubsweet/logger')
-const { Collection } = require('pubsweet-server')
 
 module.exports = async (collectionData, fragment = null) => {
   const { model: User } = require('@pubsweet/model-user')
+  const { Collection } = require('pubsweet-server/src/models')
+
   logger.info('Creating collection')
 
   const collection = await new Collection(collectionData).save()
diff --git a/packages/db-manager/src/validations.js b/packages/db-manager/src/validations.js
index 5692a8860..fea22e3ae 100644
--- a/packages/db-manager/src/validations.js
+++ b/packages/db-manager/src/validations.js
@@ -1,30 +1,14 @@
 const Joi = require('joi')
-const config = require('config')
-
-let appValidations
-try {
-  appValidations = require(config.validations)
-} catch (err) {
-  appValidations = []
-}
-const schemas = require('pubsweet-server/src/models/validations')(
-  appValidations,
-)
-const _ = require('lodash/fp')
 
 const userSchema = Joi.object({
-  username: _.get('user.username', schemas) || Joi.string().required(),
-  email:
-    _.get('user.email', schemas) ||
-    Joi.string()
-      .email()
-      .required(),
-  password:
-    _.get('user.password', schemas) ||
-    Joi.string()
-      .min(8)
-      .max(60)
-      .required(),
+  username: Joi.string().required(),
+  email: Joi.string()
+    .email()
+    .required(),
+  password: Joi.string()
+    .min(8)
+    .max(60)
+    .required(),
   admin: Joi.boolean().optional(),
 })
 
diff --git a/packages/db-manager/test/commands/add-collection.test.js b/packages/db-manager/test/commands/add-collection.test.js
index 975f7678c..0201c77f5 100644
--- a/packages/db-manager/test/commands/add-collection.test.js
+++ b/packages/db-manager/test/commands/add-collection.test.js
@@ -1,5 +1,5 @@
 const { addCollection, createTables } = require('../../src')
-const { Collection, Fragment } = require('pubsweet-server')
+const { Collection, Fragment } = require('pubsweet-server/src/models')
 const { model: User } = require('@pubsweet/model-user')
 
 describe('add-collection', () => {
diff --git a/packages/server/test/helpers/authsome_mode.js b/packages/server/test/helpers/authsome_mode.js
index 4194aa102..24fcac839 100644
--- a/packages/server/test/helpers/authsome_mode.js
+++ b/packages/server/test/helpers/authsome_mode.js
@@ -36,7 +36,7 @@ async function teamPermissions(user, operation, object, context) {
 
 function unauthenticatedUser(operation, object) {
   // Public/unauthenticated users can GET /collections, filtered by 'published'
-  if (operation === 'GET' && object && object.path === '/collections') {
+  if (operation === 'GET' && object && object.path === '/api/collections') {
     return {
       filter: collections =>
         collections.filter(collection => collection.published),
@@ -47,7 +47,7 @@ function unauthenticatedUser(operation, object) {
   if (
     operation === 'GET' &&
     object &&
-    object.path === '/collections/:id/fragments'
+    object.path === '/api/collections/:id/fragments'
   ) {
     return {
       filter: fragments => fragments.filter(fragment => fragment.published),
@@ -82,7 +82,7 @@ function unauthenticatedUser(operation, object) {
 
 async function authenticatedUser(user, operation, object, context) {
   // Allow the authenticated user to POST a collection (but not with a 'filtered' property)
-  if (operation === 'POST' && object.path === '/collections') {
+  if (operation === 'POST' && object.path === '/api/collections') {
     return {
       filter: collection => omit(collection, 'filtered'),
     }
diff --git a/packages/server/test/model-extended-fragment/src/extended_fragment.js b/packages/server/test/model-extended-fragment/src/extended_fragment.js
index e8fb055a3..557ef9767 100644
--- a/packages/server/test/model-extended-fragment/src/extended_fragment.js
+++ b/packages/server/test/model-extended-fragment/src/extended_fragment.js
@@ -3,28 +3,15 @@ const { model: Fragment } = require('@pubsweet/model-fragment')
 class ExtendedFragment extends Fragment {
   static get schema() {
     return {
-      required: ['fragmentType'],
-      anyOf: [
-        {
-          type: 'object',
-          properties: {
-            fragmentType: { type: 'string', const: 'blogpost' },
-            source: { type: 'string' },
-            kind: { type: ['string', 'null'] },
-            title: { type: 'string' },
-            presentation: { type: 'string' },
-            published: { type: ['boolean', 'null'] },
-            filtered: { type: 'string' },
-          },
-        },
-        {
-          type: 'object',
-          properties: {
-            fragmentType: { type: 'string', const: 'file' },
-            path: { type: 'string' },
-          },
-        },
-      ],
+      properties: {
+        fragmentType: { type: 'string', const: 'blogpost' },
+        source: { type: 'string' },
+        kind: { type: ['string', 'null'] },
+        title: { type: 'string' },
+        presentation: { type: 'string' },
+        published: { type: ['boolean', 'null'] },
+        filtered: { type: ['string', 'null'] },
+      },
     }
   }
 }
diff --git a/packages/server/test/model-extended-fragment/src/migrations/1543166818-add-columns-to-fragment.sql b/packages/server/test/model-extended-fragment/src/migrations/1543166818-add-columns-to-fragment.sql
index 3434573fd..439ec835d 100644
--- a/packages/server/test/model-extended-fragment/src/migrations/1543166818-add-columns-to-fragment.sql
+++ b/packages/server/test/model-extended-fragment/src/migrations/1543166818-add-columns-to-fragment.sql
@@ -4,5 +4,4 @@ ADD COLUMN kind TEXT,
 ADD COLUMN title TEXT,
 ADD COLUMN presentation TEXT,
 ADD COLUMN published BOOLEAN,
-ADD COLUMN filtered TEXT,
-ADD COLUMN path TEXT;
\ No newline at end of file
+ADD COLUMN filtered TEXT;
\ No newline at end of file
diff --git a/packages/server/test/model_test.js b/packages/server/test/model_test.js
index d9724b464..5d95d8789 100644
--- a/packages/server/test/model_test.js
+++ b/packages/server/test/model_test.js
@@ -90,17 +90,4 @@ describe('Model', () => {
       email: 'test@example.com',
     })
   })
-
-  it.skip('turns an object selector into SQL clauses', () => {
-    expect(User.selectorToSql({ foo: 'bar', 'do.re.mi': 'fa so la' })).toEqual([
-      "data->>'foo' = ?",
-      "data->'do'->'re'->>'mi' = ?",
-    ])
-  })
-
-  it.skip('escapes naughty names', () => {
-    expect(
-      User.selectorToSql({ "Robert'); DROP TABLE Students; --": '' }),
-    ).toEqual(["data->>'Robert''); DROP TABLE Students; --' = ?"])
-  })
 })
diff --git a/yarn.lock b/yarn.lock
index 7c49cfe1c..5a20d3286 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -12582,7 +12582,7 @@ promise@^7.1.1:
   dependencies:
     asap "~2.0.3"
 
-prompt@^1.0.0, prompt@flatiron/prompt#1c95d1d8d333b5fbc13fa5f0619f3dcf0d514f87, "prompt@github:flatiron/prompt#1c95d1d8d333b5fbc13fa5f0619f3dcf0d514f87":
+prompt@^1.0.0, prompt@flatiron/prompt#1c95d1d8d333b5fbc13fa5f0619f3dcf0d514f87:
   version "1.0.0"
   resolved "https://codeload.github.com/flatiron/prompt/tar.gz/1c95d1d8d333b5fbc13fa5f0619f3dcf0d514f87"
   dependencies:
-- 
GitLab