From 1ddf93621c4145db63c4a8c7581621546eb871ef Mon Sep 17 00:00:00 2001 From: trtshen Date: Wed, 10 Sep 2025 12:16:22 +0800 Subject: [PATCH] [CORE-7940] pulsecheck race-condition --- .../fast-feedback/fast-feedback.component.ts | 5 +- .../src/app/components/img/img.component.ts | 4 +- .../activity-desktop/activity-desktop.page.ts | 12 +-- projects/v3/src/app/pages/home/home.page.ts | 12 --- .../src/app/services/fast-feedback.service.ts | 90 +++++++++++++++++-- projects/v3/src/app/services/home.service.ts | 1 + projects/v3/src/app/services/modal.service.ts | 31 ++++++- .../src/app/services/notifications.service.ts | 17 ++-- 8 files changed, 137 insertions(+), 35 deletions(-) diff --git a/projects/v3/src/app/components/fast-feedback/fast-feedback.component.ts b/projects/v3/src/app/components/fast-feedback/fast-feedback.component.ts index 5214da4e7..ab82fe620 100644 --- a/projects/v3/src/app/components/fast-feedback/fast-feedback.component.ts +++ b/projects/v3/src/app/components/fast-feedback/fast-feedback.component.ts @@ -30,6 +30,7 @@ export class FastFeedbackComponent implements OnInit { loading = false; submissionCompleted: boolean; isMobile: boolean; + pulseCheckId: string; // pagination properties currentPage = 0; @@ -72,6 +73,8 @@ export class FastFeedbackComponent implements OnInit { this.totalPages = Math.ceil(this.questions.length / this.questionsPerPage); this.showPagination = this.totalPages > 1; + this.pulseCheckId = this.navParams.get('modal')?.componentProps?.pulseCheckId; + // Determine pulse check type based on question IDs this.pulseCheckType = this.determinePulseCheckType(); } @@ -227,7 +230,7 @@ export class FastFeedbackComponent implements OnInit { let submissionResult; try { submissionResult = await firstValueFrom(this.fastFeedbackService - .submit(answers, params)); + .submit(answers, params, this.pulseCheckId)); // Check if question 7's answer is 0 const question7Answer = formData['7']; // hardcoded question id 7 (1st fast feedback question) diff --git a/projects/v3/src/app/components/img/img.component.ts b/projects/v3/src/app/components/img/img.component.ts index e6bc39bba..2e3e82d8d 100644 --- a/projects/v3/src/app/components/img/img.component.ts +++ b/projects/v3/src/app/components/img/img.component.ts @@ -1,4 +1,4 @@ -import { Component, Input, isDevMode, SimpleChanges } from '@angular/core'; +import { Component, Input, isDevMode, SimpleChanges, OnChanges } from '@angular/core'; import { getData, getAllTags } from 'exif-js'; const getImageClassToFixOrientation = (orientation) => { @@ -32,7 +32,7 @@ const swapWidthAndHeight = img => { templateUrl: './img.component.html', styleUrls: ['./img.component.scss'] }) -export class ImgComponent { +export class ImgComponent implements OnChanges { @Input() alt: string; @Input() imgSrc: string; proxiedImgSrc: string; diff --git a/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts b/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts index 9407be7e4..c246a26d7 100644 --- a/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts +++ b/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts @@ -396,25 +396,25 @@ export class ActivityDesktopPage { try { // handle unexpected submission: do final status check before saving let hasSubmssion = false; - const { submission } = await this.assessmentService - .fetchAssessment( + const { submission } = await firstValueFrom( + this.assessmentService.fetchAssessment( event.assessmentId, 'assessment', this.activity.id, event.contextId, event.submissionId ) - .toPromise(); + ); if (submission?.status === 'in progress') { - const saved = await this.assessmentService - .submitAssessment( + const saved = await firstValueFrom( + this.assessmentService.submitAssessment( event.submissionId, event.assessmentId, event.contextId, event.answers ) - .toPromise(); + ); // http 200 but error if ( diff --git a/projects/v3/src/app/pages/home/home.page.ts b/projects/v3/src/app/pages/home/home.page.ts index da1a3db30..23a35a46f 100644 --- a/projects/v3/src/app/pages/home/home.page.ts +++ b/projects/v3/src/app/pages/home/home.page.ts @@ -189,18 +189,6 @@ export class HomePage implements OnInit, OnDestroy, AfterViewChecked { this.unsubscribe$.complete(); } - /** - * @name openPulseCheck - * @description This method pulls the fast feedback service (with type 'skills') to open the pulse check modal. - */ - openPulseCheck() { - this.fastFeedbackService.pullFastFeedback({ - closable: true, - skipChecking: true, - type: 'skills' - }).pipe(first()).subscribe(); - } - async updateDashboard() { await this.sharedService.refreshJWT(); // refresh JWT token [CORE-6083] this.experience = this.storageService.get("experience"); diff --git a/projects/v3/src/app/services/fast-feedback.service.ts b/projects/v3/src/app/services/fast-feedback.service.ts index 97247c285..eb3633bce 100644 --- a/projects/v3/src/app/services/fast-feedback.service.ts +++ b/projects/v3/src/app/services/fast-feedback.service.ts @@ -3,7 +3,7 @@ import { NotificationsService } from './notifications.service'; import { BrowserStorageService } from '@v3/services/storage.service'; import { UtilsService } from '@v3/services/utils.service'; import { of, from, Observable } from 'rxjs'; -import { switchMap, retry, finalize } from 'rxjs/operators'; +import { switchMap, retry, finalize, tap } from 'rxjs/operators'; import { environment } from '@v3/environments/environment'; import { DemoService } from './demo.service'; import { ApolloService } from './apollo.service'; @@ -13,6 +13,10 @@ import { ApiResponse } from '../models/api.model'; providedIn: 'root' }) export class FastFeedbackService { + private readonly SUBMISSION_COOLDOWN = 10 * 1000; // 10 seconds cooldown + + private currentPulseCheckId: string = null; // temporary store active pulse check ID + constructor( private notificationsService: NotificationsService, private storage: BrowserStorageService, @@ -114,13 +118,24 @@ export class FastFeedbackService { return of(res); } + // generate ID for this pulse check modal + const pulseCheckId = this.generatePulseCheckId(questions, meta); + + // skip showing the modal if this pulse check was recently viewed + submitted + if (this.isPulseCheckSubmitted(pulseCheckId) && !options.skipChecking) { + return of(res); + } + + // temporarily store the current pulse check ID after make sure it hasn't been submitted yet + this.currentPulseCheckId = pulseCheckId; + // popup instant feedback view if question quantity found > 0 if ( !this.utils.isEmpty(res.data) && questions?.length > 0 && !fastFeedbackIsOpened ) { - // add a flag to indicate that a fast feedback pop up is opening + // set a flag to indicate a fast feedback modal is currently opening to prevent duplicates this.storage.set("fastFeedbackOpening", true); return from( @@ -128,6 +143,7 @@ export class FastFeedbackService { { questions, meta, + pulseCheckId, }, { closable: options.closable, @@ -143,7 +159,7 @@ export class FastFeedbackService { return of(res); } catch (error) { console.error("Error in switchMap:", error); - // Return a fallback observable to allow the consumer to continue working + // fail gracefully to avoid blocking user's flow return of({ error: true, message: "An error occurred while processing fast feedback.", @@ -154,7 +170,7 @@ export class FastFeedbackService { retry({ count: 3, delay: 1000 - }) + }), ); } @@ -162,13 +178,17 @@ export class FastFeedbackService { teamId?: number; targetUserId?: number; contextId?: number; - }): Observable { + }, + pulseCheckId?: string + ): Observable { if (environment.demo) { /* eslint-disable no-console */ console.log('data', answers, 'params', params); return this.demo.normalResponse() as Observable; } + const submittedId = pulseCheckId || this.currentPulseCheckId; // fallback to temporary ID if not provided + return this.apolloService.graphQLMutate( `mutation submitPulseCheck($teamId: Int, $targetUserId: Int, $contextId: Int, $answers: [PulseCheckAnswerInput]) { submitPulseCheck(teamId: $teamId, targetUserId: $targetUserId, contextId: $contextId, answers: $answers) @@ -177,6 +197,66 @@ export class FastFeedbackService { ...params, answers, }, + ).pipe( + tap(result => { + if (result.data?.submitPulseCheck && submittedId) { + this.recordPulseCheckSubmission(submittedId); + } + }) ); } + + /** + * generates a unique id for a pulse check based on its content + */ + private generatePulseCheckId(questions: any[], meta: any): string { + if (!questions?.length || !meta) { + return null; + } + + const questionIds = questions.map(q => q.id).sort().join(','); + return `${questionIds}_${meta.teamId}_${meta.contextId || 0}`; // eg. "1,2,3_45_0" + } + + /** + * checks if this specific pulse check was recently submitted + */ + private isPulseCheckSubmitted(pulseCheckId: string): boolean { + if (!pulseCheckId) { + return false; + } + + const submittedChecks = this.storage.get('submittedPulseChecks') || {}; + const submission = submittedChecks[pulseCheckId]; + + if (!submission) { + return false; + } + + const now = Date.now(); + return (now - submission) < this.SUBMISSION_COOLDOWN; + } + + /** + * records a specific pulse check as submitted + */ + private recordPulseCheckSubmission(pulseCheckId: string): void { + if (!pulseCheckId) { + return; + } + + // Record specific pulse check submission + const submittedChecks = this.storage.get('submittedPulseChecks') || {}; + submittedChecks[pulseCheckId] = Date.now(); + + // Clean up old submissions (older than cooldown period) + const now = Date.now(); + Object.keys(submittedChecks).forEach(id => { + if (now - submittedChecks[id] > this.SUBMISSION_COOLDOWN) { + delete submittedChecks[id]; + } + }); + + this.storage.set('submittedPulseChecks', submittedChecks); + } } diff --git a/projects/v3/src/app/services/home.service.ts b/projects/v3/src/app/services/home.service.ts index e3324c17b..32cf8fea9 100644 --- a/projects/v3/src/app/services/home.service.ts +++ b/projects/v3/src/app/services/home.service.ts @@ -366,6 +366,7 @@ export class HomeService { ); } + // update skill & progress survey matrix getPulseCheckSkills(): Observable> { diff --git a/projects/v3/src/app/services/modal.service.ts b/projects/v3/src/app/services/modal.service.ts index 117396177..eb1c89721 100644 --- a/projects/v3/src/app/services/modal.service.ts +++ b/projects/v3/src/app/services/modal.service.ts @@ -7,11 +7,32 @@ import { ModalController } from '@ionic/angular'; export class ModalService { private modalQueue: any[] = []; private isShowingModal = false; - + private activeModalIds: Set = new Set(); constructor(private modalController: ModalController) { } - async addModal(modalConfig: any, callback?: Function) { - this.modalQueue.push({ modalConfig, callback }); + /** + * Adds a modal to the queue to be displayed + * @param modalConfig The configuration for the modal + * @param callback Optional callback to execute after modal is dismissed + * @param modalId Optional unique identifier to prevent duplicate modals + * @returns Promise that resolves once the modal is added to queue + */ + async addModal(modalConfig: any, callback?: Function, modalId?: string): Promise { + // check if the modalId already in queue or being shown + if (modalId && this.activeModalIds.has(modalId)) { + return; + } + + if (modalId) { + this.activeModalIds.add(modalId); + } + + this.modalQueue.push({ + modalConfig, + callback, + modalId + }); + this.showNextModal(); } @@ -26,6 +47,10 @@ export class ModalService { this.isShowingModal = true; modal.onDidDismiss().then(() => { + if (modalInfo.modalId) { + this.activeModalIds.delete(modalInfo.modalId); + } + if (modalInfo.callback) { modalInfo.callback(); } diff --git a/projects/v3/src/app/services/notifications.service.ts b/projects/v3/src/app/services/notifications.service.ts index 6b8a5a6b2..cfe35e4cf 100644 --- a/projects/v3/src/app/services/notifications.service.ts +++ b/projects/v3/src/app/services/notifications.service.ts @@ -203,16 +203,16 @@ export class NotificationsService { return modal; } - async modal(component, componentProps, options?, event?): Promise { - return this.modalOnly(component, componentProps, options, event); + async modal(component, componentProps, options?, event?, modalId?: string): Promise { + return this.modalOnly(component, componentProps, options, event, modalId); } - async modalOnly(component, componentProps, options?, event?): Promise { + async modalOnly(component, componentProps, options?, event?, modalId?: string): Promise { const modalConfig = this.modalConfig( { component, componentProps }, options ); - return this.modalService.addModal(modalConfig, event); + return this.modalService.addModal(modalConfig, event, modalId); } /** @@ -449,6 +449,7 @@ export class NotificationsService { props: { questions?: Question[]; meta?: Meta | Object; + pulseCheckId?: string; }, options: { closable?: boolean; @@ -463,11 +464,15 @@ export class NotificationsService { showBackdrop: false, ...options }; + + // use pulseCheckId to identify each modal instance to prevent duplicate + const modalId = props.pulseCheckId ? `pulse-check-${props.pulseCheckId}` : null; + if (options.modalOnly) { - return this.modalOnly(FastFeedbackComponent, props, modalConfig); + return this.modalOnly(FastFeedbackComponent, props, modalConfig, null, modalId); } - return this.modal(FastFeedbackComponent, props, modalConfig); + return this.modal(FastFeedbackComponent, props, modalConfig, null, modalId); } getTodoItems(): Observable {