Merge pull request #147 from partio-scout/bugfix/filter-perf

Fix filter performance by building filters during integration

conf/option-fields.json

@@ -0,0 +1,12 @@
+[
+  "subCamp",
+  "village",
+  "campGroup",
+  "localGroup",
+  "ageGroup",
+  "childNaps",
+  "accommodation",
+  "country",
+  "willOfTheWisp",
+  "willOfTheWispWave"
+]

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;
     }
   }
 

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;
     }
 

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() {

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);
     }

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);

src/client/stores/SearchFilterStore.js

@@ -7,15 +7,20 @@ export function getSearchFilterStore(alt, SearchFilterActions) {
       this.bindListeners({
         handleSearchFilterListUpdated: SearchFilterActions.SEARCH_FILTER_LIST_UPDATED,
         handleOptionsLoaded: SearchFilterActions.OPTIONS_LOADED,
+        handleDateOptionsLoaded: SearchFilterActions.DATE_OPTIONS_LOADED,
       });
     }
 
     handleSearchFilterListUpdated(searchFilters) {
       this.searchFilters = searchFilters;
     }
 
-    handleOptionsLoaded({ property, options }) {
-      this.options[property] = options;
+    handleOptionsLoaded(options) {
+      this.options = options;
+    }
+
+    handleDateOptionsLoaded(dates) {
+      this.dates = dates;
     }
   }
 

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],

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": []
+}

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(),
+      }));
+  }
+}
+

src/server/model-config.json

@@ -44,6 +44,10 @@
     "dataSource": "db",
     "public": true
   },
+  "Option": {
+    "dataSource": "db",
+    "public": true
+  },
   "AuditEvent": {
     "dataSource": "db",
     "public": false

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();
   });