fix: Fix "zoom to data": include deletions & correct density calc (#475)

* fix: Fix "zoom to data": include deletions and correct density calc

* fix: In territory view, zoom to data also considers observations

* chore: Update node version in Github Action

.github/workflows/nodejs.yml

@@ -28,10 +28,10 @@ jobs:
       - uses: actions/checkout@v2
         with:
           fetch-depth: 1
-      - name: Use Node.js 12.4
-        uses: actions/setup-node@v1
+      - name: Use Node.js 12.14.1
+        uses: actions/setup-node@v2
         with:
-          node-version: 12.4
+          node-version: 12.14.1
       - name: npm ci
         run: |
           npm ci

src/background/mapeo-core/mapeo.js

@@ -49,7 +49,7 @@ class MapeoRPC {
     })
 
     var idb = sublevel(this.osm.index, 'stats')
-    this.osm.core.use('stats', installStatsIndex(idb))
+    this.osm.core.use('stats', 2, installStatsIndex(idb))
 
     this.media = MediaStore(path.join(datadir, 'media'))
 

src/background/mapeo-core/osm-stats.js

@@ -7,21 +7,35 @@ function createZoomToDataIndex (ldb) {
       var bins = {}
       var pending = 1
       for (var i = 0; i < nodes.length; i++) {
-        var node = nodes[i]
+        // !!Important!! Need const here for node, binId and delete, because if
+        // var is used, it is hoisted to the top of this function, and will
+        // have a different value when the ldb.get callback is called. const and
+        // let are scoped to this for expression, and will retain their value in
+        // the callback
+        const node = nodes[i]
         if (!node.value) continue
-        if (typeof node.value.lat !== 'number' || typeof node.value.lon !== 'number') continue
-
+        if (
+          typeof node.value.lat !== 'number' ||
+          typeof node.value.lon !== 'number'
+        )
+          continue
         // update density map
-        var binId = nodeToBinId(node.value)
-        if (bins[binId]) {
-          bins[binId]++
+        const binId = nodeToBinId(node.value)
+        const deleted = node.value.deleted
+        if (typeof bins[binId] === 'number') {
+          deleted ? bins[binId]-- : bins[binId]++
         } else {
           pending++
-          ldb.get(binId, function (err, num) {
+          ldb.get(binId, function (err, val) {
+            let num
             if (err && err.notFound) num = 0
             else if (err) return next(err)
-            if (bins[binId]) bins[binId]++
-            else bins[binId] = num + 1
+            else num = Number(val)
+            if (typeof bins[binId] === 'number') {
+              deleted ? bins[binId]-- : bins[binId]++
+            } else {
+              bins[binId] = deleted ? num - 1 : num + 1
+            }
             if (!--pending) finish()
           })
         }
@@ -55,38 +69,63 @@ function createZoomToDataIndex (ldb) {
     clearIndex: function (cb) {
       // TODO: mutex to prevent other view APIs from running?
       var batch = []
-      ldb.createKeyStream()
-        .pipe(through(function (key, _, next) {
-          batch.push({ type: 'del', key: key })
-          next()
-        }, function (flush) {
-          ldb.batch(batch, function () {
-            flush()
-            cb()
-          })
-        }))
+      ldb.createKeyStream().pipe(
+        through(
+          function (key, _, next) {
+            batch.push({ type: 'del', key: key })
+            next()
+          },
+          function (flush) {
+            ldb.batch(batch, function () {
+              flush()
+              cb()
+            })
+          }
+        )
+      )
     },
     api: {
-      getMapCenter: function (core, type, cb) {
-        if (typeof type === 'function' && !cb) {
-          cb = type
-          type = 'node'
+      getMapCenter: function (core, types, cb) {
+        if (typeof types === 'function' && !cb) {
+          cb = types
+          types = ['node']
         }
-        this.ready(function () {
-          var rs = ldb.createReadStream({ gt: 'ztd/' + type + '!', lt: 'ztd/' + type + '~' })
-          var mostDense = null
-          rs.on('data', function (entry) {
-            if (mostDense === null || Number(entry.value) > Number(mostDense.value)) {
-              mostDense = entry
+        if (!Array.isArray(types)) {
+          types = [types]
+        }
+        let mostDense = null
+
+        this.ready(async () => {
+          try {
+            for (const type of types) {
+              await streamType(type)
             }
-          })
-          rs.once('end', function () {
             if (!mostDense) return cb(null, null)
             var center = binIdToLatLon(mostDense.key.substring(4))
             cb(null, center)
-          })
-          rs.once('error', cb)
+          } catch (err) {
+            cb(err)
+          }
         })
+
+        async function streamType (type) {
+          return new Promise((resolve, reject) => {
+            var rs = ldb.createReadStream({
+              gt: 'ztd/' + type + '!',
+              lt: 'ztd/' + type + '~'
+            })
+            rs.on('data', function (entry) {
+              if (
+                mostDense === null ||
+                Number(entry.value) > Number(mostDense.value)
+              ) {
+                mostDense = entry
+              }
+            })
+            rs.once('end', resolve)
+            rs.once('error', reject)
+          })
+        }
       }
     }
   }
@@ -99,15 +138,17 @@ function nodeToBinId (node) {
   var lon = Number(node.lon)
   if (Number.isNaN(lat) || lat === undefined || lat === null) return null
   if (Number.isNaN(lon) || lon === undefined || lon === null) return null
-  var latbin = Math.round(lat * 50) / 50
-  var lonbin = Math.round(lon * 50) / 50
+  // Store in bins 0.01° square ~ 1.1km at equator
+  var latbin = Math.floor(lat * 50)
+  var lonbin = Math.floor(lon * 50)
   return 'ztd/' + node.type + '/' + latbin + ',' + lonbin
 }
 
 function binIdToLatLon (binId) {
   binId = binId.split('/')[1]
-  var lat = Number(binId.split(',')[0])
-  var lon = Number(binId.split(',')[1])
+  // Get center of bin
+  var lat = (Number(binId.split(',')[0]) + 0.5) / 50
+  var lon = (Number(binId.split(',')[1]) + 0.5) / 50
   return {
     lat: lat,
     lon: lon

src/main/menu.js

@@ -202,13 +202,21 @@ function menuTemplate (ipc) {
         {
           label: t('menu-zoom-to-data'),
           click: function (item, focusedWindow) {
-            ipc.send('zoom-to-data-get-centroid', 'node', function (err, loc) {
-              if (err) logger.error(err)
-              logger.debug('RESPONSE(menu,getDatasetCentroid):', loc)
-              if (!loc) return
-              focusedWindow.webContents.send('zoom-to-data-node', loc)
-            })
-            ipc.send('zoom-to-data-get-centroid', 'observation', function (err, loc) {
+            ipc.send(
+              'zoom-to-data-get-centroid',
+              // For territory view, we want the centroid of both nodes and observations
+              ['node', 'observation'],
+              function (err, loc) {
+                if (err) logger.error(err)
+                logger.debug('RESPONSE(menu,getDatasetCentroid):', loc)
+                if (!loc) return
+                focusedWindow.webContents.send('zoom-to-data-territory', loc)
+              }
+            )
+            ipc.send('zoom-to-data-get-centroid', 'observation', function (
+              err,
+              loc
+            ) {
               if (err) logger.error(err)
               logger.debug('RESPONSE(menu,getDatasetCentroid):', loc)
               if (!loc) return

src/renderer/components/MapEditor/index.js

@@ -114,10 +114,10 @@ const MapEditor = () => {
 
   React.useEffect(
     function setupListeners () {
-      ipcRenderer.on('zoom-to-data-node', zoomToData)
+      ipcRenderer.on('zoom-to-data-territory', zoomToData)
       ipcRenderer.on('zoom-to-latlon-response', zoomToData)
       return () => {
-        ipcRenderer.removeListener('zoom-to-data-node', zoomToData)
+        ipcRenderer.removeListener('zoom-to-data-territory', zoomToData)
         ipcRenderer.removeListener('zoom-to-latlon-response', zoomToData)
       }
     },