From aba3e776d40b06fd112bf6b1a2971feeebfcc269 Mon Sep 17 00:00:00 2001 From: Claudia Asti Date: Fri, 10 Sep 2021 15:32:30 +0200 Subject: [PATCH] Refactor groups to support multiple event ids --- ...nce-control-group-dialog.component.spec.ts | 2 +- ...presence-control-group-dialog.component.ts | 8 +- .../presence-control-group.component.html | 8 +- .../presence-control-group.component.ts | 51 +++++-------- .../presence-control-header.component.html | 4 +- .../presence-control-group.service.ts | 37 ++++++---- .../presence-control-state.service.ts | 2 +- .../utils/presence-control-entries.spec.ts | 12 +-- .../shared/utils/presence-control-entries.ts | 4 +- src/app/shared/utils/user-settings.spec.ts | 74 +++++++++++++------ src/app/shared/utils/user-settings.ts | 23 +++--- 11 files changed, 122 insertions(+), 103 deletions(-) diff --git a/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.spec.ts b/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.spec.ts index 8be35ec52..205afeffe 100644 --- a/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.spec.ts +++ b/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.spec.ts @@ -26,7 +26,7 @@ describe('PresenceControlGroupDialogComponent', () => { component = fixture.componentInstance; component.dialogMode = DialogMode.Select; component.subscriptionDetail = buildSubscriptionDetail(3843); - component.savedGroupView = { eventId: 1, group: null }; + component.group = null; fixture.detectChanges(); }); diff --git a/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.ts b/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.ts index 741183797..1f8ad1a8c 100644 --- a/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.ts +++ b/src/app/presence-control/components/presence-control-group-dialog/presence-control-group-dialog.component.ts @@ -2,7 +2,6 @@ import { Component, Input, OnInit } from '@angular/core'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; import { TranslateService } from '@ngx-translate/core'; import { SubscriptionDetail } from '../../../shared/models/subscription-detail.model'; -import { GroupViewType } from '../../../shared/models/user-setting.model'; export interface GroupOptions { id: Option; @@ -22,7 +21,7 @@ export enum DialogMode { export class PresenceControlGroupDialogComponent implements OnInit { @Input() dialogMode: DialogMode; @Input() subscriptionDetail: SubscriptionDetail; - @Input() savedGroupView: GroupViewType; + @Input() group: Option; groupOptions: Array = []; selected: GroupOptions; title: string; @@ -41,9 +40,8 @@ export class PresenceControlGroupDialogComponent implements OnInit { this.groupOptions.unshift(emptyOption); this.selected = - this.groupOptions.find( - (option) => option.id === this.savedGroupView.group - ) || emptyOption; + this.groupOptions.find((option) => option.id === this.group) || + emptyOption; } private createEmtpyOption(): GroupOptions { diff --git a/src/app/presence-control/components/presence-control-group/presence-control-group.component.html b/src/app/presence-control/components/presence-control-group/presence-control-group.component.html index d6e6b071f..26fdd5800 100644 --- a/src/app/presence-control/components/presence-control-group/presence-control-group.component.html +++ b/src/app/presence-control/components/presence-control-group/presence-control-group.component.html @@ -3,7 +3,7 @@ sortCriteria: sortCriteria$ | async, sortedEntries: sortedEntries$ | async, selection: selectionService.selection$ | async, - groupView: groupService.groupView$ | async + group: groupService.group$ | async } as data" > {{ 'presence-control.groups.show' | translate }} diff --git a/src/app/presence-control/components/presence-control-group/presence-control-group.component.ts b/src/app/presence-control/components/presence-control-group/presence-control-group.component.ts index 6b037d2e9..188d907f1 100644 --- a/src/app/presence-control/components/presence-control-group/presence-control-group.component.ts +++ b/src/app/presence-control/components/presence-control-group/presence-control-group.component.ts @@ -3,15 +3,14 @@ import { ActivatedRoute } from '@angular/router'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { TranslateService } from '@ngx-translate/core'; import { ToastrService } from 'ngx-toastr'; -import { BehaviorSubject, combineLatest, EMPTY, forkJoin, of } from 'rxjs'; +import { BehaviorSubject, combineLatest, forkJoin } from 'rxjs'; import { map, mapTo, pluck, switchMap, take } from 'rxjs/operators'; -import { GroupViewType } from '../../../shared/models/user-setting.model'; import { SubscriptionDetailsRestService } from '../../../shared/services/subscription-details-rest.service'; import { UserSettingsRestService } from '../../../shared/services/user-settings-rest.service'; import { spread } from '../../../shared/utils/function'; import { parseQueryString } from '../../../shared/utils/url'; import { - updateGroupViews, + updateGroupViewSettings, getUserSetting, } from '../../../shared/utils/user-settings'; import { PresenceControlGroupSelectionService } from '../../services/presence-control-group-selection.service'; @@ -46,11 +45,7 @@ export class PresenceControlGroupComponent implements OnInit { map(parseQueryString) ); - private eventId$ = this.route.paramMap.pipe( - map((params) => Number(params.get('id'))) - ); // TODO remove?, pass multiple event ids in url? - - eventIds$ = this.state.selectedLesson$.pipe( + private eventIds$ = this.state.selectedLesson$.pipe( map((lesson) => lesson?.getEventIds() || []) ); @@ -99,16 +94,16 @@ export class PresenceControlGroupComponent implements OnInit { ): void { combineLatest([ this.groupService.getSubscriptionDetailForGroupEvent(), - this.groupService.groupView$, + this.groupService.group$, ]) .pipe(take(1)) - .subscribe(([subscriptionDetail, groupView]) => { + .subscribe(([subscriptionDetail, group]) => { const modalRef = this.modalService.open( PresenceControlGroupDialogComponent ); modalRef.componentInstance.dialogMode = dialogMode; modalRef.componentInstance.subscriptionDetail = subscriptionDetail; - modalRef.componentInstance.savedGroupView = groupView; + modalRef.componentInstance.group = group; modalRef.result.then( (selectedGroup) => { @@ -123,30 +118,20 @@ export class PresenceControlGroupComponent implements OnInit { combineLatest([this.eventIds$, this.groupService.savedGroupViews$]) .pipe( take(1), - switchMap(([eventIds, groupViews]) => { - if (eventIds.length > 0) { - const groupView: GroupViewType = { - eventId: eventIds[0], // TODO send all event ids - group: selectedGroup.id, - }; - - const propertyBody = updateGroupViews(groupView, groupViews); - const cst = getUserSetting( - 'presenceControlGroupView', - propertyBody - ); - - return this.settingsService - .updateUserSettingsCst(cst) - .pipe(mapTo(groupView)); - } - return EMPTY; + switchMap(([eventIds, savedGroupViews]) => { + const propertyBody = updateGroupViewSettings( + selectedGroup.id, + eventIds, + savedGroupViews + ); + const cst = getUserSetting('presenceControlGroupView', propertyBody); + + return this.settingsService + .updateUserSettingsCst(cst) + .pipe(mapTo(selectedGroup.id)); }) ) - .subscribe( - (propertyBody) => - propertyBody && this.groupService.selectGroupView(propertyBody) - ); + .subscribe((groupId) => this.groupService.selectGroup(groupId)); } private assignCallback(selectedGroup: GroupOptions): void { diff --git a/src/app/presence-control/components/presence-control-header/presence-control-header.component.html b/src/app/presence-control/components/presence-control-header/presence-control-header.component.html index 2715bc9e3..742fee15c 100644 --- a/src/app/presence-control/components/presence-control-header/presence-control-header.component.html +++ b/src/app/presence-control/components/presence-control-header/presence-control-header.component.html @@ -104,10 +104,10 @@ >
(); + private selectGroup$ = new Subject>(); private selectedLesson$ = new ReplaySubject>(); private lessonPresences$ = new ReplaySubject>(); private reloadSubscriptionDetails$ = new Subject(); - private defaultGroupView: GroupViewType = { eventId: null, group: null }; + private defaultGroup: Option = null; savedGroupViews$ = this.loadSavedGroupViews(); - private savedGroupView$ = this.selectedLesson$.pipe( + private savedGroup$ = this.selectedLesson$.pipe( switchMap((lesson) => this.savedGroupViews$.pipe( - map( - (views) => - views.find((view) => view.eventId === lesson?.getEventIds()[0]) || - this.defaultGroupView // TODO helper - ) + map((views) => this.findGroupByLesson(views, lesson)) ) ) ); - groupView$ = merge(this.selectGroupView$, this.savedGroupView$).pipe( - startWith(this.defaultGroupView), + group$ = merge(this.selectGroup$, this.savedGroup$).pipe( + startWith(this.defaultGroup), shareReplay(1) ); @@ -125,11 +121,11 @@ export class PresenceControlGroupService { ); subscriptionDetailPersonIds$ = combineLatest([ - this.groupView$, + this.group$, this.subscriptionDetails$, ]).pipe( - map(([groupView, details]) => - details.filter((d) => d.Value === groupView?.group).map((d) => d.IdPerson) + map(([group, details]) => + details.filter((d) => d.Value === group).map((d) => d.IdPerson) ), startWith([]) ); @@ -142,8 +138,8 @@ export class PresenceControlGroupService { @Inject(SETTINGS) private settings: Settings ) {} - selectGroupView(view: GroupViewType): void { - this.selectGroupView$.next(view); + selectGroup(groupId: Option): void { + this.selectGroup$.next(groupId); } setSelectedLesson(selected: Option): void { @@ -197,4 +193,15 @@ export class PresenceControlGroupService { defaultIfEmpty([] as ReadonlyArray) ); } + + private findGroupByLesson( + groupViews: ReadonlyArray, + lesson: Option + ): Option { + const groupView = groupViews.find( + (gv) => gv.eventId === lesson?.getEventIds()[0] // All event ids of a lesson share the same group + ); + + return groupView?.group || this.defaultGroup; + } } diff --git a/src/app/presence-control/services/presence-control-state.service.ts b/src/app/presence-control/services/presence-control-state.service.ts index 91576ffe4..bac8d1ae0 100644 --- a/src/app/presence-control/services/presence-control-state.service.ts +++ b/src/app/presence-control/services/presence-control-state.service.ts @@ -147,7 +147,7 @@ export class PresenceControlStateService ]).pipe(map(spread(getPresenceControlEntriesForLesson))); selectedPresenceControlEntriesByGroup$ = combineLatest([ - this.groupService.groupView$, + this.groupService.group$, this.selectedPresenceControlEntries$, this.groupService.subscriptionDetailPersonIds$, ]).pipe(map(spread(filterByGroup)), shareReplay(1)); diff --git a/src/app/shared/utils/presence-control-entries.spec.ts b/src/app/shared/utils/presence-control-entries.spec.ts index 2e270ada4..69a78f4f6 100644 --- a/src/app/shared/utils/presence-control-entries.spec.ts +++ b/src/app/shared/utils/presence-control-entries.spec.ts @@ -21,10 +21,9 @@ describe('PresenceControlEntries', () => { }); it('does not filter entries if group is null', () => { - const groupView = { eventId: 333, group: null }; const personIds = [1, 2, 3]; - expect(filterByGroup(groupView, entries, personIds)).toEqual([ + expect(filterByGroup(null, entries, personIds)).toEqual([ entry1, entry2, entry3, @@ -32,10 +31,9 @@ describe('PresenceControlEntries', () => { }); it('does filter entries - given all student ids', () => { - const groupView = { eventId: 333, group: 'A' }; const personIds = [1, 2, 3]; - expect(filterByGroup(groupView, entries, personIds)).toEqual([ + expect(filterByGroup('A', entries, personIds)).toEqual([ entry1, entry2, entry3, @@ -43,17 +41,15 @@ describe('PresenceControlEntries', () => { }); it('does filter entries - given one student id', () => { - const groupView = { eventId: 333, group: 'A' }; const personIds = [2]; - expect(filterByGroup(groupView, entries, personIds)).toEqual([entry2]); + expect(filterByGroup('A', entries, personIds)).toEqual([entry2]); }); it('does filter entries - given empty student ids', () => { - const groupView = { eventId: 333, group: 'A' }; const personIds: ReadonlyArray = []; - expect(filterByGroup(groupView, entries, personIds)).toEqual([]); + expect(filterByGroup('A', entries, personIds)).toEqual([]); }); }); }); diff --git a/src/app/shared/utils/presence-control-entries.ts b/src/app/shared/utils/presence-control-entries.ts index bf2033ffc..12abce8fd 100644 --- a/src/app/shared/utils/presence-control-entries.ts +++ b/src/app/shared/utils/presence-control-entries.ts @@ -31,11 +31,11 @@ export function buildPresenceControlEntries( } export function filterByGroup( - groupView: Option, + group: Option, entries: ReadonlyArray, personIds: ReadonlyArray ): ReadonlyArray { - if (groupView?.group) { + if (group) { return entries.filter((e) => personIds.find((id) => id === e.lessonPresence.StudentRef.Id) ); diff --git a/src/app/shared/utils/user-settings.spec.ts b/src/app/shared/utils/user-settings.spec.ts index aba0ce063..a96a2e74b 100644 --- a/src/app/shared/utils/user-settings.spec.ts +++ b/src/app/shared/utils/user-settings.spec.ts @@ -1,25 +1,22 @@ import { GroupViewType } from '../models/user-setting.model'; -import { updateGroupViews } from './user-settings'; +import { updateGroupViewSettings } from './user-settings'; describe('user settings', () => { - describe('.updateGroupViews', () => { - let groupViews: ReadonlyArray; - let groupView: GroupViewType; + describe('.updateGroupViewSettings', () => { + let existingSettings: ReadonlyArray; it('adds new group view to empty settings', () => { - groupViews = []; - groupView = { eventId: 1, group: 'A' }; + existingSettings = []; - const updated = updateGroupViews(groupView, groupViews); + const updated = updateGroupViewSettings('A', [1], existingSettings); expect(updated).toEqual([{ eventId: 1, group: 'A' }]); }); - it('add new group view to existing settings for other lesson', () => { - groupViews = [{ eventId: 1, group: 'A' }]; - groupView = { eventId: 2, group: 'B' }; + it('add new group view to existing settings for other event - different group', () => { + existingSettings = [{ eventId: 1, group: 'A' }]; - const updated = updateGroupViews(groupView, groupViews); + const updated = updateGroupViewSettings('B', [2], existingSettings); expect(updated).toEqual([ { eventId: 1, group: 'A' }, @@ -27,31 +24,64 @@ describe('user settings', () => { ]); }); + it('add new group view to existing settings for other event - same group', () => { + existingSettings = [{ eventId: 1, group: 'A' }]; + + const updated = updateGroupViewSettings('A', [2], existingSettings); + + expect(updated).toEqual([ + { eventId: 1, group: 'A' }, + { eventId: 2, group: 'A' }, + ]); + }); + it('replaces group setting for existing lesson', () => { - groupViews = [{ eventId: 1, group: 'A' }]; - groupView = { eventId: 1, group: 'B' }; + existingSettings = [ + { eventId: 1, group: 'A' }, + { eventId: 2, group: 'A' }, + { eventId: 3, group: 'B' }, + ]; - const updated = updateGroupViews(groupView, groupViews); + const updated = updateGroupViewSettings('B', [1, 2], existingSettings); - expect(updated).toEqual([{ eventId: 1, group: 'B' }]); + expect(updated).toEqual([ + { eventId: 1, group: 'B' }, + { eventId: 2, group: 'B' }, + { eventId: 3, group: 'B' }, + ]); }); it('does not add null group to settings', () => { - groupViews = []; - groupView = { eventId: 1, group: null }; + existingSettings = []; - const updated = updateGroupViews(groupView, groupViews); + const updated = updateGroupViewSettings(null, [1], existingSettings); expect(updated).toEqual([]); }); + it('does remove existing setting if group is null', () => { + existingSettings = [ + { eventId: 1, group: 'A' }, + { eventId: 2, group: 'A' }, + ]; + + const updated = updateGroupViewSettings(null, [1], existingSettings); + + expect(updated).toEqual([{ eventId: 2, group: 'A' }]); + }); + it('does not add duplicated settings', () => { - groupViews = [{ eventId: 1, group: 'C' }]; - groupView = { eventId: 1, group: 'C' }; + existingSettings = [ + { eventId: 1, group: 'C' }, + { eventId: 2, group: 'B' }, + ]; - const updated = updateGroupViews(groupView, groupViews); + const updated = updateGroupViewSettings('C', [1], existingSettings); - expect(updated).toEqual([{ eventId: 1, group: 'C' }]); + expect(updated).toEqual([ + { eventId: 1, group: 'C' }, + { eventId: 2, group: 'B' }, + ]); }); }); }); diff --git a/src/app/shared/utils/user-settings.ts b/src/app/shared/utils/user-settings.ts index e7f01c957..e15d1ed31 100644 --- a/src/app/shared/utils/user-settings.ts +++ b/src/app/shared/utils/user-settings.ts @@ -15,15 +15,20 @@ export function getUserSetting(key: string, propertyBody: any): UserSetting { return cst; } -export function updateGroupViews( - groupView: GroupViewType, - groupsViews: ReadonlyArray +export function updateGroupViewSettings( + group: Option, + eventIds: ReadonlyArray, + existingSettings: ReadonlyArray ): ReadonlyArray { - const updatedGroupViews = [...groupsViews].filter( - (gv) => gv.eventId !== groupView.eventId + const newSettings: ReadonlyArray = eventIds.map((eventId) => { + return { eventId, group }; + }); + + const updatedSettings = existingSettings.map( + (es) => newSettings.find((ns) => ns.eventId === es.eventId) || es + ); + + return [...new Set([...updatedSettings, ...newSettings])].filter( + (settings) => settings.group !== null ); - if (groupView.group) { - updatedGroupViews.push(groupView); - } - return updatedGroupViews; }