From 2a2f2a70341755f8ffcf94e475b27d0d594875b8 Mon Sep 17 00:00:00 2001 From: ievavold <ievavold@wisc.edu> Date: Mon, 26 Aug 2019 13:51:36 -0500 Subject: [PATCH] ROENROLL-1986, ROENROLL-1987 fix observable subscription bugs --- .../dars-audit-view.component.ts | 37 +- src/app/dars/dars-view/dars-view.component.ts | 6 +- .../metadata-table.component.ts | 65 ++-- .../new-degree-audit-dialog.component.ts | 336 +++++++++--------- .../new-what-if-audit-dialog.component.ts | 273 +++++++------- src/app/dars/store/effects.ts | 64 ++-- src/app/dars/store/state.ts | 2 + 7 files changed, 412 insertions(+), 371 deletions(-) diff --git a/src/app/dars/dars-audit-view/dars-audit-view.component.ts b/src/app/dars/dars-audit-view/dars-audit-view.component.ts index 96995a6..173e4f1 100644 --- a/src/app/dars/dars-audit-view/dars-audit-view.component.ts +++ b/src/app/dars/dars-audit-view/dars-audit-view.component.ts @@ -1,6 +1,6 @@ -import { Component, OnInit } from '@angular/core'; -import { Router, ActivatedRoute } from '@angular/router'; -import { Observable, combineLatest } from 'rxjs'; +import { Component, OnInit, OnDestroy } from '@angular/core'; +import { ActivatedRoute } from '@angular/router'; +import { Observable, combineLatest, Subscription } from 'rxjs'; import { map, filter, @@ -12,25 +12,23 @@ import { AuditStatus, MetadataStatus } from '../store/state'; import { Store } from '@ngrx/store'; import { GlobalState } from '@app/core/state'; import * as selectors from '../store/selectors'; -import { StartLoadingAudit, StartLoadingDARSView } from '../store/actions'; -import { - AuditMetadata, - AuditMetadataWithReportId, -} from '../models/audit-metadata'; +import { StartLoadingAudit, RefreshMetadata } from '../store/actions'; +import { AuditMetadataWithReportId } from '../models/audit-metadata'; @Component({ selector: 'cse-dars-audit-view', templateUrl: './dars-audit-view.component.html', styleUrls: ['./dars-audit-view.component.scss'], }) -export class AuditViewComponent implements OnInit { +export class AuditViewComponent implements OnInit, OnDestroy { public metadataStatus$: Observable<MetadataStatus['status']>; public darsDegreeAuditReportId$: Observable<number>; public metadata$: Observable<AuditMetadataWithReportId>; public audit$: Observable<AuditStatus>; + private metadataSubscription: Subscription; + private auditSubscription: Subscription; constructor( - private router: Router, private route: ActivatedRoute, private store: Store<GlobalState>, ) {} @@ -45,11 +43,13 @@ export class AuditViewComponent implements OnInit { .select(selectors.metadataStatus) .pipe(shareReplay()); - this.metadataStatus$.subscribe(metadataStatus => { - if (metadataStatus === 'NotLoaded') { - this.store.dispatch(new StartLoadingDARSView()); - } - }); + this.metadataSubscription = this.metadataStatus$.subscribe( + metadataStatus => { + if (metadataStatus === 'NotLoaded') { + this.store.dispatch(new RefreshMetadata()); + } + }, + ); /** * Get the `darsDegreeAuditReportId` from the route URL. Sanitize and parse @@ -84,7 +84,7 @@ export class AuditViewComponent implements OnInit { shareReplay(), ); - combineLatest([ + this.auditSubscription = combineLatest([ this.metadata$.pipe(distinctUntilKeyChanged('darsDegreeAuditReportId')), this.audit$.pipe(distinctUntilKeyChanged('status')), ]).subscribe(([metadata, audit]) => { @@ -94,7 +94,8 @@ export class AuditViewComponent implements OnInit { }); } - public openNewTab() { - window.open(window.location.href, '_blank', 'noopener noreferrer'); + public ngOnDestroy() { + this.metadataSubscription.unsubscribe(); + this.auditSubscription.unsubscribe(); } } diff --git a/src/app/dars/dars-view/dars-view.component.ts b/src/app/dars/dars-view/dars-view.component.ts index 2ae6026..8388e58 100644 --- a/src/app/dars/dars-view/dars-view.component.ts +++ b/src/app/dars/dars-view/dars-view.component.ts @@ -114,7 +114,8 @@ export class DARSViewComponent implements OnInit, OnDestroy { }), ); } - }); + }) + .unsubscribe(); } public openWhatIfAuditDialog() { @@ -133,6 +134,7 @@ export class DARSViewComponent implements OnInit, OnDestroy { }), ); } - }); + }) + .unsubscribe(); } } diff --git a/src/app/dars/metadata-table/metadata-table.component.ts b/src/app/dars/metadata-table/metadata-table.component.ts index 72dcba9..a9e19cc 100644 --- a/src/app/dars/metadata-table/metadata-table.component.ts +++ b/src/app/dars/metadata-table/metadata-table.component.ts @@ -5,11 +5,12 @@ import { EventEmitter, ViewChild, OnInit, + OnDestroy, } from '@angular/core'; import { AuditMetadata } from '../models/audit-metadata'; import { MatPaginator } from '@angular/material/paginator'; import { MatTableDataSource } from '@angular/material/table'; -import { Observable, combineLatest } from 'rxjs'; +import { Observable, combineLatest, Subscription } from 'rxjs'; import { PageEvent } from '@angular/material/paginator'; import { LiveAnnouncer } from '@angular/cdk/a11y'; @@ -27,7 +28,7 @@ interface MinimumAuditMetadata { templateUrl: './metadata-table.component.html', styleUrls: ['./metadata-table.component.scss'], }) -export class DarsMetadataTableComponent implements OnInit { +export class DarsMetadataTableComponent implements OnInit, OnDestroy { @Input() public waiting: Observable<{ outstanding: number; pending: number }>; @Input() public metadata: Observable<AuditMetadata[]>; @Input() public type: 'program' | 'whatIf'; @@ -49,13 +50,7 @@ export class DarsMetadataTableComponent implements OnInit { 'status', 'actions', ]; - constructor(private announcer: LiveAnnouncer) {} - onPaginationChange(event) { - this.announcer.announce( - `Switched page of the ${this.title} table`, - 'assertive', - ); - } + private dataSubscription: Subscription; public static sortMetadata(metadata: MinimumAuditMetadata[]) { return metadata.sort((a, b) => { @@ -72,28 +67,42 @@ export class DarsMetadataTableComponent implements OnInit { }); } + constructor(private announcer: LiveAnnouncer) {} + + onPaginationChange(event) { + this.announcer.announce( + `Switched page of the ${this.title} table`, + 'assertive', + ); + } + public ngOnInit() { - combineLatest([this.waiting, this.metadata]).subscribe( - ([waiting, metadata]) => { - const sorted = DarsMetadataTableComponent.sortMetadata(metadata); + this.dataSubscription = combineLatest([ + this.waiting, + this.metadata, + ]).subscribe(([waiting, metadata]) => { + const sorted = DarsMetadataTableComponent.sortMetadata(metadata); - const totalWaiting = waiting.outstanding + waiting.pending; - const synthetic = Array.from({ length: totalWaiting }).map(() => ({ - darsAuditRunDate: '', - darsDegreeAuditReportId: null, - darsHonorsOptionDescription: '', - darsInstitutionCodeDescription: '', - darsStatusOfDegreeAuditRequest: '', - degreePlannerPlanName: '', - })); + const totalWaiting = waiting.outstanding + waiting.pending; + const synthetic = Array.from({ length: totalWaiting }).map(() => ({ + darsAuditRunDate: '', + darsDegreeAuditReportId: null, + darsHonorsOptionDescription: '', + darsInstitutionCodeDescription: '', + darsStatusOfDegreeAuditRequest: '', + degreePlannerPlanName: '', + })); - this.dataSource = new MatTableDataSource<MinimumAuditMetadata>([ - ...synthetic, - ...sorted, - ]); + this.dataSource = new MatTableDataSource<MinimumAuditMetadata>([ + ...synthetic, + ...sorted, + ]); - this.dataSource.paginator = this.paginator; - }, - ); + this.dataSource.paginator = this.paginator; + }); + } + + public ngOnDestroy() { + this.dataSubscription.unsubscribe(); } } diff --git a/src/app/dars/new-degree-audit-dialog/new-degree-audit-dialog.component.ts b/src/app/dars/new-degree-audit-dialog/new-degree-audit-dialog.component.ts index b596f86..36cf2a5 100644 --- a/src/app/dars/new-degree-audit-dialog/new-degree-audit-dialog.component.ts +++ b/src/app/dars/new-degree-audit-dialog/new-degree-audit-dialog.component.ts @@ -1,23 +1,24 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, OnInit, OnDestroy } from '@angular/core'; import { DarsApiService } from '../services/api.service'; import { DegreePrograms } from '../models/degree-program'; import { StudentDegreeProgram } from '../models/student-degree-program'; -import { Observable, combineLatest, Subject, forkJoin } from 'rxjs'; +import { Observable, combineLatest, Subscription } from 'rxjs'; import { FormBuilder, FormGroup, Validators, FormControl, FormArray, + AbstractControl, } from '@angular/forms'; import { HonorsOption } from '../models/honors-option'; import { map, - share, filter, flatMap, shareReplay, distinctUntilChanged, + tap, } from 'rxjs/operators'; import { CourseBase } from '@app/core/models/course'; import { Store } from '@ngrx/store'; @@ -25,9 +26,8 @@ import { GlobalState } from '@app/core/state'; import { degreePlans } from '../store/selectors'; import { DegreePlan } from '@app/core/models/degree-plan'; import { MatDialogRef } from '@angular/material'; -import { RawTermCode } from '@app/degree-planner/shared/term-codes/without-era'; -import { DegreePlannerApiService } from '@app/degree-planner/services/api.service'; import { MatSnackBar } from '@angular/material'; +import { TermCodeFactory } from '@app/degree-planner/services/termcode.factory'; const inclusiveRange = (from: number, to: number) => { const range: number[] = []; @@ -37,41 +37,6 @@ const inclusiveRange = (from: number, to: number) => { return range; }; -// tslint:disable-next-line: interface-over-type-literal -type SimpleMap = { [name: string]: any }; -type ObservableMap<T = SimpleMap> = { [K in keyof T]: Observable<T[K]> }; - -const forkJoinWithKeys = <T = SimpleMap>(pairs: ObservableMap<T>) => { - const keys = Object.keys(pairs); - const observables = keys.map(key => pairs[key]); - return forkJoin(observables).pipe( - map<any[], T>(values => { - const valueMapping = {} as T; - - keys.forEach((key, index) => { - valueMapping[key] = values[index]; - }); - - return valueMapping; - }), - ); -}; - -const courseIsFromAnActiveOrFutureTerm = ( - course: CourseBase, - active: RawTermCode[], -): boolean => { - if (course.termCode === null || active.length === 0) { - return false; - } - - const termCode = new RawTermCode(course.termCode); - - return active.some(a => { - return a.comesBefore(termCode) || a.equals(termCode); - }); -}; - export interface NewDegreeAuditFields { darsInstitutionCode: string; darsDegreeProgramCode: string; @@ -86,12 +51,14 @@ export interface NewDegreeAuditFields { }[]; } +type CourseWithCreditRange = CourseBase & { range: number[] }; + @Component({ selector: 'cse-new-degree-audit-dialog', templateUrl: './new-degree-audit-dialog.component.html', styleUrls: ['./new-degree-audit-dialog.component.scss'], }) -export class NewDegreeAuditDialogComponent implements OnInit { +export class NewDegreeAuditDialogComponent implements OnInit, OnDestroy { // Form-builder objects public chosenProgram: FormControl; public chosenAuditSettings: FormGroup; @@ -99,18 +66,15 @@ export class NewDegreeAuditDialogComponent implements OnInit { // API observables public degreePrograms$: Observable<StudentDegreeProgram[]>; - public institutions$: Observable<DegreePrograms>; public degreePlans$: Observable<DegreePlan[]>; - public chosenRoadmapId$ = new Subject<number>(); public honorsOptions$: Observable<HonorsOption[]>; - public variableCreditCourses$: Observable< - (CourseBase & { range: number[] })[] - >; + public variableCreditCourses$: Observable<CourseWithCreditRange[]>; + public variableCreditCoursesSubscription: Subscription; constructor( private fb: FormBuilder, private api: DarsApiService, - private degreePlannerApi: DegreePlannerApiService, + private termCodeService: TermCodeFactory, private store: Store<GlobalState>, private dialogRef: MatDialogRef<NewDegreeAuditDialogComponent>, private snackBar: MatSnackBar, @@ -126,142 +90,172 @@ export class NewDegreeAuditDialogComponent implements OnInit { this.chosenCreditSettings = fb.array([]); } - public ngOnInit() { - this.degreePrograms$ = this.api.getStudentDegreePrograms().pipe(share()); - this.degreePrograms$.subscribe(programs => { - if (programs.length > 0) { - this.chosenProgram.setValue(programs[0]); - } - }); + private initDegreeProgram(): Observable<StudentDegreeProgram[]> { + /** + * Get the list of degree programs the student is enrolled in. If there are + * more than one, set the first degree program to be the initial value of + * the `chosenProgram` form input. + * + * This observable will only fire when the dialog is instantiated. + */ + return this.api.getStudentDegreePrograms().pipe(shareReplay()); + } + + private initDegreePlan(): Observable<DegreePlan[]> { + /** + * Get the list of user degree plans so that if they choose to include + * planned courses in the audit, they can be presented with a choice of + * which degree plan to load those planned courses from. + * + * Also set the primary degree plan to be the initial value of the + * `degreePlan` form input. + * + * This observable will only fire when the dialog is instantiated. + */ + return this.store.select(degreePlans).pipe(shareReplay()); + } - this.institutions$ = this.api.getStaticData().pipe(share()); + private initHonorsOptions(): Observable<HonorsOption[]> { + interface HasInstitutionCode { + darsInstitutionCode: string; + } - this.honorsOptions$ = combineLatest([ - this.institutions$, - this.chosenProgram.valueChanges.pipe( - filter(value => value.hasOwnProperty('darsInstitutionCode')), - map(value => value.darsInstitutionCode as string), - ), + /** + * Get the honors options based on the parent institution of the degree + * program chosen in step 1 of the audit dialog. Since different + * institutions can have different honors options, this subscription needs + * to be re-run whenever the degree program is switched. + */ + return combineLatest([ + this.api.getStaticData(), + this.chosenProgram.valueChanges, ]).pipe( - map(([institutions, darsInstitutionCode]) => { - return institutions.hasOwnProperty(darsInstitutionCode) - ? institutions[darsInstitutionCode].honorsOptions - : []; + filter((tuple): tuple is [DegreePrograms, HasInstitutionCode] => { + return ( + tuple[1] && + tuple[1].hasOwnProperty('darsInstitutionCode') && + typeof tuple[1].darsInstitutionCode === 'string' + ); + }), + map(([degreePrograms, hasInstitutionCode]): [DegreePrograms, string] => { + return [degreePrograms, hasInstitutionCode.darsInstitutionCode]; + }), + map(([degreePrograms, darsInstitutionCode]) => { + if (degreePrograms.hasOwnProperty(darsInstitutionCode)) { + return degreePrograms[darsInstitutionCode].honorsOptions; + } else { + return []; + } }), ); + } - this.degreePlans$ = this.store.select(degreePlans).pipe(shareReplay()); - - this.degreePlans$.subscribe(plans => { - const primaryPlan = plans.find(p => p.primary); - this.chosenAuditSettings.controls['degreePlan'].setValue(primaryPlan); - }); - - const degreePlanFormControl = this.chosenAuditSettings.get('degreePlan'); - const includeCoursesFrom = this.chosenAuditSettings.get( - 'includeCoursesFrom', - ); - - if (degreePlanFormControl) { - degreePlanFormControl.valueChanges.subscribe(currentPlan => { - this.chosenRoadmapId$.next(currentPlan.roadmapId); - - this.variableCreditCourses$ = this.chosenRoadmapId$.pipe( - distinctUntilChanged(), - flatMap(roadmapId => - forkJoinWithKeys({ - courses: this.api.getAllCourses(roadmapId), - activeTermCodes: this.degreePlannerApi.getActiveTermCodes(), - }), - ), - map(({ courses, activeTermCodes }) => { - return courses.filter(course => { - return ( - !!course.creditMin && - !!course.creditMax && - course.creditMax > course.creditMin && - courseIsFromAnActiveOrFutureTerm(course, activeTermCodes) - ); - }); - }), - map(courses => - courses.map(course => ({ + private initVariableCreditCourses(): Observable<CourseWithCreditRange[]> { + /** + * When the user chooses a degree plan: + * + * 1. Load all of the courses from that degree plan + * 2. Find all the courses that support a range of credits and that occur + * in an active or future term + * 3. Compute the range of credit values supported by those courses and add + * that range to the course objects + * 4. Populate the `chosenCreditSettings` form input with these courses + * using the credit range to populate each course's dropdown. + */ + + // prettier-ignore + const chosenDegreePlan = (this.chosenAuditSettings.get('degreePlan') as AbstractControl).valueChanges; + return chosenDegreePlan.pipe( + filter((maybeDegreePlan): maybeDegreePlan is { roadmapId: number } => { + return ( + !!maybeDegreePlan && + maybeDegreePlan.hasOwnProperty('roadmapId') && + typeof maybeDegreePlan['roadmapId'] === 'number' + ); + }), + map(maybeDegreePlan => maybeDegreePlan.roadmapId), + distinctUntilChanged(), + flatMap(roadmapId => this.api.getAllCourses(roadmapId)), + map(courses => { + return courses.filter(course => { + return ( + !!course.creditMin && + !!course.creditMax && + course.creditMax > course.creditMin && + this.courseIsFromAnActiveOrFutureTerm(course) + ); + }); + }), + map(courses => { + return courses.map( + (course): CourseWithCreditRange => { + return { ...course, range: inclusiveRange( - course.creditMin as number, - course.creditMax as number, + course.creditMin || 0, + course.creditMax || 0, ), - })), - ), - share(), + }; + }, ); - this.variableCreditCourses$.subscribe(courses => { - while (this.chosenCreditSettings.length !== 0) { - this.chosenCreditSettings.removeAt(0); - } - - courses.forEach(course => { - this.chosenCreditSettings.push( - this.fb.group({ - course, - credits: this.fb.control( - course.credits || '', - Validators.required, - ), - }), - ); - }); - }); - }); - if (includeCoursesFrom) { - includeCoursesFrom.valueChanges.subscribe(selected => { - if (selected === 'planned') { - this.variableCreditCourses$ = this.store.select(degreePlans).pipe( - map(plans => plans.find(plan => plan.primary)), - filter((plan): plan is DegreePlan => plan !== undefined), - flatMap(plan => this.api.getAllCourses(plan.roadmapId)), - map(courses => { - return courses.filter(course => { - return ( - !!course.creditMin && - !!course.creditMax && - course.creditMax > course.creditMin - ); - }); - }), - map(courses => - courses.map(course => ({ - ...course, - range: inclusiveRange( - course.creditMin as number, - course.creditMax as number, - ), - })), - ), - share(), - ); - - this.variableCreditCourses$.subscribe(courses => { - while (this.chosenCreditSettings.length !== 0) { - this.chosenCreditSettings.removeAt(0); - } - - courses.forEach(course => { - this.chosenCreditSettings.push( - this.fb.group({ - course, - credits: this.fb.control( - course.credits || '', - Validators.required, - ), - }), - ); - }); - }); - } + }), + shareReplay(), + ); + } + + public ngOnInit() { + this.degreePrograms$ = this.initDegreeProgram().pipe( + tap(dps => dps.length > 0 && this.chosenProgram.setValue(dps[0])), + ); + + this.variableCreditCourses$ = this.initVariableCreditCourses().pipe( + tap(courses => { + /** + * This feels hacky and hopefully there's a better way to remove the old + * course credit inputs and replace them with new course credit inputs. + */ + + // Remove the old credit settings form inputs + while (this.chosenCreditSettings.length > 0) { + this.chosenCreditSettings.removeAt(0); + } + + // Add the new credit settings form inputs + courses.forEach(course => { + const initialCreditValue = course.credits || ''; + const creditsDropdown = this.fb.control( + initialCreditValue, + Validators.required, + ); + const newCreditSetting = this.fb.group({ course, creditsDropdown }); + this.chosenCreditSettings.push(newCreditSetting); }); - } + }), + ); + + this.variableCreditCoursesSubscription = this.variableCreditCourses$.subscribe(); + + this.degreePlans$ = this.initDegreePlan().pipe( + tap(plans => { + const primaryPlan = plans.find(plan => plan.primary); + this.chosenAuditSettings.controls.degreePlan.setValue(primaryPlan); + }), + ); + + this.honorsOptions$ = this.initHonorsOptions(); + } + + public ngOnDestroy() { + this.variableCreditCoursesSubscription.unsubscribe(); + } + + private courseIsFromAnActiveOrFutureTerm(course: CourseBase): boolean { + if (course.termCode === null || this.termCodeService.isNotInitialized()) { + return false; } + + const termCode = this.termCodeService.fromString(course.termCode); + return termCode.isActive() || termCode.isFuture(); } public darsInstitutionCode<T>(fallback: T): string | T { diff --git a/src/app/dars/new-what-if-audit-dialog/new-what-if-audit-dialog.component.ts b/src/app/dars/new-what-if-audit-dialog/new-what-if-audit-dialog.component.ts index 549d66f..f18165c 100644 --- a/src/app/dars/new-what-if-audit-dialog/new-what-if-audit-dialog.component.ts +++ b/src/app/dars/new-what-if-audit-dialog/new-what-if-audit-dialog.component.ts @@ -1,21 +1,16 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, OnInit, OnDestroy } from '@angular/core'; import { DarsApiService } from '../services/api.service'; import { DegreePrograms, DegreeProgram } from '../models/degree-program'; -import { Observable, combineLatest, Subject, forkJoin } from 'rxjs'; -import { - FormBuilder, - FormGroup, - Validators, - FormArray, - FormControl, -} from '@angular/forms'; +import { Observable, combineLatest, Subject, Subscription } from 'rxjs'; +import { FormBuilder, FormGroup, Validators, FormArray } from '@angular/forms'; import { HonorsOption } from '../models/honors-option'; import { map, - share, flatMap, shareReplay, distinctUntilChanged, + tap, + filter, } from 'rxjs/operators'; import { CourseBase } from '@app/core/models/course'; import { Store } from '@ngrx/store'; @@ -23,9 +18,8 @@ import { GlobalState } from '@app/core/state'; import { degreePlans } from '../store/selectors'; import { DegreePlan } from '@app/core/models/degree-plan'; import { MatDialogRef } from '@angular/material'; -import { DegreePlannerApiService } from '@app/degree-planner/services/api.service'; -import { RawTermCode } from '@app/degree-planner/shared/term-codes/without-era'; import { MatSnackBar } from '@angular/material'; +import { TermCodeFactory } from '@app/degree-planner/services/termcode.factory'; const inclusiveRange = (from: number, to: number) => { const range: number[] = []; @@ -35,40 +29,7 @@ const inclusiveRange = (from: number, to: number) => { return range; }; -// tslint:disable-next-line: interface-over-type-literal -type SimpleMap = { [name: string]: any }; -type ObservableMap<T = SimpleMap> = { [K in keyof T]: Observable<T[K]> }; - -const forkJoinWithKeys = <T = SimpleMap>(pairs: ObservableMap<T>) => { - const keys = Object.keys(pairs); - const observables = keys.map(key => pairs[key]); - return forkJoin(observables).pipe( - map<any[], T>(values => { - const valueMapping = {} as T; - - keys.forEach((key, index) => { - valueMapping[key] = values[index]; - }); - - return valueMapping; - }), - ); -}; - -const courseIsFromAnActiveOrFutureTerm = ( - course: CourseBase, - active: RawTermCode[], -): boolean => { - if (course.termCode === null || active.length === 0) { - return false; - } - - const termCode = new RawTermCode(course.termCode); - - return active.some(a => { - return a.comesBefore(termCode) || a.equals(termCode); - }); -}; +type CourseWithCreditRange = CourseBase & { range: number[] }; export interface NewWhatIfAuditFields { darsInstitutionCode: string; @@ -89,7 +50,7 @@ export interface NewWhatIfAuditFields { templateUrl: './new-what-if-audit-dialog.component.html', styleUrls: ['./new-what-if-audit-dialog.component.scss'], }) -export class NewWhatIfAuditDialogComponent implements OnInit { +export class NewWhatIfAuditDialogComponent implements OnInit, OnDestroy { // Form-builder objects public chosenProgram: FormGroup; public chosenAuditSettings: FormGroup; @@ -101,14 +62,13 @@ export class NewWhatIfAuditDialogComponent implements OnInit { public degreePlans$: Observable<DegreePlan[]>; public chosenRoadmapId$ = new Subject<number>(); public honorsOptions$: Observable<HonorsOption[]>; - public variableCreditCourses$: Observable< - (CourseBase & { range: number[] })[] - >; + public variableCreditCourses$: Observable<CourseWithCreditRange[]>; + public variableCreditCoursesSubscription: Subscription; constructor( private fb: FormBuilder, private api: DarsApiService, - private degreePlannerApi: DegreePlannerApiService, + private termCodeService: TermCodeFactory, private store: Store<GlobalState>, private dialogRef: MatDialogRef< NewWhatIfAuditDialogComponent, @@ -133,101 +93,162 @@ export class NewWhatIfAuditDialogComponent implements OnInit { this.chosenCreditSettings = fb.array([]); } - public ngOnInit() { - this.institutions$ = this.api.getStaticData().pipe(share()); + private initInstitutions() { + return this.api.getStaticData().pipe(shareReplay()); + } + + private initProgramOrPlanOptions() { + // prettier-ignore + const chosenInstitution = this.chosenProgram.controls.institution.valueChanges; - this.programOrPlanOptions$ = combineLatest([ - this.institutions$, - (this.chosenProgram.get('institution') as FormControl).valueChanges, - ]).pipe( + return combineLatest([this.institutions$, chosenInstitution]).pipe( + map(([insitutions, darsInstitutionCode]) => { + if (insitutions.hasOwnProperty(darsInstitutionCode)) { + return insitutions[darsInstitutionCode].programs; + } else { + return []; + } + }), + ); + } + + private initDegreePlans(): Observable<DegreePlan[]> { + return this.store.select(degreePlans).pipe(shareReplay()); + } + + private initHonorsOptions(): Observable<HonorsOption[]> { + /** + * Get the honors options based on the parent insitution of the degree + * program chosen in step 1 of the audit dialog. Since different + * insitutions can have different honors options, this subscription needs + * to be re-run whenever the degree program is switched. + */ + + // prettier-ignore + const chosenInstitution = this.chosenProgram.controls.institution.valueChanges; + + return combineLatest([this.institutions$, chosenInstitution]).pipe( map(([institutions, darsInstitutionCode]) => { - this.chosenProgram.controls.planOrProgram.enable(); if (institutions.hasOwnProperty(darsInstitutionCode)) { - return institutions[darsInstitutionCode].programs; + return institutions[darsInstitutionCode].honorsOptions; } else { return []; } }), ); + } - this.honorsOptions$ = combineLatest([ - this.institutions$, - (this.chosenProgram.get('institution') as FormControl).valueChanges, - ]).pipe( - map(([institutions, darsInstitutionCode]) => { - return institutions.hasOwnProperty(darsInstitutionCode) - ? institutions[darsInstitutionCode].honorsOptions - : []; + private initVariableCreditCourses(): Observable<CourseWithCreditRange[]> { + /** + * When the user chooses a degree plan: + * + * 1. Load all of the courses from that degree plan + * 2. Find all the courses that support a range of credits and that occur + * in an active or future term + * 3. Compute the range of credit values supported by those courses and add + * that range to the course objects + * 4. Populate the `chosenCreditSettings` form input with these courses + * using the credit range to populate each course's dropdown. + */ + + // prettier-ignore + const chosenDegreePlan = this.chosenAuditSettings.controls.degreePlan.valueChanges; + + return chosenDegreePlan.pipe( + filter((maybeDegreePlan): maybeDegreePlan is { roadmapId: number } => { + return ( + !!maybeDegreePlan && + maybeDegreePlan.hasOwnProperty('roadmapId') && + typeof maybeDegreePlan['roadmapId'] === 'number' + ); + }), + map(maybeDegreePlan => maybeDegreePlan.roadmapId), + distinctUntilChanged(), + flatMap(roadmapId => this.api.getAllCourses(roadmapId)), + map(courses => { + return courses.filter(course => { + return ( + !!course.creditMin && + !!course.creditMax && + course.creditMax > course.creditMin && + this.courseIsFromAnActiveOrFutureTerm(course) + ); + }); + }), + map(courses => { + return courses.map( + (course): CourseWithCreditRange => { + return { + ...course, + range: inclusiveRange( + course.creditMin || 0, + course.creditMax || 0, + ), + }; + }, + ); }), + shareReplay(), ); + } - const degreePlanFormControl = this.chosenAuditSettings.get('degreePlan'); + public ngOnInit() { + this.institutions$ = this.initInstitutions().pipe( + tap(() => { + return this.chosenProgram.controls.planOrProgram.enable(); + }), + ); + + this.programOrPlanOptions$ = this.initProgramOrPlanOptions(); - const includeCoursesFrom = this.chosenAuditSettings.get( - 'includeCoursesFrom', + this.degreePlans$ = this.initDegreePlans().pipe( + tap(plans => { + const primaryPlan = plans.find(plan => plan.primary); + this.chosenAuditSettings.controls.degreePlan.setValue(primaryPlan); + }), ); - if (degreePlanFormControl) { - degreePlanFormControl.valueChanges.subscribe(plan => { - this.chosenRoadmapId$.next(plan.roadmapId); - }); - - this.variableCreditCourses$ = this.chosenRoadmapId$.pipe( - distinctUntilChanged(), - flatMap(roadmapId => - forkJoinWithKeys({ - courses: this.api.getAllCourses(roadmapId), - activeTermCodes: this.degreePlannerApi.getActiveTermCodes(), - }), - ), - map(({ courses, activeTermCodes }) => { - return courses.filter(course => { - return ( - !!course.creditMin && - !!course.creditMax && - course.creditMax > course.creditMin && - courseIsFromAnActiveOrFutureTerm(course, activeTermCodes) - ); - }); - }), - map(courses => - courses.map(course => ({ - ...course, - range: inclusiveRange( - course.creditMin as number, - course.creditMax as number, - ), - })), - ), - shareReplay(), - ); - } + this.honorsOptions$ = this.initHonorsOptions(); - this.variableCreditCourses$.subscribe(courses => { - while (this.chosenCreditSettings.length !== 0) { - this.chosenCreditSettings.removeAt(0); - } + this.variableCreditCourses$ = this.initVariableCreditCourses().pipe( + tap(courses => { + /** + * This feels hacky and hopefully there's a better way to remove the old + * course credit inputs and replace them with new course credit inputs. + */ - courses.forEach(course => { - this.chosenCreditSettings.push( - this.fb.group({ - course, - credits: this.fb.control(course.credits || '', Validators.required), - }), - ); - }); - }); + // Remove the old credit settings form inputs + while (this.chosenCreditSettings.length > 0) { + this.chosenCreditSettings.removeAt(0); + } - this.degreePlans$ = this.store.select(degreePlans).pipe(shareReplay()); + // Add the new credit settings form inputs + courses.forEach(course => { + const initialCreditValue = course.credits || ''; + const creditsDropdown = this.fb.control( + initialCreditValue, + Validators.required, + ); + const newCreditSetting = this.fb.group({ course, creditsDropdown }); + this.chosenCreditSettings.push(newCreditSetting); + }); + }), + ); - this.degreePlans$.subscribe(plans => { - const primaryPlan = plans.find(p => p.primary); - this.chosenAuditSettings.controls['degreePlan'].setValue(primaryPlan); + this.variableCreditCoursesSubscription = this.variableCreditCourses$.subscribe(); + } - if (primaryPlan) { - this.chosenRoadmapId$.next(primaryPlan.roadmapId); - } - }); + public ngOnDestroy() { + this.variableCreditCoursesSubscription.unsubscribe(); + } + + private courseIsFromAnActiveOrFutureTerm(course: CourseBase): boolean { + if (course.termCode === null || this.termCodeService.isNotInitialized()) { + return false; + } + + const termCode = this.termCodeService.fromString(course.termCode); + return termCode.isActive() || termCode.isFuture(); } public comparePlans(a: DegreePlan | null, b: DegreePlan | null): boolean { diff --git a/src/app/dars/store/effects.ts b/src/app/dars/store/effects.ts index f8d06be..7e0ead4 100644 --- a/src/app/dars/store/effects.ts +++ b/src/app/dars/store/effects.ts @@ -14,6 +14,7 @@ import { of } from 'rxjs'; import * as selectors from '@app/dars/store/selectors'; import { groupAuditMetadata } from './utils'; import { MatSnackBar } from '@angular/material'; +import { TermCodeFactory } from '@app/degree-planner/services/termcode.factory'; @Injectable() export class DARSEffects { @@ -23,47 +24,58 @@ export class DARSEffects { private degreeAPI: DegreePlannerApiService, private store$: Store<GlobalState>, private snackBar: MatSnackBar, + private termCodeService: TermCodeFactory, ) {} @Effect() load$ = this.actions$.pipe( ofType(DarsActionTypes.StartLoadingDARSView), + withLatestFrom(this.store$.select(selectors.getDARSState)), + flatMap(([_, state]) => { + if (state.hasLoaded) { + return of(new darsActions.DoneLoadingDARSView(state)); + } - flatMap(() => { return forkJoinWithKeys({ degreePlans: this.degreeAPI.getAllDegreePlans(), degreePrograms: this.api.getStudentDegreePrograms(), metadata: this.api.getAudits(), userPreferences: this.degreeAPI.getUserPreferences(), - }); - }), + activeTermCodes: this.degreeAPI.getActiveTermCodes(), + }).pipe( + map(tuple => { + if (this.termCodeService.isNotInitialized()) { + this.termCodeService.setActiveTermCodes(tuple.activeTermCodes); + } - map(({ degreePlans, degreePrograms, metadata, userPreferences }) => { - const alerts: Alert[] = []; - if (userPreferences.darsHasDismissedDisclaimer !== true) { - alerts.push( - new DarsDisclaimerAlert(() => { - this.store$.dispatch( - new UpdateUserPreferences({ - darsHasDismissedDisclaimer: true, + const alerts: Alert[] = []; + if (tuple.userPreferences.darsHasDismissedDisclaimer !== true) { + alerts.push( + new DarsDisclaimerAlert(() => { + this.store$.dispatch( + new UpdateUserPreferences({ + darsHasDismissedDisclaimer: true, + }), + ); }), ); - }), - ); - } + } - return new darsActions.DoneLoadingDARSView({ - degreePlans, - degreePrograms, - metadata: { - status: 'Loaded', - outstanding: { program: 0, whatIf: 0 }, - pending: { program: 0, whatIf: 0 }, - ...groupAuditMetadata(metadata, degreePrograms), - }, - audits: {}, - alerts, - }); + return new darsActions.DoneLoadingDARSView({ + hasLoaded: true, + degreePlans: tuple.degreePlans, + degreePrograms: tuple.degreePrograms, + metadata: { + status: 'Loaded', + outstanding: { program: 0, whatIf: 0 }, + pending: { program: 0, whatIf: 0 }, + ...groupAuditMetadata(tuple.metadata, tuple.degreePrograms), + }, + audits: {}, + alerts, + }); + }), + ); }), catchError(() => { diff --git a/src/app/dars/store/state.ts b/src/app/dars/store/state.ts index ac2f2d3..e3d0a14 100644 --- a/src/app/dars/store/state.ts +++ b/src/app/dars/store/state.ts @@ -23,6 +23,7 @@ export type AuditStatus = | { status: 'Loaded'; metadata: AuditMetadata; audit: Audit }; export interface DARSState { + hasLoaded: boolean; degreePlans: DegreePlan[]; degreePrograms: StudentDegreeProgram[]; metadata: MetadataStatus; @@ -31,6 +32,7 @@ export interface DARSState { } export const INITIAL_DARS_STATE: DARSState = { + hasLoaded: false, degreePlans: [], degreePrograms: [], metadata: { status: 'NotLoaded' }, -- GitLab