Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Commit

Permalink
🎨 improve theme results (#726)
Browse files Browse the repository at this point in the history
closes TryGhost/Ghost#8222

- there are fatal and normal errors
- fatal === can't activate a theme
- the normal errors are only returned in development mode (!)
- Separate between `fatal` and normal errors and group them
  • Loading branch information
kirrg001 authored and aileen committed Jun 6, 2017
1 parent 983a3aa commit 7ffc3c4
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 8 deletions.
32 changes: 30 additions & 2 deletions app/components/modals/upload-theme.js
Expand Up @@ -105,13 +105,38 @@ export default ModalComponent.extend({
this.set('validationWarnings', get(theme, 'warnings'));
}

// Ghost differentiates between errors and fatal errors
// You can't activate a theme with fatal errors, but with errors.
if (get(theme, 'errors.length') > 0) {
this.set('validationErrors', get(theme, 'errors'));
}

this.set('hasWarningsOrErrors', this.get('validationErrors').length || this.get('validationWarnings').length);

// invoke the passed in confirm action
invokeAction(this, 'model.uploadSuccess', theme);
},

uploadFailed(error) {
if (isThemeValidationError(error)) {
this.set('validationErrors', error.errors[0].errorDetails);
let errors = error.errors[0].errorDetails;
let fatalErrors = [];
let normalErrors = [];

// to have a proper grouping of fatal errors and none fatal, we need to check
// our errors for the fatal property
if (errors.length > 0) {
for (let i = 0; i < errors.length; i++) {
if (errors[i].fatal) {
fatalErrors.push(errors[i]);
} else {
normalErrors.push(errors[i]);
}
}
}

this.set('fatalValidationErrors', fatalErrors);
this.set('validationErrors', normalErrors);
}
},

Expand All @@ -131,7 +156,10 @@ export default ModalComponent.extend({
},

reset() {
this.set('validationErrors', null);
this.set('validationWarnings', []);
this.set('validationErrors', []);
this.set('fatalValidationErrors', []);
this.set('hasWarningsOrErrors', false);
}
}
});
1 change: 1 addition & 0 deletions app/models/theme.js
Expand Up @@ -6,6 +6,7 @@ export default Model.extend({
package: attr('raw'),
active: attr('boolean'),
warnings: attr('raw'),
errors: attr('raw'),

activate() {
let adapter = this.store.adapterFor(this.constructor.modelName);
Expand Down
17 changes: 17 additions & 0 deletions app/styles/layouts/settings.css
Expand Up @@ -374,3 +374,20 @@
margin-top: 0.2em;
font-size: 13px;
}

.theme-validation-errordescription {
margin-top: 1em;
margin-bottom: 0.5em;
}

.theme-validation-errordescription span {
font-weight: 600;
}

.theme-validation-errortype {
font-size: 18px;
}

.theme-validation-errortype.fatal {
color: var(--red);
}
43 changes: 38 additions & 5 deletions app/templates/components/modals/upload-theme.hbs
Expand Up @@ -2,7 +2,7 @@
<h1>
{{#if theme}}
{{#if validationWarnings}}
Uploaded with warnings
Upload successful with warnings/errors!
{{else}}
Upload successful!
{{/if}}
Expand All @@ -17,13 +17,30 @@

<div class="modal-body">
{{#if theme}}
{{#if validationWarnings}}
<ul class="theme-validation-errors">
{{#if hasWarningsOrErrors}}
<ul class="theme-validation-errors">
<li>
<p>
"{{themeName}}" uploaded successfully but some warnings were generated...
"{{themeName}}" uploaded successfully but some warnings/errors were detected.
You are still able to use and activate the theme. Here is your report...
</p>
</li>

{{#if validationErrors}}
<div class="theme-validation-errordescription">
<h2 class="theme-validation-errortype">Errors</h2>
<p><em>(Very recommended to fix, functionality <span>could</span> be restricted)</em></p>
</div>
{{/if}}
{{#each validationErrors as |error|}}
{{gh-theme-error-li error=error}}
{{/each}}

{{#if validationWarnings}}
<div class="theme-validation-errordescription">
<h2 class="theme-validation-errortype">Warnings</h2>
</div>
{{/if}}
{{#each validationWarnings as |error|}}
{{gh-theme-error-li error=error}}
{{/each}}
Expand All @@ -38,8 +55,24 @@
<p>
"{{fileThemeName}}" will overwrite an existing theme of the same name. Are you sure?
</p>
{{else if validationErrors}}
{{else if (or validationErrors fatalValidationErrors)}}
<ul class="theme-validation-errors">
{{#if fatalValidationErrors}}
<div class="theme-validation-errordescription">
<h2 class="theme-validation-errortype fatal">Fatal Errors</h2>
<p><em>(Must-fix to activate theme)</em></p>
</div>
{{/if}}
{{#each fatalValidationErrors as |error|}}
{{gh-theme-error-li error=error}}
{{/each}}

{{#if validationErrors}}
<div class="theme-validation-errordescription">
<h2 class="theme-validation-errortype">Errors</h2>
<p><em>(Very recommended to fix, functionality <span>could</span> be restricted)</em></p>
</div>
{{/if}}
{{#each validationErrors as |error|}}
{{gh-theme-error-li error=error}}
{{/each}}
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/settings/design-test.js
Expand Up @@ -395,7 +395,7 @@ describe('Acceptance: Settings - Design', function () {
expect(
find('.fullscreen-modal h1').text().trim(),
'modal title after uploading theme with warnings'
).to.equal('Uploaded with warnings');
).to.equal('Upload successful with warnings/errors!');

expect(
find('.theme-validation-errors').text(),
Expand Down

0 comments on commit 7ffc3c4

Please sign in to comment.