Merge pull request #62 from Inist-CNRS/fix_link_function_publish

[RFR] Fix LINK function when reference is null
Inist-CNRS · Feb 15, 2017 · 7ce9d47 · 7ce9d47
2 parents 8eb1928 + 3498ee2
commit 7ce9d47
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 19 deletions.
diff --git a/src/api/controller/api/fetchLineBy.js b/src/api/controller/api/fetchLineBy.js
@@ -0,0 +1,13 @@
+export default ctx => (fieldName, value) => (
+    value
+    ? ctx.dataset
+        .findBy(fieldName, value)
+        .then(line => (
+            line
+            ? ({
+                ...line,
+                uri: `uri to ${fieldName}: ${value}`,
+            })
+            : null),
+        )
+    : null);
diff --git a/src/api/controller/api/fetchLineBy.spec.js b/src/api/controller/api/fetchLineBy.spec.js
@@ -0,0 +1,65 @@
+import expect, { createSpy } from 'expect';
+import fetchLineBy from './fetchLineBy';
+
+describe('fetchLineBy', () => {
+    it('calls ctx.dataset.findBy', () => {
+        const line = { data: 'the line' };
+        const field = 'the field';
+        const value = 'the value';
+
+        const ctx = {
+            dataset: {
+                findBy: createSpy().andReturn(Promise.resolve(line)),
+            },
+        };
+
+        fetchLineBy(ctx)(field, value);
+        expect(ctx.dataset.findBy).toHaveBeenCalledWith(field, value);
+    });
+
+    it('returns the line with a computed uri when ctx.dataset.findBy succeeds', async () => {
+        const line = { data: 'the line' };
+        const field = 'the field';
+        const value = 'the value';
+
+        const ctx = {
+            dataset: {
+                findBy: createSpy().andReturn(Promise.resolve(line)),
+            },
+        };
+
+        const result = await fetchLineBy(ctx)(field, value);
+        expect(result).toEqual({
+            ...line,
+            uri: `uri to ${field}: ${value}`,
+        });
+    });
+
+    it('returns null when specified value is null', async () => {
+        const field = 'the field';
+        const value = null;
+
+        const ctx = {
+            dataset: {
+                findBy: createSpy().andReturn(Promise.resolve()),
+            },
+        };
+
+        const result = await fetchLineBy(ctx)(field, value);
+        expect(result).toEqual(null);
+    });
+
+    it('returns null when referenced line is not found', async () => {
+        const field = 'the field';
+        const value = 'the value';
+
+        const ctx = {
+            dataset: {
+                findBy: createSpy().andReturn(Promise.resolve()),
+            },
+        };
+
+        const result = await fetchLineBy(ctx)(field, value);
+        expect(result).toEqual(null);
+    });
+});
diff --git a/src/api/controller/api/parsing.js b/src/api/controller/api/parsing.js
@@ -1,5 +1,6 @@
 import Koa from 'koa';
 import route from 'koa-route';
+import fetchLineBy from './fetchLineBy';
 
 const app = new Koa();
 
@@ -14,11 +15,7 @@ export const getExcerpt = async (ctx) => {
 };
 
 export const findBy = async (ctx, fieldName, value) => {
-    const line = await ctx.dataset.findBy(fieldName, value);
-    ctx.body = {
-        ...line,
-        uri: `uri to ${fieldName}: ${value}`,
-    };
+    ctx.body = await fetchLineBy(ctx)(fieldName, value);
 };
 
 app.use(route.get('/', getExcerpt));

diff --git a/src/api/controller/api/publish.js b/src/api/controller/api/publish.js
@@ -1,6 +1,7 @@
 import omit from 'lodash.omit';
 import Koa from 'koa';
 import route from 'koa-route';
+import fetchLineBy from './fetchLineBy';
 
 /* eslint no-await-in-loop: off */
 import getDocumentTransformer from '../../../common/getDocumentTransformer';
@@ -38,6 +39,7 @@ export const publishCharacteristics = async (ctx, datasetCoverFields, count) =>
         .getDocumentTransformer({
             env: 'node',
             dataset: ctx.uriDataset,
+            fetchLineBy,
         }, datasetCoverFields);
 
     const [lastResource] = await ctx.uriDataset.findLimitFromSkip(1, count - 1);
