From 6cfd5aa7e9a8cc51f42be47e04fe5c4c434af8e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= <ondra@ondrovo.com>
Date: Sun, 5 Aug 2018 18:45:11 +0200
Subject: [PATCH] add some validation to row edit, better handle setting null
 cols with empty

---
 app/Exceptions/Handler.php                    |  3 +-
 app/Exceptions/ViewException.php              | 14 ----
 app/Http/Controllers/TableEditController.php  | 77 +++++++++++--------
 app/Models/Table.php                          |  7 ++
 app/Tables/Changeset.php                      |  8 +-
 app/Tables/Column.php                         |  5 +-
 .../Exceptions/SimpleValidationException.php  | 46 +++++++++++
 resources/assets/js/base-setup.js             |  2 +-
 resources/assets/js/components/RowEditor.vue  | 22 ++++--
 .../views/table/propose/edit-rows.blade.php   | 48 ++++--------
 .../propose/layout-row-pagination.blade.php   | 27 +++++++
 .../views/table/propose/layout.blade.php      |  2 +-
 routes/web.php                                |  1 +
 13 files changed, 168 insertions(+), 94 deletions(-)
 delete mode 100644 app/Exceptions/ViewException.php
 create mode 100644 porklib/Exceptions/SimpleValidationException.php
 create mode 100644 resources/views/table/propose/layout-row-pagination.blade.php

diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php
index 6de4f15..8481b6a 100644
--- a/app/Exceptions/Handler.php
+++ b/app/Exceptions/Handler.php
@@ -6,6 +6,7 @@ use Exception;
 use Illuminate\Auth\AuthenticationException;
 use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
 use Illuminate\Validation\ValidationException;
