Merge pull request #3313 from cloudfoundry-incubator/fix-list-loading…

…-indicators

Fixes double requests when single cf connected for lists with cf filter

src/frontend/app/shared/components/list/data-sources-controllers/list-data-source.ts

@@ -1,4 +1,5 @@
 import { DataSource } from '@angular/cdk/table';
+import { SortDirection } from '@angular/material';
 import { Store } from '@ngrx/store';
 import { schema } from 'normalizr';
 import {
@@ -11,8 +12,9 @@ import {
   Subscription,
 } from 'rxjs';
 import { tag } from 'rxjs-spy/operators';
-import { first, publishReplay, refCount, tap, distinctUntilChanged, map, filter } from 'rxjs/operators';
+import { distinctUntilChanged, filter, first, map, publishReplay, refCount, tap } from 'rxjs/operators';
 
+import { ListFilter, ListSort } from '../../../../store/actions/list.actions';
 import { MetricsAction } from '../../../../store/actions/metrics.actions';
 import { SetResultCount } from '../../../../store/actions/pagination.actions';
 import { AppState } from '../../../../store/app-state';
@@ -29,8 +31,6 @@ import {
 } from './list-data-source-types';
 import { getDataFunctionList } from './local-filtering-sorting';
 import { LocalListController } from './local-list-controller';
-import { SortDirection } from '@angular/material';
-import { ListSort, ListFilter } from '../../../../store/actions/list.actions';
 
 
 export class DataFunctionDefinition {

src/frontend/app/shared/components/list/list-types/app/cf-app-config.service.ts

@@ -1,6 +1,8 @@
 import { DatePipe } from '@angular/common';
 import { Injectable } from '@angular/core';
 import { Store } from '@ngrx/store';
+import { Observable } from 'rxjs';
+import { filter, first, map, switchMap } from 'rxjs/operators';
 
 import { UtilsService } from '../../../../../core/utils.service';
 import { ListView } from '../../../../../store/actions/list.actions';
@@ -23,6 +25,7 @@ import { TableCellAppStatusComponent } from './table-cell-app-status/table-cell-
 export class CfAppConfigService extends ListConfig<APIResource> implements IListConfig<APIResource> {
 
   multiFilterConfigs: IListMultiFilterConfig[];
+  initialised$: Observable<boolean>;
 
   constructor(
     private datePipe: DatePipe,
@@ -32,13 +35,25 @@ export class CfAppConfigService extends ListConfig<APIResource> implements IList
   ) {
     super();
 
-    this.appsDataSource = new CfAppsDataSource(this.store, this);
+    // Apply the initial cf guid to the data source. Normally this is done via applying the selection to the filter... however this is too
+    // late for maxedResult world
+    this.initialised$ = this.cfOrgSpaceService.cf.loading$.pipe(
+      filter(isLoading => !isLoading),
+      switchMap(() => this.cfOrgSpaceService.cf.list$),
+      first(),
+      map(cfs => {
+        const cfGuid = cfs.length === 1 ? cfs[0].guid : null;
+        this.appsDataSource = new CfAppsDataSource(this.store, this, undefined, undefined, undefined, cfGuid);
+        return true;
+      })
+    );
 
     this.multiFilterConfigs = [
       createCfOrgSpaceFilterConfig('cf', 'Cloud Foundry', this.cfOrgSpaceService.cf),
       createCfOrgSpaceFilterConfig('org', 'Organization', this.cfOrgSpaceService.org),
       createCfOrgSpaceFilterConfig('space', 'Space', this.cfOrgSpaceService.space),
     ];
+
   }
   appsDataSource: CfAppsDataSource;
   columns: Array<ITableColumn<APIResource>> = [
@@ -117,6 +132,6 @@ export class CfAppConfigService extends ListConfig<APIResource> implements IList
   getColumns = () => this.columns;
   getDataSource = () => this.appsDataSource;
   getMultiFiltersConfigs = () => this.multiFilterConfigs;
-  getInitialised = () => this.appsDataSource.initialised$;
+  getInitialised = () => this.initialised$;
 
 }

src/frontend/app/shared/components/list/list-types/app/cf-apps-data-source.ts

@@ -1,7 +1,7 @@
 import { Store } from '@ngrx/store';
-import { Observable, Subscription } from 'rxjs';
+import { Subscription } from 'rxjs';
 import { tag } from 'rxjs-spy/operators/tag';
-import { debounceTime, distinctUntilChanged, map, withLatestFrom, filter, switchMap } from 'rxjs/operators';
+import { debounceTime, distinctUntilChanged, filter, map, switchMap, withLatestFrom } from 'rxjs/operators';
 
 import { DispatchSequencer, DispatchSequencerAction } from '../../../../../core/dispatch-sequencer';
 import { cfOrgSpaceFilter, getRowMetadata } from '../../../../../features/cloud-foundry/cf.helpers';
@@ -17,7 +17,6 @@ import {
   spaceSchemaKey,
 } from '../../../../../store/helpers/entity-factory';
 import { createEntityRelationKey } from '../../../../../store/helpers/entity-relations/entity-relations.types';
-import { selectPaginationState } from '../../../../../store/selectors/pagination.selectors';
 import { APIResource } from '../../../../../store/types/api.types';
 import { PaginationParam } from '../../../../../store/types/pagination.types';
 import { createCfOrSpaceMultipleFilterFn } from '../../../../data-services/cf-org-space-service.service';
@@ -38,7 +37,6 @@ export class CfAppsDataSource extends ListDataSource<APIResource> {
   public static paginationKey = 'applicationWall';
   private subs: Subscription[];
   public action: GetAllApplications;
-  public initialised$: Observable<boolean>;
 
 
   constructor(
@@ -47,9 +45,11 @@ export class CfAppsDataSource extends ListDataSource<APIResource> {
     transformEntities?: any[],
     paginationKey = CfAppsDataSource.paginationKey,
     seedPaginationKey = CfAppsDataSource.paginationKey,
+    startingCfGuid?: string
   ) {
     const syncNeeded = paginationKey !== seedPaginationKey;
     const action = createGetAllAppAction(paginationKey);
+    action.endpointGuid = startingCfGuid;
 
     const dispatchSequencer = new DispatchSequencer(store);
 
@@ -78,17 +78,6 @@ export class CfAppsDataSource extends ListDataSource<APIResource> {
       destroy: () => this.subs.forEach(sub => sub.unsubscribe())
     });
 
-    // Reapply the cf guid to the action. Normally this is done via reapplying the selection to the filter... however this is too slow
-    // for maxedResult world
-    this.initialised$ = store.select(selectPaginationState(action.entityKey, action.paginationKey)).pipe(
-      map(pagination => {
-        if (pagination && pagination.clientPagination) {
-          action.endpointGuid = pagination.clientPagination.filter.items.cf;
-        }
-        return true;
-      })
-    );
-
     this.action = action;
 
     const statsSub = this.maxedResults$.pipe(

src/frontend/app/shared/components/list/list.component.html

@@ -97,15 +97,15 @@
             </button>
           </div>
           <button id="app-list-refresh-button" mat-icon-button *ngIf="dataSource.refresh" [disabled]="(dataSource.isLoadingPage$ | async) || (isAddingOrSelecting$ | async)" (click)="refresh()">
-            <mat-icon class="refresh-icon" [ngClass]="{refreshing: (isRefreshing$ | async)}" aria-label="Refresh list data">refresh</mat-icon>
+            <mat-icon class="refresh-icon" [ngClass]="{refreshing: isRefreshing}" aria-label="Refresh list data">refresh</mat-icon>
           </button>
         </div>
 
       </mat-card>
     </div>
     <div class="refresh-button__no-header" *ngIf="dataSource.refresh && !(hasControls$ | async) && (!(hasRowsOrIsFiltering$ | async) && noEntries)">
       <button id="app-list-refresh-button" mat-mini-fab *ngIf="!(isAddingOrSelecting$ | async)" [disabled]="dataSource.isLoadingPage$ | async" (click)="refresh()">
-        <mat-icon class="refresh-icon" [ngClass]="{refreshing: (isRefreshing$ | async)}" aria-label="Refresh list data">refresh</mat-icon>
+        <mat-icon class="refresh-icon" [ngClass]="{refreshing: isRefreshing}" aria-label="Refresh list data">refresh</mat-icon>
       </button>
     </div>
     <div class="list-component__body">

src/frontend/app/shared/components/list/list.component.ts

@@ -18,16 +18,17 @@ import { MatPaginator, PageEvent, SortDirection } from '@angular/material';
 import { Store } from '@ngrx/store';
 import { schema as normalizrSchema } from 'normalizr';
 import {
+  asapScheduler,
   BehaviorSubject,
   combineLatest as observableCombineLatest,
+  isObservable,
   Observable,
   of as observableOf,
-  Subscription,
   queueScheduler,
-  isObservable,
-  asapScheduler,
+  Subscription,
 } from 'rxjs';
 import {
+  combineLatest,
   debounceTime,
   distinctUntilChanged,
   filter,
@@ -37,18 +38,16 @@ import {
   publishReplay,
   refCount,
   startWith,
-  switchMap,
+  subscribeOn,
   takeWhile,
   tap,
-  withLatestFrom,
-  combineLatest,
   throttleTime,
-  subscribeOn,
 } from 'rxjs/operators';
 
 import { ListFilter, ListPagination, ListSort, SetListViewAction } from '../../../store/actions/list.actions';
 import { AppState } from '../../../store/app-state';
 import { entityFactory } from '../../../store/helpers/entity-factory';
+import { ActionState } from '../../../store/reducers/api-request-reducer/types';
 import { getListStateObservables } from '../../../store/reducers/list.reducer';
 import { EntityMonitor } from '../../monitors/entity-monitor';
 import { ListView } from './../../../store/actions/list.actions';
@@ -59,16 +58,14 @@ import {
   defaultPaginationPageSizeOptionsCards,
   defaultPaginationPageSizeOptionsTable,
   IGlobalListAction,
-  IListAction,
   IListConfig,
-  IListMultiFilterConfig,
   IMultiListAction,
   IOptionalAction,
   ListConfig,
   ListViewTypes,
   MultiFilterManager,
 } from './list.component.types';
-import { RequestInfoState, ActionState } from '../../../store/reducers/api-request-reducer/types';
+import { safeUnsubscribe } from '../../../core/utils.service';
 
 
 @Component({
@@ -189,7 +186,7 @@ export class ListComponent<T> implements OnInit, OnChanges, OnDestroy, AfterView
   isFiltering$: Observable<boolean>;
   noRowsNotFiltering$: Observable<boolean>;
   showProgressBar$: Observable<boolean>;
-  isRefreshing$: Observable<boolean>;
+  public isRefreshing = false;
 
 
 
@@ -514,39 +511,28 @@ export class ListComponent<T> implements OnInit, OnChanges, OnDestroy, AfterView
     this.showProgressBar$ = this.dataSource.isLoadingPage$.pipe(
       startWith(true),
       combineLatest(canShowLoading$),
-      map(([loading, canShowLoading]) => loading && canShowLoading),
+      map(([loading, canShowLoading]) => loading && canShowLoading && !this.isRefreshing),
       distinctUntilChanged(),
       throttleTime(100, queueScheduler, { leading: true, trailing: true }),
     );
 
-    this.isRefreshing$ = this.dataSource.isLoadingPage$.pipe(
-      withLatestFrom(canShowLoading$, this.showProgressBar$),
-      map(([loading, canShowLoading, showProgressBar]) => !canShowLoading && loading && !showProgressBar),
-      distinctUntilChanged()
-    );
   }
 
   ngAfterViewInit() {
     this.cd.detectChanges();
   }
 
   ngOnDestroy() {
-    this.multiFilterWidgetObservables.forEach(sub => sub.unsubscribe());
-    if (this.paginationWidgetToStore) {
-      this.paginationWidgetToStore.unsubscribe();
-    }
-    if (this.filterWidgetToStore) {
-      this.filterWidgetToStore.unsubscribe();
-    }
-    if (this.uberSub) {
-      this.uberSub.unsubscribe();
-    }
+    safeUnsubscribe(
+      ...this.multiFilterWidgetObservables,
+      this.paginationWidgetToStore,
+      this.filterWidgetToStore,
+      this.uberSub,
+      this.multiFilterChangesSub
+    );
     if (this.dataSource) {
       this.dataSource.destroy();
     }
-    if (this.multiFilterChangesSub) {
-      this.multiFilterChangesSub.unsubscribe();
-    }
     this.pendingActions.forEach(sub => sub.unsubscribe());
   }
 
@@ -605,6 +591,7 @@ export class ListComponent<T> implements OnInit, OnChanges, OnDestroy, AfterView
 
   public refresh() {
     if (this.dataSource.refresh) {
+      this.isRefreshing = true;
       this.dataSource.refresh();
       this.dataSource.isLoadingPage$.pipe(
         tap(isLoading => {
@@ -613,7 +600,7 @@ export class ListComponent<T> implements OnInit, OnChanges, OnDestroy, AfterView
           }
         }),
         takeWhile(isLoading => isLoading)
-      ).subscribe();
+      ).subscribe(null, null, () => this.isRefreshing = false);
     }
   }
 

src/frontend/app/shared/data-services/cf-org-space-service.service.ts

@@ -1,10 +1,22 @@
 import { Injectable, OnDestroy } from '@angular/core';
 import { Store } from '@ngrx/store';
 import { BehaviorSubject, combineLatest, Observable, of as observableOf, Subscription } from 'rxjs';
-import { distinctUntilChanged, filter, first, map, startWith, switchMap, tap, withLatestFrom } from 'rxjs/operators';
+import {
+  distinctUntilChanged,
+  filter,
+  first,
+  map,
+  publishReplay,
+  refCount,
+  startWith,
+  switchMap,
+  tap,
+  withLatestFrom,
+} from 'rxjs/operators';
 
 import { IOrganization, ISpace } from '../../core/cf-api.types';
 import { GetAllOrganizations } from '../../store/actions/organization.actions';
+import { ResetPagination, SetParams } from '../../store/actions/pagination.actions';
 import { AppState } from '../../store/app-state';
 import { entityFactory, organizationSchemaKey, spaceSchemaKey } from '../../store/helpers/entity-factory';
 import { createEntityRelationKey } from '../../store/helpers/entity-relations/entity-relations.types';
@@ -17,11 +29,10 @@ import { endpointsRegisteredEntitiesSelector } from '../../store/selectors/endpo
 import { selectPaginationState } from '../../store/selectors/pagination.selectors';
 import { APIResource } from '../../store/types/api.types';
 import { EndpointModel } from '../../store/types/endpoint.types';
-import { PaginationMonitorFactory } from '../monitors/pagination-monitor.factory';
+import { PaginatedAction, PaginationParam, QParam } from '../../store/types/pagination.types';
 import { ListPaginationMultiFilterChange } from '../components/list/data-sources-controllers/list-data-source-types';
-import { PaginationParam, QParam, PaginatedAction } from '../../store/types/pagination.types';
 import { valueOrCommonFalsy } from '../components/list/data-sources-controllers/list-pagination-controller';
-import { SetParams, ResetPagination } from '../../store/actions/pagination.actions';
+import { PaginationMonitorFactory } from '../monitors/pagination-monitor.factory';
 
 export function createCfOrgSpaceFilterConfig(key: string, label: string, cfOrgSpaceItem: CfOrgSpaceItem) {
   return {
@@ -200,10 +211,14 @@ export class CfOrgSpaceDataService implements OnDestroy {
   }
 
   private createCf() {
+    const list$ = this.store.select(endpointsRegisteredEntitiesSelector).pipe(
+      // Ensure we have endpoints
+      filter(endpoints => endpoints && !!Object.keys(endpoints).length),
+      publishReplay(1),
+      refCount(),
+    );
     this.cf = {
-      list$: this.store.select(endpointsRegisteredEntitiesSelector).pipe(
-        // Ensure we have endpoints
-        filter(endpoints => endpoints && !!Object.keys(endpoints).length),
+      list$: list$.pipe(
         // Filter out non-cf endpoints
         map(endpoints => Object.values(endpoints).filter(e => e.cnsi_type === 'cf')),
         // Ensure we have at least one connected cf
@@ -218,9 +233,11 @@ export class CfOrgSpaceDataService implements OnDestroy {
         first(),
         map((endpoints: EndpointModel[]) => {
           return Object.values(endpoints).sort((a: EndpointModel, b: EndpointModel) => a.name.localeCompare(b.name));
-        })
+        }),
+      ),
+      loading$: list$.pipe(
+        map(cfs => !cfs)
       ),
-      loading$: this.allOrgsLoading$,
       select: new BehaviorSubject(undefined)
     };
   }

src/frontend/app/store/actions/application.actions.ts

@@ -6,8 +6,7 @@ import { EntityInlineParentAction } from '../helpers/entity-relations/entity-rel
 import { pick } from '../helpers/reducer.helper';
 import { ActionMergeFunction } from '../types/api.types';
 import { PaginatedAction, PaginationParam } from '../types/pagination.types';
-import { ICFAction } from '../types/request.types';
-import { CFStartAction } from '../types/request.types';
+import { CFStartAction, ICFAction } from '../types/request.types';
 import { AppMetadataTypes } from './app-metadata.actions';
 
 export const GET_ALL = '[Application] Get all';

src/test-e2e/application/application-delete-e2e.spec.ts

@@ -1,6 +1,5 @@
 import { ApplicationsPage } from '../applications/applications.po';
 import { e2e } from '../e2e';
-import { CFHelpers } from '../helpers/cf-helpers';
 import { ConsoleUserType } from '../helpers/e2e-helpers';
 import { SideNavigation, SideNavMenuItem } from '../po/side-nav.po';
 import { ApplicationE2eHelper } from './application-e2e-helpers';
@@ -13,7 +12,6 @@ describe('Application Delete', function () {
   let appWall: ApplicationsPage;
   let applicationE2eHelper: ApplicationE2eHelper;
   let cfGuid, app;
-  let cfHelper: CFHelpers;
   let testAppName;
 
   beforeAll(() => {
@@ -26,7 +24,6 @@ describe('Application Delete', function () {
       .connectAllEndpoints(ConsoleUserType.admin)
       .getInfo(ConsoleUserType.admin);
     applicationE2eHelper = new ApplicationE2eHelper(setup);
-    cfHelper = new CFHelpers(setup);
   });
 
   beforeEach(() => nav.goto(SideNavMenuItem.Applications));
@@ -76,7 +73,7 @@ describe('Application Delete', function () {
       expect(appWall.isActivePage()).toBeTruthy();
 
       // We created the app after the wall loaded, so refresh to make sure app wall shows the new app
-      appWall.appList.refresh();
+      appWall.appList.header.refresh();
 
       appWall.appList.header.setSearchText(testAppName);
       expect(appWall.appList.getTotalResults()).toBe(1);