From 3135fb97613ff3cd7ac6dd76ad2ee2e98e34519e Mon Sep 17 00:00:00 2001
From: Jason Williams <jaswilli@gmail.com>
Date: Thu, 9 Jul 2015 12:10:00 -0500
Subject: [PATCH] Refactor role selects; pause transition if saving

Refs #5501
- Switch role select dropdowns from gh-select based components to
  gh-select-native.
- Prevent transition out of editor while there is an in-flight save.
---
 .../client/app/components/gh-role-selector.js | 15 ----
 core/client/app/components/gh-select.js       | 79 -------------------
 .../app/controllers/modals/invite-new-user.js | 17 ++--
 core/client/app/controllers/team/user.js      |  4 +
 .../client/app/initializers/store-injector.js | 10 ---
 core/client/app/mixins/editor-base-route.js   | 11 +++
 .../templates/components/gh-role-selector.hbs |  5 --
 .../app/templates/modals/invite-new-user.hbs  | 34 ++++----
 core/client/app/templates/team/user.hbs       | 21 +++--
 core/test/functional/client/editor_test.js    | 23 ++----
 core/test/functional/client/team_test.js      | 32 ++++----
 11 files changed, 80 insertions(+), 171 deletions(-)
 delete mode 100644 core/client/app/components/gh-role-selector.js
 delete mode 100644 core/client/app/components/gh-select.js
 delete mode 100644 core/client/app/initializers/store-injector.js
 delete mode 100644 core/client/app/templates/components/gh-role-selector.hbs

