optimise routes and guards (#676)

* reduce duplicate settings

* simplify guards/resolvers

* fetch data after 401

* unit tests

* extra unit tests

* unit tests for auth guard

* trashcan tests

src/app/app.component.ts

@@ -37,11 +37,11 @@ import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
 import { Store } from '@ngrx/store';
 import { AppExtensionService } from './extensions/extension.service';
 import {
-  SetLanguagePickerAction,
   SnackbarErrorAction,
   SetCurrentUrlAction,
   SetInitialStateAction,
-  CloseModalDialogsAction
+  CloseModalDialogsAction,
+  SetRepositoryStatusAction
 } from './store/actions';
 import {
   AppStore,
@@ -49,6 +49,9 @@ import {
   INITIAL_APP_STATE
 } from './store/states/app.state';
 import { filter } from 'rxjs/operators';
+import { ContentApiService } from './services/content-api.service';
+import { DiscoveryEntry } from 'alfresco-js-api';
+import { AppService } from './services/app.service';
 
 @Component({
   selector: 'app-root',
@@ -66,7 +69,9 @@ export class AppComponent implements OnInit {
     private authenticationService: AuthenticationService,
     private uploadService: UploadService,
     private extensions: AppExtensionService,
-    private translationService: TranslationService
+    private translationService: TranslationService,
+    private contentApi: ContentApiService,
+    private appService: AppService
   ) {}
 
   ngOnInit() {
@@ -85,6 +90,11 @@ export class AppComponent implements OnInit {
 
           this.store.dispatch(new CloseModalDialogsAction());
           this.router.navigate(['/login']);
+
+          this.appService.waitForAuth().subscribe(() => {
+            this.loadRepositoryStatus();
+            // todo: load external auth-enabled plugins here
+          });
         }
       }
     });
@@ -115,18 +125,31 @@ export class AppComponent implements OnInit {
     this.uploadService.fileUploadError.subscribe(error =>
       this.onFileUploadedError(error)
     );
+
+    this.appService.waitForAuth().subscribe(() => {
+      this.loadRepositoryStatus();
+      // todo: load external auth-enabled plugins here
+    });
+  }
+
+  private loadRepositoryStatus() {
+    this.contentApi
+      .getRepositoryInformation()
+      .subscribe((response: DiscoveryEntry) => {
+        this.store.dispatch(
+          new SetRepositoryStatusAction(response.entry.repository.status)
+        );
+      });
   }
 
   private loadAppSettings() {
-    const languagePicker = this.config.get<boolean>('languagePicker');
-    this.store.dispatch(new SetLanguagePickerAction(languagePicker));
-
     const baseShareUrl =
       this.config.get<string>('baseShareUrl') ||
       this.config.get<string>('ecmHost');
 
     const state: AppState = {
       ...INITIAL_APP_STATE,
+      languagePicker: this.config.get<boolean>('languagePicker'),
       appName: this.config.get<string>('application.name'),
       headerColor: this.config.get<string>('headerColor'),
       logoPath: this.config.get<string>('application.logo'),

src/app/app.routes.ts

@@ -30,7 +30,6 @@ import { LibrariesComponent } from './components/libraries/libraries.component';
 import { GenericErrorComponent } from './components/common/generic-error/generic-error.component';
 import { SearchResultsComponent } from './components/search/search-results/search-results.component';
 import { ProfileResolver } from './services/profile.resolver';
-import { RepositoryStatusResolver } from './services/repository-status.resolver';
 import { LoginComponent } from './components/login/login.component';
 import { AppAuthGuard } from './guards/auth.guard';
 import { AppSharedRuleGuard } from './guards/shared.guard';
@@ -57,8 +56,7 @@ export const APP_ROUTES: Routes = [
     path: '',
     component: LayoutComponent,
     resolve: {
-      profile: ProfileResolver,
+      profile: ProfileResolver
-      repository: RepositoryStatusResolver
     },
     children: [
       {
@@ -79,8 +77,6 @@ export const APP_ROUTES: Routes = [
             loadChildren:
               'src/app/components/preview/preview.module#PreviewModule',
             data: {
-              title: 'APP.PREVIEW.TITLE',
-              navigateMultiple: true,
               navigateSource: 'favorites'
             }
           }
@@ -110,8 +106,6 @@ export const APP_ROUTES: Routes = [
             loadChildren:
               'src/app/components/preview/preview.module#PreviewModule',
             data: {
-              title: 'APP.PREVIEW.TITLE',
-              navigateMultiple: true,
               navigateSource: 'libraries'
             }
           }
@@ -143,8 +137,6 @@ export const APP_ROUTES: Routes = [
             loadChildren:
               'src/app/components/preview/preview.module#PreviewModule',
             data: {
-              title: 'APP.PREVIEW.TITLE',
-              navigateMultiple: true,
               navigateSource: 'personal-files'
             }
           },
@@ -153,8 +145,6 @@ export const APP_ROUTES: Routes = [
             loadChildren:
               'src/app/components/preview/preview.module#PreviewModule',
             data: {
-              title: 'APP.PREVIEW.TITLE',
-              navigateMultiple: true,
               navigateSource: 'personal-files'
             }
           }
@@ -176,8 +166,6 @@ export const APP_ROUTES: Routes = [
             loadChildren:
               'src/app/components/preview/preview.module#PreviewModule',
             data: {
-              title: 'APP.PREVIEW.TITLE',
-              navigateMultiple: true,
               navigateSource: 'recent-files'
             }
           }
@@ -196,8 +184,6 @@ export const APP_ROUTES: Routes = [
             loadChildren:
               'src/app/components/preview/preview.module#PreviewModule',
             data: {
-              title: 'APP.PREVIEW.TITLE',
-              navigateMultiple: true,
               navigateSource: 'shared'
             }
           }
@@ -230,8 +216,6 @@ export const APP_ROUTES: Routes = [
             loadChildren:
               'src/app/components/preview/preview.module#PreviewModule',
             data: {
-              title: 'APP.PREVIEW.TITLE',
-              navigateMultiple: true,
               navigateSource: 'search'
             }
           }

src/app/components/preview/preview.module.ts

@@ -38,7 +38,11 @@ import { AppToolbarModule } from '../toolbar/toolbar.module';
 const routes: Routes = [
   {
     path: '',
-    component: PreviewComponent
+    component: PreviewComponent,
+    data: {
+      title: 'APP.PREVIEW.TITLE',
+      navigateMultiple: true
+    }
   }
 ];
 

src/app/components/shared-link-view/shared-link-view.component.spec.ts

@@ -24,9 +24,39 @@
  */
 
 import { SharedLinkViewComponent } from './shared-link-view.component';
+import { TestBed, ComponentFixture } from '@angular/core/testing';
+import { AppTestingModule } from '../../testing/app-testing.module';
+import { ActivatedRoute } from '@angular/router';
+import { of } from 'rxjs';
+import { NO_ERRORS_SCHEMA } from '@angular/core';
 
 describe('SharedLinkViewComponent', () => {
-  it('should be defined', () => {
+  let component: SharedLinkViewComponent;
-    expect(SharedLinkViewComponent).toBeDefined();
+  let fixture: ComponentFixture<SharedLinkViewComponent>;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [AppTestingModule],
+      declarations: [SharedLinkViewComponent],
+      providers: [
+        {
+          provide: ActivatedRoute,
+          useValue: {
+            snapshot: { data: { preferencePrefix: 'prefix' } },
+            params: of({ id: '123' })
+          }
+        }
+      ],
+      schemas: [NO_ERRORS_SCHEMA]
+    });
+
+    fixture = TestBed.createComponent(SharedLinkViewComponent);
+    component = fixture.componentInstance;
+
+    fixture.detectChanges();
+  });
+
+  it('should fetch link id from the active route', () => {
+    expect(component.sharedLinkId).toBe('123');
   });
 });

src/app/components/shared-link-view/shared-link-view.component.ts

@@ -6,7 +6,6 @@ import { ActivatedRoute } from '@angular/router';
   templateUrl: 'shared-link-view.component.html',
   styleUrls: ['shared-link-view.component.scss'],
   encapsulation: ViewEncapsulation.None,
-  // tslint:disable-next-line:use-host-property-decorator
   host: { class: 'app-shared-link-view' }
 })
 export class SharedLinkViewComponent implements OnInit {

src/app/components/trashcan/trashcan.component.spec.ts

@@ -89,6 +89,13 @@ describe('TrashcanComponent', () => {
     );
   });
 
+  it('should reload on nodes purged', () => {
+    component.ngOnInit();
+    spyOn(component, 'reload').and.stub();
+    contentService.nodesPurged.next({});
+    expect(component.reload).toHaveBeenCalled();
+  });
+
   describe('onRestoreNode()', () => {
     it('should call refresh()', () => {
       spyOn(component, 'reload');

src/app/directives/pagination.directive.spec.ts

@@ -24,9 +24,57 @@
  */
 
 import { PaginationDirective } from './pagination.directive';
+import { TestBed, ComponentFixture } from '@angular/core/testing';
+import { AppTestingModule } from '../testing/app-testing.module';
+import { DirectivesModule } from './directives.module';
+import {
+  UserPreferencesService,
+  AppConfigService,
+  PaginationComponent,
+  CoreModule,
+  PaginationModel
+} from '@alfresco/adf-core';
 
 describe('PaginationDirective', () => {
-  it('should be defined', () => {
+  let preferences: UserPreferencesService;
-    expect(PaginationDirective).toBeDefined();
+  let config: AppConfigService;
+  let pagination: PaginationComponent;
+  let fixture: ComponentFixture<PaginationComponent>;
+  let directive: PaginationDirective;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [AppTestingModule, DirectivesModule, CoreModule.forRoot()]
+    });
+
+    preferences = TestBed.get(UserPreferencesService);
+    config = TestBed.get(AppConfigService);
+    fixture = TestBed.createComponent(PaginationComponent);
+    pagination = fixture.componentInstance;
+    directive = new PaginationDirective(pagination, preferences, config);
+  });
+
+  afterEach(() => {
+    fixture.destroy();
+    directive.ngOnDestroy();
+  });
+
+  it('should setup supported page sizes from app config', () => {
+    spyOn(config, 'get').and.returnValue([21, 31, 41]);
+
+    directive.ngOnInit();
+
+    expect(pagination.supportedPageSizes).toEqual([21, 31, 41]);
+  });
+
+  it('should update preferences on page size change', () => {
+    directive.ngOnInit();
+
+    pagination.changePageSize.emit(
+      new PaginationModel({
+        maxItems: 100
+      })
+    );
+    expect(preferences.paginationSize).toBe(100);
   });
 });

src/app/guards/auth.guard.spec.ts

@@ -24,9 +24,50 @@
  */
 
 import { AppAuthGuard } from './auth.guard';
+import { TestBed } from '@angular/core/testing';
+import { AppTestingModule } from '../testing/app-testing.module';
+import { AuthenticationService } from '@alfresco/adf-core';
+import { RouterTestingModule } from '@angular/router/testing';
+import { Router } from '@angular/router';
 
 describe('AppAuthGuard', () => {
-  it('should be defined', () => {
+  let auth: AuthenticationService;
-    expect(AppAuthGuard).toBeDefined();
+  let guard: AppAuthGuard;
+  let router: Router;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [AppTestingModule, RouterTestingModule],
+      providers: [AppAuthGuard]
+    });
+
+    auth = TestBed.get(AuthenticationService);
+    guard = TestBed.get(AppAuthGuard);
+    router = TestBed.get(Router);
+
+    spyOn(router, 'navigateByUrl').and.stub();
+  });
+
+  it('should evaluate to [true] if logged in already', () => {
+    spyOn(auth, 'isEcmLoggedIn').and.returnValue(true);
+
+    expect(guard.checkLogin(null)).toBe(true);
+  });
+
+  it('should navigate to login with the redirect url', () => {
+    spyOn(auth, 'isEcmLoggedIn').and.returnValue(false);
+    expect(guard.checkLogin('test/url')).toBe(false);
+    expect(router.navigateByUrl).toHaveBeenCalledWith(
+      '/login?redirectUrl=test/url'
+    );
+  });
+
+  it('should evaluate to [false] with OAuth enabled', () => {
+    spyOn(auth, 'isEcmLoggedIn').and.returnValue(false);
+    spyOn(auth, 'isOauth').and.returnValue(true);
+    spyOn(guard, 'isOAuthWithoutSilentLogin').and.returnValue(false);
+
+    expect(guard.checkLogin(null)).toBe(false);
+    expect(router.navigateByUrl).not.toHaveBeenCalled();
   });
 });

src/app/guards/shared.guard.ts

@@ -27,24 +27,24 @@ import { Injectable } from '@angular/core';
 import { CanActivate } from '@angular/router';
 import { Observable } from 'rxjs';
 import { ActivatedRouteSnapshot } from '@angular/router';
-import { tap, map, filter, take } from 'rxjs/operators';
-import { Router } from '@angular/router';
 import { Store } from '@ngrx/store';
 import { AppStore } from '../store/states/app.state';
-import { GetRepositoryStatusAction } from '../store/actions';
+import { isQuickShareEnabled } from '../store/selectors/app.selectors';
-import { repositoryStatus } from '../store/selectors/app.selectors';
 
 @Injectable({
   providedIn: 'root'
 })
 export class AppSharedRuleGuard implements CanActivate {
-  constructor(private store: Store<AppStore>, private router: Router) {}
+  isQuickShareEnabled$: Observable<boolean>;
+
+  constructor(store: Store<AppStore>) {
+    this.isQuickShareEnabled$ = store.select(isQuickShareEnabled);
+  }
 
   canActivate(
     route: ActivatedRouteSnapshot
   ): Observable<boolean> | Promise<boolean> | boolean {
-    this.init();
+    return this.isQuickShareEnabled$;
-    return this.wait();
   }
 
   canActivateChild(
@@ -52,28 +52,4 @@ export class AppSharedRuleGuard implements CanActivate {
   ): Observable<boolean> | Promise<boolean> | boolean {
     return this.canActivate(route);
   }
-
-  wait(): Observable<boolean> {
-    return this.store.select(repositoryStatus).pipe(
-      map(state => state.isQuickShareEnabled),
-      filter(value => value !== null),
-      tap(value => {
-        if (value === false) {
-          this.router.navigate(['']);
-        }
-      }),
-      take(1)
-    );
-  }
-
-  init() {
-    this.store
-      .select(repositoryStatus)
-      .pipe(take(1))
-      .subscribe(state => {
-        if (state.isQuickShareEnabled === null) {
-          this.store.dispatch(new GetRepositoryStatusAction());
-        }
-      });
-  }
 }

src/app/services/app.service.ts

@@ -23,37 +23,23 @@
  * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { Effect, Actions, ofType } from '@ngrx/effects';
 import { Injectable } from '@angular/core';
-import { DiscoveryEntry } from 'alfresco-js-api';
+import { AuthenticationService } from '@alfresco/adf-core';
-import { map, mergeMap, catchError } from 'rxjs/operators';
+import { Observable, of } from 'rxjs';
-import { of } from 'rxjs';
+import { take } from 'rxjs/operators';
-import {
-  GET_REPOSITORY_STATUS,
-  GetRepositoryStatusAction
-} from '../actions/repository.actions';
-import { ContentApiService } from '../../services/content-api.service';
-import { SetRepositoryStatusAction } from '../actions';
 
-@Injectable()
+@Injectable({
-export class RepositoryEffects {
+  providedIn: 'root'
-  constructor(
+})
-    private actions$: Actions,
+export class AppService {
-    private contentApi: ContentApiService
+  constructor(private auth: AuthenticationService) {}
-  ) {}
 
-  @Effect()
+  waitForAuth(): Observable<any> {
-  getRepositoryStatus$ = this.actions$.pipe(
+    const isLoggedIn = this.auth.isLoggedIn();
-    ofType<GetRepositoryStatusAction>(GET_REPOSITORY_STATUS),
+    if (isLoggedIn) {
-    mergeMap(() => {
+      return of(true);
-      return this.contentApi.getRepositoryInformation().pipe(
+    } else {
-        map(
+      return this.auth.onLogin.pipe(take(1));
-          (response: DiscoveryEntry) =>
+    }
-            new SetRepositoryStatusAction(response.entry.repository.status)
+  }
-        ),
-        // needs proper error handling
-        catchError(() => of({ type: 'noop' }))
-      );
-    })
-  );
 }

src/app/services/repository-status.resolver.ts

@@ -1,70 +0,0 @@
-/*!
- * @license
- * Alfresco Example Content Application
- *
- * Copyright (C) 2005 - 2018 Alfresco Software Limited
- *
- * This file is part of the Alfresco Example Content Application.
- * If the software was purchased under a paid Alfresco license, the terms of
- * the paid license agreement will prevail.  Otherwise, the software is
- * provided under the following open source license terms:
- *
- * The Alfresco Example Content Application is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * The Alfresco Example Content Application is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- */
-
-import { Store } from '@ngrx/store';
-import { Injectable } from '@angular/core';
-import { Resolve } from '@angular/router';
-import { Observable } from 'rxjs';
-import { AppStore } from '../store/states/app.state';
-import { RepositoryState } from '../store/states';
-import { repositoryStatus } from '../store/selectors/app.selectors';
-import { filter, take } from 'rxjs/operators';
-import { GetRepositoryStatusAction } from '../store/actions';
-
-@Injectable({
-  providedIn: 'root'
-})
-export class RepositoryStatusResolver implements Resolve<RepositoryState> {
-  constructor(private store: Store<AppStore>) {}
-
-  resolve(): Observable<RepositoryState> {
-    this.init();
-    return this.wait();
-  }
-
-  wait(): Observable<any> {
-    return this.store.select(repositoryStatus).pipe(
-      filter(state => this.hasValuesSet(state)),
-      take(1)
-    );
-  }
-
-  init() {
-    this.store
-      .select(repositoryStatus)
-      .pipe(take(1))
-      .subscribe(state => {
-        if (!this.hasValuesSet(state)) {
-          this.store.dispatch(new GetRepositoryStatusAction());
-        }
-      });
-  }
-
-  private hasValuesSet(object): boolean {
-    return Object.keys(object)
-      .map(key => object[key])
-      .every(value => value !== null);
-  }
-}

src/app/store/actions/repository.actions.ts

@@ -24,15 +24,12 @@
  */
 
 import { Action } from '@ngrx/store';
+import { RepositoryState } from '../states';
 
 export const SET_REPOSITORY_STATUS = 'SET_REPOSITORY_STATUS';
 export const GET_REPOSITORY_STATUS = 'GET_REPOSITORY_STATUS';
 
 export class SetRepositoryStatusAction implements Action {
   readonly type = SET_REPOSITORY_STATUS;
-  constructor(public payload: any) {}
+  constructor(public payload: RepositoryState) {}
-}
-
-export class GetRepositoryStatusAction implements Action {
-  readonly type = GET_REPOSITORY_STATUS;
 }

src/app/store/app-store.module.ts

@@ -42,8 +42,7 @@ import {
   LibraryEffects,
   UploadEffects,
   FavoriteEffects,
-  ModalsEffects,
+  ModalsEffects
-  RepositoryEffects
 } from './effects';
 
 @NgModule({
@@ -61,8 +60,7 @@ import {
       LibraryEffects,
       UploadEffects,
       FavoriteEffects,
-      ModalsEffects,
+      ModalsEffects
-      RepositoryEffects
     ]),
     !environment.production
       ? StoreDevtoolsModule.instrument({ maxAge: 25 })

src/app/store/effects.ts

@@ -34,4 +34,3 @@ export * from './effects/search.effects';
 export * from './effects/library.effects';
 export * from './effects/upload.effects';
 export * from './effects/modals.effects';
-export * from './effects/repository.effects';

src/app/store/selectors/app.selectors.ts

@@ -63,6 +63,10 @@ export const repositoryStatus = createSelector(
   selectApp,
   state => state.repository
 );
+export const isQuickShareEnabled = createSelector(
+  repositoryStatus,
+  status => status.isQuickShareEnabled
+);
 
 export const ruleContext = createSelector(
   appSelection,

src/app/store/states/app.state.ts

@@ -68,7 +68,7 @@ export const INITIAL_APP_STATE: AppState = {
   infoDrawerOpened: false,
   documentDisplayMode: 'list',
   repository: {
-    isQuickShareEnabled: null
+    isQuickShareEnabled: false
   }
 };
 

src/app/testing/app-testing.module.ts

@@ -80,6 +80,13 @@ import { MaterialModule } from '../material.module';
         },
         getRedirect(): string {
           return null;
+        },
+        setRedirect() {},
+        isOauth(): boolean {
+          return false;
+        },
+        isOAuthWithoutSilentLogin(): boolean {
+          return false;
         }
       }
     },