From e1c7114bd4ebd847a59b82b4849fd70d1fc548df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= <ondra@ondrovo.com>
Date: Sat, 11 Aug 2018 23:34:48 +0200
Subject: [PATCH] correctly sort by numeric columns

---
 app/Http/Controllers/TableController.php      |  2 +-
 app/Http/Controllers/TableEditController.php  |  4 +-
 app/Models/Row.php                            | 22 ++++++
 app/Tables/BaseExporter.php                   |  2 +-
 resources/assets/js/components/RowsEditor.vue | 77 +++++++++++--------
 .../views/table/propose/edit-rows.blade.php   |  1 +
 6 files changed, 73 insertions(+), 35 deletions(-)

diff --git a/app/Http/Controllers/TableController.php b/app/Http/Controllers/TableController.php
index f809763..c7f5644 100644
--- a/app/Http/Controllers/TableController.php
+++ b/app/Http/Controllers/TableController.php
@@ -46,7 +46,7 @@ class TableController extends Controller
         $columns = Column::columnsFromJson($revision->columns);
 
         $rq = $revision->rowsData($columns);
-        if (count($columns)) $rq = $rq->orderByRaw("data->>'".$columns[0]->id."'");
+        if (count($columns)) $rq = $rq->sortByCol($columns[0]);
         $rows = $rq->paginate(25, []);
 
         return view('table.view', [
diff --git a/app/Http/Controllers/TableEditController.php b/app/Http/Controllers/TableEditController.php
index 36b3325..6f56b9d 100644
--- a/app/Http/Controllers/TableEditController.php
+++ b/app/Http/Controllers/TableEditController.php
@@ -97,7 +97,9 @@ class TableEditController extends Controller
     {
         $revision = $changeset->revision;
         $columns = $changeset->fetchAndTransformColumns();
-        $rows = $revision->rowsData($columns, true, false)->paginate(25, []);
+        $rows = $revision->rowsData($columns, true, false)
+            ->sortByCol($changeset->fetchRevisionColumns()[0])
+            ->paginate(25, []);
 
         return view('table.propose.edit-rows', [
             'changeset' => $changeset,
diff --git a/app/Models/Row.php b/app/Models/Row.php
index ca00961..c88f090 100644
--- a/app/Models/Row.php
+++ b/app/Models/Row.php
@@ -2,6 +2,9 @@
 
 namespace App\Models;
 
+use App\Tables\Column;
+use Illuminate\Database\Eloquent\Builder;
+
 /**
  * Row in a data table
  *
@@ -25,6 +28,25 @@ class Row extends BaseModel
         $this->data->_id = $value;
     }
 
+    public function scopeSortByCol(Builder $query, Column $column, $dir="asc")
+    {
+        $q = "data->>'".$column->id."'";
+        if ($column->type == 'int' || $column->type == 'float') {
+            $q = "CAST($q AS INTEGER)";
+        }
+        else if ($column->type == 'float') {
+            $q = "CAST($q AS FLOAT)";
+        }
+
+        if ($dir == 'asc') {
+            $q .= ' ASC';
+        } else {
+            $q .= ' DESC';
+        }
+
+        return $query->orderByRaw($q);
+    }
+
     /**
      * Allocate a single Global Row ID, application-unique.
      *
diff --git a/app/Tables/BaseExporter.php b/app/Tables/BaseExporter.php
index b830aaa..14723b7 100644
--- a/app/Tables/BaseExporter.php
+++ b/app/Tables/BaseExporter.php
@@ -132,7 +132,7 @@ abstract class BaseExporter
         $columns = $this->columns;
 
         // TODO raw query to allow selecting aggregates, column subsets, etc
-        if (count($columns)) $rq = $rq->orderByRaw("data->>'".$columns[0]->id."'");
+        if (count($columns)) $rq = $rq->sortByCol($columns[0]);
 
         while ($start < $count) {
             $rows = $rq->offset($start)->limit($chunkSize)->get()->toArray();
diff --git a/resources/assets/js/components/RowsEditor.vue b/resources/assets/js/components/RowsEditor.vue
index a0bab0c..ecf9542 100644
--- a/resources/assets/js/components/RowsEditor.vue
+++ b/resources/assets/js/components/RowsEditor.vue
@@ -68,7 +68,7 @@ Rows are identified by row._id, columns by col.id
         </tr>
         </thead>
         <tbody>
-          <tr v-for="row in rows" :class="row._remove ? 'remove' : ''">
+          <tr v-for="row in orderedRows" :class="row._remove ? 'remove' : ''">
             <td class="text-nowrap">
               <button :class="[
                         'mr-1', 'btn', 'btn-outline-danger',
@@ -164,6 +164,7 @@ Rows are identified by row._id, columns by col.id
       route: String,
       /** Row objects */
       xRows: Object, // key'd by _id
+      xRowOrder: Array,
       /** Array of columns */
       columns: Array,
       /** CSRF token */
@@ -182,6 +183,7 @@ Rows are identified by row._id, columns by col.id
       return {
         /** All rows */
         rows: this.xRows,
+        rowOrder: this.xRowOrder,
         /** Number of dirty rows */
         dirtyRows: 0,
       }
@@ -199,6 +201,17 @@ Rows are identified by row._id, columns by col.id
         }
       }
     },
+    computed: {
+      // JS does not respect object keys order, so we have to fix it up a little
+      orderedRows: function () {
+        if (this.newRows) return Object.values(this.rows);
+        let arr = []
+        for(let id of this.rowOrder) {
+          arr.push(this.rows[id])
+        }
+        return arr;
+      }
+    },
     methods: {
       /** Send a query to the server */
       query (data, sucfn, erfn) {
@@ -247,6 +260,35 @@ Rows are identified by row._id, columns by col.id
         })
       },
 
+      /** Save all changed rows */
+      saveAllChanges () {
+        let toChange = _.filter(this.rows, (r) => {
+          let changed = this.isRowChanged(r)
+          if (!changed && r._editing) {
+            this.rows[r._id]._editing = false
+          }
+          return changed;
+        })
+
+        if (toChange.length === 0) return;
+
+        this.query({
+          action: 'row.update-many',
+          data: toChange
+        }, (resp) => {
+          _.each(resp.data, (row) => {
+            this.$set(this.rows, row._id, row)
+          })
+          this.dirtyRows = 0
+        }, (er) => {
+          // TODO - also need to improve backend to properly report which row was bad
+          console.log(er)
+          // if (!_.isUndefined(er.errors)) {
+          //   this.$set(this.rows[row._id], '_errors', er.errors)
+          // }
+        })
+      },
+
       /** Toggle editing state - edit or save */
       toggleRowEditing (_id) {
         if (this.rows[_id]._remove) return false // can't edit row marked for removal
@@ -321,35 +363,6 @@ Rows are identified by row._id, columns by col.id
         row._dirty = isDirty
       },
 
-      /** Save all changed rows */
-      saveAllChanges () {
-        let toChange = _.filter(this.rows, (r) => {
-          let changed = this.isRowChanged(r)
-          if (!changed && r._editing) {
-            this.rows[r._id]._editing = false
-          }
-          return changed;
-        })
-
-        if (toChange.length === 0) return;
-
-        this.query({
-          action: 'row.update-many',
-          data: toChange
-        }, (resp) => {
-          _.each(resp.data, (row) => {
-            this.$set(this.rows, row._id, row)
-          })
-          this.dirtyRows = 0
-        }, (er) => {
-          // TODO
-          console.log(er)
-          // if (!_.isUndefined(er.errors)) {
-          //   this.$set(this.rows[row._id], '_errors', er.errors)
-          // }
-        })
-      },
-
       discardEdit (row) {
         let wasDirty = this.isRowChanged(row)
 
@@ -382,7 +395,7 @@ Rows are identified by row._id, columns by col.id
         if (dir == -1) {
           // up
           let last_editable_row = null
-          for (let row of Object.values(this.rows)) {
+          for (let row of this.orderedRows) {
             if (row._id === row_id) {
               break
             }
@@ -398,7 +411,7 @@ Rows are identified by row._id, columns by col.id
           // down
           let first_editable_row = null
           let foundself = false
-          for (let row of Object.values(this.rows)) {
+          for (let row of this.orderedRows) {
             if (row._id === row_id) {
               foundself = true
               continue
diff --git a/resources/views/table/propose/edit-rows.blade.php b/resources/views/table/propose/edit-rows.blade.php
index 3d00656..a617c2c 100644
--- a/resources/views/table/propose/edit-rows.blade.php
+++ b/resources/views/table/propose/edit-rows.blade.php
@@ -31,6 +31,7 @@
         route: {!! toJSON($table->draftUpdateRoute) !!},
         columns: {!! toJSON($columns) !!},
         xRows: {!! toJSON($transformed, true) !!},
+        xRowOrder: @json($transformed->keys()),
       })
     });
   </script>