From 97299c72e1c52bafbed9f2f33365a29e7b5d2ffc Mon Sep 17 00:00:00 2001 From: Santiago Date: Fri, 12 Nov 2021 15:49:20 -0300 Subject: [PATCH 01/26] typed template routes --- app/api/templates/{routes.js => routes.ts} | 11 ++-- .../specs/__snapshots__/routes.spec.ts.snap | 58 +++++++++++++++++++ app/api/templates/specs/routes.spec.js | 2 +- 3 files changed, 65 insertions(+), 6 deletions(-) rename app/api/templates/{routes.js => routes.ts} (91%) create mode 100644 app/api/templates/specs/__snapshots__/routes.spec.ts.snap diff --git a/app/api/templates/routes.js b/app/api/templates/routes.ts similarity index 91% rename from app/api/templates/routes.js rename to app/api/templates/routes.ts index 6c510c0121..bfabf858ff 100644 --- a/app/api/templates/routes.js +++ b/app/api/templates/routes.ts @@ -1,4 +1,6 @@ +/* eslint-disable max-statements */ import Joi from 'joi'; +import { Application } from 'express'; import settings from 'api/settings'; import { checkMapping, reindexAll } from 'api/search/entitiesIndex'; @@ -9,8 +11,7 @@ import templates from './templates'; import { generateNamesAndIds } from './utils'; import { checkIfReindex } from './reindex'; -export default app => { - // eslint-disable-next-line max-statements +export default (app: Application) => { app.post('/api/templates', needsAuthorization(), async (req, res, next) => { try { const { reindex: fullReindex } = req.body; @@ -29,7 +30,7 @@ export default app => { if (fullReindex) { const allTemplates = await templates.get(); - reindexAll(allTemplates, search); + await reindexAll(allTemplates, search); } res.json(response); } catch (error) { @@ -76,10 +77,10 @@ export default app => { 'query' ), (req, res, next) => { - const template = { _id: req.query._id }; + const template = { _id: req.query._id, name: req.query.name }; templates .delete(template) - .then(() => settings.removeTemplateFromFilters(template._id)) + .then(async () => settings.removeTemplateFromFilters(template._id)) .then(newSettings => { res.json(template); req.sockets.emitToCurrentTenant('updateSettings', newSettings); diff --git a/app/api/templates/specs/__snapshots__/routes.spec.ts.snap b/app/api/templates/specs/__snapshots__/routes.spec.ts.snap new file mode 100644 index 0000000000..c82ce0222d --- /dev/null +++ b/app/api/templates/specs/__snapshots__/routes.spec.ts.snap @@ -0,0 +1,58 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`templates routes /api/templates/setasdefault should have a validation schema 1`] = ` +Object { + "children": Object { + "_id": Object { + "flags": Object { + "presence": "required", + }, + "invalids": Array [ + "", + ], + "type": "string", + }, + }, + "type": "object", +} +`; + +exports[`templates routes /templates/count_by_thesauri should have a validation schema 1`] = ` +Object { + "children": Object { + "_id": Object { + "flags": Object { + "presence": "required", + }, + "invalids": Array [ + "", + ], + "type": "string", + }, + }, + "flags": Object { + "presence": "required", + }, + "type": "object", +} +`; + +exports[`templates routes DELETE should have a validation schema 1`] = ` +Object { + "children": Object { + "_id": Object { + "flags": Object { + "presence": "required", + }, + "invalids": Array [ + "", + ], + "type": "string", + }, + }, + "flags": Object { + "presence": "required", + }, + "type": "object", +} +`; diff --git a/app/api/templates/specs/routes.spec.js b/app/api/templates/specs/routes.spec.js index 2dc0c96549..04d0ddae3c 100644 --- a/app/api/templates/specs/routes.spec.js +++ b/app/api/templates/specs/routes.spec.js @@ -4,7 +4,7 @@ import settings from 'api/settings/settings'; import db from 'api/utils/testing_db'; import * as entitiesIndex from 'api/search/entitiesIndex'; import templates from '../templates'; -import templateRoutes from '../routes.js'; +import templateRoutes from '../routes'; import * as reindex from '../reindex'; From edc8c56bae1e688fb12e05757990fdbbe2923177 Mon Sep 17 00:00:00 2001 From: Santiago Date: Fri, 12 Nov 2021 16:24:11 -0300 Subject: [PATCH 02/26] combined check mappings with route --- app/api/templates/specs/routes.spec.js | 15 +-------------- app/api/templates/specs/routes.spec.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 14 deletions(-) create mode 100644 app/api/templates/specs/routes.spec.ts diff --git a/app/api/templates/specs/routes.spec.js b/app/api/templates/specs/routes.spec.js index 04d0ddae3c..b23969f7b4 100644 --- a/app/api/templates/specs/routes.spec.js +++ b/app/api/templates/specs/routes.spec.js @@ -21,9 +21,6 @@ describe('templates routes', () => { templates: [{ name: 'testing template', properties: [{ name: 'name', type: 'text' }] }], }); spyOn(entitiesIndex, 'updateMapping').and.returnValue(Promise.resolve()); - spyOn(entitiesIndex, 'checkMapping').and.returnValue( - Promise.resolve({ errors: [], valid: true }) - ); spyOn(reindex, 'checkIfReindex').and.returnValue(true); }); @@ -133,12 +130,10 @@ describe('templates routes', () => { describe('when there is an error', () => { it('should return the error in the response', async () => { - spyOn(templates, 'save').and.returnValue(Promise.reject(new Error('error'))); - try { await routes.post('/api/templates', { body: {} }); } catch (error) { - expect(error).toEqual(new Error('error')); + expect(error).toEqual(new Error('validation failed')); } }); }); @@ -188,12 +183,4 @@ describe('templates routes', () => { .catch(catchErrors(done)); }); }); - - describe('/api/templates/check_mapping', () => { - it('should check if a template is valid vs the current elasticsearch mapping', async () => { - const req = { body: { _id: 'abc1', properties: [] }, socket: mocketSocketIo() }; - const result = await routes.post('/api/templates/check_mapping', req); - expect(result).toEqual({ errors: [], valid: true }); - }); - }); }); diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts new file mode 100644 index 0000000000..d7885c9b74 --- /dev/null +++ b/app/api/templates/specs/routes.spec.ts @@ -0,0 +1,11 @@ +describe('check mappings', () => { + it('should check if a template is valid vs the current elasticsearch mapping', async () => { + const req = { + body: { _id: db.id(), name: 'my template', properties: [] }, + socket: mocketSocketIo(), + }; + const result = await routes.post('/api/templates', req); + console.log(('result: ', result)); + // expect(result).toEqual({ errors: [], valid: true }); + }); +}); From 8326867ac83992f5b6aae5b57a5b3dd8fda2e0ba Mon Sep 17 00:00:00 2001 From: Santiago Date: Sun, 14 Nov 2021 16:24:43 -0300 Subject: [PATCH 03/26] removed deprecated validateMapping --- app/react/Templates/TemplatesAPI.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/react/Templates/TemplatesAPI.js b/app/react/Templates/TemplatesAPI.js index b597a8f9e9..6eb65ff4d9 100644 --- a/app/react/Templates/TemplatesAPI.js +++ b/app/react/Templates/TemplatesAPI.js @@ -21,8 +21,4 @@ export default { delete(request) { return api.delete('templates', request).then(response => response.json); }, - - validateMapping(request) { - return api.post('templates/check_mapping', request).then(response => response.json); - }, }; From a6b5c011535d2f9edfcfdcf6ba5e605f9990c5d6 Mon Sep 17 00:00:00 2001 From: Santiago Date: Sun, 14 Nov 2021 16:25:10 -0300 Subject: [PATCH 04/26] returning response with errors for client --- app/react/Templates/actions/templateActions.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/react/Templates/actions/templateActions.js b/app/react/Templates/actions/templateActions.js index 075476a696..4790aea36f 100644 --- a/app/react/Templates/actions/templateActions.js +++ b/app/react/Templates/actions/templateActions.js @@ -110,11 +110,13 @@ export function saveTemplate(data) { return api .save(new RequestParams(template)) .then(response => { - dispatch({ type: types.TEMPLATE_SAVED, data: response }); - dispatch(actions.update('templates', response)); - - dispatch(formActions.merge('template.data', response)); - dispatch(notificationActions.notify('Saved successfully.', 'success')); + if (!response.error) { + dispatch({ type: types.TEMPLATE_SAVED, data: response }); + dispatch(actions.update('templates', response)); + dispatch(formActions.merge('template.data', response)); + dispatch(notificationActions.notify('Saved successfully.', 'success')); + } + return { response, error: response.error }; }) .catch(() => { dispatch({ type: types.TEMPLATE_SAVED, data }); From 255160f3a5c1ff34a62b7846ef8fb5041fedd0b7 Mon Sep 17 00:00:00 2001 From: Santiago Date: Sun, 14 Nov 2021 16:25:59 -0300 Subject: [PATCH 05/26] removed check_mapping + changed joi fro ajv --- app/api/templates/routes.ts | 79 ++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/app/api/templates/routes.ts b/app/api/templates/routes.ts index bfabf858ff..50ee34e035 100644 --- a/app/api/templates/routes.ts +++ b/app/api/templates/routes.ts @@ -1,7 +1,5 @@ /* eslint-disable max-statements */ -import Joi from 'joi'; import { Application } from 'express'; - import settings from 'api/settings'; import { checkMapping, reindexAll } from 'api/search/entitiesIndex'; import { search } from 'api/search'; @@ -16,14 +14,24 @@ export default (app: Application) => { try { const { reindex: fullReindex } = req.body; delete req.body.reindex; + const templateProperties = await generateNamesAndIds(req.body.properties); + const template = { ...req.body, properties: templateProperties }; + const { valid, error } = await checkMapping(template); + + if (!valid && !fullReindex) { + return res.json({ error: `Reindex requiered: ${error}` }); + } const reindex = fullReindex ? false : await checkIfReindex(req.body); const response = await templates.save(req.body, req.language, reindex); + req.sockets.emitToCurrentTenant('templateChange', response); + const updatedSettings = await settings.updateFilterName( response._id.toString(), response.name ); + if (updatedSettings) { req.sockets.emitToCurrentTenant('updateSettings', updatedSettings); } @@ -32,20 +40,33 @@ export default (app: Application) => { const allTemplates = await templates.get(); await reindexAll(allTemplates, search); } - res.json(response); + + return res.json(response); } catch (error) { next(error); } }); + // app.post('/api/templates/check_mapping', needsAuthorization(), async (req, res, next) => { + // const template = req.body; + // template.properties = await generateNamesAndIds(template.properties); + // checkMapping(template) + // .then(response => res.json(response)) + // .catch(next); + // }); + app.post( '/api/templates/setasdefault', needsAuthorization(), - validation.validateRequest( - Joi.object().keys({ - _id: Joi.string().required(), - }) - ), + validation.validateRequest({ + properties: { + body: { + properties: { + _id: { type: 'string' }, + }, + }, + }, + }), async (req, res, next) => { try { const [newDefault, oldDefault] = await templates.setAsDefault(req.body._id.toString()); @@ -70,12 +91,15 @@ export default (app: Application) => { app.delete( '/api/templates', needsAuthorization(), - validation.validateRequest( - Joi.object({ - _id: Joi.string().required(), - }).required(), - 'query' - ), + validation.validateRequest({ + properties: { + query: { + properties: { + _id: { type: 'string' }, + }, + }, + }, + }), (req, res, next) => { const template = { _id: req.query._id, name: req.query.name }; templates @@ -92,16 +116,15 @@ export default (app: Application) => { app.get( '/api/templates/count_by_thesauri', - - validation.validateRequest( - Joi.object() - .keys({ - _id: Joi.string().required(), - }) - .required(), - 'query' - ), - + validation.validateRequest({ + properties: { + query: { + properties: { + _id: { type: 'string' }, + }, + }, + }, + }), (req, res, next) => { templates .countByThesauri(req.query._id) @@ -109,12 +132,4 @@ export default (app: Application) => { .catch(next); } ); - - app.post('/api/templates/check_mapping', needsAuthorization(), async (req, res, next) => { - const template = req.body; - template.properties = await generateNamesAndIds(template.properties); - checkMapping(template) - .then(response => res.json(response)) - .catch(next); - }); }; From 87de39abac45ef8feadffcbfa03423dfbf923a09 Mon Sep 17 00:00:00 2001 From: Santiago Date: Sun, 14 Nov 2021 16:26:39 -0300 Subject: [PATCH 06/26] checking if mapping has errors to call confirm dialog --- .../Templates/components/MetadataTemplate.tsx | 31 +++++++++++++++---- .../components/specs/MetadataTemplate.spec.js | 16 +++------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/app/react/Templates/components/MetadataTemplate.tsx b/app/react/Templates/components/MetadataTemplate.tsx index ac008c20ae..8cfbbb19e4 100644 --- a/app/react/Templates/components/MetadataTemplate.tsx +++ b/app/react/Templates/components/MetadataTemplate.tsx @@ -20,7 +20,6 @@ import { addProperty, inserted, saveTemplate, - validateMapping, countByTemplate, } from 'app/Templates/actions/templateActions'; import MetadataProperty from 'app/Templates/components/MetadataProperty'; @@ -89,6 +88,26 @@ export class MetadataTemplate extends Component { this.onSubmitFailed = this.onSubmitFailed.bind(this); } + // onSubmit = async (_template: TemplateSchema) => { + // const template = { ..._template }; + // template.properties = template.properties?.map(_prop => { + // const prop = { ..._prop }; + // prop.label = _prop.label.trim(); + // return prop; + // }); + // const mappingValidation = await validateMapping(template); + // if (!mappingValidation.valid) { + // return this.confirmAndSaveTemplate(template, 'templateConflict'); + // } + // if (template._id) { + // const entitiesCountOfTemplate = await countByTemplate(template); + // const lengthyReindexFloorCount = 30000; + // if (entitiesCountOfTemplate >= lengthyReindexFloorCount) { + // return this.confirmAndSaveTemplate(template, 'largeNumberOfEntities'); + // } + // } + // this.props.saveTemplate(template); + // }; onSubmit = async (_template: TemplateSchema) => { const template = { ..._template }; template.properties = template.properties?.map(_prop => { @@ -96,10 +115,6 @@ export class MetadataTemplate extends Component { prop.label = _prop.label.trim(); return prop; }); - const mappingValidation = await validateMapping(template); - if (!mappingValidation.valid) { - return this.confirmAndSaveTemplate(template, 'templateConflict'); - } if (template._id) { const entitiesCountOfTemplate = await countByTemplate(template); const lengthyReindexFloorCount = 30000; @@ -107,7 +122,11 @@ export class MetadataTemplate extends Component { return this.confirmAndSaveTemplate(template, 'largeNumberOfEntities'); } } - this.props.saveTemplate(template); + const { error } = await this.props.saveTemplate(template); + + if (error) { + return this.confirmAndSaveTemplate(template, 'templateConflict'); + } }; onSubmitFailed() { diff --git a/app/react/Templates/components/specs/MetadataTemplate.spec.js b/app/react/Templates/components/specs/MetadataTemplate.spec.js index 8ef70a1fb1..b5b00d17d4 100644 --- a/app/react/Templates/components/specs/MetadataTemplate.spec.js +++ b/app/react/Templates/components/specs/MetadataTemplate.spec.js @@ -12,7 +12,6 @@ import thunk from 'redux-thunk'; import { shallow } from 'enzyme'; import TestBackend from 'react-dnd-test-backend'; -import api from 'app/Templates/TemplatesAPI'; import entitiesApi from 'app/Entities/EntitiesAPI'; import pagesApi from 'app/Pages/PagesAPI'; @@ -222,7 +221,6 @@ describe('MetadataTemplate', () => { describe('onSubmit', () => { it('should trim the properties labels and then call props.saveTemplate', async () => { - spyOn(api, 'validateMapping').and.returnValue({ errors: [], valid: true }); spyOn(entitiesApi, 'countByTemplate').and.returnValue(100); const component = shallow(); const template = { properties: [{ label: ' trim me please ' }] }; @@ -239,14 +237,10 @@ describe('MetadataTemplate', () => { properties: [{ name: 'dob', type: 'date', label: 'Date of birth' }], }; - async function submitTemplate(templateToSubmit, validMapping = true, entityCount = 100) { + async function submitTemplate(templateToSubmit, entityCount = 100) { context = { confirm: jasmine.createSpy('confirm'), }; - spyOn(api, 'validateMapping').and.returnValue({ - error: 'error', - valid: validMapping, - }); spyOn(entitiesApi, 'countByTemplate').and.returnValue(entityCount); const component = shallow(, { context }); await component.instance().onSubmit(templateToSubmit); @@ -254,7 +248,7 @@ describe('MetadataTemplate', () => { describe('when the mapping has conflicts', () => { it('should ask for a reindex', async () => { - await submitTemplate(templateWithId, false); + await submitTemplate(templateWithId); context.confirm.calls.mostRecent().args[0].accept(); expect(props.saveTemplate).toHaveBeenCalledWith({ ...templateWithId, @@ -264,13 +258,13 @@ describe('MetadataTemplate', () => { describe('when there is a quite amount of entities from the template', () => { it('should ask for a reindex but do not do it if the user cancels it', async () => { - await submitTemplate(templateWithId, true, 50000); + await submitTemplate(templateWithId, 50000); context.confirm.calls.mostRecent().args[0].cancel(); expect(props.saveTemplate).not.toHaveBeenCalled(); }); it('should ask for a reindex and do it if the user accepts it', async () => { - await submitTemplate(templateWithId, true, 50000); + await submitTemplate(templateWithId, 50000); context.confirm.calls.mostRecent().args[0].accept(); expect(props.saveTemplate).toHaveBeenCalledWith({ _id: templateWithId._id, @@ -282,7 +276,7 @@ describe('MetadataTemplate', () => { describe('when it is a new template', () => { it('should not check for the number of entities', async () => { - await submitTemplate({ properties: templateWithId.properties }, true, null); + await submitTemplate({ properties: templateWithId.properties }, null); expect(entitiesApi.countByTemplate).not.toHaveBeenCalled(); }); }); From d7fb39c0ac068bee00761c7b446e8be472b6921b Mon Sep 17 00:00:00 2001 From: Santiago Date: Mon, 15 Nov 2021 13:38:45 -0300 Subject: [PATCH 07/26] refactored post api/templates to avoid to many statements --- app/api/templates/routes.ts | 53 ++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/app/api/templates/routes.ts b/app/api/templates/routes.ts index 50ee34e035..44fe6ea93e 100644 --- a/app/api/templates/routes.ts +++ b/app/api/templates/routes.ts @@ -1,29 +1,49 @@ -/* eslint-disable max-statements */ import { Application } from 'express'; import settings from 'api/settings'; import { checkMapping, reindexAll } from 'api/search/entitiesIndex'; import { search } from 'api/search'; +import { TemplateSchema } from 'shared/types/templateType'; import { validation } from '../utils'; import needsAuthorization from '../auth/authMiddleware'; import templates from './templates'; import { generateNamesAndIds } from './utils'; import { checkIfReindex } from './reindex'; +const reindexAllTemplates = async () => { + const allTemplates = await templates.get(); + return reindexAll(allTemplates, search); +}; + +const checkTemplateMappings = async (template: TemplateSchema) => { + const templateProperties = await generateNamesAndIds(template.properties); + const { valid, error } = await checkMapping({ ...template, properties: templateProperties }); + return { valid, error }; +}; + +const saveTemplate = async (template: TemplateSchema, language: string, fullReindex?: boolean) => { + const reindex = fullReindex ? false : await checkIfReindex(template); + return templates.save(template, language, reindex); +}; + +const prepareRequest = async (body: TemplateSchema & { reindex?: boolean }) => { + const request = { ...body }; + const { reindex: fullReindex } = request; + + delete request.reindex; + + const template = { ...request }; + const { valid, error } = await checkTemplateMappings(template); + return { template, fullReindex, valid, error }; +}; + export default (app: Application) => { app.post('/api/templates', needsAuthorization(), async (req, res, next) => { try { - const { reindex: fullReindex } = req.body; - delete req.body.reindex; - const templateProperties = await generateNamesAndIds(req.body.properties); - const template = { ...req.body, properties: templateProperties }; - const { valid, error } = await checkMapping(template); - - if (!valid && !fullReindex) { - return res.json({ error: `Reindex requiered: ${error}` }); - } + const { template, fullReindex, valid, error } = await prepareRequest(req.body); - const reindex = fullReindex ? false : await checkIfReindex(req.body); - const response = await templates.save(req.body, req.language, reindex); + if (!valid && !fullReindex) return res.json({ error: `Reindex requiered: ${error}` }); + + const response = await saveTemplate(template, req.language, fullReindex); req.sockets.emitToCurrentTenant('templateChange', response); @@ -32,14 +52,9 @@ export default (app: Application) => { response.name ); - if (updatedSettings) { - req.sockets.emitToCurrentTenant('updateSettings', updatedSettings); - } + if (updatedSettings) req.sockets.emitToCurrentTenant('updateSettings', updatedSettings); - if (fullReindex) { - const allTemplates = await templates.get(); - await reindexAll(allTemplates, search); - } + if (fullReindex) await reindexAllTemplates(); return res.json(response); } catch (error) { From d018a10120fcb038b9e7f0ac6c82d84b9ea8f7f2 Mon Sep 17 00:00:00 2001 From: Santiago Date: Mon, 15 Nov 2021 16:03:26 -0300 Subject: [PATCH 08/26] throwing error instead of return error message --- app/api/templates/routes.ts | 17 ++++------- .../Templates/actions/templateActions.js | 14 ++++------ .../Templates/components/MetadataTemplate.tsx | 28 +++---------------- 3 files changed, 16 insertions(+), 43 deletions(-) diff --git a/app/api/templates/routes.ts b/app/api/templates/routes.ts index 44fe6ea93e..b0e501c90e 100644 --- a/app/api/templates/routes.ts +++ b/app/api/templates/routes.ts @@ -3,7 +3,7 @@ import settings from 'api/settings'; import { checkMapping, reindexAll } from 'api/search/entitiesIndex'; import { search } from 'api/search'; import { TemplateSchema } from 'shared/types/templateType'; -import { validation } from '../utils'; +import { createError, validation } from '../utils'; import needsAuthorization from '../auth/authMiddleware'; import templates from './templates'; import { generateNamesAndIds } from './utils'; @@ -41,7 +41,7 @@ export default (app: Application) => { try { const { template, fullReindex, valid, error } = await prepareRequest(req.body); - if (!valid && !fullReindex) return res.json({ error: `Reindex requiered: ${error}` }); + if (!valid && !fullReindex) throw createError(error, 409); const response = await saveTemplate(template, req.language, fullReindex); @@ -56,26 +56,19 @@ export default (app: Application) => { if (fullReindex) await reindexAllTemplates(); - return res.json(response); + res.json(response); } catch (error) { next(error); } }); - // app.post('/api/templates/check_mapping', needsAuthorization(), async (req, res, next) => { - // const template = req.body; - // template.properties = await generateNamesAndIds(template.properties); - // checkMapping(template) - // .then(response => res.json(response)) - // .catch(next); - // }); - app.post( '/api/templates/setasdefault', needsAuthorization(), validation.validateRequest({ properties: { body: { + required: ['_id'], properties: { _id: { type: 'string' }, }, @@ -109,6 +102,7 @@ export default (app: Application) => { validation.validateRequest({ properties: { query: { + required: ['_id'], properties: { _id: { type: 'string' }, }, @@ -134,6 +128,7 @@ export default (app: Application) => { validation.validateRequest({ properties: { query: { + required: ['_id'], properties: { _id: { type: 'string' }, }, diff --git a/app/react/Templates/actions/templateActions.js b/app/react/Templates/actions/templateActions.js index 4790aea36f..ba40622149 100644 --- a/app/react/Templates/actions/templateActions.js +++ b/app/react/Templates/actions/templateActions.js @@ -110,16 +110,14 @@ export function saveTemplate(data) { return api .save(new RequestParams(template)) .then(response => { - if (!response.error) { - dispatch({ type: types.TEMPLATE_SAVED, data: response }); - dispatch(actions.update('templates', response)); - dispatch(formActions.merge('template.data', response)); - dispatch(notificationActions.notify('Saved successfully.', 'success')); - } - return { response, error: response.error }; + dispatch({ type: types.TEMPLATE_SAVED, data: response }); + dispatch(actions.update('templates', response)); + dispatch(formActions.merge('template.data', response)); + dispatch(notificationActions.notify('Saved successfully.', 'success')); }) - .catch(() => { + .catch(e => { dispatch({ type: types.TEMPLATE_SAVED, data }); + throw e; }); }; } diff --git a/app/react/Templates/components/MetadataTemplate.tsx b/app/react/Templates/components/MetadataTemplate.tsx index 8cfbbb19e4..8868b70598 100644 --- a/app/react/Templates/components/MetadataTemplate.tsx +++ b/app/react/Templates/components/MetadataTemplate.tsx @@ -88,26 +88,6 @@ export class MetadataTemplate extends Component { this.onSubmitFailed = this.onSubmitFailed.bind(this); } - // onSubmit = async (_template: TemplateSchema) => { - // const template = { ..._template }; - // template.properties = template.properties?.map(_prop => { - // const prop = { ..._prop }; - // prop.label = _prop.label.trim(); - // return prop; - // }); - // const mappingValidation = await validateMapping(template); - // if (!mappingValidation.valid) { - // return this.confirmAndSaveTemplate(template, 'templateConflict'); - // } - // if (template._id) { - // const entitiesCountOfTemplate = await countByTemplate(template); - // const lengthyReindexFloorCount = 30000; - // if (entitiesCountOfTemplate >= lengthyReindexFloorCount) { - // return this.confirmAndSaveTemplate(template, 'largeNumberOfEntities'); - // } - // } - // this.props.saveTemplate(template); - // }; onSubmit = async (_template: TemplateSchema) => { const template = { ..._template }; template.properties = template.properties?.map(_prop => { @@ -122,10 +102,10 @@ export class MetadataTemplate extends Component { return this.confirmAndSaveTemplate(template, 'largeNumberOfEntities'); } } - const { error } = await this.props.saveTemplate(template); - - if (error) { - return this.confirmAndSaveTemplate(template, 'templateConflict'); + try { + await this.props.saveTemplate(template); + } catch (e) { + if (e.status === 409) return this.confirmAndSaveTemplate(template, 'templateConflict'); } }; From b090737ca4de700dbcd01ac8d37b7b27cb6d35a0 Mon Sep 17 00:00:00 2001 From: Santiago Date: Mon, 15 Nov 2021 16:05:48 -0300 Subject: [PATCH 09/26] migrated test for api/templates/delete & api/templates/get --- .../specs/__snapshots__/routes.spec.js.snap | 57 +----- .../specs/__snapshots__/routes.spec.ts.snap | 58 ------ app/api/templates/specs/routes.spec.js | 186 ------------------ app/api/templates/specs/routes.spec.ts | 102 +++++++++- 4 files changed, 97 insertions(+), 306 deletions(-) delete mode 100644 app/api/templates/specs/__snapshots__/routes.spec.ts.snap delete mode 100644 app/api/templates/specs/routes.spec.js diff --git a/app/api/templates/specs/__snapshots__/routes.spec.js.snap b/app/api/templates/specs/__snapshots__/routes.spec.js.snap index c82ce0222d..a084863e3f 100644 --- a/app/api/templates/specs/__snapshots__/routes.spec.js.snap +++ b/app/api/templates/specs/__snapshots__/routes.spec.js.snap @@ -1,58 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`templates routes /api/templates/setasdefault should have a validation schema 1`] = ` -Object { - "children": Object { - "_id": Object { - "flags": Object { - "presence": "required", - }, - "invalids": Array [ - "", - ], - "type": "string", - }, - }, - "type": "object", -} -`; +exports[`templates routes /api/templates/setasdefault should have a validation schema 1`] = `undefined`; -exports[`templates routes /templates/count_by_thesauri should have a validation schema 1`] = ` -Object { - "children": Object { - "_id": Object { - "flags": Object { - "presence": "required", - }, - "invalids": Array [ - "", - ], - "type": "string", - }, - }, - "flags": Object { - "presence": "required", - }, - "type": "object", -} -`; +exports[`templates routes /templates/count_by_thesauri should have a validation schema 1`] = `undefined`; -exports[`templates routes DELETE should have a validation schema 1`] = ` -Object { - "children": Object { - "_id": Object { - "flags": Object { - "presence": "required", - }, - "invalids": Array [ - "", - ], - "type": "string", - }, - }, - "flags": Object { - "presence": "required", - }, - "type": "object", -} -`; +exports[`templates routes DELETE should have a validation schema 1`] = `undefined`; diff --git a/app/api/templates/specs/__snapshots__/routes.spec.ts.snap b/app/api/templates/specs/__snapshots__/routes.spec.ts.snap deleted file mode 100644 index c82ce0222d..0000000000 --- a/app/api/templates/specs/__snapshots__/routes.spec.ts.snap +++ /dev/null @@ -1,58 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`templates routes /api/templates/setasdefault should have a validation schema 1`] = ` -Object { - "children": Object { - "_id": Object { - "flags": Object { - "presence": "required", - }, - "invalids": Array [ - "", - ], - "type": "string", - }, - }, - "type": "object", -} -`; - -exports[`templates routes /templates/count_by_thesauri should have a validation schema 1`] = ` -Object { - "children": Object { - "_id": Object { - "flags": Object { - "presence": "required", - }, - "invalids": Array [ - "", - ], - "type": "string", - }, - }, - "flags": Object { - "presence": "required", - }, - "type": "object", -} -`; - -exports[`templates routes DELETE should have a validation schema 1`] = ` -Object { - "children": Object { - "_id": Object { - "flags": Object { - "presence": "required", - }, - "invalids": Array [ - "", - ], - "type": "string", - }, - }, - "flags": Object { - "presence": "required", - }, - "type": "object", -} -`; diff --git a/app/api/templates/specs/routes.spec.js b/app/api/templates/specs/routes.spec.js deleted file mode 100644 index b23969f7b4..0000000000 --- a/app/api/templates/specs/routes.spec.js +++ /dev/null @@ -1,186 +0,0 @@ -import { catchErrors } from 'api/utils/jasmineHelpers'; -import instrumentRoutes from 'api/utils/instrumentRoutes'; -import settings from 'api/settings/settings'; -import db from 'api/utils/testing_db'; -import * as entitiesIndex from 'api/search/entitiesIndex'; -import templates from '../templates'; -import templateRoutes from '../routes'; - -import * as reindex from '../reindex'; - -const mocketSocketIo = () => ({ - emitToCurrentTenant: jasmine.createSpy('emitToCurrentTenant'), -}); - -describe('templates routes', () => { - let routes; - - beforeEach(async () => { - routes = instrumentRoutes(templateRoutes); - await db.clearAllAndLoad({ - templates: [{ name: 'testing template', properties: [{ name: 'name', type: 'text' }] }], - }); - spyOn(entitiesIndex, 'updateMapping').and.returnValue(Promise.resolve()); - spyOn(reindex, 'checkIfReindex').and.returnValue(true); - }); - - afterAll(async () => { - await db.disconnect(); - }); - - describe('GET', () => { - it('should return all templates by default', done => { - spyOn(templates, 'get').and.returnValue(Promise.resolve('templates')); - routes - .get('/api/templates') - .then(response => { - const tmpls = response.rows; - expect(tmpls).toBe('templates'); - done(); - }) - .catch(catchErrors(done)); - }); - - describe('when there is an error', () => { - it('should return the error in the response', async () => { - spyOn(templates, 'get').and.returnValue(Promise.reject(new Error('error'))); - try { - await routes.get('/api/templates'); - } catch (error) { - expect(error).toEqual(new Error('error')); - } - }); - }); - }); - - describe('DELETE', () => { - it('should have a validation schema', () => { - expect(routes.delete.validation('/api/templates')).toMatchSnapshot(); - }); - it('should delete a template', done => { - spyOn(templates, 'delete').and.returnValue(Promise.resolve('ok')); - spyOn(settings, 'removeTemplateFromFilters').and.returnValue(Promise.resolve()); - routes - .delete('/api/templates', { query: { _id: '123' } }) - .then(response => { - expect(templates.delete).toHaveBeenCalledWith({ _id: '123' }); - expect(response).toEqual({ _id: '123' }); - done(); - }) - .catch(catchErrors(done)); - }); - - describe('when there is an error', () => { - it('should return the error in the response', async () => { - spyOn(templates, 'delete').and.returnValue(Promise.reject(new Error('error'))); - - try { - await routes.delete('/api/templates', { query: {} }); - } catch (error) { - expect(error).toEqual(new Error('error')); - } - }); - }); - }); - - describe('POST', () => { - const aTemplate = { _id: 'id', name: 'name' }; - - it('should create a template', async () => { - const req = { - body: { name: 'created_template', properties: [{ label: 'fieldLabel' }] }, - language: 'en', - sockets: mocketSocketIo(), - }; - - spyOn(templates, 'save').and.returnValue(new Promise(resolve => resolve(aTemplate))); - spyOn(settings, 'updateFilterName').and.returnValue( - new Promise(resolve => resolve('updated settings')) - ); - - const response = await routes.post('/api/templates', req); - - expect(response).toBe(aTemplate); - expect(templates.save).toHaveBeenCalledWith(req.body, req.language, true); - expect(settings.updateFilterName).toHaveBeenCalledWith(aTemplate._id, aTemplate.name); - expect(req.sockets.emitToCurrentTenant).toHaveBeenCalledWith('templateChange', aTemplate); - expect(req.sockets.emitToCurrentTenant).toHaveBeenCalledWith( - 'updateSettings', - 'updated settings' - ); - }); - - it('should not emit settings update when settings not modified', async () => { - const req = { - body: { name: 'created_template', properties: [{ label: 'fieldLabel' }] }, - language: 'en', - sockets: mocketSocketIo(), - }; - - spyOn(templates, 'save').and.returnValue(new Promise(resolve => resolve(aTemplate))); - spyOn(settings, 'updateFilterName').and.returnValue(undefined); - - await routes.post('/api/templates', req); - - expect(req.sockets.emitToCurrentTenant).not.toHaveBeenCalledWith( - 'updateSettings', - 'updated settings' - ); - }); - - describe('when there is an error', () => { - it('should return the error in the response', async () => { - try { - await routes.post('/api/templates', { body: {} }); - } catch (error) { - expect(error).toEqual(new Error('validation failed')); - } - }); - }); - }); - - describe('/templates/count_by_thesauri', () => { - it('should have a validation schema', () => { - expect(routes.get.validation('/api/templates/count_by_thesauri')).toMatchSnapshot(); - }); - it('should return the number of templates using a thesauri', done => { - spyOn(templates, 'countByThesauri').and.returnValue(Promise.resolve(2)); - const req = { query: { _id: 'abc1' } }; - routes - .get('/api/templates/count_by_thesauri', req) - .then(result => { - expect(result).toBe(2); - expect(templates.countByThesauri).toHaveBeenCalledWith('abc1'); - done(); - }) - .catch(catchErrors(done)); - }); - }); - - describe('/api/templates/setasdefault', () => { - it('should have a validation schema', () => { - expect(routes.post.validation('/api/templates/setasdefault')).toMatchSnapshot(); - }); - - it('should call templates to set the new default', done => { - spyOn(templates, 'setAsDefault').and.returnValue( - Promise.resolve([{ name: 'newDefault' }, { name: 'oldDefault' }]) - ); - const req = { body: { _id: 'abc1' }, sockets: mocketSocketIo() }; - routes - .post('/api/templates/setasdefault', req) - .then(result => { - expect(result).toEqual({ name: 'newDefault' }); - expect(templates.setAsDefault).toHaveBeenCalledWith('abc1'); - expect(req.sockets.emitToCurrentTenant).toHaveBeenCalledWith('templateChange', { - name: 'newDefault', - }); - expect(req.sockets.emitToCurrentTenant).toHaveBeenCalledWith('templateChange', { - name: 'oldDefault', - }); - done(); - }) - .catch(catchErrors(done)); - }); - }); -}); diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts index d7885c9b74..7c9ba57691 100644 --- a/app/api/templates/specs/routes.spec.ts +++ b/app/api/templates/specs/routes.spec.ts @@ -1,11 +1,97 @@ -describe('check mappings', () => { - it('should check if a template is valid vs the current elasticsearch mapping', async () => { - const req = { - body: { _id: db.id(), name: 'my template', properties: [] }, - socket: mocketSocketIo(), +import { Application, NextFunction } from 'express'; +import request from 'supertest'; +import { setUpApp } from 'api/utils/testingRoutes'; +import { testingEnvironment } from 'api/utils/testingEnvironment'; +import { getFixturesFactory } from 'api/utils/fixturesFactory'; +import { UserRole } from 'shared/types/userSchema'; +import templateRoutes from '../routes'; +import templates from '../templates'; + +jest.mock( + '../../auth/authMiddleware.ts', + () => () => (_req: Request, _res: Response, next: NextFunction) => { + next(); + } +); + +jest.mock( + '../../utils/languageMiddleware.ts', + () => (_req: Request, _res: Response, next: NextFunction) => { + next(); + } +); + +const fixtureFactory = getFixturesFactory(); +const fixtures = { + templates: [ + fixtureFactory.template('template1', []), + fixtureFactory.template('template2', []), + fixtureFactory.template('template3', []), + ], +}; + +describe('templates routes', () => { + const app: Application = setUpApp(templateRoutes, (req, _res, next: NextFunction) => { + (req as any).user = { + role: UserRole.ADMIN, + username: 'admin', }; - const result = await routes.post('/api/templates', req); - console.log(('result: ', result)); - // expect(result).toEqual({ errors: [], valid: true }); + next(); + }); + + beforeEach(async () => { + await testingEnvironment.setUp(fixtures, 'templates_index'); + }); + + afterAll(async () => testingEnvironment.tearDown()); + + describe('GET', () => { + it('should return all templates by default', async () => { + const { body } = await request(app) + .get('/api/templates') + .expect(200); + + expect(JSON.stringify(body.rows)).toBe(JSON.stringify(fixtures.templates)); + }); + }); + + describe('DELETE', () => { + it('should delete a template', async () => { + await request(app) + .delete(`/api/templates?_id=${fixtureFactory.template('template2', [])._id}`) + .expect(200); + const remainingTemplates = await templates.get(); + expect(remainingTemplates).toContainEqual(expect.objectContaining({ name: 'template1' })); + expect(remainingTemplates).toContainEqual(expect.objectContaining({ name: 'template3' })); + expect(remainingTemplates).not.toContainEqual(expect.objectContaining({ name: 'template2' })); + }); + + it('should validate that request has _id', async () => { + const { body } = await request(app) + .delete('/api/templates') + .expect(400); + expect(body.error).toBe('validation failed'); + }); + }); + + describe('POST', () => { + it('should create a template', async () => {}); + + it('should not emit settings update when settings not modified', () => {}); + }); + + describe('/templates/count_by_thesauri', () => { + it('should have a validation schema', () => {}); + it('should return the number of templates using a thesauri', () => {}); + }); + + describe('/api/templates/setasdefault', () => { + it('should have a validation schema', () => {}); + + it('should call templates to set the new default', () => {}); + }); + + describe('check mappings', () => { + it('should check if a template is valid vs the current elasticsearch mapping', () => {}); }); }); From 4b5c0e0356a833943a95918981e434074ee59ac9 Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 16 Nov 2021 10:28:48 -0300 Subject: [PATCH 10/26] updated test --- .../specs/__snapshots__/routes.spec.js.snap | 7 ------- .../actions/specs/templateActions.spec.js | 14 +++++++------- .../components/specs/MetadataTemplate.spec.js | 4 ++++ 3 files changed, 11 insertions(+), 14 deletions(-) delete mode 100644 app/api/templates/specs/__snapshots__/routes.spec.js.snap diff --git a/app/api/templates/specs/__snapshots__/routes.spec.js.snap b/app/api/templates/specs/__snapshots__/routes.spec.js.snap deleted file mode 100644 index a084863e3f..0000000000 --- a/app/api/templates/specs/__snapshots__/routes.spec.js.snap +++ /dev/null @@ -1,7 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`templates routes /api/templates/setasdefault should have a validation schema 1`] = `undefined`; - -exports[`templates routes /templates/count_by_thesauri should have a validation schema 1`] = `undefined`; - -exports[`templates routes DELETE should have a validation schema 1`] = `undefined`; diff --git a/app/react/Templates/actions/specs/templateActions.spec.js b/app/react/Templates/actions/specs/templateActions.spec.js index 18e55f42d4..e1810775a4 100644 --- a/app/react/Templates/actions/specs/templateActions.spec.js +++ b/app/react/Templates/actions/specs/templateActions.spec.js @@ -220,15 +220,12 @@ describe('templateActions', () => { describe('on error', () => { it('should dispatch template_saved', async () => { - spyOn(api, 'save').and.callFake(() => Promise.reject(new Error())); + jest.spyOn(api, 'save').mockRejectedValueOnce('error'); spyOn(formActions, 'merge').and.returnValue({ type: 'mergeAction' }); const originalTemplateData = { name: 'my template', - properties: [ - { localID: 'a1b2', label: 'my property' }, - { localID: 'a1b3', label: 'my property' }, - ], + properties: [{ localID: 'a1b2', label: 'my property' }], }; const expectedActions = [ @@ -238,8 +235,11 @@ describe('templateActions', () => { const store = mockStore({}); - await store.dispatch(actions.saveTemplate(originalTemplateData)); - expect(store.getActions()).toEqual(expectedActions); + try { + await store.dispatch(actions.saveTemplate(originalTemplateData)); + } catch { + expect(store.getActions()).toEqual(expectedActions); + } }); }); }); diff --git a/app/react/Templates/components/specs/MetadataTemplate.spec.js b/app/react/Templates/components/specs/MetadataTemplate.spec.js index b5b00d17d4..e2fc027a55 100644 --- a/app/react/Templates/components/specs/MetadataTemplate.spec.js +++ b/app/react/Templates/components/specs/MetadataTemplate.spec.js @@ -22,6 +22,7 @@ import { } from 'app/Templates/components/MetadataTemplate'; import MetadataProperty from 'app/Templates/components/MetadataProperty'; import { dragSource } from 'app/Templates/components/PropertyOption'; +import * as templateActions from '../../actions/templateActions'; function sourceTargetTestContext(Target, Source, actions) { return DragDropContext(TestBackend)( @@ -248,6 +249,9 @@ describe('MetadataTemplate', () => { describe('when the mapping has conflicts', () => { it('should ask for a reindex', async () => { + props.saveTemplate = jest + .spyOn(templateActions, 'saveTemplate') + .mockRejectedValueOnce({ status: 409 }); await submitTemplate(templateWithId); context.confirm.calls.mostRecent().args[0].accept(); expect(props.saveTemplate).toHaveBeenCalledWith({ From c1a320a0b3fa1bc3f3655228e32290eca61557b8 Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 16 Nov 2021 16:30:04 -0300 Subject: [PATCH 11/26] test for saving/updating templates --- app/api/templates/specs/routes.spec.ts | 90 +++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts index 7c9ba57691..a4bcfff3ac 100644 --- a/app/api/templates/specs/routes.spec.ts +++ b/app/api/templates/specs/routes.spec.ts @@ -3,6 +3,7 @@ import request from 'supertest'; import { setUpApp } from 'api/utils/testingRoutes'; import { testingEnvironment } from 'api/utils/testingEnvironment'; import { getFixturesFactory } from 'api/utils/fixturesFactory'; +import translations from 'api/i18n'; import { UserRole } from 'shared/types/userSchema'; import templateRoutes from '../routes'; import templates from '../templates'; @@ -21,26 +22,70 @@ jest.mock( } ); +const templateCommonProperties = [ + { + _id: '6193bf8c86a5e87060962287', + localID: 'commonTitle', + label: 'Title', + name: 'title', + isCommonProperty: true, + type: 'text', + prioritySorting: false, + generatedId: false, + }, + { + _id: '6193bf8c86a5e87060962288', + localID: 'commonCreationDate', + label: 'Date added', + name: 'creationDate', + isCommonProperty: true, + type: 'date', + prioritySorting: false, + }, + { + _id: '6193bf8c86a5e87060962289', + localID: 'commonEditDate', + label: 'Date modified', + name: 'editDate', + isCommonProperty: true, + type: 'date', + prioritySorting: false, + }, +]; + const fixtureFactory = getFixturesFactory(); const fixtures = { templates: [ - fixtureFactory.template('template1', []), + { + ...fixtureFactory.template('template1', []), + commonProperties: templateCommonProperties, + }, fixtureFactory.template('template2', []), fixtureFactory.template('template3', []), ], }; +const emitToCurrentTenantSpy = jasmine.createSpy('emitToCurrentTenant'); + describe('templates routes', () => { const app: Application = setUpApp(templateRoutes, (req, _res, next: NextFunction) => { (req as any).user = { role: UserRole.ADMIN, username: 'admin', }; + req.sockets = { emitToCurrentTenant: emitToCurrentTenantSpy }; next(); }); + const postToEnpoint = async (route: string, body: any) => + request(app) + .post(route) + .send(body) + .expect(200); + beforeEach(async () => { await testingEnvironment.setUp(fixtures, 'templates_index'); + spyOn(translations, 'updateContext').and.returnValue(Promise.resolve()); }); afterAll(async () => testingEnvironment.tearDown()); @@ -75,9 +120,48 @@ describe('templates routes', () => { }); describe('POST', () => { - it('should create a template', async () => {}); + it('should create a template', async () => { + const templateToSave = { + name: 'template4', + properties: [], + commonProperties: templateCommonProperties, + }; + + await postToEnpoint('/api/templates', templateToSave); + + const savedTemplates = await templates.get(); + + expect(savedTemplates).toContainEqual(expect.objectContaining({ name: 'template4' })); + }); + + it('should update a existing template', async () => { + const [firstTemplate] = await templates.get(); + const templateToUpdate = { + ...firstTemplate, + properties: [{ label: 'Numeric', type: 'numeric', localID: 'z0x8wx8xy4' }], + commonProperties: templateCommonProperties, + __v: 0, + }; + + await postToEnpoint('/api/templates', templateToUpdate); + + const [updatedTemplate] = await templates.get({ _id: templateToUpdate._id }); + expect(updatedTemplate.properties).toContainEqual( + expect.objectContaining({ label: 'Numeric', type: 'numeric', localID: 'z0x8wx8xy4' }) + ); + }); - it('should not emit settings update when settings not modified', () => {}); + it('should not emit settings update when settings not modified', async () => { + const templateToSave = { + name: 'new template', + properties: [], + commonProperties: templateCommonProperties, + }; + + await postToEnpoint('/api/templates', templateToSave); + + expect(emitToCurrentTenantSpy).not.toHaveBeenCalledWith('updateSettings'); + }); }); describe('/templates/count_by_thesauri', () => { From bfd450caeca029b209e0748f3ea7d59958a39140 Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 16 Nov 2021 16:32:11 -0300 Subject: [PATCH 12/26] extracted fixtures to fixtures file and moved to folder --- .../specs/extractedMetadataFunctions.spec.ts | 2 +- .../specs/{ => fixtures}/fixtures.js | 0 .../specs/fixtures/routesFixtures.ts | 46 ++++++++++++++++++ app/api/templates/specs/reindex.spec.js | 2 +- app/api/templates/specs/routes.spec.ts | 47 +------------------ app/api/templates/specs/templates.spec.js | 2 +- 6 files changed, 51 insertions(+), 48 deletions(-) rename app/api/templates/specs/{ => fixtures}/fixtures.js (100%) create mode 100644 app/api/templates/specs/fixtures/routesFixtures.ts diff --git a/app/api/templates/specs/extractedMetadataFunctions.spec.ts b/app/api/templates/specs/extractedMetadataFunctions.spec.ts index 1271f7550c..aacb2ed931 100644 --- a/app/api/templates/specs/extractedMetadataFunctions.spec.ts +++ b/app/api/templates/specs/extractedMetadataFunctions.spec.ts @@ -8,7 +8,7 @@ import fixtures, { propertyC, propertyD, templateWithExtractedMetadata, -} from './fixtures'; +} from './fixtures/fixtures'; describe('updateExtractedMetadataProperties()', () => { beforeEach(async () => { diff --git a/app/api/templates/specs/fixtures.js b/app/api/templates/specs/fixtures/fixtures.js similarity index 100% rename from app/api/templates/specs/fixtures.js rename to app/api/templates/specs/fixtures/fixtures.js diff --git a/app/api/templates/specs/fixtures/routesFixtures.ts b/app/api/templates/specs/fixtures/routesFixtures.ts new file mode 100644 index 0000000000..6e35459d89 --- /dev/null +++ b/app/api/templates/specs/fixtures/routesFixtures.ts @@ -0,0 +1,46 @@ +import { getFixturesFactory } from 'api/utils/fixturesFactory'; + +const templateCommonProperties = [ + { + _id: '6193bf8c86a5e87060962287', + localID: 'commonTitle', + label: 'Title', + name: 'title', + isCommonProperty: true, + type: 'text', + prioritySorting: false, + generatedId: false, + }, + { + _id: '6193bf8c86a5e87060962288', + localID: 'commonCreationDate', + label: 'Date added', + name: 'creationDate', + isCommonProperty: true, + type: 'date', + prioritySorting: false, + }, + { + _id: '6193bf8c86a5e87060962289', + localID: 'commonEditDate', + label: 'Date modified', + name: 'editDate', + isCommonProperty: true, + type: 'date', + prioritySorting: false, + }, +]; + +const fixtureFactory = getFixturesFactory(); +const fixtures = { + templates: [ + { + ...fixtureFactory.template('template1', []), + commonProperties: templateCommonProperties, + }, + fixtureFactory.template('template2', []), + fixtureFactory.template('template3', []), + ], +}; + +export { templateCommonProperties, fixtures, fixtureFactory }; diff --git a/app/api/templates/specs/reindex.spec.js b/app/api/templates/specs/reindex.spec.js index a8ea86a3c3..ae98a1c870 100644 --- a/app/api/templates/specs/reindex.spec.js +++ b/app/api/templates/specs/reindex.spec.js @@ -3,7 +3,7 @@ import { search } from 'api/search'; import { propertyTypes } from 'shared/propertyTypes'; import templates from '../templates'; import { checkIfReindex } from '../reindex'; -import fixtures, { templateWithContents } from './fixtures'; +import fixtures, { templateWithContents } from './fixtures/fixtures'; const getAndUpdateTemplate = async props => { const [template] = await templates.get({ _id: templateWithContents }); diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts index a4bcfff3ac..8ed6183fa2 100644 --- a/app/api/templates/specs/routes.spec.ts +++ b/app/api/templates/specs/routes.spec.ts @@ -2,11 +2,11 @@ import { Application, NextFunction } from 'express'; import request from 'supertest'; import { setUpApp } from 'api/utils/testingRoutes'; import { testingEnvironment } from 'api/utils/testingEnvironment'; -import { getFixturesFactory } from 'api/utils/fixturesFactory'; import translations from 'api/i18n'; import { UserRole } from 'shared/types/userSchema'; import templateRoutes from '../routes'; import templates from '../templates'; +import { templateCommonProperties, fixtures, fixtureFactory } from './fixtures/routesFixtures'; jest.mock( '../../auth/authMiddleware.ts', @@ -22,49 +22,6 @@ jest.mock( } ); -const templateCommonProperties = [ - { - _id: '6193bf8c86a5e87060962287', - localID: 'commonTitle', - label: 'Title', - name: 'title', - isCommonProperty: true, - type: 'text', - prioritySorting: false, - generatedId: false, - }, - { - _id: '6193bf8c86a5e87060962288', - localID: 'commonCreationDate', - label: 'Date added', - name: 'creationDate', - isCommonProperty: true, - type: 'date', - prioritySorting: false, - }, - { - _id: '6193bf8c86a5e87060962289', - localID: 'commonEditDate', - label: 'Date modified', - name: 'editDate', - isCommonProperty: true, - type: 'date', - prioritySorting: false, - }, -]; - -const fixtureFactory = getFixturesFactory(); -const fixtures = { - templates: [ - { - ...fixtureFactory.template('template1', []), - commonProperties: templateCommonProperties, - }, - fixtureFactory.template('template2', []), - fixtureFactory.template('template3', []), - ], -}; - const emitToCurrentTenantSpy = jasmine.createSpy('emitToCurrentTenant'); describe('templates routes', () => { @@ -165,8 +122,8 @@ describe('templates routes', () => { }); describe('/templates/count_by_thesauri', () => { - it('should have a validation schema', () => {}); it('should return the number of templates using a thesauri', () => {}); + it('should have a validation schema', () => {}); }); describe('/api/templates/setasdefault', () => { diff --git a/app/api/templates/specs/templates.spec.js b/app/api/templates/specs/templates.spec.js index 022e4dd1be..f8d80b0e07 100644 --- a/app/api/templates/specs/templates.spec.js +++ b/app/api/templates/specs/templates.spec.js @@ -23,7 +23,7 @@ import fixtures, { relatedTo, thesauriId1, thesauriId2, -} from './fixtures.js'; +} from './fixtures/fixtures.js'; describe('templates', () => { const elasticIndex = 'templates_spec_index'; From 1bf0381e2ee50e50525df1ee9ef97b6991094bfd Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 16 Nov 2021 18:06:18 -0300 Subject: [PATCH 13/26] set as default and count by thesauri tests --- .../specs/fixtures/routesFixtures.ts | 45 ++++++++++++++++++- app/api/templates/specs/routes.spec.ts | 40 +++++++++++++---- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/app/api/templates/specs/fixtures/routesFixtures.ts b/app/api/templates/specs/fixtures/routesFixtures.ts index 6e35459d89..b07a87d5ba 100644 --- a/app/api/templates/specs/fixtures/routesFixtures.ts +++ b/app/api/templates/specs/fixtures/routesFixtures.ts @@ -33,13 +33,54 @@ const templateCommonProperties = [ const fixtureFactory = getFixturesFactory(); const fixtures = { + dictionaries: [ + fixtureFactory.thesauri('select_thesaurus', ['A']), + fixtureFactory.thesauri('multiselect_thesaurus', ['A', 'B']), + { + name: 'select_thesaurus_2', + _id: '123456789', + values: [ + { + _id: 'abc', + id: 'A', + label: 'A', + }, + ], + }, + ], templates: [ { ...fixtureFactory.template('template1', []), commonProperties: templateCommonProperties, + default: true, + }, + fixtureFactory.template('template2', [ + fixtureFactory.property('select_property', 'select', { + content: fixtureFactory.id('select_thesaurus'), + }), + fixtureFactory.property('multiselect_property', 'multiselect', { + content: fixtureFactory.id('multiselect_thesaurus'), + }), + ]), + fixtureFactory.template('template3', [ + fixtureFactory.property('select_property', 'select', { + content: fixtureFactory.id('select_thesaurus'), + }), + ]), + { + _id: 'template_with_select_id', + name: 'template_with_select', + properties: [ + { + _id: 'zxc', + id: 'select_property', + label: 'select_property', + name: 'select_property', + type: 'select', + content: '123456789', + }, + ], }, - fixtureFactory.template('template2', []), - fixtureFactory.template('template3', []), ], }; diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts index 8ed6183fa2..d5e1398d8d 100644 --- a/app/api/templates/specs/routes.spec.ts +++ b/app/api/templates/specs/routes.spec.ts @@ -59,8 +59,9 @@ describe('templates routes', () => { describe('DELETE', () => { it('should delete a template', async () => { + const templateId = fixtureFactory.id('template2'); await request(app) - .delete(`/api/templates?_id=${fixtureFactory.template('template2', [])._id}`) + .delete(`/api/templates?_id=${templateId}`) .expect(200); const remainingTemplates = await templates.get(); expect(remainingTemplates).toContainEqual(expect.objectContaining({ name: 'template1' })); @@ -122,17 +123,40 @@ describe('templates routes', () => { }); describe('/templates/count_by_thesauri', () => { - it('should return the number of templates using a thesauri', () => {}); - it('should have a validation schema', () => {}); + it('should return the number of templates using a thesauri', async () => { + const { body } = await request(app) + .get('/api/templates/count_by_thesauri?_id=123456789') + .expect(200); + + expect(body).toBe(1); + }); + it('should have a validation schema', async () => { + const { body } = await request(app).get('/api/templates/count_by_thesauri'); + expect(body.error).toBe('validation failed'); + }); }); describe('/api/templates/setasdefault', () => { - it('should have a validation schema', () => {}); + it('should call templates to set the new default', async () => { + const template2Id = fixtureFactory.id('template2'); + await request(app) + .post('/api/templates/setasdefault') + .send({ _id: template2Id }) + .expect(200); - it('should call templates to set the new default', () => {}); - }); + const savedTemplates = await templates.get(); + + expect(savedTemplates).toContainEqual( + expect.objectContaining({ name: 'template1', default: false }) + ); + expect(savedTemplates).toContainEqual( + expect.objectContaining({ name: 'template2', default: true }) + ); + }); - describe('check mappings', () => { - it('should check if a template is valid vs the current elasticsearch mapping', () => {}); + it('should have a validation schema', async () => { + const { body } = await request(app).post('/api/templates/setasdefault'); + expect(body.error).toBe('validation failed'); + }); }); }); From 1fc845b143840c7cde04c3d75c208d04cafcc340 Mon Sep 17 00:00:00 2001 From: Santiago Date: Wed, 17 Nov 2021 09:56:22 -0300 Subject: [PATCH 14/26] test check mappings logic --- app/api/templates/specs/routes.spec.ts | 55 ++++++++++++++++++++------ 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts index d5e1398d8d..fbd60e5cbc 100644 --- a/app/api/templates/specs/routes.spec.ts +++ b/app/api/templates/specs/routes.spec.ts @@ -22,6 +22,12 @@ jest.mock( } ); +const templateToSave = { + name: 'template4', + properties: [], + commonProperties: templateCommonProperties, +}; + const emitToCurrentTenantSpy = jasmine.createSpy('emitToCurrentTenant'); describe('templates routes', () => { @@ -79,12 +85,6 @@ describe('templates routes', () => { describe('POST', () => { it('should create a template', async () => { - const templateToSave = { - name: 'template4', - properties: [], - commonProperties: templateCommonProperties, - }; - await postToEnpoint('/api/templates', templateToSave); const savedTemplates = await templates.get(); @@ -110,12 +110,6 @@ describe('templates routes', () => { }); it('should not emit settings update when settings not modified', async () => { - const templateToSave = { - name: 'new template', - properties: [], - commonProperties: templateCommonProperties, - }; - await postToEnpoint('/api/templates', templateToSave); expect(emitToCurrentTenantSpy).not.toHaveBeenCalledWith('updateSettings'); @@ -159,4 +153,41 @@ describe('templates routes', () => { expect(body.error).toBe('validation failed'); }); }); + + describe('check mappings', () => { + beforeEach(async () => { + await postToEnpoint('/api/templates', { + ...templateToSave, + properties: [ + { + label: 'Numeric', + type: 'numeric', + localID: 'byhrp7qv54i', + name: 'numeric', + id: 'Numeric', + }, + ], + }); + }); + it('should throw an error if template is invalid vs the current elasticsearch mapping', async () => { + const savedTemplate = await templates.get({ name: 'template4' }); + try { + await postToEnpoint('/api/templates', { + ...savedTemplate, + properties: [ + { + label: 'Numeric', + type: 'text', + localID: 'byhrp7qv54i', + name: 'numeric', + id: 'text', + }, + ], + reindex: false, + }); + } catch (error) { + expect(error.message).toContain('expected 200 "OK", got 409 "Conflict"'); + } + }); + }); }); From e06505a80a9648e32f46dcf200d0daf6a6c19f79 Mon Sep 17 00:00:00 2001 From: Santiago Date: Wed, 17 Nov 2021 09:56:52 -0300 Subject: [PATCH 15/26] refactor --- app/api/templates/routes.ts | 13 ++++--------- app/api/templates/specs/routes.spec.ts | 4 +--- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/app/api/templates/routes.ts b/app/api/templates/routes.ts index b0e501c90e..551cf9c9ca 100644 --- a/app/api/templates/routes.ts +++ b/app/api/templates/routes.ts @@ -14,12 +14,6 @@ const reindexAllTemplates = async () => { return reindexAll(allTemplates, search); }; -const checkTemplateMappings = async (template: TemplateSchema) => { - const templateProperties = await generateNamesAndIds(template.properties); - const { valid, error } = await checkMapping({ ...template, properties: templateProperties }); - return { valid, error }; -}; - const saveTemplate = async (template: TemplateSchema, language: string, fullReindex?: boolean) => { const reindex = fullReindex ? false : await checkIfReindex(template); return templates.save(template, language, reindex); @@ -28,11 +22,12 @@ const saveTemplate = async (template: TemplateSchema, language: string, fullRein const prepareRequest = async (body: TemplateSchema & { reindex?: boolean }) => { const request = { ...body }; const { reindex: fullReindex } = request; - delete request.reindex; - const template = { ...request }; - const { valid, error } = await checkTemplateMappings(template); + + const templateProperties = await generateNamesAndIds(template.properties); + const { valid, error } = await checkMapping({ ...template, properties: templateProperties }); + return { template, fullReindex, valid, error }; }; diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts index fbd60e5cbc..e986e78ae2 100644 --- a/app/api/templates/specs/routes.spec.ts +++ b/app/api/templates/specs/routes.spec.ts @@ -155,7 +155,7 @@ describe('templates routes', () => { }); describe('check mappings', () => { - beforeEach(async () => { + it('should throw an error if template is invalid vs the current elasticsearch mapping', async () => { await postToEnpoint('/api/templates', { ...templateToSave, properties: [ @@ -168,8 +168,6 @@ describe('templates routes', () => { }, ], }); - }); - it('should throw an error if template is invalid vs the current elasticsearch mapping', async () => { const savedTemplate = await templates.get({ name: 'template4' }); try { await postToEnpoint('/api/templates', { From 032ee0c953ca65347d0bf6fdf1b600f41884f5af Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 23 Nov 2021 10:56:56 -0300 Subject: [PATCH 16/26] removed unneeded user setup --- app/api/templates/specs/routes.spec.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/api/templates/specs/routes.spec.ts b/app/api/templates/specs/routes.spec.ts index e986e78ae2..1e662c5acc 100644 --- a/app/api/templates/specs/routes.spec.ts +++ b/app/api/templates/specs/routes.spec.ts @@ -3,7 +3,6 @@ import request from 'supertest'; import { setUpApp } from 'api/utils/testingRoutes'; import { testingEnvironment } from 'api/utils/testingEnvironment'; import translations from 'api/i18n'; -import { UserRole } from 'shared/types/userSchema'; import templateRoutes from '../routes'; import templates from '../templates'; import { templateCommonProperties, fixtures, fixtureFactory } from './fixtures/routesFixtures'; @@ -32,10 +31,6 @@ const emitToCurrentTenantSpy = jasmine.createSpy('emitToCurrentTenant'); describe('templates routes', () => { const app: Application = setUpApp(templateRoutes, (req, _res, next: NextFunction) => { - (req as any).user = { - role: UserRole.ADMIN, - username: 'admin', - }; req.sockets = { emitToCurrentTenant: emitToCurrentTenantSpy }; next(); }); From 3e53fb4de7b683f4cf3c969eaf03b78797247c6e Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 23 Nov 2021 15:40:04 -0300 Subject: [PATCH 17/26] not rendering options with 'any' id --- app/react/Forms/components/MultiSelect.tsx | 37 ++++++++++--------- .../components/specs/MultiSelect.spec.js | 11 ++++++ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/app/react/Forms/components/MultiSelect.tsx b/app/react/Forms/components/MultiSelect.tsx index 6a96ecabc1..06b671db37 100644 --- a/app/react/Forms/components/MultiSelect.tsx +++ b/app/react/Forms/components/MultiSelect.tsx @@ -317,24 +317,27 @@ abstract class MultiSelectBase extends Component< renderOption(option: Option, index: number, groupIndex = '') { const { optionsValue, optionsLabel, prefix } = this.props; const key = `${groupIndex}${index}`; + const isNotAnyOption = option.id !== 'any'; return ( -
  • - - {this.label(option)} -
    • + isNotAnyOption && ( +
  • + + {this.label(option)} +
    • + ) ); } diff --git a/app/react/Forms/components/specs/MultiSelect.spec.js b/app/react/Forms/components/specs/MultiSelect.spec.js index bc46d9c5d0..310219ac40 100644 --- a/app/react/Forms/components/specs/MultiSelect.spec.js +++ b/app/react/Forms/components/specs/MultiSelect.spec.js @@ -131,6 +131,17 @@ describe('MultiSelect', () => { .simulate('change'); expect(props.onChange).toHaveBeenCalledWith(['option2']); }); + + it('should not render options with the `any` id', () => { + props.options.push({ + id: 'any', + value: 'any', + label: 'Any', + results: 13, + }); + render(); + expect(component.find('li[title="Any"]').length).toBe(0); + }); }); describe('checking a group', () => { From f42fd95c08f4d41f16864503302504a17c6476e4 Mon Sep 17 00:00:00 2001 From: Santiago Date: Wed, 24 Nov 2021 10:20:18 -0300 Subject: [PATCH 18/26] Revert "not rendering options with 'any' id" This reverts commit 3e53fb4de7b683f4cf3c969eaf03b78797247c6e. --- app/react/Forms/components/MultiSelect.tsx | 37 +++++++++---------- .../components/specs/MultiSelect.spec.js | 11 ------ 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/app/react/Forms/components/MultiSelect.tsx b/app/react/Forms/components/MultiSelect.tsx index 06b671db37..6a96ecabc1 100644 --- a/app/react/Forms/components/MultiSelect.tsx +++ b/app/react/Forms/components/MultiSelect.tsx @@ -317,27 +317,24 @@ abstract class MultiSelectBase extends Component< renderOption(option: Option, index: number, groupIndex = '') { const { optionsValue, optionsLabel, prefix } = this.props; const key = `${groupIndex}${index}`; - const isNotAnyOption = option.id !== 'any'; return ( - isNotAnyOption && ( -
  • - - {this.label(option)} -
    • - ) +
  • + + {this.label(option)} +
    • ); } diff --git a/app/react/Forms/components/specs/MultiSelect.spec.js b/app/react/Forms/components/specs/MultiSelect.spec.js index 310219ac40..bc46d9c5d0 100644 --- a/app/react/Forms/components/specs/MultiSelect.spec.js +++ b/app/react/Forms/components/specs/MultiSelect.spec.js @@ -131,17 +131,6 @@ describe('MultiSelect', () => { .simulate('change'); expect(props.onChange).toHaveBeenCalledWith(['option2']); }); - - it('should not render options with the `any` id', () => { - props.options.push({ - id: 'any', - value: 'any', - label: 'Any', - results: 13, - }); - render(); - expect(component.find('li[title="Any"]').length).toBe(0); - }); }); describe('checking a group', () => { From 008fecc3deb0a4629e521d795014ecb95042d859 Mon Sep 17 00:00:00 2001 From: Santiago Date: Wed, 24 Nov 2021 11:07:29 -0300 Subject: [PATCH 19/26] removed 'any' from options --- .../components/FiltersFromProperties.js | 41 +++-- .../specs/FiltersFromProperties.spec.js | 164 ++++++++++++------ 2 files changed, 138 insertions(+), 67 deletions(-) diff --git a/app/react/Library/components/FiltersFromProperties.js b/app/react/Library/components/FiltersFromProperties.js index 60cc942d6f..d959f8ff33 100644 --- a/app/react/Library/components/FiltersFromProperties.js +++ b/app/react/Library/components/FiltersFromProperties.js @@ -13,6 +13,28 @@ import NumberRangeFilter from './NumberRangeFilter'; import SelectFilter from './SelectFilter'; import TextFilter from './TextFilter'; +const prepareOptions = property => { + const filteredProperty = { + ...property, + options: property.options.filter(option => option.id !== 'any'), + }; + return filteredProperty.options.map(option => { + const finalTranslatedOption = { + ...option, + label: t(filteredProperty.content, option.label, undefined, false), + }; + + if (option.options) { + const translatedSubOptions = option.options.map(subOption => ({ + ...subOption, + label: t(filteredProperty.content, subOption.label, undefined, false), + })); + finalTranslatedOption.options = translatedSubOptions; + } + return finalTranslatedOption; + }); +}; + export const FiltersFromProperties = ({ onChange, properties, @@ -32,24 +54,7 @@ export const FiltersFromProperties = ({ onChange, }; - const propertyOptions = property.options - ? property.options.map(option => { - const finalTranslatedOption = { - ...option, - label: t(property.content, option.label, undefined, false), - }; - - if (option.options) { - const translatedSubOptions = option.options.map(subOption => ({ - ...subOption, - label: t(property.content, subOption.label, undefined, false), - })); - finalTranslatedOption.options = translatedSubOptions; - } - - return finalTranslatedOption; - }) - : []; + const propertyOptions = property.options ? prepareOptions(property) : []; let filter = ; diff --git a/app/react/Library/components/specs/FiltersFromProperties.spec.js b/app/react/Library/components/specs/FiltersFromProperties.spec.js index dc58fc8fed..4827224b3e 100644 --- a/app/react/Library/components/specs/FiltersFromProperties.spec.js +++ b/app/react/Library/components/specs/FiltersFromProperties.spec.js @@ -17,28 +17,33 @@ jest.mock('app/I18N', () => ({ default: jest.fn(), })); -describe('FiltersFromProperties', () => { - let props = {}; - - beforeEach(() => { - t.mockImplementation((_context, label) => label); - const state = { - settings: { collection: Immutable.fromJS({ dateFormat: 'dateFormat' }) }, - library: { aggregations: Immutable.fromJS({ aggregations: 'aggregations' }) }, - templates: Immutable.fromJS([ - { - name: 'blah', - properties: [ - { _id: '1234', type: 'date', name: 'age' }, - { _id: '4567', type: 'text', name: 'name' }, - ], - }, - ]), - }; +let props = {}; +let component; - props = mapStateToProps(state, { storeKey: 'library' }); - }); +beforeEach(() => { + t.mockImplementation((_context, label) => label); + const state = { + settings: { collection: Immutable.fromJS({ dateFormat: 'dateFormat' }) }, + library: { aggregations: Immutable.fromJS({ aggregations: 'aggregations' }) }, + templates: Immutable.fromJS([ + { + name: 'blah', + properties: [ + { _id: '1234', type: 'date', name: 'age' }, + { _id: '4567', type: 'text', name: 'name' }, + ], + }, + ]), + }; + props = mapStateToProps(state, { storeKey: 'library' }); + props.translationContext = 'oneContext'; +}); +const render = () => { + component = shallow(); +}; + +describe('FiltersFromProperties', () => { it('should concat the modelPrefix with the model', () => { props.properties = [ { @@ -46,11 +51,13 @@ describe('FiltersFromProperties', () => { label: 'textLabel', }, ]; - props.translationContext = 'oneContext'; + props.modelPrefix = '.prefix'; - const component = shallow().find(TextFilter); - expect(component).toMatchSnapshot(); + render(); + + const textFilter = component.find(TextFilter); + expect(textFilter).toMatchSnapshot(); }); describe('when type is text', () => { @@ -61,10 +68,11 @@ describe('FiltersFromProperties', () => { label: 'textLabel', }, ]; - props.translationContext = 'oneContext'; - const component = shallow().find(TextFilter); - expect(component).toMatchSnapshot(); + render(); + + const textFilter = component.find(TextFilter); + expect(textFilter).toMatchSnapshot(); }); }); @@ -93,10 +101,10 @@ describe('FiltersFromProperties', () => { options: [{ label: 'option2' }], }, ]; - props.translationContext = 'oneContext'; + render(); - const component = shallow().find(SelectFilter); - expect(component).toMatchSnapshot(); + const selectFilter = component.find(SelectFilter); + expect(selectFilter).toMatchSnapshot(); }); it('should translate the options of filter with thesaurus', () => { @@ -116,15 +124,18 @@ describe('FiltersFromProperties', () => { options: [{ label: 'option2' }], }, ]; - props.translationContext = 'oneContext'; + t.mockImplementation(() => 'translatedOption'); - const component = shallow().find(SelectFilter); + + render(); + + const selectFilter = component.find(SelectFilter); const _text = undefined; const returnComponent = false; expect(t).toHaveBeenCalledWith('thesaurus1', 'option1', _text, returnComponent); expect(t).toHaveBeenCalledWith('thesaurus2', 'option2', _text, returnComponent); - expect(component.get(0).props.options[0].label).toBe('translatedOption'); - expect(component.get(1).props.options[0].label).toBe('translatedOption'); + expect(selectFilter.get(0).props.options[0].label).toBe('translatedOption'); + expect(selectFilter.get(1).props.options[0].label).toBe('translatedOption'); }); it('should translate the options of filter with nested thesauris', () => { @@ -144,15 +155,18 @@ describe('FiltersFromProperties', () => { options: [{ label: 'option2', options: [{ label: 'suboption2' }] }], }, ]; - props.translationContext = 'oneContext'; + t.mockImplementation(() => 'translatedOption'); - const component = shallow().find(SelectFilter); + + render(); + + const selectFilter = component.find(SelectFilter); const _text = undefined; const returnComponent = false; expect(t).toHaveBeenCalledWith('thesaurus1', 'suboption1', _text, returnComponent); expect(t).toHaveBeenCalledWith('thesaurus2', 'suboption2', _text, returnComponent); - expect(component.get(0).props.options[0].options[0].label).toBe('translatedOption'); - expect(component.get(1).props.options[0].options[0].label).toBe('translatedOption'); + expect(selectFilter.get(0).props.options[0].options[0].label).toBe('translatedOption'); + expect(selectFilter.get(1).props.options[0].options[0].label).toBe('translatedOption'); }); }); @@ -168,9 +182,11 @@ describe('FiltersFromProperties', () => { options: [{ label: 'option2' }], }, ]; - props.translationContext = 'oneContext'; - const component = shallow().find(DateFilter); - expect(component.length).toBe(1); + + render(); + + const dateFilter = component.find(DateFilter); + expect(dateFilter.length).toBe(1); }); }); @@ -202,10 +218,11 @@ describe('FiltersFromProperties', () => { type: 'multidaterange', }, ]; - props.translationContext = 'oneContext'; - const component = shallow().find(DateFilter); - expect(component).toMatchSnapshot(); + render(); + + const dateFilter = component.find(DateFilter); + expect(dateFilter).toMatchSnapshot(); }); }); @@ -218,10 +235,11 @@ describe('FiltersFromProperties', () => { type: 'numeric', }, ]; - props.translationContext = 'oneContext'; - const component = shallow().find(NumberRangeFilter); - expect(component).toMatchSnapshot(); + render(); + + const numberRangeFilter = component.find(NumberRangeFilter); + expect(numberRangeFilter).toMatchSnapshot(); }); }); @@ -234,10 +252,58 @@ describe('FiltersFromProperties', () => { type: 'nested', }, ]; - props.translationContext = 'oneContext'; - const component = shallow().find(NestedFilter); - expect(component).toMatchSnapshot(); + render(); + + const nestedFilter = component.find(NestedFilter); + expect(nestedFilter).toMatchSnapshot(); }); }); + + it('should not render `any` options', () => { + props.properties = [ + { + localID: 'srhkbn1bbqi', + name: 'property', + type: 'multiselect', + options: [ + { + id: '7ycel666l65vobt9', + value: '7ycel666l65vobt9', + label: 'Option 1', + results: 10, + }, + { + id: 'mbpzxhzlfep6tj4i', + value: 'mbpzxhzlfep6tj4i', + label: 'Option 2', + results: 5, + }, + { + id: 'any', + value: 'any', + label: 'Any', + results: 15, + }, + ], + }, + ]; + + render(); + const { options } = component.find(SelectFilter).props(); + expect(options).toMatchObject([ + { + id: '7ycel666l65vobt9', + value: '7ycel666l65vobt9', + label: 'Option 1', + results: 10, + }, + { + id: 'mbpzxhzlfep6tj4i', + value: 'mbpzxhzlfep6tj4i', + label: 'Option 2', + results: 5, + }, + ]); + }); }); From 135abbd4f853cf29257289d0145357cd6704c867 Mon Sep 17 00:00:00 2001 From: Santiago Date: Wed, 24 Nov 2021 11:39:36 -0300 Subject: [PATCH 20/26] refactored test --- .../specs/FiltersFromProperties.spec.js | 91 +------------------ .../fixtures/FiltersFromPropertiesFixtures.ts | 63 +++++++++++++ 2 files changed, 68 insertions(+), 86 deletions(-) create mode 100644 app/react/Library/components/specs/fixtures/FiltersFromPropertiesFixtures.ts diff --git a/app/react/Library/components/specs/FiltersFromProperties.spec.js b/app/react/Library/components/specs/FiltersFromProperties.spec.js index 4827224b3e..862fa62d57 100644 --- a/app/react/Library/components/specs/FiltersFromProperties.spec.js +++ b/app/react/Library/components/specs/FiltersFromProperties.spec.js @@ -10,6 +10,7 @@ import NestedFilter from '../NestedFilter'; import NumberRangeFilter from '../NumberRangeFilter'; import SelectFilter from '../SelectFilter'; import TextFilter from '../TextFilter'; +import { defaultProperties } from './fixtures/FiltersFromPropertiesFixtures'; jest.mock('app/I18N', () => ({ __esModule: true, @@ -44,33 +45,21 @@ const render = () => { }; describe('FiltersFromProperties', () => { - it('should concat the modelPrefix with the model', () => { - props.properties = [ - { - name: 'textFilter', - label: 'textLabel', - }, - ]; + beforeEach(() => { + props.properties = defaultProperties; + }); + it('should concat the modelPrefix with the model', () => { props.modelPrefix = '.prefix'; render(); - const textFilter = component.find(TextFilter); expect(textFilter).toMatchSnapshot(); }); describe('when type is text', () => { it('should render a text filter', () => { - props.properties = [ - { - name: 'textFilter', - label: 'textLabel', - }, - ]; - render(); - const textFilter = component.find(TextFilter); expect(textFilter).toMatchSnapshot(); }); @@ -78,31 +67,7 @@ describe('FiltersFromProperties', () => { describe('when type is select, multiselect or relationship', () => { it('should render a select filter', () => { - props.properties = [ - { - content: 'aContent', - name: 'selectFilter', - label: 'selectLabel', - type: 'select', - options: [{ label: 'option1' }], - }, - { - content: 'aContent', - name: 'multiselectFilter', - label: 'multiselectLabel', - type: 'multiselect', - options: [{ label: 'option3' }], - }, - { - content: 'aContent', - name: 'relationshipFilter', - label: 'relationshipLabel', - type: 'relationship', - options: [{ label: 'option2' }], - }, - ]; render(); - const selectFilter = component.find(SelectFilter); expect(selectFilter).toMatchSnapshot(); }); @@ -192,35 +157,7 @@ describe('FiltersFromProperties', () => { describe('when type is date, multidate, multidaterange or daterange', () => { it('should render a date filter', () => { - props.properties = [ - { - content: 'oneContent', - name: 'dateFilter', - label: 'dateLabel', - type: 'date', - }, - { - content: 'oneContent', - name: 'daterange', - label: 'daterangeLabel', - type: 'daterange', - }, - { - content: 'oneContent', - name: 'multidate', - label: 'multidateLabel', - type: 'multidate', - }, - { - content: 'oneContent', - name: 'multidaterange', - label: 'multidaterangeLabel', - type: 'multidaterange', - }, - ]; - render(); - const dateFilter = component.find(DateFilter); expect(dateFilter).toMatchSnapshot(); }); @@ -228,16 +165,7 @@ describe('FiltersFromProperties', () => { describe('when type is numeric', () => { it('should render a number range filter', () => { - props.properties = [ - { - name: 'numericFilter', - label: 'numericLabel', - type: 'numeric', - }, - ]; - render(); - const numberRangeFilter = component.find(NumberRangeFilter); expect(numberRangeFilter).toMatchSnapshot(); }); @@ -245,16 +173,7 @@ describe('FiltersFromProperties', () => { describe('when type is nested', () => { it('should render a number range filter', () => { - props.properties = [ - { - name: 'nestedFilter', - label: 'nestedLabel', - type: 'nested', - }, - ]; - render(); - const nestedFilter = component.find(NestedFilter); expect(nestedFilter).toMatchSnapshot(); }); diff --git a/app/react/Library/components/specs/fixtures/FiltersFromPropertiesFixtures.ts b/app/react/Library/components/specs/fixtures/FiltersFromPropertiesFixtures.ts new file mode 100644 index 0000000000..df5fda707b --- /dev/null +++ b/app/react/Library/components/specs/fixtures/FiltersFromPropertiesFixtures.ts @@ -0,0 +1,63 @@ +const defaultProperties = [ + { + name: 'textFilter', + label: 'textLabel', + }, + { + name: 'numericFilter', + label: 'numericLabel', + type: 'numeric', + }, + { + name: 'nestedFilter', + label: 'nestedLabel', + type: 'nested', + }, + { + content: 'aContent', + name: 'selectFilter', + label: 'selectLabel', + type: 'select', + options: [{ label: 'option1' }], + }, + { + content: 'aContent', + name: 'multiselectFilter', + label: 'multiselectLabel', + type: 'multiselect', + options: [{ label: 'option3' }], + }, + { + content: 'aContent', + name: 'relationshipFilter', + label: 'relationshipLabel', + type: 'relationship', + options: [{ label: 'option2' }], + }, + { + content: 'oneContent', + name: 'dateFilter', + label: 'dateLabel', + type: 'date', + }, + { + content: 'oneContent', + name: 'daterange', + label: 'daterangeLabel', + type: 'daterange', + }, + { + content: 'oneContent', + name: 'multidate', + label: 'multidateLabel', + type: 'multidate', + }, + { + content: 'oneContent', + name: 'multidaterange', + label: 'multidaterangeLabel', + type: 'multidaterange', + }, +]; + +export { defaultProperties }; From ca28c9501b51dc0b1a4c002750ba1e873f7d80fa Mon Sep 17 00:00:00 2001 From: Santiago Date: Fri, 19 Nov 2021 10:28:08 -0300 Subject: [PATCH 21/26] sidepanel starts closed if entity has page view --- app/react/Entities/components/EntityViewer.js | 2 +- .../components/specs/EntityViewer.spec.js | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/react/Entities/components/EntityViewer.js b/app/react/Entities/components/EntityViewer.js index fe38e0fd26..b07f3d2daf 100644 --- a/app/react/Entities/components/EntityViewer.js +++ b/app/react/Entities/components/EntityViewer.js @@ -36,7 +36,7 @@ export class EntityViewer extends Component { constructor(props, context) { super(props, context); this.state = { - panelOpen: true, + panelOpen: !this.props.hasPageView, copyFrom: false, copyFromProps: [], }; diff --git a/app/react/Entities/components/specs/EntityViewer.spec.js b/app/react/Entities/components/specs/EntityViewer.spec.js index de2eb2bb60..0bdf0b91eb 100644 --- a/app/react/Entities/components/specs/EntityViewer.spec.js +++ b/app/react/Entities/components/specs/EntityViewer.spec.js @@ -45,6 +45,7 @@ describe('EntityViewer', () => { ], }, ]), + hasPageView: false, deleteConnection: jasmine.createSpy('deleteConnection'), startNewConnection: jasmine.createSpy('startNewConnection'), deleteEntity: jasmine.createSpy('deleteEntity'), @@ -128,17 +129,18 @@ describe('EntityViewer', () => { }); describe('closing side panel', () => { - beforeEach(() => { + it('should close the side panel when close button is clicked', () => { render(); component.find('.closeSidepanel').simulate('click'); component.update(); - }); - it('should close the side panel when close button is clicked', () => { expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(false); expect(component.find('.entity-connections').prop('open')).toBe(false); expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(true); }); it('should reveal side panel when context menu is clicked', () => { + render(); + component.find('.closeSidepanel').simulate('click'); + component.update(); expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(false); component.find('.show-info-sidepanel-menu').prop('openPanel')(); @@ -148,6 +150,13 @@ describe('EntityViewer', () => { expect(component.find('.entity-connections').prop('open')).toBe(true); expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(false); }); + it('should have the sidepanel closed by default when entity has page view', () => { + props.hasPageView = true; + render(); + expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(false); + expect(component.find('.entity-connections').prop('open')).toBe(false); + expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(true); + }); }); describe('mapStateToProps', () => { From 948ef09dcc8f06210069081f5a0742bcfb4fe7f4 Mon Sep 17 00:00:00 2001 From: Santiago Date: Fri, 19 Nov 2021 10:46:39 -0300 Subject: [PATCH 22/26] small test refactor --- .../components/specs/EntityViewer.spec.js | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/app/react/Entities/components/specs/EntityViewer.spec.js b/app/react/Entities/components/specs/EntityViewer.spec.js index 0bdf0b91eb..d7e2a57bbd 100644 --- a/app/react/Entities/components/specs/EntityViewer.spec.js +++ b/app/react/Entities/components/specs/EntityViewer.spec.js @@ -45,7 +45,6 @@ describe('EntityViewer', () => { ], }, ]), - hasPageView: false, deleteConnection: jasmine.createSpy('deleteConnection'), startNewConnection: jasmine.createSpy('startNewConnection'), deleteEntity: jasmine.createSpy('deleteEntity'), @@ -128,28 +127,14 @@ describe('EntityViewer', () => { }); }); - describe('closing side panel', () => { - it('should close the side panel when close button is clicked', () => { + describe('Side panel', () => { + it('should have the sidepanel open by default', () => { render(); - component.find('.closeSidepanel').simulate('click'); - component.update(); - expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(false); - expect(component.find('.entity-connections').prop('open')).toBe(false); - expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(true); - }); - it('should reveal side panel when context menu is clicked', () => { - render(); - component.find('.closeSidepanel').simulate('click'); - component.update(); - expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(false); - - component.find('.show-info-sidepanel-menu').prop('openPanel')(); - component.update(); - expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(true); expect(component.find('.entity-connections').prop('open')).toBe(true); expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(false); }); + it('should have the sidepanel closed by default when entity has page view', () => { props.hasPageView = true; render(); @@ -157,6 +142,31 @@ describe('EntityViewer', () => { expect(component.find('.entity-connections').prop('open')).toBe(false); expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(true); }); + + describe('toggling the side panel', () => { + beforeEach(() => { + render(); + component.find('.closeSidepanel').simulate('click'); + component.update(); + }); + + it('should close the side panel when close button is clicked', () => { + expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(false); + expect(component.find('.entity-connections').prop('open')).toBe(false); + expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(true); + }); + + it('should reveal side panel when context menu is clicked', () => { + expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(false); + + component.find('.show-info-sidepanel-menu').prop('openPanel')(); + component.update(); + + expect(component.find('.entity-viewer').hasClass('with-panel')).toBe(true); + expect(component.find('.entity-connections').prop('open')).toBe(true); + expect(component.find('.show-info-sidepanel-context-menu').prop('show')).toBe(false); + }); + }); }); describe('mapStateToProps', () => { From 7d8a96a634638f0dadf3e147dafcb8d284abf1ff Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 23 Nov 2021 11:08:59 -0300 Subject: [PATCH 23/26] changed open side panel button --- app/react/Entities/components/ShowSidepanelMenu.js | 2 +- app/react/UI/Icon/library.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/react/Entities/components/ShowSidepanelMenu.js b/app/react/Entities/components/ShowSidepanelMenu.js index b0bf6b9750..e09324735c 100644 --- a/app/react/Entities/components/ShowSidepanelMenu.js +++ b/app/react/Entities/components/ShowSidepanelMenu.js @@ -13,7 +13,7 @@ export class ShowSidepanelMenu extends Component {
    openPanel(e)}> - +
    diff --git a/app/react/UI/Icon/library.js b/app/react/UI/Icon/library.js index a9fa408f72..6a7b7e2617 100644 --- a/app/react/UI/Icon/library.js +++ b/app/react/UI/Icon/library.js @@ -104,6 +104,7 @@ import { infoCircleHollow } from 'UI/Icon/info-circle-hollow'; import { faTasks } from '@fortawesome/free-solid-svg-icons/faTasks'; import { faMap } from '@fortawesome/free-solid-svg-icons/faMap'; import { faMapMarkerAlt } from '@fortawesome/free-solid-svg-icons/faMapMarkerAlt'; +import { faColumns } from '@fortawesome/free-solid-svg-icons'; import { saveAndNext } from './save-and-next'; import { exportCsv } from './export-csv'; import { importCsv } from './import-csv'; @@ -221,6 +222,7 @@ const icons = { twoFactorAuth, infoCircleHollow, faBullseye, + faColumns, }; export const loadIcons = () => { From 3a49a6c17ac18815e4f6cca5ccdcff01d33f3105 Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 23 Nov 2021 11:19:03 -0300 Subject: [PATCH 24/26] made icon bigger --- app/react/App/scss/modules/_ContextMenu.scss | 22 +++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/app/react/App/scss/modules/_ContextMenu.scss b/app/react/App/scss/modules/_ContextMenu.scss index 1f98b3f236..792743df58 100644 --- a/app/react/App/scss/modules/_ContextMenu.scss +++ b/app/react/App/scss/modules/_ContextMenu.scss @@ -14,7 +14,7 @@ .document-viewer.is-active ~ &, .document-viewer.is-active & { - left: calc((100% - 400px)/2); + left: calc((100% - 400px) / 2); } } @@ -24,7 +24,7 @@ width: $header-height; height: $header-height; line-height: $header-height; - font-size: $f-size-lg; + font-size: $f-size-xl; padding: 0; margin: 0 6px; opacity: 0.9; @@ -35,9 +35,17 @@ animation-name: ContextMenu-center; animation-duration: 225ms; @keyframes ContextMenu-center { - 0% { transform: scale(0.5); opacity: 0; } - 25% { transform: scale(1.1); opacity: 0.9; } - 100% { transform: scale(1); } + 0% { + transform: scale(0.5); + opacity: 0; + } + 25% { + transform: scale(1.1); + opacity: 0.9; + } + 100% { + transform: scale(1); + } } } @@ -50,7 +58,7 @@ left: 50%; transform: translateX(-50%) translateY(-10px); padding: 5px; - background-color: transparentize($c-black, .25); + background-color: transparentize($c-black, 0.25); color: $c-white; border-radius: $border-radius; text-align: center; @@ -60,7 +68,7 @@ content: ''; border-left: 5px solid transparent; border-right: 5px solid transparent; - border-top: 5px solid transparentize($c-black, .25); + border-top: 5px solid transparentize($c-black, 0.25); position: absolute; top: 100%; left: 50%; From 48596fa17b6862ce068d27dd2861319b1ae9db4a Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 23 Nov 2021 11:44:40 -0300 Subject: [PATCH 25/26] updated snapshot --- .../specs/__snapshots__/ShowSidepanelMenu.spec.js.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/react/Entities/components/specs/__snapshots__/ShowSidepanelMenu.spec.js.snap b/app/react/Entities/components/specs/__snapshots__/ShowSidepanelMenu.spec.js.snap index b17ffccbe8..9cd2f13e4c 100644 --- a/app/react/Entities/components/specs/__snapshots__/ShowSidepanelMenu.spec.js.snap +++ b/app/react/Entities/components/specs/__snapshots__/ShowSidepanelMenu.spec.js.snap @@ -12,7 +12,7 @@ exports[`ShowSidepanelMenu should show button if panel is not open 1`] = ` onClick={[Function]} > From 23b960a86036d572af5a4454a144aef4c8d71728 Mon Sep 17 00:00:00 2001 From: Daneryl Date: Mon, 29 Nov 2021 09:22:16 +0100 Subject: [PATCH 26/26] version 1.45.0-rc1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c92d735e27..2662d1378a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "uwazi", - "version": "1.44.0-rc3", + "version": "1.45.0-rc1", "description": "Uwazi is a free, open-source solution for organising, analysing and publishing your documents.", "keywords": [ "react"