+use MightyPork\Exceptions\Exceptions\SimpleValidationException;
 
 class Handler extends ExceptionHandler
 {
@@ -15,7 +16,7 @@ class Handler extends ExceptionHandler
      * @var array
      */
     protected $dontReport = [
-        //
+        SimpleValidationException::class,
     ];
 
     /**
diff --git a/app/Exceptions/ViewException.php b/app/Exceptions/ViewException.php
deleted file mode 100644
index 5c21332..0000000
--- a/app/Exceptions/ViewException.php
+++ /dev/null
@@ -1,14 +0,0 @@
-<?php
-
-namespace FlowBox\Exceptions;
-
-class ViewException extends FBRuntimeException
-{
-	public $captured;
-
-	public function __construct(\Exception $cause, $captured)
-	{
-		$this->captured = $captured;
-		parent::__construct($cause);
-	}
-}
diff --git a/app/Http/Controllers/TableEditController.php b/app/Http/Controllers/TableEditController.php
index f546b47..16ef366 100644
--- a/app/Http/Controllers/TableEditController.php
+++ b/app/Http/Controllers/TableEditController.php
@@ -3,14 +3,12 @@
 
 namespace App\Http\Controllers;
 
-use App\Models\Row;
 use App\Models\Table;
 use App\Models\User;
 use App\Tables\Changeset;
-use App\Tables\Column;
-use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Input;
+use MightyPork\Exceptions\SimpleValidationException;
 
 class TableEditController extends Controller
 {
@@ -22,14 +20,12 @@ class TableEditController extends Controller
      */
     private function getChangeset(Table $table)
     {
-        $session_key = "proposal_{$table->id}";
-
         if (Input::has('reset')) {
-            session()->forget($session_key);
+            session()->forget($table->draftSessionKey);
         }
 
         /** @var Changeset $changeset */
-        return session()->remember($session_key, function () use ($table) {
+        return session()->remember($table->draftSessionKey, function () use ($table) {
             $changeset = new Changeset();
             $changeset->table = $table;
             $changeset->revision = $table->revision;
@@ -39,10 +35,21 @@ class TableEditController extends Controller
 
     private function storeChangeset(Changeset $chs)
     {
-        $session_key = "proposal_{$chs->table->id}";
+        session()->put($chs->table->draftSessionKey, $chs);
+    }
+
+    /**
+     * Discard draft and redirect to table view
+     */
+    public function discard(User $user, string $table)
+    {
+        /** @var Table $tableModel */
+        $tableModel = $user->tables()->where('name', $table)->first();
+        if ($tableModel === null) abort(404, "No such table.");
+
+        session()->forget($tableModel->draftSessionKey);
 
-        session()->put($session_key, $chs);
-        session()->save();
+        return redirect($tableModel->viewRoute);
     }
 
     public function draft(User $user, string $table, $tab = null)
@@ -103,29 +110,33 @@ class TableEditController extends Controller
 
         $input = objBag($request->all(), false);
 
-        $code = 200;
-        switch ($input->action) {
-            case 'row.update':
-                $data = (object)$input->data;
-                $changeset->rowUpdate($data);
-
-                $resp = $changeset->fetchAndTransformRow($data->_id);
-                break;
-
-            case 'row.remove':
-                $changeset->rowRemove($input->id);
-                $resp = $changeset->fetchAndTransformRow($input->id);
-                break;
-
-            case 'row.restore':
-                $changeset->rowRestore($input->id);
-                $resp = $changeset->fetchAndTransformRow($input->id);
-                break;
-
-            default:
-                $resp = "Bad Action";
-                $code = 400;
-                break;
+        try {
+            $code = 200;
+            switch ($input->action) {
+                case 'row.update':
+                    $data = (object)$input->data;
+                    $changeset->rowUpdate($data);
+
+                    $resp = $changeset->fetchAndTransformRow($data->_id);
+                    break;
+
+                case 'row.remove':
+                    $changeset->rowRemove($input->id);
+                    $resp = $changeset->fetchAndTransformRow($input->id);
+                    break;
+
+                case 'row.restore':
+                    $changeset->rowRestore($input->id);
+                    $resp = $changeset->fetchAndTransformRow($input->id);
+                    break;
+
+                default:
+                    $resp = "Bad Action";
+                    $code = 400;
+                    break;
+            }
+        } catch (SimpleValidationException $e) {
+            return $this->jsonResponse(['errors' => $e->getMessageBag()->getMessages()], 400);
         }
 
         $this->storeChangeset($changeset);
diff --git a/app/Models/Table.php b/app/Models/Table.php
index f5c351b..381cd3f 100644
--- a/app/Models/Table.php
+++ b/app/Models/Table.php
@@ -25,6 +25,9 @@ use Illuminate\Database\Eloquent\Collection;
  * @property-read string $draftRoute
  * @property-read string $settingsRoute
  * @property-read string $deleteRoute
+ * @property-read string $draftSessionKey
+ * @property-read string $draftDiscardRoute
+ * @property-read string $draftUpdateRoute
  * @property-read User $owner
  * @property-read Table $parentTable
  * @property-read Table[]|Collection $forks
@@ -136,9 +139,13 @@ class Table extends BaseModel
                 case 'settingsRoute': return route('table.conf', $arg);
                 case 'draftRoute':    return route('table.draft', $arg);
                 case 'deleteRoute':   return route('table.delete', $arg);
+                case 'draftDiscardRoute':    return route('table.draft-discard', $arg);
+                case 'draftUpdateRoute':    return route('table.draft-update', $arg);
             }
         }
 
+        if ($name == 'draftSessionKey') return "proposal.{$this->id}";
+
         return parent::__get($name);
     }
 
diff --git a/app/Tables/Changeset.php b/app/Tables/Changeset.php
index 2b33ad9..8ba6d96 100644
--- a/app/Tables/Changeset.php
+++ b/app/Tables/Changeset.php
@@ -325,8 +325,12 @@ class Changeset
         foreach ($newVals as $colId => $value) {
             if (starts_with($colId, '_')) continue; // internals
 
-            if (!isset($origRow->$colId) || $value != $origRow->$colId) {
-                $updateObj[$colId] = $cols[$colId]->cast($value);
+            $col = $cols[$colId];
+            $value = $col->cast($value);
+            $origValue = $col->cast(isset($origRow->$colId) ? $origRow->$colId : null);
+
+            if ($value !== $origValue) {
+                $updateObj[$colId] = $value;
             }
         }
 
diff --git a/app/Tables/Column.php b/app/Tables/Column.php
index 3741e1a..d0fb9e4 100644
--- a/app/Tables/Column.php
+++ b/app/Tables/Column.php
@@ -4,6 +4,7 @@ namespace App\Tables;
 
 use Illuminate\Contracts\Support\Arrayable;
 use JsonSerializable;
+use MightyPork\Exceptions\SimpleValidationException;
 use MightyPork\Exceptions\NotApplicableException;
 use MightyPork\Utils\Utils;
 
@@ -172,13 +173,13 @@ class Column implements JsonSerializable, Arrayable
                 if (is_int($value)) return $value;
                 if (is_float($value)) return round($value);
                 if (is_numeric($value)) return intval($value);
-                throw new NotApplicableException("Could not convert value \"$value\" to int!");
+                throw new SimpleValidationException($this->id, "Could not convert value \"$value\" to int!");
 
             case 'float':
                 if (is_null($value)) return 0;
                 if (is_int($value) || is_float($value)) return (float)$value;
                 if (is_numeric($value)) return floatval($value);
-                throw new NotApplicableException("Could not convert value \"$value\" to float!");
+                throw new SimpleValidationException($this->id, "Could not convert value \"$value\" to float!");
 
             case 'bool':
                 if (is_null($value)) return false;
diff --git a/porklib/Exceptions/SimpleValidationException.php b/porklib/Exceptions/SimpleValidationException.php
new file mode 100644
index 0000000..9e51f9e
--- /dev/null
+++ b/porklib/Exceptions/SimpleValidationException.php
@@ -0,0 +1,46 @@
+<?php
+
+namespace MightyPork\Exceptions;
+
+use Illuminate\Contracts\Support\MessageProvider;
+use Illuminate\Support\MessageBag;
+
+class SimpleValidationException extends RuntimeException implements MessageProvider
+{
+    /** @var MessageBag */
+    private $mb;
+
+    /**
+     * FBValidationException constructor.
+     *
+     * @param string|MessageProvider $key
+     * @param string $message
+     */
+    public function __construct($key, $message = null)
+    {
+        if (is_null($message)) {
+            $this->mb = $key->getMessageBag();
+        } else {
+            $mb = new MessageBag();
+            $mb->add($key, $message);
+            $this->mb = $mb;
+        }
+
+        $str = '';
+        foreach ($this->mb->getMessages() as $key => $errors) {
+            $str .= $key . ': ' . implode(', ', $errors) . "\n";
+        }
+
+        parent::__construct($str);
+    }
+
+    /**
+     * Get the messages for the instance.
+     *
+     * @return MessageBag
+     */
+    public function getMessageBag()
+    {
+        return $this->mb;
+    }
+}
diff --git a/resources/assets/js/base-setup.js b/resources/assets/js/base-setup.js
index 4f30037..4379bb3 100644
--- a/resources/assets/js/base-setup.js
+++ b/resources/assets/js/base-setup.js
@@ -1,4 +1,4 @@
-//window._ = require('lodash');
+window._ = require('lodash');
 window.Popper = require('popper.js').default
 
 /**
diff --git a/resources/assets/js/components/RowEditor.vue b/resources/assets/js/components/RowEditor.vue
index 19a115b..10dd476 100644
--- a/resources/assets/js/components/RowEditor.vue
+++ b/resources/assets/js/components/RowEditor.vue
@@ -26,14 +26,16 @@
 
         <template v-if="row._editing">
           <td v-for="col in columns" class="pr-0">
-            <input v-model="row[col.id]" class="form-control" type="text">
+            <input v-model="row[col.id]" :class="['form-control', { 'is-invalid': row._errors && row._errors[col.id] }]"
+                   :title="(row._errors && row._errors[col.id]) ? row._errors[col.id][0] : null"
+                   type="text" @keyup.enter="toggleRowEditing(row._id)">
           </td>
         </template>
 
         <template v-else>
           <td v-for="col in columns">
             <span class="text-danger strike" title="Original value" v-if="isChanged(row, col.id)">{{row._orig[col.id]}}</span>
-            <span>{{ row[col.id] || '' }}</span>
+            <span>{{ row[col.id] }}</span>
             <v-icon v-if="isChanged(row, col.id)"
                     @click="revertCell(row, col.id)"
                     class="fa-undo text-danger pointer"
@@ -52,8 +54,6 @@
 </style>
 
 <script>
-import { merge } from 'lodash';
-
 export default {
   props: {
     route: String,
@@ -69,11 +69,13 @@ export default {
     busy (yes) {
       $('#draft-busy').css('opacity', yes ? 1 : 0)
     },
-    query (data, sucfn) {
+    query (data, sucfn, erfn) {
       this.busy(true)
       if (!sucfn) sucfn = ()=>{}
-      window.axios.post(this.route, data).then(sucfn).catch((e) => {
-        console.error(e)
+      if (!sucfn) erfn = ()=>{}
+      window.axios.post(this.route, data).then(sucfn).catch((error) => {
+        console.error(error.message)
+        erfn(error.response.data)
       }).then(() => {
         this.busy(false)
       })
@@ -94,6 +96,10 @@ export default {
         data: row
       }, (resp) => {
         this.$set(this.rows, resp.data._id, resp.data);
+      }, (er) => {
+        if (!_.isUndefined(er.errors)) {
+          this.$set(this.rows[row._id], '_errors', er.errors);
+        }
       })
     },
     toggleRowEditing(_id) {
@@ -127,7 +133,7 @@ export default {
       return row._changed.indexOf(colId) > -1
     },
     revertCell(row, colId) {
-      this.submitRowChange(merge({}, row, { [colId]: row._orig[colId] }))
+      this.submitRowChange(_.merge({}, row, { [colId]: row._orig[colId] }))
     }
   }
 }
diff --git a/resources/views/table/propose/edit-rows.blade.php b/resources/views/table/propose/edit-rows.blade.php
index 1ac4791..a59b866 100644
--- a/resources/views/table/propose/edit-rows.blade.php
+++ b/resources/views/table/propose/edit-rows.blade.php
@@ -4,40 +4,24 @@
   /** @var \App\Tables\Changeset $changeset */
   /** @var \App\Models\Row[]|Illuminate\Pagination\Paginator $rows */
   /** @var \App\Models\Table $table */
+  /** @var \App\Tables\Column[] $columns */
+  /** @var \App\Tables\Changeset $changeset */
+  /** @var \App\Models\Table $table */
 @endphp
 
-@extends('table.propose.layout')
-
-@section('tab-content')
-
-  @if($rows->hasPages())
-    <div class="col-md-12 d-flex">
-      <nav class="text-center" aria-label="Pages of the table">
-        {{ $rows->links(null, ['ulClass' => 'mb-0']) }}
-      </nav>
-    </div>
-  @endif
-
-  <div class="col-12">
-    @php
-      $transformed = $rows->keyBy('_id')->map(function($r) use ($changeset) {
-        /** @var \App\Tables\Changeset $changeset */
-        return $changeset->transformRow($r, true);
-      });
-    @endphp
+@extends('table.propose.layout-row-pagination')
 
-    <table is="row-editor"
-           route="{{$table->getDraftRoute('update')}}"
-           :columns="{{toJSON($columns)}}"
-           :x-rows="{{toJSON($transformed)}}">
-    </table>
-  </div>
+@section('rows')
+  @php
+    $transformed = $rows->keyBy('_id')->map(function($r) use ($changeset) {
+      /** @var \App\Tables\Changeset $changeset */
+      return $changeset->transformRow($r, true);
+    });
+  @endphp
 
-  @if($rows->hasPages())
-    <div class="col-md-12 d-flex">
-      <nav class="text-center" aria-label="Pages of the table">
-        {{ $rows->links(null, ['ulClass' => 'mb-0']) }}
-      </nav>
-    </div>
-  @endif
+  <table is="row-editor"
+         route="{{$table->draftUpdateRoute}}"
+         :columns="{{toJSON($columns)}}"
+         :x-rows="{{toJSON($transformed)}}">
+  </table>
 @stop
diff --git a/resources/views/table/propose/layout-row-pagination.blade.php b/resources/views/table/propose/layout-row-pagination.blade.php
new file mode 100644
index 0000000..3b078e8
--- /dev/null
+++ b/resources/views/table/propose/layout-row-pagination.blade.php
@@ -0,0 +1,27 @@
+@php
+  /** @var \App\Models\Row[]|Illuminate\Pagination\Paginator $rows */
+@endphp
+
+@extends('table.propose.layout')
+
+@section('tab-content')
+  @if($rows->hasPages())
+    <div class="col-md-12 d-flex">
+      <nav class="text-center" aria-label="Pages of the table">
+        {{ $rows->links(null, ['ulClass' => 'mb-0']) }}
+      </nav>
+    </div>
+  @endif
+
+  <div class="col-12">
+    @yield('rows')
+  </div>
+
+  @if($rows->hasPages())
+    <div class="col-md-12 d-flex">
+      <nav class="text-center" aria-label="Pages of the table">
+        {{ $rows->links(null, ['ulClass' => 'mb-0']) }}
+      </nav>
+    </div>
+  @endif
+@stop
diff --git a/resources/views/table/propose/layout.blade.php b/resources/views/table/propose/layout.blade.php
index c2cd99e..ded5dfe 100644
--- a/resources/views/table/propose/layout.blade.php
+++ b/resources/views/table/propose/layout.blade.php
@@ -22,7 +22,7 @@ if (!isset($tab) || $tab == '') $tab = 'edit-rows';
     <h1 class="mx-3">
       <a href="{{ $table->viewRoute }}" class="link-no-color">{{ $table->title }}</a>
     </h1>
-    <a href="" class="btn btn-outline-danger mr-2" @tooltip(Discard changes)>
+    <a href="{{$table->draftDiscardRoute}}" class="btn btn-outline-danger mr-2" @tooltip(Discard changes)>
       @icon(fa-trash-o, sr:Discard)
     </a>
     @if(user()->ownsTable($table))
diff --git a/routes/web.php b/routes/web.php
index c68376e..2a24bf7 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -51,6 +51,7 @@ Route::group(['middleware' => ['auth', 'activated']], function () {
     Route::post('@{user}/{table}/delete', 'TableController@delete')->name('table.delete');
 
     Route::post('@{user}/{table}/draft/update', 'TableEditController@draftUpdate')->name('table.draft-update');
+    Route::get('@{user}/{table}/draft/discard', 'TableEditController@discard')->name('table.draft-discard');
     Route::get('@{user}/{table}/draft/{tab?}', 'TableEditController@draft')->name('table.draft');
 });