[ACS-6190] prefer-promise-reject-errors rule and fixes

.eslintrc.js

@@ -101,7 +101,7 @@ module.exports = {
                 ],
                 '@typescript-eslint/member-ordering': 'off',
                 'prefer-arrow/prefer-arrow-functions': 'off',
-
+                'prefer-promise-reject-errors': 'error',
                 'brace-style': 'off',
                 '@typescript-eslint/brace-style': 'error',
                 'comma-dangle': 'error',

lib/content-services/src/lib/common/services/people-content.service.spec.ts

@@ -26,17 +26,14 @@ import {
     AlfrescoApiService,
     AlfrescoApiServiceMock,
     AuthenticationService,
-    CoreTestingModule,
-    LogService
+    CoreTestingModule
 } from '@alfresco/adf-core';
 import { PeopleContentQueryRequestModel, PeopleContentService } from './people-content.service';
 import { TranslateModule } from '@ngx-translate/core';
 import { TestBed } from '@angular/core/testing';
 
 describe('PeopleContentService', () => {
-
     let peopleContentService: PeopleContentService;
-    let logService: LogService;
     let authenticationService: AuthenticationService;
 
     beforeEach(() => {
@@ -52,7 +49,6 @@ describe('PeopleContentService', () => {
 
         authenticationService = TestBed.inject(AuthenticationService);
         peopleContentService = TestBed.inject(PeopleContentService);
-        logService = TestBed.inject(LogService);
     });
 
     it('should be able to fetch person details based on id', async () => {
@@ -110,14 +106,11 @@ describe('PeopleContentService', () => {
     });
 
     it('should be able to throw an error if createPerson api failed', (done) => {
-        spyOn(peopleContentService.peopleApi, 'createPerson').and.returnValue(Promise.reject('failed to create new person'));
-        const logErrorSpy = spyOn(logService, 'error');
+        spyOn(peopleContentService.peopleApi, 'createPerson').and.returnValue(Promise.reject(new Error('failed to create new person')));
         peopleContentService.createPerson(createNewPersonMock).subscribe(
-            () => {
-            },
-            (error) => {
-                expect(logErrorSpy).toHaveBeenCalledWith('failed to create new person');
-                expect(error).toEqual('failed to create new person');
+            () => {},
+            (error: Error) => {
+                expect(error.message).toEqual('failed to create new person');
                 done();
             }
         );

lib/content-services/src/lib/common/services/people-content.service.ts

@@ -16,9 +16,9 @@
  */
 
 import { Injectable } from '@angular/core';
-import { from, Observable, of, throwError } from 'rxjs';
-import { AlfrescoApiService, AuthenticationService, LogService } from '@alfresco/adf-core';
-import { catchError, map, tap } from 'rxjs/operators';
+import { from, Observable, of } from 'rxjs';
+import { AlfrescoApiService, AuthenticationService } from '@alfresco/adf-core';
+import { map, tap } from 'rxjs/operators';
 import { Pagination, PeopleApi, PersonBodyCreate, PersonBodyUpdate } from '@alfresco/js-api';
 import { EcmUserModel } from '../models/ecm-user.model';
 import { ContentService } from './content.service';
@@ -59,7 +59,6 @@ export class PeopleContentService {
     constructor(
         private apiService: AlfrescoApiService,
         authenticationService: AuthenticationService,
-        private logService: LogService,
         private contentService: ContentService
     ) {
         authenticationService.onLogout.subscribe(() => {
@@ -76,8 +75,8 @@ export class PeopleContentService {
     getPerson(personId: string): Observable<EcmUserModel> {
         return from(this.peopleApi.getPerson(personId))
             .pipe(
-                map((personEntry) => new EcmUserModel(personEntry.entry)),
-                catchError((error) => this.handleError(error)));
+                map((personEntry) => new EcmUserModel(personEntry.entry))
+            );
     }
 
     getCurrentPerson(): Observable<EcmUserModel> {
@@ -130,8 +129,7 @@ export class PeopleContentService {
             map(response => ({
                 pagination: response.list.pagination,
                 entries: response.list.entries.map((person) => person.entry as EcmUserModel)
-            })),
-            catchError((err) => this.handleError(err))
+            }))
         );
     }
 
@@ -144,8 +142,7 @@ export class PeopleContentService {
      */
     createPerson(newPerson: PersonBodyCreate, opts?: any): Observable<EcmUserModel> {
         return from(this.peopleApi.createPerson(newPerson, opts)).pipe(
-            map((res) => res?.entry as EcmUserModel),
-            catchError((error) => this.handleError(error))
+            map((res) => res?.entry as EcmUserModel)
         );
     }
 
@@ -159,8 +156,7 @@ export class PeopleContentService {
      */
     updatePerson(personId: string, details: PersonBodyUpdate, opts?: any): Observable<EcmUserModel> {
         return from(this.peopleApi.updatePerson(personId, details, opts)).pipe(
-            map((res) => res?.entry as EcmUserModel),
-            catchError((error) => this.handleError(error))
+            map((res) => res?.entry as EcmUserModel)
         );
     }
 
@@ -177,9 +173,4 @@ export class PeopleContentService {
     private buildOrderArray(sorting: PeopleContentSortingModel): string[] {
         return sorting?.orderBy && sorting?.direction ? [`${sorting.orderBy} ${sorting.direction.toUpperCase()}`] : [];
     }
-
-    private handleError(error: any) {
-        this.logService.error(error);
-        return throwError(error || 'Server error');
-    }
 }

lib/content-services/src/lib/common/services/rendition.service.ts

@@ -201,7 +201,7 @@ export class RenditionService {
                                 clearInterval(intervalId);
                                 return resolve(this.handleNodeRendition(nodeId, rendition.entry.content.mimeType, versionId));
                             }
-                        }, () => reject());
+                        }, () => reject(new Error('Error geting version rendition')));
                     } else {
                         this.renditionsApi.getRendition(nodeId, renditionId).then((rendition: RenditionEntry) => {
                             const status: string = rendition.entry.status.toString();
@@ -210,11 +210,11 @@ export class RenditionService {
                                 clearInterval(intervalId);
                                 return resolve(this.handleNodeRendition(nodeId, renditionId, versionId));
                             }
-                        }, () => reject());
+                        }, () => reject(new Error('Error getting rendition')));
                     }
                 } else {
                     clearInterval(intervalId);
-                    return reject();
+                    return reject(new Error('Error getting rendition'));
                 }
             }, this.TRY_TIMEOUT);
         });