@@ -84,7 +86,11 @@ export const doPublish = async (ctx) => {
     const datasetCoverFields = fields.filter(c => c.cover === 'dataset');
 
     const uriCol = fields.find(col => col.name === 'uri');
-    const getUri = ctx.getDocumentTransformer({ env: 'node', dataset: ctx.dataset }, [uriCol]);
+    const getUri = ctx.getDocumentTransformer({
+        env: 'node',
+        dataset: ctx.dataset,
+        fetchLineBy,
+    }, [uriCol]);
     const addUri = ctx.addTransformResultToDoc(getUri);
 
     await ctx.tranformAllDocuments(
@@ -98,6 +104,7 @@ export const doPublish = async (ctx) => {
         .getDocumentTransformer({
             env: 'node',
             dataset: ctx.uriDataset,
+            fetchLineBy,
         }, collectionCoverFields.filter(col => col.name !== 'uri'));
 
     const transformDocumentAndKeepUri = ctx.versionTransformResult(transformDocument);

diff --git a/src/api/controller/api/publish.spec.js b/src/api/controller/api/publish.spec.js
@@ -11,6 +11,7 @@ import {
     publishCharacteristics,
 } from './publish';
 import getDocumentTransformer from '../../../common/getDocumentTransformer';
+import fetchLineBy from './fetchLineBy';
 
 describe('publish', () => {
     describe('doPublish', () => {
@@ -64,7 +65,11 @@ describe('publish', () => {
         });
 
         it('should call getDocumentTransformer to get the uri transformers', () => {
-            expect(ctx.getDocumentTransformer).toHaveBeenCalledWith({ env: 'node', dataset: ctx.dataset }, [fields[0]]);
+            expect(ctx.getDocumentTransformer).toHaveBeenCalledWith({
+                env: 'node',
+                dataset: ctx.dataset,
+                fetchLineBy,
+            }, [fields[0]]);
         });
 
         it('should call ctx.addTransformResultToDoc with transformDocument', () => {
@@ -79,6 +84,7 @@ describe('publish', () => {
             expect(ctx.getDocumentTransformer).toHaveBeenCalledWith({
                 env: 'node',
                 dataset: ctx.uriDataset,
+                fetchLineBy,
             }, [fields[1]]);
         });
 
@@ -129,6 +135,7 @@ describe('publish', () => {
                 {
                     env: 'node',
                     dataset: ctx.uriDataset,
+                    fetchLineBy,
                 },
                 datasetFields,
             );

diff --git a/src/app/e2e/admin/linked_sample_csv.csv b/src/app/e2e/admin/linked_sample_csv.csv
@@ -2,3 +2,4 @@
 "1";"rock";"3";
 "2";"paper";"1";
 "3";"scissor";"2";
+"4";"invalid_reference";"5";
diff --git a/src/app/e2e/admin/publication.js b/src/app/e2e/admin/publication.js
@@ -7,7 +7,7 @@ import { clear } from '../../../common/tests/fixtures';
 import { elementIsClicked, inputElementIsFocusable, elementValueIs } from '../../../common/tests/conditions';
 
 describe('Admin', () => {
-    describe('Admin page', function homeTests() {
+    describe('Publication', function homeTests() {
         this.timeout(10000);
         const DEFAULT_WAIT_TIMEOUT = 9000; // A bit less than mocha's timeout to get explicit errors from selenium
 
@@ -60,7 +60,7 @@ describe('Admin', () => {
                 expect(text).toBe('uri');
 
                 const tds = await driver.findElements(By.css('.publication-preview tr td:first-child'));
-                expect(tds.length).toBe(3);
+                expect(tds.length).toBe(4);
                 await Promise.all(tds.map(td =>
                     driver.wait(until.elementTextIs(td, ''), DEFAULT_WAIT_TIMEOUT)),
                 );
@@ -85,7 +85,7 @@ describe('Admin', () => {
 
             it('should have completed uri column with generated uri', async () => {
                 const tds = await driver.findElements(By.css('.publication-preview tr td:first-child'));
-                expect(tds.length).toBe(3);
+                expect(tds.length).toBe(4);
                 await Promise.all(tds.map(td =>
                     driver.wait(until.elementTextMatches(td, /[A-Z0-9]{8}/), DEFAULT_WAIT_TIMEOUT)),
                 );
@@ -144,11 +144,12 @@ describe('Admin', () => {
                     DEFAULT_WAIT_TIMEOUT,
                 );
                 const tds = await driver.findElements(By.css('.publication-preview tr td:nth-child(2)'));
-                expect(tds.length).toBe(3);
+                expect(tds.length).toBe(4);
                 const expectedTexts = [
                     'uri to id: 3',
                     'uri to id: 1',
                     'uri to id: 2',
+                    '',
                 ];
                 await Promise.all(tds.map((td, index) =>
                     driver.wait(until.elementTextIs(td, expectedTexts[index]), DEFAULT_WAIT_TIMEOUT)),
@@ -166,9 +167,9 @@ describe('Admin', () => {
 
             it('should have updated the preview', async () => {
                 const tds = await driver.findElements(By.css('.publication-preview tr td:last-child'));
-                expect(tds.length).toBe(3);
+                expect(tds.length).toBe(4);
                 await Promise.all(tds.map(td =>
-                    driver.wait(until.elementTextMatches(td, /rock|paper|scissor/), DEFAULT_WAIT_TIMEOUT)),
+                    driver.wait(until.elementTextMatches(td, /rock|paper|scissor|invalid_reference/), DEFAULT_WAIT_TIMEOUT)),
                 );
             });
         });
@@ -197,7 +198,9 @@ describe('Admin', () => {
                 const headersText = await Promise.all(headers.map(h => h.getText()));
                 expect(headersText).toEqual(['uri', 'stronger', 'name']);
 
-                const rows = await Promise.all([1, 2, 3].map(index =>
+                await driver.sleep(5000);
+
+                const rows = await Promise.all([1, 2, 3, 4].map(index =>
                     Promise.all([
                         driver
                             .findElement(By.css(`.dataset table tbody tr:nth-child(${index}) td.dataset-uri`))
@@ -219,10 +222,11 @@ describe('Admin', () => {
                     rock: 'scissor',
                     paper: 'rock',
                     scissor: 'paper',
+                    invalid_reference: '',
                 };
 
                 rows.forEach(({ stronger, name }) => {
-                    expect(rows.find(r => r.uri === stronger).name).toEqual(expected[name]);
+                    expect((rows.find(r => r.uri === stronger) || { name: '' }).name).toEqual(expected[name]);
                 });
             });
         });

diff --git a/src/app/js/admin/publicationPreview/sagas.js b/src/app/js/admin/publicationPreview/sagas.js
@@ -2,6 +2,8 @@ import { takeLatest } from 'redux-saga';
 import { call, put, select } from 'redux-saga/effects';
 
 import getDocumentTransformer from '../../../../common/getDocumentTransformer';
+import fetchLineBy from '../../lib/fetchLineBy';
+
 import { getToken } from '../../user';
 import {
     COMPUTE_PREVIEW,
@@ -27,7 +29,7 @@ export function* handleComputePreview() {
         const token = yield select(getToken);
         const fields = yield select(getFields);
 
-        const transformDocument = yield call(getDocumentTransformer, { env: 'browser', token }, fields);
+        const transformDocument = yield call(getDocumentTransformer, { env: 'browser', token, fetchLineBy }, fields);
 
         const lines = yield select(getExcerptLines);
         const preview = yield lines.map(line => call(transformDocument, line));

diff --git a/src/common/transformers/getOtherLine.js b/src/common/transformers/getOtherLine.js
@@ -1,9 +1,7 @@
-import fetchLineBy from '../../app/js/lib/fetchLineBy';
-
 export default (context, targetColumn) => async (prev) => {
     if (context.env === 'node') {
         return context.dataset.findBy(targetColumn, prev);
     }
 
-    return fetchLineBy(targetColumn, prev, context.token);
+    return context.fetchLineBy(targetColumn, prev, context.token);
 };