From 3aa7cb5ec4530be1bb50c829993555329eedbe6f Mon Sep 17 00:00:00 2001
From: An Phan <an@construct.sg>
Date: Wed, 16 Dec 2015 00:28:54 +0800
Subject: [PATCH] Better error handling for settings saving

---
 app/Http/Requests/API/SettingRequest.php      |  2 +-
 app/Providers/AppServiceProvider.php          |  2 +-
 resources/assets/js/app.vue                   | 44 ++++++++++++----
 .../main-wrapper/main-content/settings.vue    |  9 +++-
 .../assets/js/components/shared/overlay.vue   | 50 +++++++++++++++++--
 resources/assets/js/services/http.js          |  4 +-
 resources/assets/js/services/utils.js         | 11 ++++
 resources/assets/js/stores/setting.js         |  4 +-
 .../assets/js/tests/services/utilsTest.js     | 19 +++++++
 resources/assets/sass/partials/_vars.scss     |  3 +-
 resources/lang/en/validation.php              |  8 +--
 11 files changed, 131 insertions(+), 25 deletions(-)

diff --git a/app/Http/Requests/API/SettingRequest.php b/app/Http/Requests/API/SettingRequest.php
index 3bac2778..fa484039 100644
--- a/app/Http/Requests/API/SettingRequest.php
+++ b/app/Http/Requests/API/SettingRequest.php
@@ -22,7 +22,7 @@ class SettingRequest extends Request
     public function rules()
     {
         return [
-            'media_path' => 'string|required|valid_path',
+            'media_path' => 'string|required|path.valid',
         ];
     }
 }
diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php
index 2d32c55c..c53edd64 100644
--- a/app/Providers/AppServiceProvider.php
+++ b/app/Providers/AppServiceProvider.php
@@ -15,7 +15,7 @@ class AppServiceProvider extends ServiceProvider
     public function boot()
     {
         // Add some custom validation rules
-        Validator::extend('valid_path', function ($attribute, $value, $parameters, $validator) {
+        Validator::extend('path.valid', function ($attribute, $value, $parameters, $validator) {
             return is_dir($value) && is_readable($value);
         });
     }
diff --git a/resources/assets/js/app.vue b/resources/assets/js/app.vue
index 053a5f68..e5b5137f 100644
--- a/resources/assets/js/app.vue
+++ b/resources/assets/js/app.vue
@@ -8,7 +8,7 @@
         <site-header></site-header>
         <main-wrapper></main-wrapper>
         <site-footer></site-footer>
-        <overlay v-show="loading"></overlay>
+        <overlay :state.sync="overlayState"></overlay>
     </div>
 </template>
 
@@ -36,13 +36,19 @@
 
         data() {
             return {
-                loading: false,
                 prefs: preferenceStore.state,
+
+                overlayState: {
+                    showing: true,
+                    dismissable: false,
+                    type: 'loading',
+                    message: '',
+                },
             };
         },
 
         ready() {
-            this.toggleOverlay();
+            this.showOverlay();
 
             // Make the most important HTTP request to get all necessary data from the server.
             // Afterwards, init all mandatory stores and services.
@@ -50,8 +56,7 @@
                 this.initStores();
                 playback.init(this);
 
-                // Hide the overlaying loading screen.
-                this.toggleOverlay();
+                this.hideOverlay();
 
                 // Ask for user's notificatio permission.
                 this.requestNotifPermission();
@@ -174,11 +179,32 @@
             },
 
             /**
-             * Show or hide the loading overlay.
+             * Shows the overlay.
+             * 
+             * @param {String}  message     The message to display.
+             * @param {String}  type        (loading|success|info|warning|error)
+             * @param {Boolean} dismissable Whether to show the Close button
              */
-            toggleOverlay() {
-                this.loading = !this.loading;
-            }
+            showOverlay(message = 'Just a little patience…', type = 'loading', dismissable = false) {
+                this.overlayState.message = message;
+                this.overlayState.type = type;
+                this.overlayState.dismissable = dismissable;
+                this.overlayState.showing = true;
+            },
+
+            /**
+             * Hides the overlay.
+             */
+            hideOverlay() {
+                this.overlayState.showing = false;
+            },
+
+            /**
+             * Shows the close button, allowing the user to close the overlay.
+             */
+            setOverlayDimissable() {
+                this.overlayState.dismissable = true;
+            },
         },
     };
 
