✨ detailed theme validation errors (#226)

no issue
- display the detailed validation errors that we get back from gscan so that users know how to fix their themes

ghost/admin/app/components/modals/upload-theme.js

@@ -2,7 +2,10 @@ import ModalComponent from 'ghost-admin/components/modals/base';
 import computed, {mapBy, or} from 'ember-computed';
 import {invokeAction} from 'ember-invoke-action';
 import ghostPaths from 'ghost-admin/utils/ghost-paths';
-import {UnsupportedMediaTypeError} from 'ghost-admin/services/ajax';
+import {
+    UnsupportedMediaTypeError,
+    isThemeValidationError
+} from 'ghost-admin/services/ajax';
 import {isBlank} from 'ember-utils';
 import run from 'ember-runloop';
 import injectService from 'ember-service/inject';
@@ -91,6 +94,12 @@ export default ModalComponent.extend({
             invokeAction(this, 'model.uploadSuccess', this.get('theme'));
         },
 
+        uploadFailed(error) {
+            if (isThemeValidationError(error)) {
+                this.set('validationErrors', error.errors[0].errorDetails);
+            }
+        },
+
         confirm() {
             // noop - we don't want the enter key doing anything
         },
@@ -104,6 +113,10 @@ export default ModalComponent.extend({
             if (!this.get('closeDisabled')) {
                 this._super(...arguments);
             }
+        },
+
+        reset() {
+            this.set('validationErrors', null);
         }
     }
 });

ghost/admin/app/services/ajax.js

@@ -88,6 +88,24 @@ export function isMaintenanceError(errorOrStatus) {
     }
 }
 