diff --git a/core/client/app/components/gh-role-selector.js b/core/client/app/components/gh-role-selector.js
deleted file mode 100644
index 20a5e75542..0000000000
--- a/core/client/app/components/gh-role-selector.js
+++ /dev/null
@@ -1,15 +0,0 @@
-import Ember from 'ember';
-import GhostSelect from 'ghost/components/gh-select';
-
-var RolesSelector = GhostSelect.extend({
-    roles: Ember.computed.alias('options'),
-
-    options: Ember.computed(function () {
-        var rolesPromise = this.store.find('role', {permissions: 'assign'});
-
-        return Ember.ArrayProxy.extend(Ember.PromiseProxyMixin)
-            .create({promise: rolesPromise});
-    })
-});
-
-export default RolesSelector;
diff --git a/core/client/app/components/gh-select.js b/core/client/app/components/gh-select.js
deleted file mode 100644
index f053bc3422..0000000000
--- a/core/client/app/components/gh-select.js
+++ /dev/null
@@ -1,79 +0,0 @@
-import Ember from 'ember';
-// GhostSelect is a solution to Ember.Select being evil and worthless.
-// (Namely, this solves problems with async data in Ember.Select)
-// Inspired by (that is, totally ripped off from) this JSBin
-// http://emberjs.jsbin.com/rwjblue/40/edit
-
-// Usage:
-// Extend this component and create a template for your component.
-// Your component must define the `options` property.
-// Optionally use `initialValue` to set the object
-//     you want to have selected to start with.
-// Both options and initalValue are promise safe.
-// Set onChange in your template to be the name
-//    of the action you want called in your
-// For an example, see gh-roles-selector
-
-var GhostSelect = Ember.Component.extend({
-    tagName: 'span',
-    classNames: ['gh-select'],
-    attributeBindings: ['tabindex'],
-
-    tabindex: '0', // 0 must be a string, or else it's interpreted as false
-
-    options: null,
-    initialValue: null,
-
-    resolvedOptions: null,
-    resolvedInitialValue: null,
-
-    // Convert promises to their values
-    init: function () {
-        var self = this;
-
-        this._super.apply(this, arguments);
-
-        Ember.RSVP.hash({
-            resolvedOptions: this.get('options'),
-            resolvedInitialValue: this.get('initialValue')
-        }).then(function (resolvedHash) {
-            self.setProperties(resolvedHash);
-
-            // Run after render to ensure the <option>s have rendered
-            Ember.run.schedule('afterRender', function () {
-                self.setInitialValue();
-            });
-        });
-    },
-
-    setInitialValue: function () {
-        var initialValue = this.get('resolvedInitialValue'),
-            options = this.get('resolvedOptions'),
-            initialValueIndex = options.indexOf(initialValue);
-
-        if (initialValueIndex > -1) {
-            this.$('option:eq(' + initialValueIndex + ')').prop('selected', true);
-        }
-    },
-
-    // Called by DOM events
-    change: function () {
-        this._changeSelection();
-    },
-
-    // Send value to specified action
-    _changeSelection: function () {
-        var value = this._selectedValue();
-
-        Ember.set(this, 'value', value);
-        this.sendAction('onChange', value);
-    },
-
-    _selectedValue: function () {
-        var selectedIndex = this.$('select')[0].selectedIndex;
-
-        return this.get('options').objectAt(selectedIndex);
-    }
-});
-
-export default GhostSelect;
diff --git a/core/client/app/controllers/modals/invite-new-user.js b/core/client/app/controllers/modals/invite-new-user.js
index 707da53f2c..4af00cc725 100644
--- a/core/client/app/controllers/modals/invite-new-user.js
+++ b/core/client/app/controllers/modals/invite-new-user.js
@@ -3,18 +3,25 @@ import Ember from 'ember';
 export default Ember.Controller.extend({
     notifications: Ember.inject.service(),
 
+    role: null,
+    authorRole: null,
+
+    roles: Ember.computed(function () {
+        return this.store.find('role', {permissions: 'assign'});
+    }),
+
     // Used to set the initial value for the dropdown
-    authorRole: Ember.computed(function () {
+    authorRoleObserver: Ember.observer('roles.@each.role', function () {
         var self = this;
 
-        return this.store.find('role').then(function (roles) {
+        this.get('roles').then(function (roles) {
             var authorRole = roles.findBy('name', 'Author');
 
-            // Initialize role as well.
-            self.set('role', authorRole);
             self.set('authorRole', authorRole);
 
-            return authorRole;
+            if (!self.get('role')) {
+                self.set('role', authorRole);
+            }
         });
     }),
 
diff --git a/core/client/app/controllers/team/user.js b/core/client/app/controllers/team/user.js
index 5bcb687a4c..6ad63f3f0d 100644
--- a/core/client/app/controllers/team/user.js
+++ b/core/client/app/controllers/team/user.js
@@ -77,6 +77,10 @@ export default Ember.Controller.extend({
         });
     }),
 
+    roles: Ember.computed(function () {
+        return this.store.find('role', {permissions: 'assign'});
+    }),
+
     actions: {
         changeRole: function (newRole) {
             this.set('model.role', newRole);
diff --git a/core/client/app/initializers/store-injector.js b/core/client/app/initializers/store-injector.js
deleted file mode 100644
index e867d49b87..0000000000
--- a/core/client/app/initializers/store-injector.js
+++ /dev/null
@@ -1,10 +0,0 @@
-var StoreInjector = {
-    name: 'store-injector',
-    after: 'store',
-
-    initialize: function (container, application) {
-        application.inject('component:gh-role-selector', 'store', 'store:main');
-    }
-};
-
-export default StoreInjector;
diff --git a/core/client/app/mixins/editor-base-route.js b/core/client/app/mixins/editor-base-route.js
index 7ca7f161fb..14974f20e2 100644
--- a/core/client/app/mixins/editor-base-route.js
+++ b/core/client/app/mixins/editor-base-route.js
@@ -39,6 +39,17 @@ var EditorBaseRoute = Ember.Mixin.create(styleBody, ShortcutsRoute, {
                 fromNewToEdit,
                 deletedWithoutChanges;
 
+            // if a save is in-flight we don't know whether or not it's safe to leave
+            // so we abort the transition and retry after the save has completed.
+            if (state.isSaving) {
+                transition.abort();
+                return Ember.run.later(this, function () {
+                    Ember.RSVP.resolve(controller.get('lastPromise')).then(function () {
+                        transition.retry();
+                    });
+                }, 100);
+            }
+
             fromNewToEdit = this.get('routeName') === 'editor.new' &&
                 transition.targetName === 'editor.edit' &&
                 transition.intent.contexts &&
diff --git a/core/client/app/templates/components/gh-role-selector.hbs b/core/client/app/templates/components/gh-role-selector.hbs
deleted file mode 100644
index 9f534be421..0000000000
--- a/core/client/app/templates/components/gh-role-selector.hbs
+++ /dev/null
@@ -1,5 +0,0 @@
-<select id="{{selectId}}" name="{{selectName}}">
-{{#each roles as |role|}}
-<option value={{role.id}}>{{role.name}}</option>
-{{/each}}
-</select>
diff --git a/core/client/app/templates/modals/invite-new-user.hbs b/core/client/app/templates/modals/invite-new-user.hbs
index 2cf1074729..db59e84fd0 100644
--- a/core/client/app/templates/modals/invite-new-user.hbs
+++ b/core/client/app/templates/modals/invite-new-user.hbs
@@ -1,21 +1,25 @@
 {{#gh-modal-dialog action="closeModal" showClose=true type="action"
     title="Invite a New User" confirm=confirm class="invite-new-user"}}
 
-        <fieldset>
-            <div class="form-group">
-                <label for="new-user-email">Email Address</label>
-                {{input enter="confirmAccept" class="gh-input email" id="new-user-email" type="email" placeholder="Email Address" name="email" autofocus="autofocus"
-                autocapitalize="off" autocorrect="off" value=email}}
-            </div>
+    <fieldset>
+        <div class="form-group">
+            <label for="new-user-email">Email Address</label>
+            {{input enter="confirmAccept" class="gh-input email" id="new-user-email" type="email" placeholder="Email Address" name="email" autofocus="autofocus"
+            autocapitalize="off" autocorrect="off" value=email}}
+        </div>
 
-            <div class="form-group for-select">
-                <label for="new-user-role">Role</label>
-                {{gh-role-selector
-                    initialValue=authorRole
-                    onChange="setRole"
-                    selectId="new-user-role"}}
-            </div>
-
-        </fieldset>
+        <div class="form-group for-select">
+            <label for="new-user-role">Role</label>
+            <span class="gh-select" tabindex="0">
+                {{gh-select-native id="new-user-role"
+                    content=roles
+                    optionValuePath="id"
+                    optionLabelPath="name"
+                    selection=role
+                    action="setRole"
+                }}
+            </span>
+        </div>
+    </fieldset>
 
 {{/gh-modal-dialog}}
diff --git a/core/client/app/templates/team/user.hbs b/core/client/app/templates/team/user.hbs
index 15bffc63be..00a18cef51 100644
--- a/core/client/app/templates/team/user.hbs
+++ b/core/client/app/templates/team/user.hbs
@@ -80,14 +80,19 @@
                     <p>Used for notifications</p>
                 </div>
                 {{#if rolesDropdownIsVisible}}
-                <div class="form-group">
-                    <label for="user-role">Role</label>
-                    {{gh-role-selector
-                        initialValue=user.role
-                        onChange="changeRole"
-                        selectId="user-role"}}
-                    <p>What permissions should this user have?</p>
-                </div>
+                    <div class="form-group">
+                        <label for="user-role">Role</label>
+                        <span class="gh-select" tabindex="0">
+                            {{gh-select-native id="new-user-role"
+                                content=roles
+                                optionValuePath="id"
+                                optionLabelPath="name"
+                                selection=model.role
+                                action="changeRole"
+                            }}
+                        </span>
+                        <p>What permissions should this user have?</p>
+                    </div>
                 {{/if}}
                 <div class="form-group">
                     <label for="user-location">Location</label>
diff --git a/core/test/functional/client/editor_test.js b/core/test/functional/client/editor_test.js
index ccf474bca5..6ad5f6e5cc 100644
--- a/core/test/functional/client/editor_test.js
+++ b/core/test/functional/client/editor_test.js
@@ -528,7 +528,6 @@ CasperTest.begin('Publish menu - delete post', 7, function testDeleteModal(test)
     });
 });
 
-// TODO: Change number of tests back to 4 once the commented-out tests are fixed
 CasperTest.begin('Publish menu - new post status is correct after failed save', 2, function suite(test) {
     casper.thenOpenAndWaitForPageLoad('editor', function testTitleAndUrl() {
         test.assertTitle('Editor - Test Blog', 'Ghost admin has incorrect title');
@@ -553,27 +552,15 @@ CasperTest.begin('Publish menu - new post status is correct after failed save',
     // attempt to save
     casper.thenClick('.js-publish-button');
 
-    // ... check status, label, class
-    // TODO: re-implement these tests once #5933 is merged
-    // casper.waitForSelector('.notification-error', function onSuccess() {
-    //     test.assertExists('.js-publish-button.btn-blue', 'Update button should have .btn-blue');
-    //     // wait for button to settle
-    //     casper.wait(500);
-    //     test.assertSelectorHasText('.js-publish-button', 'Save Draft', '.js-publish-button says Save Draft');
-    // }, function onTimeout() {
-    //     test.assert(false, 'Saving post with invalid title should trigger an error');
-    // });
-
     // Click on "Content" in the main nav
     casper.thenClick('.gh-nav-main-content');
 
-    // TODO: FIX THIS TEST!!!!
     // The "Are you sure?" modal appears
-    // casper.waitUntilVisible('.modal-content', function onSuccess() {
-    //     casper.thenClick('.btn-red');
-    // }, function onTimeout() {
-    //     test.assert(false, 'Are you sure you want to leave modal did not appear.');
-    // });
+    casper.waitUntilVisible('.modal-content', function onSuccess() {
+        casper.thenClick('.btn-red');
+    }, function onTimeout() {
+        test.assert(false, 'Are you sure you want to leave modal did not appear.');
+    });
 });
 
 // TODO: Change number of tests back to 6 once the commented-out tests are fixed
diff --git a/core/test/functional/client/team_test.js b/core/test/functional/client/team_test.js
index 3f4fac1bfb..f347198bc6 100644
--- a/core/test/functional/client/team_test.js
+++ b/core/test/functional/client/team_test.js
@@ -27,8 +27,7 @@ CasperTest.begin('Team tab is correct', 4, function suite(test) {
     });
 });
 
-// TODO: reset to 9 tests once test is fixed
-CasperTest.begin('Users screen is correct', 8, function suite(test) {
+CasperTest.begin('Users screen is correct', 9, function suite(test) {
     casper.thenOpenAndWaitForPageLoad('settings.general');
     casper.thenTransitionAndWaitForScreenLoad('team', function canTransition() {
         test.assert(true, 'Can transition to users screen from settings.general');
@@ -45,7 +44,7 @@ CasperTest.begin('Users screen is correct', 8, function suite(test) {
     casper.thenClick('.view-actions .btn-green');
     casper.waitForOpaque('.invite-new-user .modal-content', function then() {
         test.assertEval(function testOwnerRoleNotAnOption() {
-            var options = document.querySelectorAll('.invite-new-user select#new-user-role option'),
+            var options = document.querySelectorAll('.invite-new-user #new-user-role select option'),
                 i = 0;
             for (; i < options.length; i = i + 1) {
                 if (options[i].text === 'Owner') {
@@ -55,21 +54,22 @@ CasperTest.begin('Users screen is correct', 8, function suite(test) {
             return true;
         }, '"Owner" is not a role option for new users');
     });
-    // TODO: FIX THIS TEST!!
+
     // role options get loaded asynchronously; give them a chance to come in
-    // casper.waitForSelector('.invite-new-user select#new-user-role option', function then() {
-    //     test.assertEval(function authorIsSelectedByDefault() {
-    //         var options = document.querySelectorAll('.invite-new-user select#new-user-role option'),
-    //             i = 0;
-    //         for (; i < options.length; i = i + 1) {
-    //             if (options[i].selected) {
-    //                 return options[i].text === 'Author';
-    //             }
-    //         }
-    //         return false;
-    //     }, 'The "Author" role is selected by default when adding a new user');
-    // });
+    casper.waitForSelector('.invite-new-user #new-user-role select option', function then() {
+        test.assertEval(function authorIsSelectedByDefault() {
+            var options = document.querySelectorAll('.invite-new-user #new-user-role select option'),
+                i = 0;
+            for (; i < options.length; i = i + 1) {
+                if (options[i].selected) {
+                    return options[i].text === 'Author';
+                }
+            }
+            return false;
+        }, 'The "Author" role is selected by default when adding a new user');
+    });
 });
+
 // ### User settings tests
 CasperTest.begin('Can save settings', 7, function suite(test) {
     casper.thenOpenAndWaitForPageLoad('team.user', function testTitleAndUrl() {