Subscribers: Admin UI updates & fixes

Update for synchronous feature service

Add client-side handling of server-side errors when adding subscribers
- display server-provided error message when we get a server error
- fix the ajax util's `getRequestErrorMessage` method so that it works correctly with Ember's `InvalidError` object instead of the previous request object that it was receiving (*TODO:* this really needs looking at properly so we aren't losing details and Ember Data can do it's stuff)

Styling updates
- proper icon for ascending/descending
- change hover colour to green for "Import CSV" button

Delete subscriber button with confirm modal
- display delete button when hovering over a subscriber row (WARN: really ugly button, styles definitely want looking at)
- show confirm modal when clicking the delete button
- delete subscriber, remove from table, and update total on confirm

ghost/admin/app/components/gh-nav-menu.js

@@ -3,8 +3,7 @@ import Ember from 'ember';
 const {
     Component,
     inject: {service},
-    computed,
-    observer
+    computed
 } = Ember;
 
 export default Component.extend({
@@ -13,7 +12,6 @@ export default Component.extend({
     classNameBindings: ['open'],
 
     open: false,
-    subscribersEnabled: false,
 
     navMenuIcon: computed('ghostPaths.subdir', function () {
         let url = `${this.get('ghostPaths.subdir')}/ghost/img/ghosticon.jpg`;
@@ -26,22 +24,6 @@ export default Component.extend({
     ghostPaths: service(),
     feature: service(),
 
-    // TODO: the features service should offer some way to propogate raw values
-    // rather than promises so we don't need to jump through the hoops below
-    didInsertElement() {
-        this.updateSubscribersEnabled();
-    },
-
-    updateFeatures: observer('feature.labs.subscribers', function () {
-        this.updateSubscribersEnabled();
-    }),
-
-    updateSubscribersEnabled() {
-        this.get('feature.subscribers').then((enabled) => {
-            this.set('subscribersEnabled', enabled);
-        });
-    },
-
     mouseEnter() {
         this.sendAction('onMouseEnter');
     },

ghost/admin/app/components/modals/delete-subscriber.js

@@ -0,0 +1,23 @@
+import Ember from 'ember';
+import ModalComponent from 'ghost/components/modals/base';
+import {invokeAction} from 'ember-invoke-action';
+
+const {computed} = Ember;
+const {alias} = computed;
+
+export default ModalComponent.extend({
+
+    submitting: false,
+
+    subscriber: alias('model'),
+
+    actions: {
+        confirm() {
+            this.set('submitting', true);
+
+            invokeAction(this, 'confirm').finally(() => {
+                this.set('submitting', false);
+            });
+        }
+    }
+});

ghost/admin/app/components/modals/new-subscriber.js

@@ -16,6 +16,12 @@ export default ModalComponent.extend({
 
             confirmAction().then(() => {
                 this.send('closeModal');
+            }).catch((errors) => {
+                let [error] = errors;
+                if (error && error.match(/email/)) {
+                    this.get('model.errors').add('email', error);
+                    this.get('model.hasValidated').pushObject('email');
+                }
             }).finally(() => {
                 if (!this.get('isDestroying') && !this.get('isDestroyed')) {
                     this.set('submitting', false);

ghost/admin/app/controllers/subscribers.js

@@ -20,6 +20,7 @@ export default Ember.Controller.extend(PaginationMixin, {
 
     total: 0,
     table: null,
+    subscriberToDelete: null,
 
     session: service(),
 
@@ -66,6 +67,11 @@ export default Ember.Controller.extend(PaginationMixin, {
             valuePath: 'status',
             sorted: order === 'status',
             ascending: direction === 'asc'
+        }, {
+            label: '',
+            sortable: false,
+            cellComponent: 'gh-subscribers-table-delete-cell',
+            align: 'right'
         }];
     }),
 
@@ -84,8 +90,6 @@ export default Ember.Controller.extend(PaginationMixin, {
         loadFirstPage() {
             let table = this.get('table');
 
-            console.log('loadFirstPage', this.get('paginationSettings'));
-
             return this._super(...arguments).then((results) => {
                 table.addRows(results);
                 return results;
@@ -119,6 +123,24 @@ export default Ember.Controller.extend(PaginationMixin, {
             this.incrementProperty('total');
         },
 
+        deleteSubscriber(subscriber) {
+            this.set('subscriberToDelete', subscriber);
+        },
+
+        confirmDeleteSubscriber() {
+            let subscriber = this.get('subscriberToDelete');
+
+            return subscriber.destroyRecord().then(() => {
+                this.set('subscriberToDelete', null);
+                this.get('table').removeRow(subscriber);
+                this.decrementProperty('total');
+            });
+        },
+
+        cancelDeleteSubscriber() {
+            this.set('subscriberToDelete', null);
+        },
+
         reset() {
             this.get('table').setRows([]);
             this.send('loadFirstPage');

ghost/admin/app/mirage/config.js

@@ -55,8 +55,17 @@ function mockSubscribers(server) {
 
     server.post('/subscribers/', function (db, request) {
         let [attrs] = JSON.parse(request.requestBody).subscribers;
-        let subscriber;
+        let [subscriber] = db.subscribers.where({email: attrs.email});
 
+        if (subscriber) {
+            return new Mirage.Response(422, {}, {
+                errors: [{
+                    errorType: 'DataImportError',
+                    message: 'duplicate email',
+                    property: 'email'
+                }]
+            });
+        } else {
             attrs.created_at = new Date();
             attrs.created_by = 0;
 
@@ -65,6 +74,7 @@ function mockSubscribers(server) {
             return {
                 subscriber
             };
+        }
     });
 
     server.put('/subscribers/:id/', function (db, request) {

ghost/admin/app/styles/layouts/subscribers.css

@@ -22,6 +22,20 @@
     margin: 0;
 }
 
+.subscribers-table table .btn {
+    visibility: hidden;
+}
+
+.subscribers-table table tr:hover .btn {
+    visibility: visible;
+}
+
+.subscribers-table tbody td:last-of-type {
+    padding-top: 0;
+    padding-bottom: 0;
+    padding-left: 0;
+}
+
 
 /* Sidebar (right pane)
 /* ---------------------------------------------------------- */

ghost/admin/app/styles/patterns/buttons.css

@@ -65,6 +65,13 @@ fieldset[disabled] .btn {
     vertical-align: middle;
 }
 
+.btn-hover-green:hover,
+.btn-hover-green:active,
+.btn-hover-green:focus {
+    border-color: var(--green);
+    color: color(var(--green) lightness(-10%));
+}
+
 
 /* Blue button
 /* ---------------------------------------------------------- */

ghost/admin/app/styles/patterns/icons.css

@@ -87,9 +87,15 @@
 .icon-idea:before {
     content: "\e00e";
 }
-.icon-arrow:before {
+.icon-arrow:before,
+.icon-ascending:before,
+.icon-descending:before {
     content: "\e00f";
 }
+.icon-ascending:before {
+    display: inline-block;
+    transform: rotate(180deg);
+}
 .icon-pen:before {
     content: "\e010";
 }

ghost/admin/app/styles/patterns/tables.css

@@ -63,17 +63,16 @@ table td,
 /* Ember Light Table
 /* ---------------------------------------------------------- */
 
+.ember-light-table th {
+    white-space: nowrap;
+}
+
 .ember-light-table .lt-column .lt-sort-icon {
     float: none;
-    margin-left: 0.5em;
-}
-
-.lt-sort-icon.icon-ascending:before {
-    content: "▲";
-    font-size: 0.7em;
+    margin-left: 0.3rem;
 }
 
+.lt-sort-icon.icon-ascending:before,
 .lt-sort-icon.icon-descending:before {
-    content: "▼";
-    font-size: 0.5em;
+    font-size: 0.6em;
 }

ghost/admin/app/templates/components/gh-nav-menu.hbs

@@ -25,7 +25,7 @@
         {{!<li><a href="#"><i class="icon-user"></i>My Posts</a></li>}}
         <li>{{#link-to "team" classNames="gh-nav-main-users"}}<i class="icon-team"></i>Team{{/link-to}}</li>
         {{!<li><a href="#"><i class="icon-idea"></i>Ideas</a></li>}}
-        {{#if subscribersEnabled}}
+        {{#if feature.subscribers}}
             {{#if (gh-user-can-admin session.user)}}
                 <li>{{#link-to "subscribers" classNames="gh-nav-main-subscribers"}}<i class="icon-mail"></i>Subscribers{{/link-to}}</li>
             {{/if}}

ghost/admin/app/templates/components/gh-subscribers-table-delete-cell.hbs

@@ -0,0 +1 @@
+<button class="btn btn-minor btn-sm" {{action tableActions.delete row.content}}><i class="icon-trash"></i></button>

ghost/admin/app/templates/components/gh-subscribers-table.hbs

@@ -1,7 +1,7 @@
 {{#gh-light-table table scrollContainer=".subscribers-table" scrollBuffer=100 onScrolledToBottom=(action 'onScrolledToBottom') as |t|}}
     {{t.head onColumnClick=(action sortByColumn) iconAscending="icon-ascending" iconDescending="icon-descending"}}
 
-    {{#t.body canSelect=false as |body|}}
+    {{#t.body canSelect=false tableActions=(hash delete=(action delete)) as |body|}}
         {{#if isLoading}}
             {{#body.loader}}
                 Loading...

ghost/admin/app/templates/components/modals/delete-subscriber.hbs

@@ -0,0 +1,13 @@
+<header class="modal-header">
+    <h1>Are you sure?</h1>
+</header>
+<a class="close icon-x" href="" title="Close" {{action "closeModal"}}><span class="hidden">Close</span></a>
+
+<div class="modal-body">
+    <strong>WARNING:</strong> All data for this subscriber will be deleted. There is no way to recover this.
+</div>
+
+<div class="modal-footer">
+    <button {{action "closeModal"}} class="btn btn-default btn-minor">Cancel</button>
+    {{#gh-spin-button action="confirm" class="btn btn-red" submitting=submitting}}Delete{{/gh-spin-button}}
+</div>

ghost/admin/app/templates/subscribers.hbs

@@ -11,14 +11,15 @@
                 table=table
                 isLoading=isLoading
                 loadNextPage=(action 'loadNextPage')
-                sortByColumn=(action 'sortByColumn')}}
+                sortByColumn=(action 'sortByColumn')
+                delete=(action 'deleteSubscriber')}}
 
         <div class="subscribers-sidebar">
             <div class="settings-menu-header">
                 <h4>Import Subscribers</h4>
             </div>
             <div class="settings-menu-content subscribers-import-buttons">
-                {{#link-to "subscribers.import" class="btn"}}Import CSV{{/link-to}}
+                {{#link-to "subscribers.import" class="btn btn-hover-green"}}Import CSV{{/link-to}}
                 <a {{action 'exportData'}} class="btn">Export CSV</a>
             </div>
 
@@ -37,4 +38,12 @@
     </section>
 </section>
 
+{{#if subscriberToDelete}}
+    {{gh-fullscreen-modal "delete-subscriber"
+        model=subscriberToDelete
+        confirm=(action "confirmDeleteSubscriber")
+        close=(action "cancelDeleteSubscriber")
+        modifier="action wide"}}
+{{/if}}
+
 {{outlet}}

ghost/admin/app/utils/ajax.js

@@ -2,6 +2,9 @@ import Ember from 'ember';
 
 const {isArray} = Ember;
 
+// TODO: this should be removed and instead have our app serializer properly
+// process the response so that errors can be tied to the model
+
 // Used in API request fail handlers to parse a standard api error
 // response json for the message to display
 export default function getRequestErrorMessage(request, performConcat) {
@@ -20,12 +23,12 @@ export default function getRequestErrorMessage(request, performConcat) {
     if (request.status !== 200) {
         try {
             // Try to parse out the error, or default to 'Unknown'
-            if (request.responseJSON.errors && isArray(request.responseJSON.errors)) {
-                message = request.responseJSON.errors.map((errorItem) => {
+            if (request.errors && isArray(request.errors)) {
+                message = request.errors.map((errorItem) => {
                     return errorItem.message;
                 });
             } else {
-                message =  request.responseJSON.error || 'Unknown Error';
+                message =  request.error || 'Unknown Error';
             }
         } catch (e) {
             msgDetail = request.status ? `${request.status} - ${request.statusText}` : 'Server was not available';

ghost/admin/tests/acceptance/subscribers-test.js

@@ -169,6 +169,62 @@ describe('Acceptance: Subscribers', function() {
                     .to.equal('41');
             });
 
+            // saving a duplicate subscriber
+            click('.btn:contains("Add Subscriber")');
+            fillIn('.fullscreen-modal input[name="email"]', 'test@example.com');
+            click('.fullscreen-modal .btn:contains("Add")');
+
+            andThen(function () {
+                // the validation error is displayed
+                expect(find('.fullscreen-modal .error .response').text().trim(), 'duplicate email validation')
+                    .to.match(/duplicate/);
+
+                // the subscriber is not added to the table
+                expect(find('.lt-cell:contains(test@example.com)').length, 'number of "test@example.com rows"')
+                    .to.equal(1);
+
+                // the subscriber total is unchanged
+                expect(find('#total-subscribers').text().trim(), 'subscribers total after failed add')
+                    .to.equal('41');
+            });
+
+            // deleting a subscriber
+            click('.fullscreen-modal .btn:contains("Cancel")');
+            click('.subscribers-table tbody tr:first-of-type button:last-of-type');
+
+            andThen(function () {
+                // it displays the delete subscriber modal
+                expect(find('.fullscreen-modal').length, 'delete subscriber modal displayed')
+                    .to.equal(1);
+            });
+
+            // cancel the modal
+            click('.fullscreen-modal .btn:contains("Cancel")');
+
+            andThen(function () {
+                // return pauseTest();
+                // it closes the add subscriber modal
+                expect(find('.fullscreen-modal').length, 'delete subscriber modal displayed after cancel')
+                    .to.equal(0);
+            });
+
+            click('.subscribers-table tbody tr:first-of-type button:last-of-type');
+            click('.fullscreen-modal .btn:contains("Delete")');
+
+            andThen(function () {
+                // the add subscriber modal is closed
+                expect(find('.fullscreen-modal').length, 'delete subscriber modal displayed after confirm')
+                    .to.equal(0);
+
+                // the subscriber is removed from the table
+                expect(find('.subscribers-table .lt-body .lt-row:first-of-type .lt-cell:first-of-type').text().trim(), 'first email in list after addition')
+                    .to.not.equal('test@example.com');
+
+                // the subscriber total is updated
+                expect(find('#total-subscribers').text().trim(), 'subscribers total after addition')
+                    .to.equal('40');
+            });
+
             // click the import subscribers button
             click('.btn:contains("Import CSV")');
 

ghost/admin/tests/integration/components/gh-subscribers-table-test.js

@@ -17,8 +17,9 @@ describeComponent(
         it('renders', function() {
             this.set('table', new Table([], []));
             this.set('sortByColumn', function () {});
+            this.set('delete', function () {});
 
-            this.render(hbs`{{gh-subscribers-table table=table sortByColumn=(action sortByColumn)}}`);
+            this.render(hbs`{{gh-subscribers-table table=table sortByColumn=(action sortByColumn) delete=(action delete)}}`);
             expect(this.$()).to.have.length(1);
         });
     }

ghost/admin/tests/integration/components/modals/delete-subscriber-test.js

@@ -0,0 +1,30 @@
+/* jshint expr:true */
+import { expect } from 'chai';
+import {
+    describeComponent,
+    it
+} from 'ember-mocha';
+import hbs from 'htmlbars-inline-precompile';
+
+describeComponent(
+    'modals/delete-subscriber',
+    'Integration: Component: modals/delete-subscriber',
+    {
+        integration: true
+    },
+    function() {
+        it('renders', function() {
+            // Set any properties with this.set('myProperty', 'value');
+            // Handle any actions with this.on('myAction', function(val) { ... });
+            // Template block usage:
+            // this.render(hbs`
+            //   {{#modals/delete-subscriber}}
+            //     template content
+            //   {{/modals/delete-subscriber}}
+            // `);
+
+            this.render(hbs`{{modals/delete-subscriber}}`);
+            expect(this.$()).to.have.length(1);
+        });
+    }
+);

ghost/admin/tests/unit/components/gh-file-uploader-test.js

@@ -49,6 +49,7 @@ describeComponent(
         needs: [
             'service:ajax',
             'service:session', // used by ajax service
+            'service:feature',
             'component:x-file-input'
         ],
         unit: true