diff --git a/resources/assets/js/components/main-wrapper/main-content/settings.vue b/resources/assets/js/components/main-wrapper/main-content/settings.vue
index f326f4df..658e4259 100644
--- a/resources/assets/js/components/main-wrapper/main-content/settings.vue
+++ b/resources/assets/js/components/main-wrapper/main-content/settings.vue
@@ -26,6 +26,7 @@
 
 <script>
     import settingStore from '../../../stores/setting';
+    import utils from '../../../services/utils';
 
     export default {
         data() {
@@ -39,7 +40,7 @@
              * Save the settings.
              */
             save() {
-                this.$root.toggleOverlay();
+                this.$root.showOverlay();
 
                 settingStore.update(() => {
                     // Data changed. 
@@ -48,6 +49,12 @@
                     // We need refresh the page.
                     // Goodbye.
                     document.location.reload();
+                }, (error, status) => {
+                    if (status === 422) {
+                        error = utils.parseValidationError(error)[0];
+                    }
+
+                    this.$root.showOverlay(`Error: ${error}`, 'error', true);
                 });
             },
         },
diff --git a/resources/assets/js/components/shared/overlay.vue b/resources/assets/js/components/shared/overlay.vue
index c0040d8f..eec75e02 100644
--- a/resources/assets/js/components/shared/overlay.vue
+++ b/resources/assets/js/components/shared/overlay.vue
@@ -1,7 +1,16 @@
 <template>
-    <div id="overlay">
-        <sound-bar></sound-bar>
-        <span class="gnr">Just a little patience…</span>
+    <div id="overlay" v-show="state.showing" class="{{ state.type }}">
+        <div class="display">
+            <sound-bar v-show="state.type === 'loading'"></sound-bar>
+            <i class="fa fa-exclamation-circle" v-show="state.type === 'error'"></i>
+            <i class="fa fa-exclamation-triangle" v-show="state.type === 'warning'"></i>
+            <i class="fa fa-info-circle" v-show="state.type === 'info'"></i>
+            <i class="fa fa-check-circle" v-show="state.type === 'success'"></i>
+
+            <span>{{{ state.message }}}</span>
+        </div>
+
+        <button v-show="state.dismissable" @click.prevent="state.showing = false">Close</button>
     </div>
 </template>
 
@@ -9,6 +18,7 @@
     import soundBar from './sound-bar.vue';
 
     export default {
+        props: ['state'],
         components: { soundBar },
     };
 </script>
@@ -27,9 +37,39 @@
         background-color: rgba(0, 0, 0, 1);
 
         @include vertical-center();
+        flex-direction: column;
 
-        .gnr {
-            opacity: .7;
+        .display {
+            @include vertical-center();
+
+            i {
+                margin-right: 6px;
+            }
+        }
+
+        button {
+            font-size: 12px;
+            margin-top: 16px;
+        }
+
+        &.error {
+            color: $colorRed;
+        }
+
+        &.success {
+            color: $colorGreen;
+        }
+
+        &.info {
+            color: $colorBlue;
+        }
+
+        &.loading {
+            color: $color2ndText;
+        }
+
+        &.warning {
+            color: $colorOrange;
         }
     }
 </style>
diff --git a/resources/assets/js/services/http.js b/resources/assets/js/services/http.js
index d8d0fd53..fba9078d 100644
--- a/resources/assets/js/services/http.js
+++ b/resources/assets/js/services/http.js
@@ -10,13 +10,13 @@ import { extend } from 'lodash';
  */
 export default {
     request(method, url, data, cb = null, options = {}) {
-        options = extend(options, {
+        options = extend({
             error: (data, status, request) => {
                 if (status === 401) {
                     document.location.href = "/login";
                 }
             },
-        });
+        }, options);
 
         switch (method) {
             case 'get':
diff --git a/resources/assets/js/services/utils.js b/resources/assets/js/services/utils.js
index 98302d27..9a3652ea 100644
--- a/resources/assets/js/services/utils.js
+++ b/resources/assets/js/services/utils.js
@@ -26,4 +26,15 @@ export default {
 
         return (h === '00' ? '' : h + ':') + i + ':' + s;
     },
+
+    /**
+     * Parse the validation error from the server into a flattened array of messages.
+     * 
+     * @param  {Object}  error  The error object in JSON format.
+     *                                    
+     * @return {Array}
+     */
+    parseValidationError(error) {
+        return Object.keys(error).reduce((messages, field) => messages.concat(error[field]), []);
+    }
 };
diff --git a/resources/assets/js/stores/setting.js b/resources/assets/js/stores/setting.js
index bccc943a..70f45411 100644
--- a/resources/assets/js/stores/setting.js
+++ b/resources/assets/js/stores/setting.js
@@ -17,11 +17,11 @@ export default {
         return this.state.settings;
     },
 
-    update(cb = null) {
+    update(cb = null, error = null) {
         http.post('settings', this.all(), msg => {
             if (cb) {
                 cb();
             }
-        });
+        }, { error });
     },
 };
diff --git a/resources/assets/js/tests/services/utilsTest.js b/resources/assets/js/tests/services/utilsTest.js
index 6842ac72..e6894b2c 100644
--- a/resources/assets/js/tests/services/utilsTest.js
+++ b/resources/assets/js/tests/services/utilsTest.js
@@ -12,4 +12,23 @@ describe('services/utils', () => {
             utils.secondsToHis(314).should.equal('05:14');
         });
     });
