0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-24 23:48:13 -05:00

💅🏼 Imprve theme activation error messages (#756)

refs TryGhost/Ghost#8530

This PR takes care that the modals for theme activation gets the same treatment as theme upload modal:
- differentiate between normal and fatal errors
- list headings for each error type (fatal, normal or warning)
- update test
This commit is contained in:
Aileen Nowak 2017-06-23 00:19:01 +07:00 committed by Katharina Irrgang
parent fa391217b9
commit 7eb60b0086
6 changed files with 70 additions and 8 deletions

View file

@ -5,6 +5,8 @@ export default ModalComponent.extend({
title: reads('model.title'), title: reads('model.title'),
message: reads('model.message'), message: reads('model.message'),
warnings: reads('model.warnings'), warnings: reads('model.warnings'),
errors: reads('model.errors'),
fatalErrors: reads('model.fatalErrors'),
'data-test-theme-warnings-modal': true 'data-test-theme-warnings-modal': true
}); });

View file

@ -48,7 +48,6 @@ export default Controller.extend({
try { try {
yield RSVP.all(validationPromises); yield RSVP.all(validationPromises);
return yield this.get('model').save(); return yield this.get('model').save();
} catch (error) { } catch (error) {
if (error) { if (error) {
notifications.showAPIError(error); notifications.showAPIError(error);
@ -125,13 +124,43 @@ export default Controller.extend({
activateTheme(theme) { activateTheme(theme) {
return theme.activate().then((theme) => { return theme.activate().then((theme) => {
let themeName = theme.get('name');
if (!isEmpty(theme.get('warnings'))) { if (!isEmpty(theme.get('warnings'))) {
this.set('themeWarnings', theme.get('warnings')); this.set('themeWarnings', theme.get('warnings'));
this.set('showThemeWarningsModal', true); this.set('showThemeWarningsModal', true);
} }
if (!isEmpty(theme.get('errors'))) {
this.set('themeErrors', theme.get('errors'));
this.set('showThemeWarningsModal', true);
}
if (this.get('themeErrors') || this.get('themeWarnings')) {
let message = `${themeName} activated successfully but some warnings/errors were detected.
You are still able to use and activate the theme. Here is your report...`;
this.set('message', message);
}
}).catch((error) => { }).catch((error) => {
if (isThemeValidationError(error)) { if (isThemeValidationError(error)) {
this.set('themeWarnings', 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('themeErrors', normalErrors);
this.set('themeFatalErrors', fatalErrors);
this.set('showThemeErrorsModal', true); this.set('showThemeErrorsModal', true);
return; return;
} }
@ -167,6 +196,8 @@ export default Controller.extend({
hideThemeWarningsModal() { hideThemeWarningsModal() {
this.set('themeWarnings', null); this.set('themeWarnings', null);
this.set('themeErrors', null);
this.set('themeFatalErrors', null);
this.set('showThemeWarningsModal', false); this.set('showThemeWarningsModal', false);
this.set('showThemeErrorsModal', false); this.set('showThemeErrorsModal', false);
}, },

View file

@ -10,9 +10,35 @@
<p data-test-theme-warnings-message>{{message}}</p> <p data-test-theme-warnings-message>{{message}}</p>
</li> </li>
{{/if}} {{/if}}
{{#if fatalErrors}}
<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 fatalErrors as |error|}}
{{gh-theme-error-li error=error}}
{{/each}}
{{#if errors}}
<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 errors as |error|}}
{{gh-theme-error-li error=error}}
{{/each}}
{{#if warnings}}
<div class="theme-validation-errordescription">
<h2 class="theme-validation-errortype">Warnings</h2>
</div>
{{/if}}
{{#each warnings as |error|}} {{#each warnings as |error|}}
{{gh-theme-error-li error=error}} {{gh-theme-error-li error=error}}
{{/each}} {{/each}}
</ul> </ul>
</div> </div>

View file

@ -1,7 +1,7 @@
<header class="modal-header"> <header class="modal-header">
<h1> <h1>
{{#if theme}} {{#if theme}}
{{#if validationWarnings}} {{#if hasWarningsOrErrors}}
Upload successful with warnings/errors! Upload successful with warnings/errors!
{{else}} {{else}}
Upload successful! Upload successful!

View file

@ -46,8 +46,10 @@
{{#if showThemeWarningsModal}} {{#if showThemeWarningsModal}}
{{gh-fullscreen-modal "theme-warnings" {{gh-fullscreen-modal "theme-warnings"
model=(hash model=(hash
title="Theme activated with warnings" title="Activated successful with warnings/errors!"
warnings=themeWarnings warnings=themeWarnings
errors=themeErrors
message=message
) )
close=(action "hideThemeWarningsModal") close=(action "hideThemeWarningsModal")
modifier="action wide"}} modifier="action wide"}}
@ -56,8 +58,9 @@
{{#if showThemeErrorsModal}} {{#if showThemeErrorsModal}}
{{gh-fullscreen-modal "theme-warnings" {{gh-fullscreen-modal "theme-warnings"
model=(hash model=(hash
title="Theme activation failed" title="Activation failed"
warnings=themeWarnings errors=themeErrors
fatalErrors=themeFatalErrors
) )
close=(action "hideThemeWarningsModal") close=(action "hideThemeWarningsModal")
modifier="action wide"}} modifier="action wide"}}

View file

@ -511,7 +511,7 @@ describe('Acceptance: Settings - Design', function () {
expect( expect(
find(testSelector('theme-warnings-title')).text().trim(), find(testSelector('theme-warnings-title')).text().trim(),
'modal title after activating invalid theme' 'modal title after activating invalid theme'
).to.equal('Theme activation failed'); ).to.equal('Activation failed');
expect( expect(
find(testSelector('theme-warnings')).text(), find(testSelector('theme-warnings')).text(),
@ -570,7 +570,7 @@ describe('Acceptance: Settings - Design', function () {
expect( expect(
find(testSelector('theme-warnings-title')).text().trim(), find(testSelector('theme-warnings-title')).text().trim(),
'modal title after activating theme with warnings' 'modal title after activating theme with warnings'
).to.equal('Theme activated with warnings'); ).to.equal('Activated successful with warnings/errors!');
expect( expect(
find(testSelector('theme-warnings')).text(), find(testSelector('theme-warnings')).text(),