Merge branch 'development' into release

app/api/entities/specs/entities.spec.js

@@ -843,7 +843,10 @@ describe('entities', () => {
         ['shared'],
         {
           diffMetadata: {
-            multiselect: { added: [{ value: 'country_two' }], removed: [{ value: 'country_one' }] },
+            multiselect: {
+              added: [{ value: 'country_two' }],
+              removed: [{ value: 'country_one' }],
+            },
           },
         },
         { language: 'en' }

app/api/migrations/migrations/39-remove_wrong_text_references/index.js

@@ -0,0 +1,47 @@
+/* eslint-disable no-await-in-loop */
+export default {
+  delta: 39,
+
+  name: 'remove_wrong_text_references',
+
+  description: 'remove wrongly created text references and log them',
+
+  async up(db) {
+    process.stdout.write(`${this.name}...\r\n`);
+
+    await db
+      .collection('connections')
+      .updateMany(
+        { 'reference.text': { $exists: false }, 'reference.selectionRectangles': { $size: 0 } },
+        { $unset: { reference: '' } }
+      );
+
+    const cursor = await db
+      .collection('connections')
+      .find({ 'reference.text': { $exists: true }, 'reference.selectionRectangles': { $size: 0 } });
+
+    while (await cursor.hasNext()) {
+      const connection = await cursor.next();
+
+      const connectionsOnHub = await db
+        .collection('connections')
+        .find({ hub: connection.hub })
+        .toArray();
+
+      if (connectionsOnHub.length === 2) {
+        await db
+          .collection('connections')
+          .deleteMany({ _id: { $in: connectionsOnHub.map(c => c._id) } });
+
+        connectionsOnHub.forEach(c => {
+          process.stdout.write(JSON.stringify(c, null, ' '));
+          process.stdout.write('\r\n');
+        });
+      } else {
+        await db.collection('connections').deleteOne(connection);
+        process.stdout.write(JSON.stringify(connection, null, ' '));
+        process.stdout.write('\r\n');
+      }
+    }
+  },
+};

app/api/migrations/migrations/39-remove_wrong_text_references/specs/39-remove_wrong_text_references.spec.js

@@ -0,0 +1,67 @@
+import testingDB from 'api/utils/testing_db';
+import migration from '../index.js';
+import fixtures from './fixtures.js';
+
+describe('migration remove_wrong_text_references', () => {
+  beforeEach(async () => {
+    spyOn(process.stdout, 'write');
+    await testingDB.clearAllAndLoad(fixtures);
+    await migration.up(testingDB.mongodb);
+  });
+
+  afterAll(async () => {
+    await testingDB.disconnect();
+  });
+
+  it('should have a delta number', () => {
+    expect(migration.delta).toBe(39);
+  });
+
+  it('should remove remove all connections with text but empty selectionRectangles', async () => {
+    const connections = await testingDB.mongodb
+      .collection('connections')
+      .find({ 'reference.text': { $exists: true } })
+      .toArray();
+
+    expect(connections).toEqual([
+      expect.objectContaining({ reference: { text: 'reference 1', selectionRectangles: [{}] } }),
+      expect.objectContaining({ reference: { text: 'reference 2', selectionRectangles: [{}] } }),
+    ]);
+  });
+
+  it('should remove references belonging to deleted references hubs (when only one remains)', async () => {
+    const connections = await testingDB.mongodb
+      .collection('connections')
+      .find({ hub: { $exists: true } })
+      .toArray();
+
+    expect(connections).toEqual([
+      expect.objectContaining({ hub: 'hub3' }),
+      expect.objectContaining({ hub: 'hub3' }),
+    ]);
+  });
+
+  it('should log the removed connections to stdout', async () => {
+    expect(process.stdout.write).toHaveBeenCalledWith(expect.stringMatching('wrong reference 1'));
+    expect(process.stdout.write).toHaveBeenCalledWith(
+      expect.stringMatching('wrong reference 1 partner')
+    );
+    expect(process.stdout.write).toHaveBeenCalledWith(expect.stringMatching('wrong reference 2'));
+    expect(process.stdout.write).toHaveBeenCalledWith(
+      expect.stringMatching('wrong reference 2 partner')
+    );
+    expect(process.stdout.write).toHaveBeenCalledWith(expect.stringMatching('wrong reference 3'));
+  });
+
+  it(`should remove empty SelectionRectangles from references that do not have reference text
+    (mongoose was creating this empty array by default)`, async () => {
+    const connections = await testingDB.mongodb
+      .collection('connections')
+      .find({
+        reference: { $exists: false },
+      })
+      .toArray();
+
+    expect(connections.length).toEqual(2);
+  });
+});

app/api/migrations/migrations/39-remove_wrong_text_references/specs/fixtures.js

@@ -0,0 +1,19 @@
+export default {
+  connections: [
+    { reference: { text: 'reference 1', selectionRectangles: [{}] } },
+    { reference: { text: 'reference 2', selectionRectangles: [{}] } },
+    { hub: 'hub1', reference: { text: 'wrong reference 1', selectionRectangles: [] } },
+    { hub: 'hub2', reference: { text: 'wrong reference 2', selectionRectangles: [] } },
+    { hub: 'hub3', reference: { text: 'wrong reference 3', selectionRectangles: [] } },
+    { hub: 'hub1', reference: { text: 'wrong reference 1 partner' } },
+    { hub: 'hub2', reference: { text: 'wrong reference 2 partner' } },
+
+    //
+    { hub: 'hub3', reference: {} },
+    { hub: 'hub3', reference: {} },
+
+    //
+    { reference: { selectionRectangles: [] } },
+    { reference: { selectionRectangles: [] } },
+  ],
+};

app/api/relationships/model.js

@@ -2,16 +2,18 @@ import mongoose from 'mongoose';
 import { instanceModel } from 'api/odm';
 
 const relationshipsSchema = new mongoose.Schema({
+  __v: { type: Number, select: false },
   entity: { type: String, index: true },
   hub: { type: mongoose.Schema.Types.ObjectId, index: true },
   sharedId: { type: mongoose.Schema.Types.ObjectId, index: true },
   template: { type: mongoose.Schema.Types.ObjectId, ref: 'relationTypes', index: true },
   metadata: mongoose.Schema.Types.Mixed,
   file: String,
   reference: {
-    selectionRectangles: [
-      { top: Number, left: Number, width: Number, height: Number, page: String },
-    ],
+    selectionRectangles: {
+      type: [{ top: Number, left: Number, width: Number, height: Number, page: String }],
+      default: undefined,
+    },
     text: String,
   },
 });

app/api/relationships/relationships.js

@@ -12,6 +12,7 @@ import { generateNamesAndIds } from '../templates/utils';
 
 import { filterRelevantRelationships, groupRelationships } from './groupByRelationships';
 import { RelationshipCollection, groupByHubs } from './relationshipsHelpers';
+import { validateConnectionSchema } from './validateConnectionSchema';
 
 const normalizeConnectedDocumentData = (relationship, connectedDocument) => {
   relationship.entityData = connectedDocument;
@@ -271,6 +272,8 @@ export default {
 
     const rel = !Array.isArray(_relationships) ? [_relationships] : _relationships;
 
+    await validateConnectionSchema(rel);
+
     const existingEntities = (
       await entities.get({
         sharedId: { $in: rel.map(r => r.entity) },

app/api/relationships/routes.js

@@ -1,17 +1,33 @@
 /** @format */
 
 import Joi from 'joi';
+import Ajv from 'ajv';
 import relationships from './relationships.js';
 import { validation } from '../utils';
 import needsAuthorization from '../auth/authMiddleware';
 
 export default app => {
-  app.post('/api/relationships/bulk', needsAuthorization(['admin', 'editor']), (req, res, next) => {
-    relationships
-      .bulk(req.body, req.language)
-      .then(response => res.json(response))
-      .catch(next);
-  });
+  app.post(
+    '/api/relationships/bulk',
+    needsAuthorization(['admin', 'editor']),
+    async (req, res, next) => {
+      try {
+        const response = await relationships.bulk(req.body, req.language);
+        res.json(response);
+      } catch (e) {
+        if (
+          e instanceof Ajv.ValidationError &&
+          e.errors.find(
+            error => error.dataPath.match(/selectionRectangles/) && error.keyword === 'minItems'
+          )
+        ) {
+          next(new Error('selectionRectangles should not be empty'));
+        } else {
+          next(e);
+        }
+      }
+    }
+  );
 
   app.post(
     '/api/references',

app/api/relationships/specs/__snapshots__/relationships.spec.js.snap

@@ -26,7 +26,6 @@ Array [
       },
       "hub": "hub2",
       "reference": Object {
-        "selectionRectangles": Array [],
         "text": "changed text",
       },
       "template": "relation2",

app/api/relationships/specs/relationships.spec.js

@@ -211,6 +211,7 @@ describe('relationships', () => {
         _id: _savedItem._id.equals(connectionID5) ? 'connectionID5' : _savedItem._id,
         template: _savedItem.template.equals(relation2) ? 'relation2' : _savedItem.relation2,
         hub: _savedItem.hub.equals(hub2) ? 'hub2' : _savedItem.hub2,
+        reference: { text: _savedItem.reference.text },
       };
 
       savedItem.entityData = {
@@ -232,7 +233,10 @@ describe('relationships', () => {
             entity: 'entity3',
             hub: hub2,
             template: relation2,
-            reference: { text: 'changed text' },
+            reference: {
+              text: 'changed text',
+              selectionRectangles: [{ width: 1, height: 1, top: 1, left: 1, page: '1' }],
+            },
           },
         ],
         delete: [{ _id: connectionID2 }, { _id: connectionID3 }],
@@ -366,7 +370,7 @@ describe('relationships', () => {
     describe('when saving one reference without hub', () => {
       it('should throw an error', done => {
         relationships
-          .save({ entity: 'entity3', range: { text: 'range' } }, 'en')
+          .save({ entity: 'entity3' }, 'en')
           .then(() => {
             done.fail('Should throw an error');
           })

app/api/relationships/specs/routes.spec.ts

@@ -0,0 +1,47 @@
+import { Application, Request, Response, NextFunction } from 'express';
+import request from 'supertest';
+
+import { testingDB } from 'api/utils/testing_db';
+import errorLog from 'api/log/errorLog';
+import { setUpApp } from 'api/utils/testingRoutes';
+
+import routes from '../routes';
+
+jest.mock(
+  '../../auth/authMiddleware.ts',
+  () => () => (_req: Request, _res: Response, next: NextFunction) => {
+    next();
+  }
+);
+
+describe('relationships routes', () => {
+  const app: Application = setUpApp(routes);
+
+  beforeEach(async () => {
+    spyOn(errorLog, 'error');
+    await testingDB.clearAllAndLoad({
+      settings: [{ languages: [{ key: 'en', default: true }] }],
+    });
+  });
+
+  afterAll(async () => testingDB.disconnect());
+
+  describe('POST/bulk', () => {
+    it('should validate connections', async () => {
+      const { body } = await request(app)
+        .post('/api/relationships/bulk')
+        .send({ save: [{ notAllowedProperty: 'test' }], delete: [] });
+
+      expect(body.error).toBe('validation failed');
+    });
+
+    it('should throw an especial 500 error when selectionRectangles is sent empty', async () => {
+      const { body, status } = await request(app)
+        .post('/api/relationships/bulk')
+        .send({ save: [{ reference: { text: 'test', selectionRectangles: [] } }], delete: [] });
+
+      expect(status).toBe(500);
+      expect(body.error.match(/selectionRectangles should not be empty/)).not.toBe(null);
+    });
+  });
+});

app/api/relationships/validateConnectionSchema.ts

@@ -0,0 +1,10 @@
+/* eslint-disable max-statements */
+import Ajv from 'ajv';
+import { wrapValidator } from 'shared/tsUtils';
+import { connectionSchema } from 'shared/types/connectionSchema';
+
+const ajv = Ajv({ allErrors: true });
+
+export const validateConnectionSchema = wrapValidator(
+  ajv.compile({ $async: true, type: 'array', items: connectionSchema })
+);

app/react/Connections/actions/specs/actions.spec.js

@@ -167,11 +167,11 @@ describe('Connections actions', () => {
           [
             {
               entity: 'sourceId',
-              template: null,
               reference: {
                 selectionRectangles: [{ top: 20, left: 42, height: 13, width: 84 }],
                 text: 'source text',
               },
+              template: null,
             },
             { entity: 'targetId', template: 'relationTypeId' },
           ],
@@ -195,11 +195,11 @@ describe('Connections actions', () => {
           [
             {
               entity: 'sourceId',
-              template: null,
               reference: {
                 selectionRectangles: [{ top: 20, left: 42, height: 13, width: 84 }],
                 text: 'source text',
               },
+              template: null,
             },
             {
               entity: 'targetId',

app/react/Relationships/actions/actions.js

@@ -114,7 +114,6 @@ export function saveRelationships() {
 
                 return relationships.concat(newRelationships);
               }
-
               return relationships;
             },
             []

app/react/Viewer/components/PageReferences.tsx

@@ -60,23 +60,24 @@ const indexdReferencesByPage = createSelector(
             return mappedReferences;
           }
 
-          const pages = (connection.reference?.selectionRectangles || [])
-            .map(selection => selection.page)
-            .filter(unique);
+          if (connection.reference) {
+            const pages = connection.reference.selectionRectangles
+              .map(selection => selection.page)
+              .filter(unique);
 
-          pages.forEach(page => {
-            if (!page) {
-              return;
-            }
+            pages.forEach(page => {
+              if (!page) {
+                return;
+              }
 
-            if (!mappedReferences[page]) {
-              // eslint-disable-next-line no-param-reassign
-              mappedReferences[page] = [];
-            }
-
-            mappedReferences[page].push(connection);
-          });
+              if (!mappedReferences[page]) {
+                // eslint-disable-next-line no-param-reassign
+                mappedReferences[page] = [];
+              }
 
+              mappedReferences[page].push(connection);
+            });
+          }
           return mappedReferences;
         },
         {}