Merge pull request #714 from rfcx/bug/CE-1143-slow-unvalidated-filter

bug/CE-1143 slow unvalidated filter; feature/CE-1158 filters load site batches

CHANGELOG.md

@@ -1,10 +1,12 @@
 # Arbimon Release Notes
 
-## v3.0.33 - June 01, 2021
+## v3.0.33 - July XX, 2021
 
 Performance improvements:
 
 - CE-988 The datime_local column is removed
+- CE-1143 Update "Unvalidated", "All", "Present", "Not present", "200 top scores per site", "Score per site" filters on Pattern Matching details page to return results per site
+- CE-1158 update PM filters logic so "Best per Site" and "Best per Site, Day" return results by 20 sites batches
 
 ## v3.0.32 - June 26, 2021
 

TEST_NOTES.md

@@ -1,6 +1,20 @@
 # Arbimon Test Notes
 Test Notes are used to list what pages / components / features / user flows are affected by each update.
 
+## v3.0.33
+
+- CE-1143 Update "Unvalidated", "All", "Present", "Not present", "200 top scores per site", "Score per site" filters on Pattern Matching details page to return results per site
+  - Check that all filters still return correct data
+  - Check that pagination works correctly for every filter
+  - Check that selecting a site with Site List dropdown (icon with map pin) works fine
+  - Check that switching between filters does not break any logic
+  - Check that you validation feature still works correctly
+- CE-1158 update PM filters logic so "Best per Site" and "Best per Site, Day" return results by 20 sites batches
+  - Check that these two filters return 20 sites per page (or less if project is small)
+  - Check that pagination works correctly for these two filters
+  - Check that other filters still work correctly
+  - Check that switching between filters, pages, sites, etc works correctly
+
 ## v3.0.32
 
 - CE-1065 Patten Matching list loads quicker

app/model/pattern_matchings.js

