From 72541f2810ece7dc277f11321b220ebfcc0a3efe Mon Sep 17 00:00:00 2001 From: Emil Virkki Date: Wed, 20 Jul 2016 04:31:24 +0300 Subject: [PATCH] Fix filter performance by building filters during integration --- conf/option-fields.json | 12 ++++ src/client/actions/SearchFilterActions.js | 45 +++++++------ .../ParticipantListPage.jsx | 6 +- .../containers/DateFilterContainer.jsx | 2 +- .../containers/PropertyFilterContainer.jsx | 4 -- src/client/main.jsx | 3 +- src/client/stores/SearchFilterStore.js | 9 ++- src/common/models-list.js | 1 + src/common/models/option.json | 66 +++++++++++++++++++ src/kuksa-integration/rebuild-tables.js | 26 +++++++- src/server/model-config.json | 4 ++ test/kuksa-integration.test.js | 15 +++++ 12 files changed, 163 insertions(+), 30 deletions(-) create mode 100644 conf/option-fields.json create mode 100644 src/common/models/option.json diff --git a/conf/option-fields.json b/conf/option-fields.json new file mode 100644 index 00000000..2682efa9 --- /dev/null +++ b/conf/option-fields.json @@ -0,0 +1,12 @@ +[ + "subCamp", + "village", + "campGroup", + "localGroup", + "ageGroup", + "childNaps", + "accommodation", + "country", + "willOfTheWisp", + "willOfTheWispWave" +] diff --git a/src/client/actions/SearchFilterActions.js b/src/client/actions/SearchFilterActions.js index 60312ab8..8a7906cc 100644 --- a/src/client/actions/SearchFilterActions.js +++ b/src/client/actions/SearchFilterActions.js @@ -1,6 +1,6 @@ import _ from 'lodash'; -export function getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, errorActions) { +export function getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, optionResource, errorActions) { class SearchFilterActions { saveSearchFilter(name, filter) { return dispatch => { @@ -43,23 +43,27 @@ export function getSearchFilterActions(alt, searchFilterResource, participantRes return searchFilters; } - loadOptions(property) { + loadOptions() { return dispatch => { dispatch(); - if (!property) { - this.optionsLoaded('', null); // dispatch empty list - } else { - participantResource.findAll(`filter[fields][${property}]=true`) - .then(response => this.optionsLoaded(property, processResults(response)), - err => errorActions.error(err, `Hakusuodatinta ${property} ei voitu ladata`)); - } + optionResource.findAll() + .then(response => this.optionsLoaded(processResults(response)), + err => errorActions.error(err, `Hakusuodattimia ei voitu ladata`)); }; - function processResults(result) { - const optionStrings = result.map(obj => obj[property]); - const uniqueStrings = _.uniq(optionStrings); - uniqueStrings.sort(); - return _.concat([''], uniqueStrings); + function processResults(response) { + //TODO Clean up after a good night's sleep + const x = {}; + for (let i = 0; i < response.length; i++) { + const property = response[i].property; + if (x[property]) { + x[property].push(response[i].value); + } else { + x[property] = []; + x[property].push(response[i].value); + } + } + return x; } } @@ -69,16 +73,17 @@ export function getSearchFilterActions(alt, searchFilterResource, participantRes return dispatch => { dispatch(); participantDateResource.findAll(`filter[fields][date]=true`) - .then(response => this.optionsLoaded('dates', processResults(response)), + .then(response => this.dateOptionsLoaded(processResults(response)), err => this.optionsLoadingFailed(err)); }; } - optionsLoaded(property, options) { - return { - property: property, - options: options, - }; + optionsLoaded(options) { + return options; + } + + dateOptionsLoaded(options) { + return options; } } diff --git a/src/client/components/ParticipantListPage/ParticipantListPage.jsx b/src/client/components/ParticipantListPage/ParticipantListPage.jsx index c5cf6501..5ae9dbb3 100644 --- a/src/client/components/ParticipantListPage/ParticipantListPage.jsx +++ b/src/client/components/ParticipantListPage/ParticipantListPage.jsx @@ -192,6 +192,10 @@ export function getParticipantListPage(participantStore, participantActions, sea ); } + componentWillMount() { + searchFilterActions.loadOptions.defer(); + } + componentDidMount() { participantStore.listen(this.checkNoneOnParticipantsChanged); searchFilterStore.listen(this.onSearchFilterStoreChanged); @@ -216,7 +220,7 @@ export function getParticipantListPage(participantStore, participantActions, sea extractDatesFromSearchFilters() { const state = this.state; - state.availableDates = searchFilterStore.getState().options.dates || []; + state.availableDates = searchFilterStore.getState().dates || []; return state; } diff --git a/src/client/components/ParticipantListPage/containers/DateFilterContainer.jsx b/src/client/components/ParticipantListPage/containers/DateFilterContainer.jsx index c1dabd7c..e9ce14ac 100644 --- a/src/client/components/ParticipantListPage/containers/DateFilterContainer.jsx +++ b/src/client/components/ParticipantListPage/containers/DateFilterContainer.jsx @@ -30,7 +30,7 @@ export function getDateFilterContainer(searchFilterStore, searchFilterActions) { } extractState() { - return { options: searchFilterStore.getState().options[this.props.property] || [] }; + return { options: searchFilterStore.getState().dates || [] }; } render() { diff --git a/src/client/components/ParticipantListPage/containers/PropertyFilterContainer.jsx b/src/client/components/ParticipantListPage/containers/PropertyFilterContainer.jsx index ceef7c58..3b090c54 100644 --- a/src/client/components/ParticipantListPage/containers/PropertyFilterContainer.jsx +++ b/src/client/components/ParticipantListPage/containers/PropertyFilterContainer.jsx @@ -13,10 +13,6 @@ export function getPropertyFilterContainer(searchFilterStore, searchFilterAction this.extractState = this.extractState.bind(this); } - componentWillMount() { - searchFilterActions.loadOptions.defer(this.props.property); - } - componentDidMount() { searchFilterStore.listen(this.onStoreChanged); } diff --git a/src/client/main.jsx b/src/client/main.jsx index 2dea8b18..7c688530 100644 --- a/src/client/main.jsx +++ b/src/client/main.jsx @@ -28,12 +28,13 @@ const participantResource = new RestfulResource('/api/participants', accessToken const participantDateResource = new RestfulResource('/api/participantDates', accessToken); const registryUserResource = new RestfulResource('/api/registryusers', accessToken); const searchFilterResource = new RestfulResource('/api/searchfilters', accessToken); +const optionResource = new RestfulResource('/api/options', accessToken); const alt = new Alt(); const errorActions = actions.getErrorActions(alt); const participantActions = actions.getParticipantActions(alt, participantResource, errorActions); -const searchFilterActions = actions.getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, errorActions); +const searchFilterActions = actions.getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, optionResource, errorActions); const registryUserActions = actions.getRegistryUserActions(alt, registryUserResource, errorActions); const errorStore = stores.getErrorStore(alt, errorActions); diff --git a/src/client/stores/SearchFilterStore.js b/src/client/stores/SearchFilterStore.js index 537132e0..29d62b51 100644 --- a/src/client/stores/SearchFilterStore.js +++ b/src/client/stores/SearchFilterStore.js @@ -7,6 +7,7 @@ export function getSearchFilterStore(alt, SearchFilterActions) { this.bindListeners({ handleSearchFilterListUpdated: SearchFilterActions.SEARCH_FILTER_LIST_UPDATED, handleOptionsLoaded: SearchFilterActions.OPTIONS_LOADED, + handleDateOptionsLoaded: SearchFilterActions.DATE_OPTIONS_LOADED, }); } @@ -14,8 +15,12 @@ export function getSearchFilterStore(alt, SearchFilterActions) { this.searchFilters = searchFilters; } - handleOptionsLoaded({ property, options }) { - this.options[property] = options; + handleOptionsLoaded(options) { + this.options = options; + } + + handleDateOptionsLoaded(dates) { + this.dates = dates; } } diff --git a/src/common/models-list.js b/src/common/models-list.js index 62d55bd9..1587f59a 100644 --- a/src/common/models-list.js +++ b/src/common/models-list.js @@ -15,6 +15,7 @@ const modelList = [ ['ParticipantAllergy', false], ['ParticipantDate', false], ['PresenceHistory', false], + ['Option', false], ['AuditEvent', false], ['SearchFilter', false], ['Selection', false], diff --git a/src/common/models/option.json b/src/common/models/option.json new file mode 100644 index 00000000..1a825e23 --- /dev/null +++ b/src/common/models/option.json @@ -0,0 +1,66 @@ +{ + "name": "Option", + "base": "PersistedModel", + "idInjection": false, + "options": { + "validateUpsert": true, + "postgresql": { + "schema": "public", + "table": "option" + } + }, + "properties": { + "id": { + "type": "Number", + "id": true, + "generated": true, + "postgresql": { + "columnName": "id", + "dataType": "integer", + "dataLength": null, + "dataPrecision": 32, + "dataScale": 0, + "nullable": "NO" + } + }, + "property": { + "type": "string", + "required": true, + "postgresql": { + "columnName": "property", + "dataType": "character varying", + "dataLength": 255, + "dataPrecision": null, + "dataScale": null, + "nullable": "NO" + } + }, + "value": { + "type": "string", + "required": true, + "postgresql": { + "columnName": "value", + "dataType": "character varying", + "dataLength": 255, + "dataPrecision": null, + "dataScale": null, + "nullable": "NO" + } + } + }, + "validations": [], + "acls": [ + { + "principalType": "ROLE", + "principalId": "$everyone", + "permission": "DENY" + }, + { + "accessType": "READ", + "principalType": "ROLE", + "principalId": "registryUser", + "permission": "ALLOW" + } + ], + "methods": [] +} diff --git a/src/kuksa-integration/rebuild-tables.js b/src/kuksa-integration/rebuild-tables.js index 12f1419e..a549f74b 100644 --- a/src/kuksa-integration/rebuild-tables.js +++ b/src/kuksa-integration/rebuild-tables.js @@ -3,6 +3,7 @@ import Promise from 'bluebird'; import { _ } from 'lodash'; import moment from 'moment'; import paymentToDateMappings from '../../conf/payment-date-mappings.json'; +import optionFields from '../../conf/option-fields.json'; const KuksaParticipant = app.models.KuksaParticipant; const findKuksaParticipants = Promise.promisify(KuksaParticipant.find, { context: KuksaParticipant }); @@ -12,6 +13,10 @@ const upsertParticipant = Promise.promisify(Participant.upsert, { context: Parti const findParticipants = Promise.promisify(Participant.find, { context: Participant }); const destroyAllParticipants = Promise.promisify(Participant.destroyAll, { context: Participant }); +const Option = app.models.Option; +const destroyAllOptions = Promise.promisify(Option.destroyAll, { context: Option }); +const upsertOption = Promise.promisify(Option.upsert, { context: Option }); + if (require.main === module) { main().then( () => { console.log('Finished successfully.'); process.exit(0); }, @@ -25,7 +30,8 @@ function main() { .then(addAllergiesToParticipants) .then(addDatesToParticipants) .then(buildSelectionTable) - .then(deleteCancelledParticipants); + .then(deleteCancelledParticipants) + .then(buildOptionTable); } function buildAllergyTable() { @@ -217,3 +223,21 @@ function deleteCancelledParticipants() { .then(idsToDelete => destroyAllParticipants({ participantId: { inq: idsToDelete } })) .then(info => console.log(`Deleted ${info.count} cancelled participants.`)); } + +function buildOptionTable() { + const addFieldValues = ({ field, values }) => Promise.each(values, value => upsertOption({ property: field, value: value })); + return destroyAllOptions() + .then(() => Promise.mapSeries(optionFields, getFieldValues)) + .then(items => Promise.each(items, addFieldValues)); + + function getFieldValues(field) { + const filter = { fields: { } }; + filter['fields'][field] = true; + return findParticipants(filter) + .then(values => ({ + field: field, + values: _(values).map(obj => obj[field]).uniq().reject(_.isNull).value().sort(), + })); + } +} + diff --git a/src/server/model-config.json b/src/server/model-config.json index f43671af..d988d3df 100644 --- a/src/server/model-config.json +++ b/src/server/model-config.json @@ -44,6 +44,10 @@ "dataSource": "db", "public": true }, + "Option": { + "dataSource": "db", + "public": true + }, "AuditEvent": { "dataSource": "db", "public": false diff --git a/test/kuksa-integration.test.js b/test/kuksa-integration.test.js index 3b168ad1..2b6d62e7 100644 --- a/test/kuksa-integration.test.js +++ b/test/kuksa-integration.test.js @@ -20,6 +20,7 @@ describe('Kuksa integration', () => { const findAllergyById = Promise.promisify(app.models.Allergy.findById, { context: app.models.Allergy }); const findSelections = Promise.promisify(app.models.Selection.find, { context: app.models.Selection }); const countSelections = Promise.promisify(app.models.Selection.count, { context: app.models.Selection }); + const countOptions = Promise.promisify(app.models.Option.count, { context: app.models.Option }); before(function(done) { this.timeout(80000); @@ -167,6 +168,20 @@ describe('Kuksa integration', () => { }) ); + it('builds options in advance', + () => expect(countOptions()).to.eventually.be.above(0) + ); + + it('creates each option only once', + () => expect(countOptions({ property: 'subCamp', value: 'Unity' })).to.eventually.equal(1) + ); + + it('builds correct amount of options', + () => expect(countOptions({ property: 'village' })).to.eventually.equal(4) + ); + + //TODO Check it saves correct optinos for each field + after(() => { mockKuksa.stop(); });