Skip to content

Commit ad9e678

Browse files
VanessaIniguezGVanessaIniguezG
committed
refactor: TT-106 Use the ngOnDestroy to destroy subscriptions running in the background when changing components and create test to validate the call to unsubscribe in the ngOnDestroy
1 parent 0e2cc72 commit ad9e678

File tree

6 files changed

+39
-148
lines changed

6 files changed

+39
-148
lines changed

‎src/app/modules/time-clock/components/entry-fields/entry-fields.component.spec.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Observable, of } from 'rxjs';
1+
import { Observable, of, Subscription } from 'rxjs';
22
import { LoadActiveEntry, EntryActionTypes, UpdateEntry } from './../../store/entry.actions';
33
import { ActivityManagementActionTypes } from './../../../activities-management/store/activity-management.actions';
44
import {waitForAsync, ComponentFixture, TestBed} from '@angular/core/testing';
@@ -16,7 +16,6 @@ import { formatDate } from '@angular/common';
1616
import { NgxMaterialTimepickerModule } from 'ngx-material-timepicker';
1717
import * as moment from 'moment';
1818
import { DATE_FORMAT_YEAR } from 'src/environments/environment';
19-
import { FeatureManagerService } from 'src/app/modules/shared/feature-toggles/feature-toggle-manager.service';
2019