+/* Theme validation error */
+
+export function ThemeValidationError(errors) {
+    AjaxError.call(this, errors, 'Theme is not compatible or contains errors.');
+}
+
+ThemeValidationError.prototype = Object.create(AjaxError.prototype);
+
+export function isThemeValidationError(errorOrStatus, payload) {
+    if (isAjaxError(errorOrStatus)) {
+        return errorOrStatus instanceof ThemeValidationError;
+    } else if (errorOrStatus && get(errorOrStatus, 'isAdapterError')) {
+        return get(errorOrStatus, 'errors.firstObject.errorType') === 'ThemeValidationError';
+    } else {
+        return get(payload || {}, 'errors.firstObject.errorType') === 'ThemeValidationError';
+    }
+}
+
 /* end: custom error types */
 
 export default AjaxService.extend({
@@ -119,6 +137,8 @@ export default AjaxService.extend({
             return new UnsupportedMediaTypeError(payload.errors);
         } else if (this.isMaintenanceError(status, headers, payload)) {
             return new MaintenanceError(payload.errors);
+        } else if (this.isThemeValidationError(status, headers, payload)) {
+            return new ThemeValidationError(payload.errors);
         }
 
         return this._super(...arguments);
@@ -160,5 +180,9 @@ export default AjaxService.extend({
 
     isMaintenanceError(status, headers, payload) {
         return isMaintenanceError(status, payload);
+    },
+
+    isThemeValidationError(status, headers, payload) {
+        return isThemeValidationError(status, payload);
     }
 });

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

@@ -251,3 +251,22 @@ a.theme-list-action {
         margin-right: 20px;
     }
 }
+
+.theme-validation-errors {
+    padding-left: 0;
+}
+
+.theme-validation-errors p {
+    margin-bottom: 0;
+}
+
+.theme-validation-errors > li {
+    margin-bottom: 1.2em;
+    list-style: none;
+    font-size: 15px;
+}
+
+.theme-validation-errors > li > ul {
+    margin-top: 0.2em;
+    font-size: 13px;
+}

ghost/admin/app/templates/components/modals/upload-theme.hbs

@@ -2,6 +2,8 @@
     <h1>
         {{#if theme}}
             Upload successful!
+        {{else if validationErrors}}
+            Invalid theme
         {{else}}
             Upload a theme
         {{/if}}
@@ -19,6 +21,24 @@
         <p>
             "{{fileThemeName}}" will overwrite an existing theme of the same name. Are you sure?
         </p>
+    {{else if validationErrors}}
+        <ul class="theme-validation-errors">
+            {{#each validationErrors as |error|}}
+                <li>
+                    {{#if error.details}}
+                        {{{error.details}}}
+                    {{else}}
+                        {{{error.rule}}}
+                    {{/if}}
+
+                    <ul>
+                        {{#each error.failures as |failure|}}
+                            <li><code>{{failure.ref}}</code>{{#if failure.message}}: {{failure.message}}{{/if}}</li>
+                        {{/each}}
+                    </ul>
+                </li>
+            {{/each}}
+        </ul>
     {{else}}
         {{gh-file-uploader
             url=uploadUrl
@@ -29,6 +49,7 @@
             uploadStarted=(action "uploadStarted")
             uploadFinished=(action "uploadFinished")
             uploadSuccess=(action "uploadSuccess")
+            uploadFailed=(action "uploadFailed")
             listenTo="themeUploader"}}
     {{/if}}
 </div>
@@ -42,6 +63,11 @@
             Overwrite
         </button>
     {{/if}}
+    {{#if validationErrors}}
+        <button {{action "reset"}} class="btn btn-green">
+            Try Again
+        </button>
+    {{/if}}
     {{#if canActivateTheme}}
         <button {{action "activate"}} class="btn btn-green">
             Activate Now

ghost/admin/tests/acceptance/settings/general-test.js

@@ -411,7 +411,7 @@ describe('Acceptance: Settings - General', function () {
                 ).to.match(/default Casper theme cannot be overwritten/);
             });
 
-            // theme upload handles validation errors
+            // theme upload handles upload errors
             andThen(() => {
                 server.post('/themes/upload/', function () {
                     return new Mirage.Response(422, {}, {
@@ -433,8 +433,90 @@ describe('Acceptance: Settings - General', function () {
                 mockThemes(server);
             });
 
-            // theme upload handles success then close
+            // theme upload handles validation errors
+            andThen(() => {
+                server.post('/themes/upload/', function () {
+                    return new Mirage.Response(422, {}, {
+                        errors: [
+                            {
+                                message: 'Theme is not compatible or contains errors.',
+                                errorType: 'ThemeValidationError',
+                                errorDetails: [
+                                    {
+                                        level: 'error',
+                                        rule: 'Templates must contain valid Handlebars.',
+                                        failures: [
+                                            {
+                                                ref: 'index.hbs',
+                                                message: 'The partial index_meta could not be found'
+                                            },
+                                            {
+                                                ref: 'tag.hbs',
+                                                message: 'The partial index_meta could not be found'
+                                            }
+                                        ]
+                                    },
+                                    {
+                                        level: 'error',
+                                        rule: 'Assets such as CSS & JS must use the <code>{{asset}}</code> helper',
+                                        details: '<p>The listed files should be included using the <code>{{asset}}</code> helper.</p>',
+                                        failures: [
+                                            {
+                                                ref: '/assets/javascripts/ui.js'
+                                            }
+                                        ]
+                                    }
+                                ]
+                            }
+                        ]
+                    });
+                });
+            });
             click('button:contains("Try Again")');
+            fileUpload('.fullscreen-modal input[type="file"]', ['test'], {name: 'bad-theme.zip', type: 'application/zip'});
+            andThen(() => {
+                expect(
+                    find('.fullscreen-modal h1').text().trim(),
+                    'modal title after uploading invalid theme'
+                ).to.equal('Invalid theme');
+
+                expect(
+                    find('.theme-validation-errors').text(),
+                    'top-level errors are displayed'
+                ).to.match(/Templates must contain valid Handlebars/);
+
+                expect(
+                    find('.theme-validation-errors').text(),
+                    'top-level errors do not escape HTML'
+                ).to.match(/The listed files should be included using the {{asset}} helper/);
+
+                expect(
+                    find('.theme-validation-errors').text(),
+                    'individual failures are displayed'
+                ).to.match(/index\.hbs: The partial index_meta could not be found/);
+
+                // reset to default mirage handlers
+                mockThemes(server);
+            });
+            click('button:contains("Try Again")');
+            andThen(() => {
+                expect(
+                    find('.theme-validation-errors').length,
+                    '"Try Again" resets form after theme validation error'
+                ).to.equal(0);
+
+                expect(
+                    find('.gh-image-uploader').length,
+                    '"Try Again" resets form after theme validation error'
+                ).to.equal(1);
+
+                expect(
+                    find('.fullscreen-modal h1').text().trim(),
+                    '"Try Again" resets form after theme validation error'
+                ).to.equal('Upload a theme');
+            });
+
+            // theme upload handles success then close
             fileUpload('.fullscreen-modal input[type="file"]', ['test'], {name: 'theme-1.zip', type: 'application/zip'});
             andThen(() => {
                 expect(