@@ -221,6 +221,8 @@ var PatternMatchings = {
 
     SEARCH_ROIS_SCHEMA : {
         patternMatching: joi.number().required(),
+        site: joi.string(),
+        sites: joi.array().items(joi.string()),
         csValidationsFor: joi.number().integer(),
         expertCSValidations: joi.boolean(),
         perUserCSValidations: joi.boolean(),
@@ -253,7 +255,6 @@ var PatternMatchings = {
     buildRoisQuery(parameters){
         var builder = new SQLBuilder();
         return q.ninvoke(joi, 'validate', parameters, PatternMatchings.SEARCH_ROIS_SCHEMA).then(function(parameters){
-            var outputs = parameters.output instanceof Array ? parameters.output : [parameters.output];
             var show = parameters.show || {};
             var presteps=[];
 
@@ -269,15 +270,11 @@ var PatternMatchings = {
 
             builder.addTable("JOIN recordings", "R", "R.recording_id = PMR.recording_id");
             builder.addTable("JOIN sites", "S", "S.site_id = R.site_id");
+
             builder.addProjection(
-                'SUBSTRING_INDEX(R.`uri`, "/", -1) as `recording`, R.meta ',
-                'S.`name` as `site`',
-                'S.`site_id`',
-                'EXTRACT(year FROM R.`datetime`) as `year`',
-                'EXTRACT(month FROM R.`datetime`) as `month`',
-                'EXTRACT(day FROM R.`datetime`) as `day`',
-                'EXTRACT(hour FROM R.`datetime`) as `hour`',
-                'EXTRACT(minute FROM R.`datetime`) as `min`'
+                'R.uri as recording, R.meta ',
+                'S.name as site, S.site_id',
+                'PMR.denorm_recording_datetime as datetime',
             );
 
             if(show.datetime){
@@ -312,9 +309,15 @@ var PatternMatchings = {
                 }
             }
 
-            builder.addConstraint("PMR.pattern_matching_id = ?", [
-                parameters.patternMatching
-            ]);
+            builder.addConstraint("PMR.pattern_matching_id = ?", [ parameters.patternMatching ]);
+
+            if (parameters.site) {
+                builder.addConstraint("PMR.denorm_site_id = ?", [ parameters.site ]);
+            }
+
+            if (parameters.sites) {
+                builder.addConstraint("PMR.denorm_site_id IN ?", [ parameters.sites ]);
+            }
 
             if(parameters.expertCSValidations){
                 if(show.names){
@@ -483,14 +486,13 @@ var PatternMatchings = {
 
     getRoisForId(options) {
         let sortBy = [['S.name', 1], ['R.datetime', 1]] // default sorting
-        if (options.byScorePerSite) {
-            sortBy = [['S.name', 1], ['PMR.score', 0]]
-        }
-        else if (options.byScore) {
+        if (options.byScore || options.byScorePerSite) {
             sortBy = [['PMR.score', 0]]
         }
         return this.buildRoisQuery({
             patternMatching: options.patternMatchingId,
+            site: options.site,
+            sites: options.sites,
             perSiteCount: options.perSiteCount,
             csValidationsFor: options.csValidationsFor,
             expertCSValidations: options.expertCSValidations,
@@ -510,9 +512,11 @@ var PatternMatchings = {
             offset: options.offset,
             show: { patternMatchingId: true, datetime: true, names: options.showNames },
             sortBy,
-        }).then(
-            builder => dbpool.query(builder.getSQL())
-        ).then(function (results) {
+        }).then((builder) => {
+            return dbpool.query(builder.getSQL())
+        })
+        .then(this.completePMRResults)
+        .then(function (results) {
             // Fill the original filename from the meta column.
             for (let _1 of results) {
                 _1.meta = _1.meta ? PatternMatchings.__parse_meta_data(_1.meta) : null;
@@ -530,6 +534,45 @@ var PatternMatchings = {
             })
     },
 
+    async getPmRois (req) {
+        // let prom
+        let rois = []
+        let sites = [undefined] // a hack to make a loop workable even if no sites are specified
+        if (req.query.site) {
+            sites = [req.query.site]
+        }
+        if (req.query.sites) {
+            sites = req.query.sites.split(',')
+        }
+        for (let site of sites) {
+            const opts = {
+                patternMatchingId: req.params.patternMatching,
+                site: site,
+                limit: req.paging.limit || 100
+            }
+            switch (req.query.search) {
+                case 'best_per_site':
+                case 'top_200_per_site':
+                    rois = rois.concat(await this.getTopRoisByScoresPerSite(opts));
+                    break;
+                case 'best_per_site_day':
+                    rois = rois.concat(await this.getTopRoisByScoresPerSiteDay(opts));
+                    break;
+                default:
+                    rois = rois.concat(await this.getRoisForId({
+                        ...opts,
+                        wherePresent: req.query.search == 'present',
+                        whereNotPresent: req.query.search == 'not_present',
+                        whereUnvalidated: req.query.search == 'unvalidated',
+                        byScorePerSite: req.query.search == 'by_score_per_site',
+                        byScore: req.query.search == 'by_score',
+                        offset: req.paging.offset || 0,
+                    }))
+            }
+        }
+        return rois
+    },
+
     pmrSqlSelect: `SELECT S.site_id, S.name as site, PMR.score, R.uri as recording, PMR.pattern_matching_roi_id as id, PMR.pattern_matching_id,
         PMR.denorm_recording_datetime as datetime, PMR.recording_id, PMR.species_id, PMR.songtype_id, PMR.x1, PMR.y1, PMR.x2, PMR.y2, PMR.uri,
         PMR.score, PMR.validated FROM pattern_matching_rois AS PMR`,
@@ -548,17 +591,19 @@ var PatternMatchings = {
         return pmrs
     },
 
-    getTopRoisByScoresPerSite (pmId, siteId, limit = 200) {
+    getTopRoisByScoresPerSite (opts) {
         const base = `${this.pmrSqlSelect}
             JOIN recordings AS R ON R.recording_id = PMR.recording_id
             JOIN sites AS S ON PMR.denorm_site_id = S.site_id
             WHERE PMR.pattern_matching_id = ? AND PMR.denorm_site_id = ?
             ORDER BY PMR.score DESC LIMIT ?`
-        return dbpool.query({ sql: base, typeCast: sqlutil.parseUtcDatetime }, [pmId, siteId, limit])
+        return dbpool.query({ sql: base, typeCast: sqlutil.parseUtcDatetime }, [opts.patternMatchingId, opts.site, opts.limit || 200])
             .then(this.completePMRResults)
     },
 
-    getTopRoisByScoresPerSiteDay (pmId, siteId) {
+    getTopRoisByScoresPerSiteDay (opts) {
+        const pmId = opts.patternMatchingId
+        const site = opts.site
         const base = `${this.pmrSqlSelect}
             JOIN (
                 SELECT MAX(score) as max_score, denorm_recording_date
@@ -570,7 +615,7 @@ var PatternMatchings = {
             JOIN recordings AS R ON R.recording_id = PMR.recording_id
             JOIN sites AS S ON PMR.denorm_site_id = S.site_id
             WHERE PMR.pattern_matching_id = ? AND PMR.denorm_site_id = ?;`
-        return dbpool.query({ sql: base, typeCast: sqlutil.parseUtcDatetime }, [pmId, siteId, pmId, siteId])
+        return dbpool.query({ sql: base, typeCast: sqlutil.parseUtcDatetime }, [pmId, site, pmId, site])
             .then((data) => {
                 const pmrs = this.completePMRResults(data)
                 return pmrs.sort((a, b) => {
@@ -640,7 +685,7 @@ var PatternMatchings = {
             rois,
         ]) : Promise.resolve();
     },
-    
+
     getRoi(patternMatchingId, roisId){
         return dbpool.query(
             "SELECT *\n" +

app/routes/data-api/project/pattern_matchings.js

@@ -79,31 +79,10 @@ router.param('paging', function(req, res, next, paging){
     return next();
 });
 
-router.get('/:patternMatching/rois/:paging', function(req, res, next) {
+router.get('/:patternMatching/rois/:paging', async function(req, res, next) {
     res.type('json');
-    let prom
-    switch (req.query.search) {
-        case 'best_per_site':
-        case 'top_200_per_site':
-            prom = model.patternMatchings.getTopRoisByScoresPerSite(req.params.patternMatching, req.query.site, req.paging.limit);
-            break;
-        case 'best_per_site_day':
-            prom = model.patternMatchings.getTopRoisByScoresPerSiteDay(req.params.patternMatching, req.query.site, req.paging.limit);
-            break;
-        default:
-            prom = model.patternMatchings.getRoisForId({
-                patternMatchingId: req.params.patternMatching,
-                wherePresent: req.query.search == 'present',
-                whereNotPresent: req.query.search == 'not_present',
-                whereUnvalidated: req.query.search == 'unvalidated',
-                byScorePerSite: req.query.search == 'by_score_per_site',
-                site: req.query.site,
-                byScore: req.query.search == 'by_score',
-                limit: req.paging.limit || 100,
-                offset: req.paging.offset || 0,
-            })
-    }
-    prom.then((json) => res.json(json))
+    model.patternMatchings.getPmRois(req)
+        .then((json) => res.json(json))
         .catch(next);
 });
 
@@ -185,7 +164,7 @@ router.post('/:patternMatching/validate', function(req, res, next) {
 
         const updatedRois = rois.filter(function(roi) { return roi.validated != validation });
         const updatedRoiIds = updatedRois.map(function(roi) { return roi.pattern_matching_roi_id });
-        
+
         model.patternMatchings.validateRois(req.params.patternMatching, updatedRoiIds, validation)
             .then(async function(validatedRois) {
                 for (let roi of updatedRois) {

assets/app/a2services/patternmatching-service.js

@@ -47,7 +47,7 @@ angular.module('a2.srv.patternmatching', [
 
             return a2APIService.get('/pattern-matchings/' + patternMatchingId + '/rois/' + (offset||0) + '_' + (limit||0) + (query ? '?'+query : '')).catch(notify.serverError);
         },
-        getSiteIndexFor: function(patternMatchingId, options) {
+        getSitesListFor: function(patternMatchingId, options) {
             var query = Object.keys(options || {}).map(function(option){
                 return option + '=' + encodeURIComponent(options[option]);
             }).join('&');

assets/app/app/analysis/patternmatching/details.html

@@ -101,7 +101,7 @@
                             <ui-select class="btn-group-item" ng-model="controller.selected.siteBookmark" theme="bootstrap" style="display:inline-block; width:4em;"
                                 ng-change="controller.setSiteBookmark($select.selected)">
                                 <ui-select-match class="a2-uism-btn-icon icon-site"> </ui-select-match>
-                                <ui-select-choices repeat="$item in controller.siteIndex | filter:$select.search">
+                                <ui-select-choices repeat="$item in controller.sitesList | filter:$select.search">
                                     <span class="text-wrap"><i class="fa fa-map-marker"></i> {{$item.site}}</span>
                                 </ui-select-choices>
                             </ui-select>
@@ -169,7 +169,7 @@
         <div class="row panel-body" style="text-align: center;" ng-if="!controller.loading.rois && !controller.rois.length && controller.isTopRoisResults()">
             No results found per selected site.
         </div>
-        <div class="row" ng-if="controller.patternMatching && controller.patternMatching.matches > 100 && this.search.value !== 'best_per_site' && this.search.value !== 'best_per_site_day' && !controller.isTopRoisResults()">
+        <div class="row" ng-if="controller.patternMatching && controller.patternMatching.matches && !controller.shouldGetPerSite()">
             <div class="col-sm-12 text-center">
                 <pagination
                     total-items="controller.patternMatching.matches"
@@ -184,18 +184,18 @@
                 </pagination>
             </div>
         </div>
-        <div class="row" ng-if="controller.sitesTotal && controller.isTopRoisResults()">
+        <div class="row" ng-if="controller.sitesList && controller.shouldGetPerSite()">
             <div class="col-sm-12 text-center">
                 <pagination
-                    total-items="controller.sitesTotal"
+                    total-items="controller.sitesList.length"
                     ng-model="controller.selected.page"
                     ng-change="controller.setPage(controller.selected.page, true)"
                     class="pagination-sm"
                     boundary-links="true"
                     max-size="5"
                     rotate="false"
                     ng-disabled="controller.loading.rois"
-                    items-per-page="200">
+                    items-per-page="controller.sitesListBatchSize">
                 </pagination>
             </div>
         </div>