2120
describe('EntryFieldsComponent', () => {
2221
type Merged = TechnologyState & ProjectState;
@@ -26,7 +25,6 @@ describe('EntryFieldsComponent', () => {
2625
let mockTechnologySelector;
2726
let mockProjectsSelector;
2827
let entryForm;
29-
let featureManagerService: FeatureManagerService;
3028
const actionSub: ActionsSubject = new ActionsSubject();
3129
const toastrServiceStub = {
3230
error: (message?: string, title?: string, override?: Partial<IndividualConfig>) => { },
@@ -117,7 +115,6 @@ describe('EntryFieldsComponent', () => {
117115
entryForm = TestBed.inject(FormBuilder);
118116
mockTechnologySelector = store.overrideSelector(allTechnologies, state.technologies);
119117
mockProjectsSelector = store.overrideSelector(getCustomerProjects, state.projects);
120-
// featureManagerService = TestBed.inject(FeatureManagerService);
121118
}));
122119

123120
beforeEach(() => {
@@ -153,16 +150,6 @@ describe('EntryFieldsComponent', () => {
153150
expect(component.selectedTechnologies).toEqual([]);
154151
});
155152

156-
// const exponentialGrowth = [true, false];
157-
// exponentialGrowth.map((toggleValue) => {
158-
// it(`when FeatureToggle is ${toggleValue} should return ${toggleValue}`, () => {
159-
// spyOn(featureManagerService, 'isToggleEnabled').and.returnValue(of(toggleValue));
160-
// const isFeatureToggleActivated: Promise<boolean> = component.isFeatureToggleActivated();
161-
// expect(featureManagerService.isToggleEnabled).toHaveBeenCalled();
162-
// isFeatureToggleActivated.then((value) => expect(value).toEqual(toggleValue));
163-
// });
164-
// });
165-
166153
it('displays error message when the date selected is in the future', () => {
167154
const mockEntry = { ...entry,
168155
start_date : moment().format(DATE_FORMAT_YEAR),
@@ -424,4 +411,19 @@ describe('EntryFieldsComponent', () => {
424411

425412
expect(component.selectedTechnologies).toBe(initialTechnologies);
426413
});
414+
415+
it('calls unsubscribe on ngDestroy', () => {
416+
component.loadActivitiesSubscription = new Subscription();
417+
component.loadActiveEntrySubscription = new Subscription();
418+
component.actionSetDateSubscription = new Subscription();
419+
spyOn(component.loadActivitiesSubscription, 'unsubscribe');
420+
spyOn(component.loadActiveEntrySubscription, 'unsubscribe');
421+
spyOn(component.actionSetDateSubscription, 'unsubscribe');
422+
423+
component.ngOnDestroy();
424+
425+
expect(component.loadActivitiesSubscription.unsubscribe).toHaveBeenCalled();
426+
expect(component.loadActiveEntrySubscription.unsubscribe).toHaveBeenCalled();
427+
expect(component.actionSetDateSubscription.unsubscribe).toHaveBeenCalled();
428+
});
427429
});

‎src/app/modules/time-clock/components/entry-fields/entry-fields.component.ts‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { Activity, NewEntry } from '../../../shared/models';
88
import { ProjectState } from '../../../customer-management/components/projects/components/store/project.reducer';
99
import { TechnologyState } from '../../../shared/store/technology.reducers';
1010
import { ActivityState, LoadActivities } from '../../../activities-management/store';
11-
import { FeatureManagerService } from 'src/app/modules/shared/feature-toggles/feature-toggle-manager.service';
1211
import * as entryActions from '../../store/entry.actions';
1312
import { get } from 'lodash';
1413
import * as moment from 'moment';
@@ -38,7 +37,6 @@ export class EntryFieldsComponent implements OnInit, OnDestroy {
3837
actionSetDateSubscription: Subscription;
3938

4039
constructor(
41-
private featureManagerService: FeatureManagerService,
4240
private formBuilder: FormBuilder,
4341
private store: Store<Merged>,
4442
private actionsSubject$: ActionsSubject,
@@ -175,17 +173,9 @@ export class EntryFieldsComponent implements OnInit, OnDestroy {
175173
this.store.dispatch(new entryActions.UpdateEntryRunning({ ...this.newData, technologies: $event }));
176174
}
177175

178-
179-
ngOnDestroy(): void {
180-
if (this.isFeatureToggleActivated()) {
181-
this.loadActivitiesSubscription.unsubscribe();
182-
this.loadActiveEntrySubscription.unsubscribe();
183-
this.actionSetDateSubscription.unsubscribe();
184-
}
185-
}
186-
187-
isFeatureToggleActivated() {
188-
return this.featureManagerService.isToggleEnabledForUser('exponential-growth').pipe(
189-
map((enabled) => enabled));
176+
ngOnDestroy(): void {
177+
this.loadActivitiesSubscription.unsubscribe();
178+
this.loadActiveEntrySubscription.unsubscribe();
179+
this.actionSetDateSubscription.unsubscribe();
190180
}
191181
}

‎src/app/modules/time-clock/components/project-list-hover/project-list-hover.component.spec.ts‎

Lines changed: 7 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,12 @@ import { getCustomerProjects } from '../../../customer-management/components/pro
1111
import { FilterProjectPipe } from '../../../shared/pipes';
1212
import { UpdateEntryRunning } from '../../store/entry.actions';
1313
import { ProjectListHoverComponent } from './project-list-hover.component';
14-
import { FeatureManagerService } from 'src/app/modules/shared/feature-toggles/feature-toggle-manager.service';
1514

1615
describe('ProjectListHoverComponent', () => {
1716
let component: ProjectListHoverComponent;
1817
let fixture: ComponentFixture<ProjectListHoverComponent>;
1918
let store: MockStore<ProjectState>;
2019
let mockProjectsSelector;
21-
let featureManagerService: FeatureManagerService;
2220
const toastrServiceStub = {
2321
error: (message?: string, title?: string, override?: Partial<IndividualConfig>) => { }
2422
};
@@ -58,7 +56,6 @@ describe('ProjectListHoverComponent', () => {
5856
fixture = TestBed.createComponent(ProjectListHoverComponent);
5957
component = fixture.componentInstance;
6058
fixture.detectChanges();
61-
featureManagerService = TestBed.inject(FeatureManagerService);
6259
});
6360

6461
it('should create', () => {
@@ -103,36 +100,19 @@ describe('ProjectListHoverComponent', () => {
103100

104101
it('calls unsubscribe on ngDestroy', () => {
105102
component.updateEntrySubscription = new Subscription();
103+
component.projectsSubscription = new Subscription();
104+
component.activeEntrySubscription = new Subscription();
106105
spyOn(component.updateEntrySubscription, 'unsubscribe');
106+
spyOn(component.projectsSubscription, 'unsubscribe');
107+
spyOn(component.activeEntrySubscription, 'unsubscribe');
107108

108109
component.ngOnDestroy();
109110

110111
expect(component.updateEntrySubscription.unsubscribe).toHaveBeenCalled();
112+
expect(component.projectsSubscription.unsubscribe).toHaveBeenCalled();
113+
expect(component.activeEntrySubscription.unsubscribe).toHaveBeenCalled();
111114
});
112115

113-
// it('calls projectsSubscribe unsubscribe on ngDestroy', () => {
114-
// component.projectsSubscription = new Subscription();
115-
// spyOn(component.projectsSubscribe, 'unsubscribe');
116-
117-
// component.ngOnDestroy();
118-
119-
// expect(component.projectsSubscribe.unsubscribe).toHaveBeenCalledTimes(1);
120-
// });
121-
122-
// fit('When Component is created, should call the feature toggle method', () => {
123-
// // component.exponentialGrowth = true;
124-
// component.projectsSubscribe = new Subscription();
125-
// component.activeEntrySubscribe = new Subscription();
126-
// spyOn(component.projectsSubscribe, 'unsubscribe');
127-
// spyOn(component.activeEntrySubscribe, 'unsubscribe');
128-
129-
// component.ngOnDestroy();
130-
131-
// expect(component.projectsSubscribe.unsubscribe).toHaveBeenCalled();
132-
// expect(component.activeEntrySubscribe.unsubscribe).toHaveBeenCalled();
133-
134-
// });
135-
136116
it('sets customer name and project name on setSelectedProject', () => {
137117
spyOn(component.projectsForm, 'setValue');
138118
component.activeEntry = { project_id : 'p1'};
@@ -141,61 +121,9 @@ describe('ProjectListHoverComponent', () => {
141121
component.setSelectedProject();
142122

143123
expect(component.projectsForm.setValue)
144-
.toHaveBeenCalledWith({ project_id: 'customer - xyz'});
124+
.toHaveBeenCalledWith({ project_id: 'customer - xyz'});
145125
});
146-
147-
var toggleValue = true;
148-
fit(`when FeatureToggle is ${toggleValue} should return ${toggleValue}`, () => {
149-
spyOn(featureManagerService, 'isToggleEnabledForUser').and.returnValue(of(toggleValue));
150-
component.projectsSubscription = new Subscription();
151-
component.activeEntrySubscription = new Subscription();
152-
153-
spyOn(component.projectsSubscription, 'unsubscribe');
154-
spyOn(component.activeEntrySubscription, 'unsubscribe');
155-
156-
component.ngOnDestroy();
157-
158-
expect(component.projectsSubscription.unsubscribe).toHaveBeenCalled();
159-
expect(component.activeEntrySubscription.unsubscribe).toHaveBeenCalled();
160-
});
161-
162-
toggleValue = false;
163-
fit(`when FeatureToggle is ${toggleValue} should return ${toggleValue}`, () => {
164-
spyOn(component, 'isFeatureToggleActivated').and.returnValue(of(toggleValue));
165-
component.projectsSubscription = new Subscription();
166-
component.activeEntrySubscription = new Subscription();
167-
168-
spyOn(component.projectsSubscription, 'unsubscribe');
169-
spyOn(component.activeEntrySubscription, 'unsubscribe');
170-
171-
component.ngOnDestroy();
172-
173-
expect(component.projectsSubscription.unsubscribe).not.toHaveBeenCalled();
174-
expect(component.activeEntrySubscription.unsubscribe).not.toHaveBeenCalled();
175-
});
176-
177-
// const exponentialGrowth = [true, false];
178-
// exponentialGrowth.map((toggleValue) => {
179-
// it(`when FeatureToggle is ${toggleValue} should return ${toggleValue}`, () => {
180-
// spyOn(featureManagerService, 'isToggleEnabled').and.returnValue(of(toggleValue));
181-
// const isFeatureToggleActivated: Promise<boolean> = component.isFeatureToggleActivated();
182-
// expect(featureManagerService.isToggleEnabled).toHaveBeenCalled();
183-
// isFeatureToggleActivated.then((value) => expect(value).toEqual(toggleValue));
184-
// });
185-
// });
186-
187-
// it('deleteProject, should dispatch DeleteProject action', () => {
188-
// component.projectsSubscription = new Subscription();
189-
// const subscription = spyOn(component.projectsSubscription, 'unsubscribe');
190126

191-
// component.idToDelete = project.id;
192-
// spyOn(store, 'dispatch');
193-
// component.deleteProject();
194-
// component.ngOnDestroy();
195-
// expect(subscription).toHaveBeenCalledTimes(1);
196-
// expect(store.dispatch).toHaveBeenCalledTimes(1);
197-
// expect(store.dispatch).toHaveBeenCalledWith(new DeleteProject(project.id));
198-
// });
199127
// TODO Fix this test since it is throwing this error
200128
// Expected spy dispatch to have been called with:
201129
// [CreateEntry({ payload: Object({ project_id: '1', start_date: '2020-07-27T22:30:26.743Z', timezone_offset: 300 }),
@@ -217,5 +145,4 @@ describe('ProjectListHoverComponent', () => {
217145
// })
218146
// );
219147
// });
220-
221148
});

‎src/app/modules/time-clock/components/project-list-hover/project-list-hover.component.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import * as entryActions from '../../store/entry.actions';
1111
import { getIsLoading, getProjects } from './../../../customer-management/components/projects/components/store/project.selectors';
1212
import { EntryActionTypes } from './../../store/entry.actions';
1313
import { getActiveTimeEntry } from './../../store/entry.selectors';
14-
import { FeatureManagerService } from 'src/app/modules/shared/feature-toggles/feature-toggle-manager.service';
1514
@Component({
1615
selector: 'app-project-list-hover',
1716
templateUrl: './project-list-hover.component.html',
@@ -28,8 +27,7 @@ export class ProjectListHoverComponent implements OnInit, OnDestroy {
2827
projectsSubscription: Subscription;
2928
activeEntrySubscription: Subscription;
3029

31-
constructor(private featureManagerService: FeatureManagerService,
32-
private formBuilder: FormBuilder, private store: Store<ProjectState>,
30+
constructor(private formBuilder: FormBuilder, private store: Store<ProjectState>,
3331
private actionsSubject$: ActionsSubject, private toastrService: ToastrService) {
3432
this.projectsForm = this.formBuilder.group({ project_id: null, });
3533
this.isLoading$ = this.store.pipe(delay(0), select(getIsLoading));
@@ -78,7 +76,7 @@ export class ProjectListHoverComponent implements OnInit, OnDestroy {
7876
if (project.id === this.activeEntry.project_id) {
7977
this.projectsForm.setValue(
8078
{ project_id: `${project.customer_name} - ${project.name}`, }
81-
);
79+
);
8280
}
8381
});
8482
}
@@ -110,15 +108,8 @@ export class ProjectListHoverComponent implements OnInit, OnDestroy {
110108
}
111109

112110
ngOnDestroy(): void {
113-
if(this.isFeatureToggleActivated()){
114-
this.projectsSubscription.unsubscribe();
115-
this.activeEntrySubscription.unsubscribe();
116-
}
111+
this.projectsSubscription.unsubscribe();
112+
this.activeEntrySubscription.unsubscribe();
117113
this.updateEntrySubscription.unsubscribe();
118114
}
119-
120-
isFeatureToggleActivated() {
121-
return this.featureManagerService.isToggleEnabledForUser('exponential-growth').pipe(
122-
map((enabled) => enabled === true ? true : false));
123-
}
124115
}

‎src/app/modules/time-clock/pages/time-clock.component.spec.ts‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@ import { AzureAdB2CService } from '../../login/services/azure.ad.b2c.service';
1212
import { ActionsSubject } from '@ngrx/store';
1313
import { EntryFieldsComponent } from '../components/entry-fields/entry-fields.component';
1414
import { ToastrService } from 'ngx-toastr';
15-
import { FeatureManagerService } from 'src/app/modules/shared/feature-toggles/feature-toggle-manager.service';
1615

1716
describe('TimeClockComponent', () => {
1817
let component: TimeClockComponent;
1918
let fixture: ComponentFixture<TimeClockComponent>;
2019
let store: MockStore<ProjectState>;
2120
let azureAdB2CService: AzureAdB2CService;
22-
let featureManagerService: FeatureManagerService;
2321
const actionSub: ActionsSubject = new ActionsSubject();
2422

2523
let injectedToastrService;
@@ -72,7 +70,6 @@ describe('TimeClockComponent', () => {
7270
fixture.detectChanges();
7371
azureAdB2CService = TestBed.inject(AzureAdB2CService);
7472
injectedToastrService = TestBed.inject(ToastrService);
75-
// featureManagerService = TestBed.inject(FeatureManagerService);
7673
});
7774

7875
it('should be created', () => {
@@ -99,12 +96,14 @@ describe('TimeClockComponent', () => {
9996
expect(component.reloadSummariesOnClockOut).toHaveBeenCalled();
10097
});
10198

102-
it('unsubscribe clockOutSubscription onDestroy', () => {
99+
it('unsubscribe clockOutSubscription, storeSubscription onDestroy', () => {
103100
spyOn(component.clockOutSubscription, 'unsubscribe');
101+
spyOn(component.storeSubscription, 'unsubscribe');
104102

105103
component.ngOnDestroy();
106104

107105
expect(component.clockOutSubscription.unsubscribe).toHaveBeenCalled();
106+
expect(component.storeSubscription.unsubscribe).toHaveBeenCalled();
108107
});
109108

110109
it('onInit checks if isLogin and gets the userName', () => {
@@ -146,14 +145,4 @@ describe('TimeClockComponent', () => {
146145

147146
expect(injectedToastrService.error).toHaveBeenCalled();
148147
});
149-
150-
// const exponentialGrowth = [true, false];
151-
// exponentialGrowth.map((toggleValue) => {
152-
// it(`when FeatureToggle is ${toggleValue} should return true`, () => {
153-
// spyOn(featureManagerService, 'isToggleEnabled').and.returnValue(of(toggleValue));
154-
// const isFeatureToggleActivated: Promise<boolean> = component.isFeatureToggleActivated();
155-
// expect(featureManagerService.isToggleEnabled).toHaveBeenCalled();
156-
// isFeatureToggleActivated.then((value) => expect(value).toEqual(toggleValue));
157-
// });
158-
// });
159148
});

‎src/app/modules/time-clock/pages/time-clock.component.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { EntryFieldsComponent } from '../components/entry-fields/entry-fields.co
1010
import { Entry } from './../../shared/models/entry.model';
1111
import { EntryActionTypes, LoadEntriesSummary, StopTimeEntryRunning } from './../store/entry.actions';
1212
import { getActiveTimeEntry } from './../store/entry.selectors';
13-
import { FeatureManagerService } from 'src/app/modules/shared/feature-toggles/feature-toggle-manager.service';
1413
@Component({
1514
selector: 'app-time-clock',
1615
templateUrl: './time-clock.component.html',
@@ -26,15 +25,13 @@ export class TimeClockComponent implements OnInit, OnDestroy {
2625
storeSubscription: Subscription;
2726

2827
constructor(
29-
private featureManagerService: FeatureManagerService,
3028
private azureAdB2CService: AzureAdB2CService,
3129
private store: Store<Entry>,
3230
private toastrService: ToastrService,
3331
private actionsSubject$: ActionsSubject
3432
) {}
3533

36-
ngOnInit(): void{
37-
34+
ngOnInit(): void {
3835
this.username = this.azureAdB2CService.isLogin() ? this.azureAdB2CService.getName() : '';
3936
this.storeSubscription = this.store.pipe(select(getActiveTimeEntry)).subscribe((activeTimeEntry) => {
4037
this.activeTimeEntry = activeTimeEntry;
@@ -72,16 +69,11 @@ export class TimeClockComponent implements OnInit, OnDestroy {
7269
}
7370
}
7471

75-
ngOnDestroy(): void {
76-
this.isFeatureToggleActivated() && this.storeSubscription.unsubscribe();
77-
this.clockOutSubscription.unsubscribe();
78-
this.storeSubscription.unsubscribe();
79-
}
80-
81-
isFeatureToggleActivated() {
82-
return this.featureManagerService.isToggleEnabledForUser('exponential-growth').pipe(
83-
map((enabled) => enabled));
72+
ngOnDestroy(): void {
73+
this.clockOutSubscription.unsubscribe();
74+
this.storeSubscription.unsubscribe();
8475
}
76+
8577
}
8678

8779

0 commit comments

Comments
 (0)