diff --git a/src/api/controller/api/fetchLineBy.js b/src/api/controller/api/fetchLineBy.js new file mode 100644 index 0000000000..9e0c8c4491 --- /dev/null +++ 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 new file mode 100644 index 0000000000..0a4317739b --- /dev/null +++ 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 index de3aff4c3d..d46840f720 100644 --- 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 index d4bb763fc3..4b90105306 100644 --- 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 index 47e804250a..4fe1a84696 100644 --- 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 index bfc774c403..00ac579eac 100644 --- 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 index 2f6e0c79b5..e49fde5294 100644 --- 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 index 850fa94a29..bd4a915053 100644 --- 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 index 6d27146a07..c3273f0785 100644 --- 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); };