Mathijs den Burger pushed to branch feature/visual-editing-psp1-CHANNELMGR-1014 
at cms-community / hippo-addon-channel-manager


Commits:
a6617110 by Mathijs den Burger at 2016-11-30T09:47:38+01:00
CHANNELMGR-1014 Code improvements after review

- use FeedbackService.showError to avoid undefined parameters
- shorten errorKey logic
- clarify promise logic in viewFullContent with a comment
- move helper function _checkSaveChanges closer to only usage
- improve unit test description

- - - - -


2 changed files:

- 
frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.component.js
- 
frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.spec.js


Changes:

=====================================
frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.component.js
=====================================
--- 
a/frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.component.js
+++ 
b/frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.component.js
@@ -177,11 +177,8 @@ export class ChannelRightSidePanelCtrl {
         if (this._isDocument(response.data)) {
           // CHANNELMGR-898: handle validation error on a per-field basis
         } else {
-          let defaultKey = 'ERROR_UNABLE_TO_SAVE';
-          if (this._isErrorInfo(response.data)) {
-            defaultKey = `ERROR_${response.data.reason}`;
-          }
-          this.FeedbackService.showErrorResponse(undefined, defaultKey, 
undefined, this.$element);
+          const errorKey = this._isErrorInfo(response.data) ? 
`ERROR_${response.data.reason}` : 'ERROR_UNABLE_TO_SAVE';
+          this.FeedbackService.showError(errorKey, {}, this.$element);
         }
         return this.$q.reject(); // tell the caller that saving has failed.
       });
@@ -198,12 +195,28 @@ export class ChannelRightSidePanelCtrl {
   viewFullContent() {
     this._checkSaveChanges()
       .then(() => {
+        // don't return the result of saveDocument so a failing save does not 
switch to the full content
         this.saveDocument()
           .then(() => this._closeAndBrowseToView());
       })
       .catch(() => this._closeAndBrowseToView());
   }
 
+  _checkSaveChanges() {
+    if (!this._isFormDirty()) {
+      return this.$q.reject();
+    }
+    const messageParams = {
+      documentName: this.doc.displayName,
+    };
+    const confirm = this.DialogService.confirm()
+      .textContent(this.$translate.instant('CONFIRM_SAVE_CHANGES_MESSAGE', 
messageParams))
+      .ok(this.$translate.instant('SAVE'))
+      .cancel(this.$translate.instant('DISCARD'));
+
+    return this.DialogService.show(confirm);
+  }
+
   _closeAndBrowseToView() {
     // The CMS automatically unlocks content that is being viewed, so close 
the side-panel to reflect that.
     // It will will unlock the document if needed, so don't delete the draft 
here.
@@ -252,21 +265,6 @@ export class ChannelRightSidePanelCtrl {
     return this.DialogService.show(confirm);
   }
 
-  _checkSaveChanges() {
-    if (!this._isFormDirty()) {
-      return this.$q.reject();
-    }
-    const messageParams = {
-      documentName: this.doc.displayName,
-    };
-    const confirm = this.DialogService.confirm()
-      .textContent(this.$translate.instant('CONFIRM_SAVE_CHANGES_MESSAGE', 
messageParams))
-      .ok(this.$translate.instant('SAVE'))
-      .cancel(this.$translate.instant('DISCARD'));
-
-    return this.DialogService.show(confirm);
-  }
-
   _deleteDraft() {
     if (this.editing) {
       this.ContentService.deleteDraft(this.documentId);


=====================================
frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.spec.js
=====================================
--- 
a/frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.spec.js
+++ 
b/frontend-ng/src/angularjs/channel/sidePanels/rightSidePanel/rightSidePanel.spec.js
@@ -82,7 +82,7 @@ describe('ChannelRightSidePanel', () => {
 
     ChannelSidePanelService = jasmine.createSpyObj('ChannelSidePanelService', 
['initialize', 'isOpen', 'close']);
     ContentService = jasmine.createSpyObj('ContentService', ['createDraft', 
'getDocumentType', 'saveDraft', 'deleteDraft']);
-    FeedbackService = jasmine.createSpyObj('FeedbackService', 
['showErrorResponse']);
+    FeedbackService = jasmine.createSpyObj('FeedbackService', ['showError']);
 
     CmsService = jasmine.createSpyObj('CmsService', ['publish']);
     DialogService = jasmine.createSpyObj('DialogService', ['confirm', 'show']);
@@ -420,7 +420,7 @@ describe('ChannelRightSidePanel', () => {
     expect(ContentService.saveDraft).not.toHaveBeenCalled();
   });
 
-  it('shows a toast when document save fails', () => {
+  it('shows an error when document save fails', () => {
     const response = {
       reason: 'TEST',
     };
@@ -434,10 +434,10 @@ describe('ChannelRightSidePanel', () => {
 
     $rootScope.$apply();
 
-    expect(FeedbackService.showErrorResponse).toHaveBeenCalledWith(undefined, 
'ERROR_TEST', undefined, $ctrl.$element);
+    expect(FeedbackService.showError).toHaveBeenCalledWith('ERROR_TEST', {}, 
$ctrl.$element);
   });
 
-  it('shows a toast when document save fails and there is no data returned', 
() => {
+  it('shows an error when document save fails and there is no data returned', 
() => {
     ContentService.saveDraft.and.returnValue($q.reject({}));
 
     $ctrl.doc = testDocument;
@@ -448,7 +448,7 @@ describe('ChannelRightSidePanel', () => {
 
     $rootScope.$apply();
 
-    expect(FeedbackService.showErrorResponse).toHaveBeenCalledWith(undefined, 
'ERROR_UNABLE_TO_SAVE', undefined, $ctrl.$element);
+    
expect(FeedbackService.showError).toHaveBeenCalledWith('ERROR_UNABLE_TO_SAVE', 
{}, $ctrl.$element);
   });
 
   it('directly views the full content if the form is not dirty', () => {
@@ -464,7 +464,7 @@ describe('ChannelRightSidePanel', () => {
     expect(CmsService.publish).toHaveBeenCalledWith('view-content', 'test');
   });
 
-  it('can skip saving pending changes when viewing the full content', () => {
+  it('can discard pending changes before viewing the full content', () => {
     const dialog = jasmine.createSpyObj('dialog', ['textContent', 'ok', 
'cancel']);
     dialog.textContent.and.returnValue(dialog);
     dialog.ok.and.returnValue(dialog);



View it on GitLab: 
https://code.onehippo.org/cms-community/hippo-addon-channel-manager/commit/a6617110f89726767595d38db2c7fe3346c54da8
_______________________________________________
Hippocms-svn mailing list
Hippocms-svn@lists.onehippo.org
https://lists.onehippo.org/mailman/listinfo/hippocms-svn

Reply via email to