From a4aab76f0f5520c0b3635e4e2cc13cd9cb58be82 Mon Sep 17 00:00:00 2001 From: Jordan Powers Date: Thu, 25 Jan 2024 20:34:15 -0800 Subject: [PATCH] [Web] Lock interface after polling update Carl was noticing that an update from another mentor checking a student in or out could cause the interface to shift right as the user was about to push a button, potentially causing the wrong student to be checked in or out. The solution is to disable the interface for 100ms after such an update is received, to give the user time to readjust. --- .../add-attendance-event-list.component.ts | 39 ++++++++++++++++--- .../src/app/models/attendance-event.model.ts | 9 +++++ .../src/app/models/student.model.ts | 26 ++++++++++++- .../src/app/services/students.service.ts | 12 +++++- .../src/environments/environment.prod.ts | 3 +- .../src/environments/environment.ts | 3 +- 6 files changed, 82 insertions(+), 10 deletions(-) diff --git a/attendance-web/src/app/components/add-attendance-event-list/add-attendance-event-list.component.ts b/attendance-web/src/app/components/add-attendance-event-list/add-attendance-event-list.component.ts index f795835..eee96a5 100644 --- a/attendance-web/src/app/components/add-attendance-event-list/add-attendance-event-list.component.ts +++ b/attendance-web/src/app/components/add-attendance-event-list/add-attendance-event-list.component.ts @@ -1,5 +1,5 @@ import { AfterViewInit, Component, OnDestroy, OnInit } from '@angular/core'; -import { BehaviorSubject, combineLatest, finalize, forkJoin, interval, map, Observable, of, ReplaySubject, shareReplay, startWith, Subject, Subscription, switchMap, take, tap } from 'rxjs'; +import { BehaviorSubject, combineLatest, finalize, forkJoin, interval, map, Observable, of, ReplaySubject, shareReplay, startWith, Subject, Subscription, switchMap, take, tap, timer } from 'rxjs'; import { StudentsService } from 'src/app/services/students.service'; import { Student, StudentList, compareStudents } from 'src/app/models/student.model'; import { AttendanceService } from 'src/app/services/attendance.service'; @@ -35,6 +35,7 @@ export class AddAttendanceEventListComponent implements OnInit, AfterViewInit, O protected mode: AttendanceEventType protected pendingStudentIds = new BehaviorSubject([]); + protected inUpdateLockout = new BehaviorSubject(false); constructor( private pollService: PollService, @@ -67,11 +68,31 @@ export class AddAttendanceEventListComponent implements OnInit, AfterViewInit, O // Set up polling for new check-ins/check-outs this.polling = this.pollService.getPollingObservable().subscribe(pollData => { - this.studentsService.updateStudentsInCache(pollData.updated_students); + // If we get new data, disable the UI for a configurable period (in case a user is about + // to tap an option that will move with the update) + let shouldLockout: Observable; if(pollData.meeting_events.length > 0) { - // TODO: Verify that the 0th meeting event is always the most recent - this.lastEndOfMeeting.next(pollData.meeting_events[0]); + shouldLockout = of(true); + } else if(pollData.updated_students.length > 0) { + shouldLockout = this.studentsService.matchesCache(pollData.updated_students).pipe(map(matches => !matches)); + } else { + shouldLockout = of(false); } + + shouldLockout.subscribe(shouldLockout => { + if(shouldLockout) { + this.inUpdateLockout.next(true); + timer(environment.updateLockoutInterval).subscribe(() => { + this.inUpdateLockout.next(false); + }); + + this.studentsService.updateStudentsInCache(pollData.updated_students); + if(pollData.meeting_events.length > 0) { + // TODO: Verify that the 0th meeting event is always the most recent + this.lastEndOfMeeting.next(pollData.meeting_events[0]); + } + } + }); }); // Combine search, sort filters, and student roster into the final observable which @@ -131,6 +152,11 @@ export class AddAttendanceEventListComponent implements OnInit, AfterViewInit, O } private attendance(student: Student, action: AttendanceEventType) : void { + // If we're locked-out due to a recent update, ignore the event + if(this.inUpdateLockout.getValue()) { + return; + } + this.pendingStudentIds.next(this.pendingStudentIds.getValue().concat([student.id])); this.authService.getUser().pipe(map(user => { @@ -218,9 +244,10 @@ export class AddAttendanceEventListComponent implements OnInit, AfterViewInit, O ), isPending: this.pendingStudentIds.pipe( map(ids => ids.includes(student.id)) - ) + ), + inUpdateLockout: this.inUpdateLockout }).pipe( - map(({availableActionDoesNotMatchTab, isPending}) => availableActionDoesNotMatchTab || isPending) + map(({availableActionDoesNotMatchTab, isPending, inUpdateLockout}) => availableActionDoesNotMatchTab || isPending || inUpdateLockout) ); } diff --git a/attendance-web/src/app/models/attendance-event.model.ts b/attendance-web/src/app/models/attendance-event.model.ts index 6abd81b..35f9344 100644 --- a/attendance-web/src/app/models/attendance-event.model.ts +++ b/attendance-web/src/app/models/attendance-event.model.ts @@ -13,3 +13,12 @@ export interface AttendanceEvent { created_at: DateTime, updated_at: DateTime } + +export function areAttendanceEventsEqual(a: AttendanceEvent, b: AttendanceEvent) { + return a.id === b.id + && a.student_id === b.student_id + && a.registered_by === b.registered_by + && a.type === b.type + && a.created_at.equals(b.created_at) + && a.updated_at.equals(b.updated_at); +} diff --git a/attendance-web/src/app/models/student.model.ts b/attendance-web/src/app/models/student.model.ts index 8976459..bb75405 100644 --- a/attendance-web/src/app/models/student.model.ts +++ b/attendance-web/src/app/models/student.model.ts @@ -1,5 +1,5 @@ import { DateTime } from "luxon" -import { AttendanceEvent } from "./attendance-event.model" +import { AttendanceEvent, areAttendanceEventsEqual } from "./attendance-event.model" export interface Student { id: number, @@ -13,6 +13,30 @@ export interface Student { last_check_out?: AttendanceEvent } +export function areStudentsEqual(a: Student, b: Student): boolean { + const compare_optional_dates = (d1: DateTime|undefined, d2: DateTime|undefined) => { + if(d1 == undefined || d2 == undefined) { + return d1 === d2; + } + return d1.equals(d2); + }; + const compare_optional_events = (e1: AttendanceEvent|undefined, e2: AttendanceEvent|undefined) => { + if(e1 == undefined || e2 == undefined) { + return e1 === e2; + } + return areAttendanceEventsEqual(e1, e2); + }; + return a.id === b.id + && a.name === b.name + && a.graduation_year === b.graduation_year + && a.registered_by === b.registered_by + && a.created_at.equals(b.created_at) + && a.updated_at.equals(b.updated_at) + && compare_optional_dates(a.deleted_at, b.deleted_at) + && compare_optional_events(a.last_check_in, b.last_check_in) + && compare_optional_events(a.last_check_out, b.last_check_out); +} + export function compareStudents(a: Student, b: Student): number { const aName = a.name.toLocaleLowerCase(); const bName = b.name.toLocaleLowerCase(); diff --git a/attendance-web/src/app/services/students.service.ts b/attendance-web/src/app/services/students.service.ts index 728bda6..d8b3da5 100644 --- a/attendance-web/src/app/services/students.service.ts +++ b/attendance-web/src/app/services/students.service.ts @@ -4,7 +4,7 @@ import { HttpClient, HttpContext, HttpErrorResponse, HttpParams } from '@angular import { catchError, filter, map, Observable, of, OperatorFunction, ReplaySubject, shareReplay, switchMap, take, tap } from 'rxjs'; import { environment } from 'src/environments/environment'; -import { Student } from 'src/app/models/student.model'; +import { areStudentsEqual, Student } from 'src/app/models/student.model'; import { DateTime } from 'luxon'; import { CATCH_ERRORS } from '../http-interceptors/error-interceptor'; import { ErrorService } from './error.service'; @@ -87,6 +87,16 @@ export class StudentsService { ).subscribe(student => this.updateStudentsInCache([student])); } + public matchesCache(students: Student[]): Observable { + return this.cachedStudents.pipe( + take(1), + map(cache => students.reduce((eqSoFar, student) => { + const cachedStudent = cache.get(student.id); + return eqSoFar && cachedStudent != undefined && areStudentsEqual(student, cachedStudent); + }, true)) + ); + } + public updateStudentsInCache(new_students: Student[]) { this.cachedStudents.pipe( take(1), diff --git a/attendance-web/src/environments/environment.prod.ts b/attendance-web/src/environments/environment.prod.ts index 21b984c..3fc693d 100644 --- a/attendance-web/src/environments/environment.prod.ts +++ b/attendance-web/src/environments/environment.prod.ts @@ -2,5 +2,6 @@ export const environment = { production: true, apiRoot: "/attendance/api", authRoot: "/attendance/auth", - pollInterval: 500 + pollInterval: 500, + updateLockoutInterval: 100 }; diff --git a/attendance-web/src/environments/environment.ts b/attendance-web/src/environments/environment.ts index 6b2780b..e7b951d 100644 --- a/attendance-web/src/environments/environment.ts +++ b/attendance-web/src/environments/environment.ts @@ -6,7 +6,8 @@ export const environment = { production: false, apiRoot: "/api", authRoot: "/auth", - pollInterval: 500 + pollInterval: 500, + updateLockoutInterval: 100 }; /*