@@ -287,6 +287,7 @@ export class RenditionService {
      * These are: images, PDF files, or PDF rendition of files.
      * We also force PDF rendition for TEXT type objects, otherwise the default URL is to download.
      * TODO there are different TEXT type objects, (HTML, plaintext, xml, etc. we should determine how these are handled)
+     *
      * @param objectId object it
      * @param mimeType mime type
      */

lib/content-services/src/lib/common/services/rendition.services.spec.ts

@@ -20,22 +20,20 @@ import { AlfrescoApiService, LogService, TranslationService, ViewUtilService } f
 import { Rendition, RenditionEntry, RenditionPaging, RenditionsApi } from '@alfresco/js-api';
 import { RenditionService } from '@alfresco/adf-content-services';
 
-const getRenditionEntry = (status: Rendition.StatusEnum): RenditionEntry => {
-    return {
-        entry: {
-            id: 'pdf',
-            status: status,
-            content: { mimeType: 'application/pdf', mimeTypeName: 'application/pdf', sizeInBytes: 10 }
-        }
-    };
-};
-const getRenditionPaging = (status: Rendition.StatusEnum): RenditionPaging =>  {
-    return {
-        list: {
-            entries: [getRenditionEntry(status)]
-        }
-    };
-};
+const getRenditionEntry = (status: Rendition.StatusEnum): RenditionEntry => ({
+    entry: {
+        id: 'pdf',
+        status,
+        content: { mimeType: 'application/pdf', mimeTypeName: 'application/pdf', sizeInBytes: 10 }
+    }
+});
+
+const getRenditionPaging = (status: Rendition.StatusEnum): RenditionPaging =>  ({
+    list: {
+        entries: [getRenditionEntry(status)]
+    }
+});
+
 describe('RenditionService', () => {
     let renditionService: RenditionService;
     let renditionsApi: any;

lib/content-services/src/lib/dialogs/node-lock.dialog.spec.ts

@@ -92,7 +92,7 @@ describe('NodeLockDialogComponent', () => {
         }));
 
         it('should call onError if submit fails', fakeAsync(() => {
-            spyOn(component.nodesApi, 'lockNode').and.returnValue(Promise.reject('error'));
+            spyOn(component.nodesApi, 'lockNode').and.returnValue(Promise.reject(new Error('error')));
             spyOn(component.data, 'onError');
 
             component.submit();

lib/content-services/src/lib/directives/library-favorite.directive.spec.ts

@@ -53,39 +53,39 @@ describe('LibraryFavoriteDirective', () => {
     });
 
     it('should not check for favorite if no selection exists', () => {
-        spyOn(component.directive['favoritesApi'], 'getFavoriteSite');
+        spyOn(component.directive.favoritesApi, 'getFavoriteSite');
         fixture.detectChanges();
 
-        expect(component.directive['favoritesApi'].getFavoriteSite).not.toHaveBeenCalled();
+        expect(component.directive.favoritesApi.getFavoriteSite).not.toHaveBeenCalled();
     });
 
     it('should mark selection as favorite', async () => {
-        spyOn(component.directive['favoritesApi'], 'getFavoriteSite').and.returnValue(Promise.resolve(null));
+        spyOn(component.directive.favoritesApi, 'getFavoriteSite').and.returnValue(Promise.resolve(null));
 
         delete selection.isFavorite;
 
         fixture.detectChanges();
         await fixture.whenStable();
 
-        expect(component.directive['favoritesApi'].getFavoriteSite).toHaveBeenCalled();
+        expect(component.directive.favoritesApi.getFavoriteSite).toHaveBeenCalled();
         expect(component.directive.isFavorite()).toBe(true);
     });
 
     it('should mark selection not favorite', async () => {
-        spyOn(component.directive['favoritesApi'], 'getFavoriteSite').and.returnValue(Promise.reject());
+        spyOn(component.directive.favoritesApi, 'getFavoriteSite').and.returnValue(Promise.reject(new Error('error')));
 
         delete selection.isFavorite;
 
         fixture.detectChanges();
         await fixture.whenStable();
 
-        expect(component.directive['favoritesApi'].getFavoriteSite).toHaveBeenCalled();
+        expect(component.directive.favoritesApi.getFavoriteSite).toHaveBeenCalled();
         expect(component.directive.isFavorite()).toBe(false);
     });
 
     it('should call addFavorite() on click event when selection is not a favorite', async () => {
-        spyOn(component.directive['favoritesApi'], 'getFavoriteSite').and.returnValue(Promise.reject());
-        spyOn(component.directive['favoritesApi'], 'createFavorite').and.returnValue(Promise.resolve(null));
+        spyOn(component.directive.favoritesApi, 'getFavoriteSite').and.returnValue(Promise.reject(new Error('error')));
+        spyOn(component.directive.favoritesApi, 'createFavorite').and.returnValue(Promise.resolve(null));
 
         fixture.detectChanges();
         await fixture.whenStable();
@@ -94,12 +94,12 @@ describe('LibraryFavoriteDirective', () => {
 
         fixture.nativeElement.querySelector('button').dispatchEvent(new MouseEvent('click'));
         fixture.detectChanges();
-        expect(component.directive['favoritesApi'].createFavorite).toHaveBeenCalled();
+        expect(component.directive.favoritesApi.createFavorite).toHaveBeenCalled();
     });
 
     it('should call removeFavoriteSite() on click event when selection is favorite', async () => {
-        spyOn(component.directive['favoritesApi'], 'getFavoriteSite').and.returnValue(Promise.resolve(null));
-        spyOn(component.directive['favoritesApi'], 'deleteFavorite').and.returnValue(Promise.resolve());
+        spyOn(component.directive.favoritesApi, 'getFavoriteSite').and.returnValue(Promise.resolve(null));
+        spyOn(component.directive.favoritesApi, 'deleteFavorite').and.returnValue(Promise.resolve());
 
         selection.isFavorite = true;
 
@@ -113,6 +113,6 @@ describe('LibraryFavoriteDirective', () => {
         fixture.detectChanges();
         await fixture.whenStable();
 
-        expect(component.directive['favoritesApi'].deleteFavorite).toHaveBeenCalled();
+        expect(component.directive.favoritesApi.deleteFavorite).toHaveBeenCalled();
     });
 });

lib/content-services/src/lib/directives/library-membership.directive.spec.ts

@@ -70,7 +70,7 @@ describe('LibraryMembershipDirective', () => {
 
     describe('markMembershipRequest', () => {
         beforeEach(() => {
-            getMembershipSpy = spyOn(directive['sitesApi'], 'getSiteMembershipRequestForPerson').and.returnValue(Promise.resolve({ entry: requestedMembershipResponse }));
+            getMembershipSpy = spyOn(directive.sitesApi, 'getSiteMembershipRequestForPerson').and.returnValue(Promise.resolve({ entry: requestedMembershipResponse }));
         });
 
         it('should not check membership requests if no entry is selected', fakeAsync(() => {
@@ -98,7 +98,7 @@ describe('LibraryMembershipDirective', () => {
         }));
 
         it('should remember when a membership request is not found for selected library', fakeAsync(() => {
-            getMembershipSpy.and.returnValue(Promise.reject());
+            getMembershipSpy.and.returnValue(Promise.reject(new Error('error')));
 
             const selection = { entry: testSiteEntry };
             const change = new SimpleChange(null, selection, true);
@@ -111,9 +111,9 @@ describe('LibraryMembershipDirective', () => {
     describe('toggleMembershipRequest', () => {
         beforeEach(() => {
             mockSupportedVersion = false;
-            getMembershipSpy = spyOn(directive['sitesApi'], 'getSiteMembershipRequestForPerson').and.returnValue(Promise.resolve({ entry: requestedMembershipResponse }));
-            addMembershipSpy = spyOn(directive['sitesApi'], 'createSiteMembershipRequestForPerson').and.returnValue(Promise.resolve({ entry: requestedMembershipResponse }));
-            deleteMembershipSpy = spyOn(directive['sitesApi'], 'deleteSiteMembershipRequestForPerson').and.returnValue(Promise.resolve());
+            getMembershipSpy = spyOn(directive.sitesApi, 'getSiteMembershipRequestForPerson').and.returnValue(Promise.resolve({ entry: requestedMembershipResponse }));
+            addMembershipSpy = spyOn(directive.sitesApi, 'createSiteMembershipRequestForPerson').and.returnValue(Promise.resolve({ entry: requestedMembershipResponse }));
+            deleteMembershipSpy = spyOn(directive.sitesApi, 'deleteSiteMembershipRequestForPerson').and.returnValue(Promise.resolve());
         });
 
         it('should do nothing if there is no selected library ', fakeAsync(() => {

lib/content-services/src/lib/directives/node-delete.directive.spec.ts

@@ -111,10 +111,10 @@ describe('NodeDeleteDirective', () => {
         element = fixture.debugElement.query(By.directive(NodeDeleteDirective));
         elementWithPermanentDelete = fixtureWithPermanentComponent.debugElement.query(By.directive(NodeDeleteDirective));
 
-        deleteNodeSpy = spyOn(component.deleteDirective['nodesApi'], 'deleteNode').and.returnValue(Promise.resolve());
+        deleteNodeSpy = spyOn(component.deleteDirective.nodesApi, 'deleteNode').and.returnValue(Promise.resolve());
 
-        deleteNodePermanentSpy = spyOn(componentWithPermanentDelete.deleteDirective['nodesApi'], 'deleteNode').and.returnValue(Promise.resolve());
-        purgeDeletedNodePermanentSpy = spyOn(componentWithPermanentDelete.deleteDirective['trashcanApi'], 'deleteDeletedNode').and.returnValue(Promise.resolve());
+        deleteNodePermanentSpy = spyOn(componentWithPermanentDelete.deleteDirective.nodesApi, 'deleteNode').and.returnValue(Promise.resolve());
+        purgeDeletedNodePermanentSpy = spyOn(componentWithPermanentDelete.deleteDirective.trashcanApi, 'deleteDeletedNode').and.returnValue(Promise.resolve());
 
     });
 
@@ -152,7 +152,7 @@ describe('NodeDeleteDirective', () => {
         });
 
         it('should notify failed node deletion', async () => {
-            deleteNodeSpy.and.returnValue(Promise.reject('error'));
+            deleteNodeSpy.and.returnValue(Promise.reject(new Error('error')));
 
             component.selection = [{ entry: { id: '1', name: 'name1' } }];
             fixture.detectChanges();
@@ -187,7 +187,7 @@ describe('NodeDeleteDirective', () => {
         });
 
         it('should notify failed nodes deletion', async () => {
-            deleteNodeSpy.and.returnValue(Promise.reject('error'));
+            deleteNodeSpy.and.returnValue(Promise.reject(new Error('error')));
 
             component.selection = [
                 { entry: { id: '1', name: 'name1' } },
@@ -209,7 +209,7 @@ describe('NodeDeleteDirective', () => {
         it('should notify partial deletion when only one node is successful', async () => {
             deleteNodeSpy.and.callFake((id) => {
                 if (id === '1') {
-                    return Promise.reject('error');
+                    return Promise.reject(new Error('error'));
                 } else {
                     return Promise.resolve();
                 }
@@ -235,7 +235,7 @@ describe('NodeDeleteDirective', () => {
         it('should notify partial deletion when some nodes are successful', async () => {
             deleteNodeSpy.and.callFake((id) => {
                 if (id === '1') {
-                    return Promise.reject(null);
+                    return Promise.reject(new Error('error'));
                 }
 
                 return Promise.resolve();