Skip to content

Commit

Permalink
Merge pull request #3512 from anotherchrisberry/no-scope-refresh
Browse files Browse the repository at this point in the history
refactor(core/application): make $scope nullable for onRefresh regist…
  • Loading branch information
Justin Reynolds committed Apr 11, 2017
2 parents 520db3f + fd45e62 commit 808eff3
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 34 deletions.
39 changes: 26 additions & 13 deletions app/scripts/modules/core/application/application.model.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {ILogService, IPromise, IQService, IScope} from 'angular';
import {map, union, uniq} from 'lodash';
import {Subject, Subscription} from 'rxjs';

import {ApplicationDataSource} from './service/applicationDataSource';
import {Subject} from 'rxjs';
import {ICluster} from '../domain/ICluster';

export class Application {
Expand Down Expand Up @@ -77,7 +79,7 @@ export class Application {

private refreshFailureStream: Subject<any> = new Subject();

constructor(private applicationName: string, private scheduler: any, private $q: ng.IQService, private $log: ng.ILogService) {}
constructor(private applicationName: string, private scheduler: any, private $q: IQService, private $log: ILogService) {}

/**
* Returns a data source based on its key. Data sources can be accessed on the application directly via the key,
Expand All @@ -95,7 +97,7 @@ export class Application {
* @returns {IPromise<void>} a promise that resolves when the application finishes loading, rejecting with an error if
* one of the data sources fails to refresh
*/
public refresh(forceRefresh?: boolean): ng.IPromise<any> {
public refresh(forceRefresh?: boolean): IPromise<any> {
// refresh hidden data sources but do not consider their results when determining when the refresh completes
this.dataSources.filter(ds => !ds.visible).forEach(ds => ds.refresh(forceRefresh));
return this.$q.all(
Expand All @@ -111,10 +113,10 @@ export class Application {
/**
* A promise that resolves immediately if all data sources are ready (i.e. loaded), or once all data sources have
* loaded
* @returns {IPromise<any>|IPromise<any[]>|IPromise<{}>|IPromise<T>} the return value is a promise, but its value is
* @returns {IPromise<any>} the return value is a promise, but its value is
* not useful - it's only useful to watch the promise itself
*/
public ready(): ng.IPromise<any> {
public ready(): IPromise<any> {
return this.$q.all(
this.dataSources
.filter(ds => ds.onLoad !== undefined && ds.visible)
Expand All @@ -124,26 +126,37 @@ export class Application {
/**
* Used to subscribe to the application's refresh cycle. Will automatically be disposed when the $scope is destroyed.
* @param $scope the $scope that will manage the lifecycle of the subscription
* If you pass in null for the $scope, you are responsible for unsubscribing when your component unmounts.
* @param method the method to call when the refresh completes
* @param failureMethod a method to call if the refresh fails
* @return a method to call to unsubscribe
*/
public onRefresh($scope: ng.IScope, method: any, failureMethod: any): void {
const success = this.refreshStream.subscribe(method);
$scope.$on('$destroy', () => success.unsubscribe());
public onRefresh($scope: IScope, method: any, failureMethod: any): () => void {
const success: Subscription = this.refreshStream.subscribe(method);
let failure: Subscription = null;
if (failureMethod) {
const failure = this.refreshFailureStream.subscribe(failureMethod);
$scope.$on('$destroy', () => failure.unsubscribe());
failure = this.refreshFailureStream.subscribe(failureMethod);
}
const unsubscribe = () => {
success.unsubscribe();
if (failure) {
failure.unsubscribe();
}
};
if ($scope) {
$scope.$on('$destroy', () => unsubscribe());
}
return unsubscribe;
}

/**
* This is really only used by the ApplicationController - it manages the refresh cycle for the overall application
* and halts refresh when switching applications or navigating to a non-application view
* @param scope
* @param $scope
*/
public enableAutoRefresh(scope: ng.IScope): void {
public enableAutoRefresh($scope: IScope): void {
const dataLoader = this.scheduler.subscribe(() => this.refresh());
scope.$on('$destroy', () => {
$scope.$on('$destroy', () => {
dataLoader.unsubscribe();
this.scheduler.unsubscribe();
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {Subject} from 'rxjs';
import {get} from 'lodash';
import {ILogService, IPromise, IQService, IScope} from 'angular';
import {Subject, Subscription} from 'rxjs';

import {Application} from '../application.model';
import {IEntityTags} from 'core/domain/IEntityTags';
import {get} from 'lodash';

export class DataSourceConfig {

Expand Down Expand Up @@ -94,7 +96,7 @@ export class DataSourceConfig {
* It does *not* automatically populate the "data" field of the data source - that is the responsibility of the
* "onLoad" method.
*/
public loader: {(fn: any): ng.IPromise<any>};
public loader: {(fn: any): IPromise<any>};

/**
* A method that is called when the "loader" method resolves. The method must return a promise. If the "loader"
Expand All @@ -104,7 +106,7 @@ export class DataSourceConfig {
* If the onLoad method resolves with a null value, the result will be discarded and the data source's "data" field
* will remain unchanged.
*/
public onLoad: {(fn: any): ng.IPromise<any>};
public onLoad: {(fn: any): IPromise<any>};

/**
* (Optional) A method that is called after the "onLoad" method resolves. The data source's data will be populated
Expand Down Expand Up @@ -274,7 +276,7 @@ export class ApplicationDataSource {
/**
* See DataSourceConfig#onLoad
*/
public onLoad: {(application: Application, fn: any): ng.IPromise<any>};
public onLoad: {(application: Application, fn: any): IPromise<any>};

/**
* See DataSourceConfig#afterLoad
Expand All @@ -284,7 +286,7 @@ export class ApplicationDataSource {
/**
* See DataSourceConfig#loader
*/
public loader: {(fn: any): ng.IPromise<any>};
public loader: {(fn: any): IPromise<any>};

private refreshStream: Subject<any> = new Subject();

Expand All @@ -300,7 +302,7 @@ export class ApplicationDataSource {
}
}

constructor(config: DataSourceConfig, private application: Application, private $q?: ng.IQService, private $log?: ng.ILogService, private $filter?: any) {
constructor(config: DataSourceConfig, private application: Application, private $q?: IQService, private $log?: ILogService, private $filter?: any) {
Object.assign(this, config);
if (!config.sref && config.visible !== false) {
this.sref = '.insight.' + config.key;
Expand All @@ -318,34 +320,56 @@ export class ApplicationDataSource {
/**
* A method that allows another method to be called the next time the data source refreshes
*
* @param $scope the controller scope of the calling method. If the $scope is destroyed, the subscription is disposed
* @param $scope the controller scope of the calling method. If the $scope is destroyed, the subscription is disposed.
* If you pass in null for the $scope, you are responsible for unsubscribing when your component unmounts.
* @param method the method to call the next time the data source refreshes
* @param failureMethod (optional) a method to call if the data source refresh fails
* @return a method to call to unsubscribe
*/
public onNextRefresh($scope: ng.IScope, method: any, failureMethod?: any): void {
const success = this.refreshStream.take(1).subscribe(method);
$scope.$on('$destroy', () => success.unsubscribe());
public onNextRefresh($scope: IScope, method: any, failureMethod?: any): () => void {
const success: Subscription = this.refreshStream.take(1).subscribe(method);
let failure: Subscription = null;
if (failureMethod) {
const failure = this.refreshFailureStream.take(1).subscribe(failureMethod);
$scope.$on('$destroy', () => failure.unsubscribe());
failure = this.refreshFailureStream.take(1).subscribe(failureMethod);
}
const unsubscribe = () => {
success.unsubscribe();
if (failure) {
failure.unsubscribe();
}
};
if ($scope) {
$scope.$on('$destroy', () => unsubscribe());
}
return unsubscribe;
}

/**
* A method that allows another method to be called the whenever the data source refreshes. The subscription will be
* automatically disposed when the $scope is destroyed.
*
* @param $scope the controller scope of the calling method. If the $scope is destroyed, the subscription is disposed
* @param $scope the controller scope of the calling method. If the $scope is destroyed, the subscription is disposed.
* If you pass in null for the $scope, you are responsible for unsubscribing when your component unmounts.
* @param method the method to call the next time the data source refreshes
* @param failureMethod (optional) a method to call if the data source refresh fails
* @return a method to call to unsubscribe
*/
public onRefresh($scope: ng.IScope, method: any, failureMethod?: any): void {
const success = this.refreshStream.subscribe(method);
$scope.$on('$destroy', () => success.unsubscribe());
public onRefresh($scope: IScope, method: any, failureMethod?: any): () => void {
const success: Subscription = this.refreshStream.subscribe(method);
let failure: Subscription = null;
if (failureMethod) {
const failure = this.refreshFailureStream.subscribe(failureMethod);
$scope.$on('$destroy', () => failure.unsubscribe());
failure = this.refreshFailureStream.subscribe(failureMethod);
}
const unsubscribe = () => {
success.unsubscribe();
if (failure) {
failure.unsubscribe();
}
};
if ($scope) {
$scope.$on('$destroy', () => unsubscribe());
}
return unsubscribe;
}

/**
Expand All @@ -360,7 +384,7 @@ export class ApplicationDataSource {
*
* @returns {IPromise<T>}
*/
public ready(): ng.IPromise<any> {
public ready(): IPromise<any> {
const deferred = this.$q.defer();
if (this.disabled || this.loaded || (this.lazy && !this.active)) {
deferred.resolve();
Expand Down Expand Up @@ -396,7 +420,7 @@ export class ApplicationDataSource {
* @param forceRefresh
* @returns {any}
*/
public refresh(forceRefresh?: boolean): ng.IPromise<any> {
public refresh(forceRefresh?: boolean): IPromise<any> {
if (!this.loader || this.disabled) {
this.data.length = 0;
this.loading = false;
Expand Down

0 comments on commit 808eff3

Please sign in to comment.