[New] Drop Queue Window Scheduler #160
Conversation
…/extended from payloads
enum/Scheduler.ts
Outdated
| @@ -1,4 +1,5 @@ | |||
| export enum JobName { | |||
| SESSION_MAINTENANCE = 'session-maintenance', | |||
| EVENT_SCHEDULER = 'event-scheduler', | |||
| QUEUE_WINDOW_SCHEDULER = 'queue-window-scheduler', | |||
There was a problem hiding this comment.
Can we rename it to something relatable? How about CONTINUE_BLACKOUT_TIMEOUT_SCHEDULER?
lib/QueueWindowProcessor.ts
Outdated
| public async processor(jobContext: IJobContext, read: IRead, modify: IModify, http: IHttp, persistence: IPersistence): Promise<void> { | ||
| const sessionId = jobContext.rid; | ||
| try { | ||
| await setIsQueueWindowActive(read, persistence, sessionId, false); | ||
| } catch (error) { | ||
| // Failed to send event, so close blackout window | ||
| console.error(`${Logs.DIALOGFLOW_REST_API_ERROR}: { roomID: ${sessionId} } ${getError(error)}`); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
You are trying to setsetIsQueueWindowActive flag to false But the actual flag that blocks user messages is setIsProcessingMessage So we need to use that maybe?
lib/payloadAction.ts
Outdated
| const continueBlackoutTask = { | ||
| id: JobName.QUEUE_WINDOW_SCHEDULER, | ||
| when: `120 seconds`, | ||
| data: { | ||
| rid, | ||
| sessionId: rid, | ||
| jobName: JobName.EVENT_SCHEDULER, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Here, the id is JobName.QUEUE_WINDOW_SCHEDULER, but the jobName is JobName.EVENT_SCHEDULER. It should be same right?
lib/payloadAction.ts
Outdated
| @@ -74,6 +74,17 @@ export const handlePayloadActions = async (app: IApp, read: IRead, modify: IMo | |||
|
|
|||
| // Start blackout window | |||
| if (params.continue_blackout) { | |||
| const continueBlackoutTask = { | |||
| id: JobName.QUEUE_WINDOW_SCHEDULER, | |||
| when: `120 seconds`, | |||
There was a problem hiding this comment.
In the past we faced some issue with passing literal strings here. It was scheduling job with some invalid date. And using Date object works fine https://github.com/WideChat/Apps.Dialogflow/blob/master/lib/payloadAction.ts#L63 Can you please try it?
lib/payloadAction.ts
Outdated
| @@ -93,6 +104,7 @@ export const handlePayloadActions = async (app: IApp, read: IRead, modify: IMo | |||
| } | |||
| } else if (actionName === ActionIds.DROP_QUEUE) { | |||
| await setIsQueueWindowActive(read, persistence, rid, false); | |||
| await modify.getScheduler().cancelJobByDataQuery({sessionId : rid, jobName: JobName.QUEUE_WINDOW_SCHEDULER}); | |||
There was a problem hiding this comment.
In the lib/Scheduler.ts file we made some functions to cancel scheduled tasks. i.e. cancelAllEventSchedulerJobForSession We should create one for this scheduler as well.
lib/EventTimeoutProcessor.ts
Outdated
| @@ -46,6 +57,7 @@ export class EventScheduler implements IProcessor { | |||
| const queuedMessage = await getQueuedMessage(read, sessionId); | |||
|
|
|||
| await setIsQueueWindowActive(read, persistence, sessionId, false); | |||
| await modify.getScheduler().cancelJobByDataQuery({sessionId, jobName: JobName.QUEUE_WINDOW_SCHEDULER}); | |||
There was a problem hiding this comment.
We are setting and unsetting setIsQueueWindowActive in this function. The actual flag that blocks user messages is setIsProcessingMessage So maybe we don't need this here?
|
@Shailesh351 it looks like Alexander responded to the comments? Please verify when you have a chance. Thanks. |
lib/EventTimeoutProcessor.ts
Outdated
| import { Global } from '../Global'; | ||
| import { getError } from '../lib/Helper'; | ||
| import { Dialogflow } from './Dialogflow'; | ||
| import { handleResponse } from './payloadAction'; | ||
| import { getQueuedMessage, getRoomAssoc, retrieveDataByAssociation, setIsProcessingMessage, setIsQueueWindowActive, setQueuedMessage } from './Persistence'; | ||
| import { cancelAllBlackoutTimeoutSchedulerJobForSession } from './Scheduler'; |
There was a problem hiding this comment.
This import is unused
lib/EventTimeoutProcessor.ts
Outdated
| const continueBlackoutTask = { | ||
| id: JobName.CONTINUE_BLACKOUT_TIMEOUT_SCHEDULER, | ||
| when: `120 seconds`, | ||
| data: { | ||
| rid: sessionId, | ||
| sessionId, | ||
| jobName: JobName.CONTINUE_BLACKOUT_TIMEOUT_SCHEDULER, | ||
| }, | ||
| }; | ||
| await modify.getScheduler().scheduleOnce(continueBlackoutTask); |
There was a problem hiding this comment.
I think we don't need this. Because in this function when we receive a response from the DF, and if it is having continue_blackout flag we are already scheduling a timer. Can you please check and confirm if it's working. Thanks!
lib/QueueWindowProcessor.ts
Outdated
| import { IJobContext, IProcessor } from '@rocket.chat/apps-engine/definition/scheduler'; | ||
| import { Logs } from '../enum/Logs'; | ||
| import { getError } from './Helper'; | ||
| import { setIsProcessingMessage, setIsQueueWindowActive} from './Persistence'; |
There was a problem hiding this comment.
Import setIsQueueWindowActive is unused
For issue: WideChat/Rocket.Chat#1225
-Activates 120 second timer when queue window is set as active which drops the queue window.
-Resets timeout to 120 seconds if DF sends payload with
continue_blackoutparameter.