+
+    describe('#parseValidationError', () => {
+        it('correctly parses single-level validation error', () => {
+            let error = {
+                err_1: ['Foo'],
+            };
+
+            utils.parseValidationError(error).should.eql(['Foo']);
+        });
+
+        it('correctly parses multi-level validation error', () => {
+            let error = {
+                err_1: ['Foo', 'Bar'],
+                err_2: ['Baz', 'Qux'],
+            };
+
+            utils.parseValidationError(error).should.eql(['Foo', 'Bar', 'Baz', 'Qux']);
+        });
+    });
 });
diff --git a/resources/assets/sass/partials/_vars.scss b/resources/assets/sass/partials/_vars.scss
index afc1dd9b..b130e560 100644
--- a/resources/assets/sass/partials/_vars.scss
+++ b/resources/assets/sass/partials/_vars.scss
@@ -10,6 +10,7 @@ $colorHeart: #bf2043;
 $colorGreen: #56a052;
 $colorBlue: #4c769a;
 $colorRed: #c34848;
+$colorOrange: #ff7d2e;
 
 $colorSidebarBgr: #212121;
 $colorExtraBgr: #212121;
@@ -19,7 +20,7 @@ $colorPlayerControlsBgr: transparent;
 $colorLink: #aaa;
 $colorLinkHovered: #fff;
 
-$colorHighlight: #ff7d2e;
+$colorHighlight: $colorOrange;
 
 $fontFamily: 'Roboto', sans-serif;
 $fontSize: 13px;
diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php
index b0a1f143..c80fdc55 100644
--- a/resources/lang/en/validation.php
+++ b/resources/lang/en/validation.php
@@ -77,6 +77,10 @@ return [
     'unique'               => 'The :attribute has already been taken.',
     'url'                  => 'The :attribute format is invalid.',
 
+    'path'                 => [
+        'valid'   => 'The :attribute is not a valid or readable path.',
+    ],
+
     /*
     |--------------------------------------------------------------------------
     | Custom Validation Language Lines
@@ -89,9 +93,7 @@ return [
     */
 
     'custom' => [
-        'attribute-name' => [
-            'rule-name' => 'custom-message',
-        ],
+        
     ],
 
     /*