Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Contributor
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds TTS pre-warming and a dispatch-readiness retry flow for Twilio voice calls, introduces dispatch scheduling (model, controller, views, JS), refactors weather alerts to be system-originated notifications, adds voice-model warm-up support, adjusts FFmpeg codec to pcm_mulaw, and removes TwilioProviderController. ChangesTwilio Voice Dispatch Readiness & TTS Pre-Warming
Dispatch Call Scheduling
Weather Alert System Refactor
Sequence Diagram(s)sequenceDiagram
participant Client as Twilio Client
participant Controller as TwilioController
participant VoiceService as ITwilioVoiceResponseService
participant TtsService as TTS/Audio System
participant Cache as Audio Cache
Client->>Controller: VoiceCall(userId, callId, retry?)
Controller->>Controller: TryAppendDispatchPlaybackAsync()
alt Dispatch audio ready
Controller->>VoiceService: GetPromptUrlAsync(...)
VoiceService->>TtsService: GetOrCreatePromptUrlAsync(...)
TtsService->>Cache: Return/generated URL
Controller->>Client: Play dispatch audio + Gather menu
else Dispatch audio not ready
Controller->>VoiceService: PreWarmPromptAsync("Please wait...")
VoiceService->>TtsService: GetOrCreatePromptUrlAsync(...) (background)
Controller->>Client: Play please-wait prompt then Redirect VoiceCall?retry=N
end
sequenceDiagram
participant Admin as User/Admin
participant UI as Dispatch UI
participant Controller as DispatchController
participant Service as Dispatch Service
participant DB as Database
Admin->>UI: Submit NewCall with ScheduleDispatchDate
UI->>Controller: POST NewCall
Controller->>Service: Validate & Save (DispatchOn set if ≥15min)
Service->>DB: Insert/Update call with DispatchOn
Controller->>UI: Redirect/Response
Admin->>UI: View ScheduledCalls
UI->>Controller: GET GetScheduledCallsList
Controller->>Service: Query scheduled, non-dispatched calls ordered by DispatchOn
Service->>DB: SELECT WHERE DispatchOn > now AND !HasBeenDispatched
Service->>Controller: Return list
Controller->>UI: Render DataTable
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (9)
Web/Resgrid.Web.Services/Twilio/ITwilioVoiceResponseService.cs (1)
32-32: 💤 Low value
CancellationTokenparameter lacks= default— inconsistent with the rest of the interface.All four existing
AppendPrompt*overloads useCancellationToken cancellationToken = default, butGetPromptUrlAsyncomits the default. This forces callers to always pass an explicit token.♻️ Proposed fix
-System.Threading.Tasks.Task<Uri> GetPromptUrlAsync(string text, string voice, CancellationToken cancellationToken); +System.Threading.Tasks.Task<Uri> GetPromptUrlAsync(string text, string voice, CancellationToken cancellationToken = default);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Services/Twilio/ITwilioVoiceResponseService.cs` at line 32, The GetPromptUrlAsync declaration omits the optional default for its CancellationToken parameter, forcing callers to supply a token; update the interface method GetPromptUrlAsync(string text, string voice, CancellationToken cancellationToken) to make the token optional by adding = default so its signature matches the other AppendPrompt* overloads and callers can omit the token.Web/Resgrid.Web.Tts/Services/AudioProcessingService.cs (1)
106-137: ⚡ Quick win
GetDistinctVoiceIdentifiers— redundant outer guard and O(n²) inner loop can be simplified.The outer
!distinctVoices.Contains(languageCode)check is alwaystruebecauseVoiceModelMapis aDictionarywhose keys are inherently unique — the samelanguageCodecan never appear twice during iteration. The innerforeachoverdistinctVoicesproduces an O(n²) dedup that is better expressed with a secondHashSettracking seen model names:♻️ Proposed refactor
-public IReadOnlySet<string> GetDistinctVoiceIdentifiers() -{ - var distinctVoices = new HashSet<string>(StringComparer.OrdinalIgnoreCase); - - foreach (var (languageCode, modelName) in VoiceModelMap) - { - // We select the first language code we encounter for each model file. - // Since SortedDictionary-like ordering isn't needed here (any code that - // maps to a given model will load it), this simple dedup suffices. - if (!distinctVoices.Contains(languageCode)) - { - // Check if this model has already been represented - var alreadyRepresented = false; - foreach (var existingVoice in distinctVoices) - { - if (VoiceModelMap.TryGetValue(existingVoice, out var existingModel) - && string.Equals(existingModel, modelName, StringComparison.OrdinalIgnoreCase)) - { - alreadyRepresented = true; - break; - } - } - - if (!alreadyRepresented) - { - distinctVoices.Add(languageCode); - } - } - } - - return distinctVoices; -} +/// <summary> +/// Returns the set of distinct voice identifiers (one per unique model file) +/// that should be warmed at startup. The identifiers are chosen as the first +/// language code that maps to each model, providing a deterministic and +/// minimal set of voices to pre-load. +/// </summary> +public IReadOnlySet<string> GetDistinctVoiceIdentifiers() +{ + var seenModels = new HashSet<string>(StringComparer.OrdinalIgnoreCase); + var result = new HashSet<string>(StringComparer.OrdinalIgnoreCase); + + foreach (var (languageCode, modelName) in VoiceModelMap) + { + if (seenModels.Add(modelName)) + result.Add(languageCode); + } + + return result; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Tts/Services/AudioProcessingService.cs` around lines 106 - 137, GetDistinctVoiceIdentifiers currently uses an unnecessary outer distinctVoices.Contains(languageCode) guard and an O(n²) inner loop to detect duplicate model names; replace that logic by keeping a second HashSet<string> seenModels (StringComparer.OrdinalIgnoreCase) and iterate VoiceModelMap entries, adding the languageCode to distinctVoices only when seenModels does not contain the modelName, then add modelName to seenModels; update the method (GetDistinctVoiceIdentifiers, VoiceModelMap, distinctVoices) to return the distinctVoices set.Web/Resgrid.Web.Services/Controllers/TwilioController.cs (2)
521-526: 💤 Low valueEncode
userIdwhen interpolating into the action URL.
userIdis a route parameter passed verbatim into the query string of theGatheraction URI. ASP.NET will accept identifier-like values fine, but characters such as&,+,#, or spaces (which can legitimately appear in some ASP.NET Identity user IDs depending on provider) would corrupt the resulting URL. Other endpoints in this file (e.g., line 542, 548, 558, 625, 657) have the same shape and would benefit from the same treatment, but at minimum the dispatch flow added here shouldUri.EscapeDataString(userId)to be consistent with the proposed encoding in the redirect path above.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Services/Controllers/TwilioController.cs` around lines 521 - 526, The Gather action URI is interpolating userId directly (in the Gather creation near Gather, AppendVoicePromptAsync, TwilioVoicePromptCatalog.OutboundDispatchMenu and Config.SystemBehaviorConfig.ResgridApiBaseUrl with callId), which can break the querystring for IDs containing special characters; fix by encoding the userId with Uri.EscapeDataString(userId) when building the action URL (and apply the same change to other similar interpolations in this controller such as the redirect/voice action URIs).
1115-1131: ⚡ Quick winCatch transient TTS failures so they fall back to the redirect path instead of erroring the call.
The
tryonly handlesOperationCanceledExceptiontriggered by the timeout. Any other exception fromGetPromptUrlAsync(HTTP failure to the TTS microservice, transient cache exception, malformed response, etc.) will bubble up, causing a 500 from this Twilio webhook and the outbound call to fail outright. PerTwilioVoiceResponseService.GetOrCreatePromptUrlAsync(Web/Resgrid.Web.Services/Twilio/TwilioVoiceResponseService.cs:196-228), the cache entry is removed on non-cancellation exceptions, so a generic catch here would let the next redirect retry generation cleanly. Consider catching, logging viaResgrid.Framework.Logging.LogException, and returningfalseso the caller's pre-warm/redirect path takes over.🛡️ Proposed graceful fallback
try { var url = await _twilioVoiceResponseService.GetPromptUrlAsync(dispatchText, ttsLanguage, linkedCts.Token); response.Append(new Play { Url = url }); return true; } catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested) { // TTS generation is taking too long — return false so the caller // can pre-warm in the background and redirect. return false; } + catch (Exception ex) + { + Framework.Logging.LogException(ex); + return false; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Services/Controllers/TwilioController.cs` around lines 1115 - 1131, The code only catches OperationCanceledException; update the try/catch around _twilioVoiceResponseService.GetPromptUrlAsync(dispatchText, ttsLanguage, linkedCts.Token) to also catch general exceptions, log them with Resgrid.Framework.Logging.LogException (including context such as dispatchText and ttsLanguage or a short message), and return false so the caller can follow the pre-warm/redirect path; keep the existing OperationCanceledException branch for timeout handling and ensure the catch-all does not rethrow.Tests/Resgrid.Tests/Web/Services/TwilioControllerVoiceVerificationTests.cs (1)
116-122: ⚡ Quick winConsider adding a test that exercises the redirect path when
GetPromptUrlAsynctimes out.The mock here always returns a ready URL, so the new
dispatchReady == falsebranch inTwilioController.VoiceCall(the pre-warm + redirect withretry=1) is not covered by any test. A test that configuresGetPromptUrlAsyncto throwOperationCanceledException(or to delay long enough for the controller's 3-second linked CTS to fire) would lock down the redirect URL, thePleaseWaitForDispatchprompt, and aPreWarmPromptAsyncinvocation — all of which are the new contract introduced in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/Resgrid.Tests/Web/Services/TwilioControllerVoiceVerificationTests.cs` around lines 116 - 122, Add a unit test in TwilioControllerVoiceVerificationTests that simulates the timeout branch by configuring the _twilioVoiceResponseServiceMock.Setup(x => x.GetPromptUrlAsync(...)) to throw an OperationCanceledException (or to delay past the controller’s 3s linked CTS) and ensure TwilioController.VoiceCall responds with a redirect URL containing retry=1, that the voice response contains the "PleaseWaitForDispatch" prompt, and that _twilioVoiceResponseServiceMock.PreWarmPromptAsync(...) is invoked; use the existing test arrangement for request creation and asserts to verify the redirect location, the presence of the wait prompt, and a single PreWarmPromptAsync call.Web/Resgrid.Web/Areas/User/Views/Dispatch/ScheduledCalls.cshtml (1)
44-44: 💤 Low valueOptional: declare table header explicitly.
DataTables can render headers from the
columnsconfig, but it's slightly more robust (and avoids brief layout flicker before init) to declare an explicit<thead>row matching the JS columns. Not required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/ScheduledCalls.cshtml` at line 44, The table with id "scheduledCallsList" lacks an explicit header which can cause a brief layout flicker; add a <thead> element inside the table with a single <tr> whose <th> cells match the DataTables columns configuration (labels/order) used when initializing "scheduledCallsList" so the HTML header aligns with the JS columns and prevents flicker before DataTables initializes.Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js (1)
22-22: 💤 Low valueSort by
DispatchOnmay not be chronological.
order: [[3, 'asc']]sorts theDispatchOncolumn, but the data returned is an ISO-ish string and DataTables will sort it lexically. That's "usually" chronological for ISO 8601, but only when the strings have a consistent shape (Z vs offset vs no suffix). If you switchDispatchOnto a Unix-secondslong?(per the suggestion onCallListJson.cs), numeric ordering is unambiguous; otherwise consider providing a sort-friendly value via DataTables' orthogonal data (renderreturning different values fortype === 'sort'vs'display').🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js` at line 22, The DataTables sort currently uses order: [[3, 'asc']] on the DispatchOn column which sorts the ISO-ish string lexically and can be incorrect; either change the server-side CallListJson.cs to return DispatchOn as a numeric Unix-seconds long (so numeric ordering is correct) or, in Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js add a columnDefs/render for the DispatchOn column that returns a numeric value when type === 'sort' (and the human-friendly formatted string for type === 'display') so DataTables sorts chronologically while keeping the display format.Web/Resgrid.Web/Areas/User/Models/Dispatch/CallListJson.cs (1)
21-21: ⚡ Quick winInconsistent timestamp serialization vs.
LoggedOn.
LoggedOnis along(Unix seconds) so the client doesn't have to deal withDateTime.Kind/timezone parsing, butDispatchOnis exposed asDateTime?. InDispatchController.GetScheduledCallsList,callJson.DispatchOn = call.DispatchOnis assigned withoutSpecifyKind(..., Utc), so the serialized value typically ships without aZ/offset and the JS consumer inresgrid.dispatch.scheduledcalls.js(new Date(data).toLocaleString()) will interpret it as local time on some browsers and as UTC on others — producing inconsistent displays.For consistency and to remove the timezone ambiguity, prefer the same shape as
LoggedOn:♻️ Proposed fix
- public DateTime? DispatchOn { get; set; } + public long? DispatchOn { get; set; }And in
DispatchController.GetScheduledCallsList:callJson.DispatchOn = call.DispatchOn.HasValue ? new DateTimeOffset(DateTime.SpecifyKind(call.DispatchOn.Value, DateTimeKind.Utc)).ToUnixTimeSeconds() : (long?)null;Then on the JS side render with
new Date(data * 1000).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Models/Dispatch/CallListJson.cs` at line 21, Change DispatchOn from DateTime? to a Unix-seconds long? to match LoggedOn (update the property on CallListJson: DispatchOn -> long?), and in DispatchController.GetScheduledCallsList set callJson.DispatchOn by converting call.DispatchOn to UTC then to Unix seconds (use DateTime.SpecifyKind(call.DispatchOn.Value, DateTimeKind.Utc) wrapped in DateTimeOffset and ToUnixTimeSeconds()), returning null if no value; update client code to use new Date(data * 1000) when rendering.Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs (1)
429-440: ⚡ Quick winConsider extracting the scheduled-dispatch handling into a shared helper.
NewCallandUpdateCallcarry near-identical 15-minute validation, error-key, andDispatchOn/HasBeenDispatchedassignment logic. Extracting a private helper (e.g.,TryApplyScheduledDispatch(Call call, DateTime? scheduleDate, out string error)) would eliminate the duplication and make the validation rule (and any future timezone handling) authoritative in one place. This is non-blocking; raising it here so the two paths don't drift.Also applies to: 851-867
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs` around lines 429 - 440, There is duplicated scheduled-dispatch logic in DispatchController.NewCall and DispatchController.UpdateCall (the 15-minute validation, ModelState error key "ScheduleDispatchDate" and assignments to Call.DispatchOn / Call.HasBeenDispatched); create a private helper like TryApplyScheduledDispatch(Call call, DateTime? scheduleDate, out string error) (or bool TryApplyScheduledDispatch(..., out string error)) that performs the DateTime.UtcNow.AddMinutes(15) check, returns the localized error via _dispatchLocalizer["ScheduleDispatchValidationError"].Value when invalid, and sets call.DispatchOn and call.HasBeenDispatched when valid; replace the duplicated blocks in NewCall and UpdateCall with calls to this helper and add ModelState.AddModelError("ScheduleDispatchDate", error) when the helper reports failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Web/Resgrid.Web.Services/Controllers/TwilioController.cs`:
- Around line 499-518: The redirect can loop indefinitely because retry is set
but never read; update the TwilioController logic that calls
TryAppendDispatchPlaybackAsync to parse and increment a retry query param (e.g.,
read "retry" from the incoming request), cap retries (e.g., MAX_RETRY = 3) and
when the cap is exceeded fall back to a static error prompt or
TwilioVoicePromptCatalog.CallClosed instead of redirecting; also change the
fire-and-forget PreWarmPromptAsync call so its returned task is observed and
faults are logged (wrap
_twilioVoiceResponseService.PreWarmPromptAsync(dispatchText, ttsLanguage) with a
continuation that catches exceptions and calls
Resgrid.Framework.Logging.LogException) — update code paths involving
TryAppendDispatchPlaybackAsync, PreWarmPromptAsync, BuildDispatchPrompt,
GetDepartmentTtsLanguageAsync, AppendVoicePromptAsync and
CreateVoiceContentResult accordingly.
In `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml`:
- Around line 8034-8057: The XML docs mistakenly describe personnel result
wrappers as "populate the New Call form"; update the <summary> text for the
types Resgrid.Web.Services.Models.v4.Personnel.GetAllPersonnelInfosResult,
Resgrid.Web.Services.Models.v4.Personnel.GetPersonnelFilterOptionsResult, and
Resgrid.Web.Services.Models.v4.Personnel.PersonnelInfoResult to accurate
personnel-related descriptions (e.g., "Result containing personnel data" or
"Result containing personnel information for API responses") while leaving the
existing Data property summaries as-is.
- Around line 7671-7679: Update the XML documentation for the
SavePersonnelStaffingInput properties to describe personnel staffing rather than
units: change the summary for member
P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.UserId
to state this is the Id of the user/personnel whose staffing is being set, and
change the summary for member
P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.Type
to describe the staffing type/role (e.g., PersonnelStaffingType or equivalent)
being applied to that user; ensure the text matches the model semantics for
personnel staffing.
- Around line 7711-7714: Update the XML summary for the type
SavePersonnelStaffingsInput to reflect that it handles bulk staffing updates
rather than a single user; replace the current text mentioning “Personnel
Status” and “single user” with a concise description such as “Saves or sets
personnel staffing statuses in the system for multiple users” so the summary
matches the plural intent of the model.
- Around line 7561-7574: The XML summaries for the PersonnelLocation types
incorrectly call them "unit" locations; update the <summary> text for
T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult,
P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult.Data,
and
T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData
to refer to "personnel" (or "personnel member") locations and response data
instead of "unit" to accurately describe the PersonnelLocation model in the
generated docs.
In `@Web/Resgrid.Web.Services/Twilio/TwilioVoiceResponseService.cs`:
- Around line 186-199: Add fast-fail argument validation to the new public
methods PreWarmPromptAsync and GetPromptUrlAsync: check
string.IsNullOrWhiteSpace(text) at the start of each method and throw an
appropriate exception (ArgumentNullException or ArgumentException) with a clear
parameter name/message before calling GetOrCreatePromptUrlAsync or using the
text for cache-key generation so null/blank input cannot reach the cache logic.
- Around line 186-194: PreWarmPromptAsync currently fires
GetOrCreatePromptUrlAsync(text, voice, CancellationToken.None) without observing
its result so any fault is unobserved; capture the returned Task from
GetOrCreatePromptUrlAsync inside PreWarmPromptAsync and attach a continuation
(or await if making the method async) that observes exceptions and logs them
(using the existing logger in this class) to ensure failures are not swallowed,
then return Task.CompletedTask (or the awaited task if you change the signature)
so pre-warm errors are recorded instead of ignored.
In `@Web/Resgrid.Web.Tts/Services/TtsService.cs`:
- Around line 91-101: The loop in TtsService uses raw identifiers from
GetDistinctVoiceIdentifiers() and compares them directly to
_options.DefaultVoice, causing the default voice branch to never hit; instead
resolve each voice to its effective synthesis profile using
GetEffectiveSynthesisProfile (or the service method that maps voice -> resolved
model name) and compare that resolved model name to the resolved model for
_options.DefaultVoice, then skip warming when they match so the English model is
not warmed twice; update the check inside the foreach (and any variable names
like modelWarmPrompt) to use the resolved profile comparison (e.g.,
Resolve/GetEffectiveSynthesisProfile for both the iterated voice and
_options.DefaultVoice) before continuing.
In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs`:
- Around line 851-867: The ScheduleDispatchDate validation currently runs on
every update; change the logic in the UpdateCall handling so you only apply the
">= 15 minutes in future" check when the incoming model.ScheduleDispatchDate
actually differs from the persisted call.DispatchOn (compare values taking nulls
into account) and only then set call.DispatchOn and call.HasBeenDispatched
accordingly; remove the redundant else-if check for model.ScheduleDispatchDate
== null, and when the user clears a previously scheduled dispatch
(model.ScheduleDispatchDate is null but call.DispatchOn had a value) set
call.DispatchOn = null and also reset call.HasBeenDispatched = false to keep
entity state consistent; keep the
ModelState.AddModelError("ScheduleDispatchDate", ...) behavior when the changed
schedule fails the 15-minute rule.
- Around line 2551-2589: GetScheduledCallsList currently filters scheduled calls
to future DispatchOn times and assigns callJson.DispatchOn as a raw DateTime? —
remove or adjust the .Where(x => x.DispatchOn.HasValue && x.DispatchOn.Value >
DateTime.UtcNow) predicate on the calls enumeration so overdue (DispatchOn in
the past but not yet dispatched) items are included like other endpoints, and
change the DispatchOn serialization to match LoggedOn by converting to a Unix
epoch (use new DateTimeOffset(DateTime.SpecifyKind(call.DispatchOn.Value,
DateTimeKind.Utc)).ToUnixTimeSeconds()) when setting callJson.DispatchOn in the
GetScheduledCallsList method so timezone handling is consistent.
- Around line 429-440: The ScheduleDispatchDate value in DispatchController's
NewCall and UpdateCall paths must be converted from the form's unspecified/local
time to UTC before comparing to DateTime.UtcNow.AddMinutes(15) and before
assigning to model.Call.DispatchOn; update the logic around ScheduleDispatchDate
to call
DateTimeHelpers.ConvertToUtc(DateTime.SpecifyKind(model.ScheduleDispatchDate.Value,
DateTimeKind.Unspecified), department.Timezone) (or equivalent) then perform the
>= 15 minutes validation against DateTime.UtcNow and assign the converted UTC
value to model.Call.DispatchOn while preserving model.Call.HasBeenDispatched =
false.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/UpdateCall.cshtml`:
- Around line 612-617: The JS prefill block is overwriting the form-bound
ScheduleDispatchDate with raw UTC Model.Call.DispatchOn and ignores department
timezone; instead, set UpdateCallView.ScheduleDispatchDate from the controller
GET (e.g., view.ScheduleDispatchDate =
call.DispatchOn?.TimeConverter(department) or equivalent
FormatForDepartment/TimeConverterToString helper) so the rendered asp-for input
already contains the timezone-converted value, then remove the `@if`
(Model.Call.DispatchOn.HasValue) JS branch (or, if JS must remain, read
Model.ScheduleDispatchDate rather than Model.Call.DispatchOn) and ensure the
datetimepicker format matches the rendered value.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js`:
- Around line 27-36: The Scheduled Dispatch column is timezone-ambiguous and not
localized: ensure
CallListJson.DispatchOn/DispatchController.GetScheduledCallsList emits an
unambiguous UTC value (either change DispatchOn to a Unix-seconds long? like
LoggedOn, or call DateTime.SpecifyKind(value, DateTimeKind.Utc) before
serializing) so the browser won't interpret an unspecified ISO string
inconsistently, and update the column's title to use getText(...) (replace
title: 'Scheduled Dispatch' with title: getText('ScheduledDispatch') or similar
key) so it is localized consistently with the other columns.
---
Nitpick comments:
In `@Tests/Resgrid.Tests/Web/Services/TwilioControllerVoiceVerificationTests.cs`:
- Around line 116-122: Add a unit test in TwilioControllerVoiceVerificationTests
that simulates the timeout branch by configuring the
_twilioVoiceResponseServiceMock.Setup(x => x.GetPromptUrlAsync(...)) to throw an
OperationCanceledException (or to delay past the controller’s 3s linked CTS) and
ensure TwilioController.VoiceCall responds with a redirect URL containing
retry=1, that the voice response contains the "PleaseWaitForDispatch" prompt,
and that _twilioVoiceResponseServiceMock.PreWarmPromptAsync(...) is invoked; use
the existing test arrangement for request creation and asserts to verify the
redirect location, the presence of the wait prompt, and a single
PreWarmPromptAsync call.
In `@Web/Resgrid.Web.Services/Controllers/TwilioController.cs`:
- Around line 521-526: The Gather action URI is interpolating userId directly
(in the Gather creation near Gather, AppendVoicePromptAsync,
TwilioVoicePromptCatalog.OutboundDispatchMenu and
Config.SystemBehaviorConfig.ResgridApiBaseUrl with callId), which can break the
querystring for IDs containing special characters; fix by encoding the userId
with Uri.EscapeDataString(userId) when building the action URL (and apply the
same change to other similar interpolations in this controller such as the
redirect/voice action URIs).
- Around line 1115-1131: The code only catches OperationCanceledException;
update the try/catch around
_twilioVoiceResponseService.GetPromptUrlAsync(dispatchText, ttsLanguage,
linkedCts.Token) to also catch general exceptions, log them with
Resgrid.Framework.Logging.LogException (including context such as dispatchText
and ttsLanguage or a short message), and return false so the caller can follow
the pre-warm/redirect path; keep the existing OperationCanceledException branch
for timeout handling and ensure the catch-all does not rethrow.
In `@Web/Resgrid.Web.Services/Twilio/ITwilioVoiceResponseService.cs`:
- Line 32: The GetPromptUrlAsync declaration omits the optional default for its
CancellationToken parameter, forcing callers to supply a token; update the
interface method GetPromptUrlAsync(string text, string voice, CancellationToken
cancellationToken) to make the token optional by adding = default so its
signature matches the other AppendPrompt* overloads and callers can omit the
token.
In `@Web/Resgrid.Web.Tts/Services/AudioProcessingService.cs`:
- Around line 106-137: GetDistinctVoiceIdentifiers currently uses an unnecessary
outer distinctVoices.Contains(languageCode) guard and an O(n²) inner loop to
detect duplicate model names; replace that logic by keeping a second
HashSet<string> seenModels (StringComparer.OrdinalIgnoreCase) and iterate
VoiceModelMap entries, adding the languageCode to distinctVoices only when
seenModels does not contain the modelName, then add modelName to seenModels;
update the method (GetDistinctVoiceIdentifiers, VoiceModelMap, distinctVoices)
to return the distinctVoices set.
In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs`:
- Around line 429-440: There is duplicated scheduled-dispatch logic in
DispatchController.NewCall and DispatchController.UpdateCall (the 15-minute
validation, ModelState error key "ScheduleDispatchDate" and assignments to
Call.DispatchOn / Call.HasBeenDispatched); create a private helper like
TryApplyScheduledDispatch(Call call, DateTime? scheduleDate, out string error)
(or bool TryApplyScheduledDispatch(..., out string error)) that performs the
DateTime.UtcNow.AddMinutes(15) check, returns the localized error via
_dispatchLocalizer["ScheduleDispatchValidationError"].Value when invalid, and
sets call.DispatchOn and call.HasBeenDispatched when valid; replace the
duplicated blocks in NewCall and UpdateCall with calls to this helper and add
ModelState.AddModelError("ScheduleDispatchDate", error) when the helper reports
failure.
In `@Web/Resgrid.Web/Areas/User/Models/Dispatch/CallListJson.cs`:
- Line 21: Change DispatchOn from DateTime? to a Unix-seconds long? to match
LoggedOn (update the property on CallListJson: DispatchOn -> long?), and in
DispatchController.GetScheduledCallsList set callJson.DispatchOn by converting
call.DispatchOn to UTC then to Unix seconds (use
DateTime.SpecifyKind(call.DispatchOn.Value, DateTimeKind.Utc) wrapped in
DateTimeOffset and ToUnixTimeSeconds()), returning null if no value; update
client code to use new Date(data * 1000) when rendering.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/ScheduledCalls.cshtml`:
- Line 44: The table with id "scheduledCallsList" lacks an explicit header which
can cause a brief layout flicker; add a <thead> element inside the table with a
single <tr> whose <th> cells match the DataTables columns configuration
(labels/order) used when initializing "scheduledCallsList" so the HTML header
aligns with the JS columns and prevents flicker before DataTables initializes.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js`:
- Line 22: The DataTables sort currently uses order: [[3, 'asc']] on the
DispatchOn column which sorts the ISO-ish string lexically and can be incorrect;
either change the server-side CallListJson.cs to return DispatchOn as a numeric
Unix-seconds long (so numeric ordering is correct) or, in
Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js
add a columnDefs/render for the DispatchOn column that returns a numeric value
when type === 'sort' (and the human-friendly formatted string for type ===
'display') so DataTables sorts chronologically while keeping the display format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f8f8de3e-9cc9-4cb9-bd50-24ce975ee69d
⛔ Files ignored due to path filters (18)
Core/Resgrid.Localization/Areas/User/Dispatch/Call.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.uk.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.uk.resxis excluded by!**/*.resx
📒 Files selected for processing (22)
Core/Resgrid.Model/TwilioVoicePromptCatalog.csCore/Resgrid.Services/WeatherAlertService.csTests/Resgrid.Tests/Web/Services/TwilioControllerVoiceVerificationTests.csTests/Resgrid.Tests/Web/Tts/TtsServiceTests.csWeb/Resgrid.Web.Services/Controllers/TwilioController.csWeb/Resgrid.Web.Services/Controllers/TwilioProviderController.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xmlWeb/Resgrid.Web.Services/Twilio/ITwilioVoiceResponseService.csWeb/Resgrid.Web.Services/Twilio/TwilioVoiceResponseService.csWeb/Resgrid.Web.Tts/Configuration/TtsOptions.csWeb/Resgrid.Web.Tts/Services/AudioProcessingService.csWeb/Resgrid.Web.Tts/Services/IAudioProcessingService.csWeb/Resgrid.Web.Tts/Services/TtsService.csWeb/Resgrid.Web/Areas/User/Controllers/DispatchController.csWeb/Resgrid.Web/Areas/User/Models/Calls/NewCallView.csWeb/Resgrid.Web/Areas/User/Models/Calls/UpdateCallView.csWeb/Resgrid.Web/Areas/User/Models/Dispatch/CallListJson.csWeb/Resgrid.Web/Areas/User/Views/Dispatch/Dashboard.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/ScheduledCalls.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/UpdateCall.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js
💤 Files with no reviewable changes (1)
- Web/Resgrid.Web.Services/Controllers/TwilioProviderController.cs
Comment on lines
+7561
to
7574
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult"> | ||
| <summary> | ||
| Information about a User | ||
| A unit location in the Resgrid system | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.Personnel.PersonnelInfoResultData.UserId"> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult.Data"> | ||
| <summary> | ||
| The UserId GUID/UUID for the user | ||
| Response Data | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.Personnel.PersonnelInfoResultData.DepartmentId"> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData"> | ||
| <summary> | ||
| DepartmentId of the deparment the user belongs to | ||
| The information about a specific unit's location | ||
| </summary> |
Contributor
There was a problem hiding this comment.
Fix entity mismatch in personnel-location summaries.
Line [7563] and Line [7573] describe these types as “unit” location objects, but they are under PersonnelLocation. This will mislead API consumers reading generated docs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml` around lines 7561 - 7574,
The XML summaries for the PersonnelLocation types incorrectly call them "unit"
locations; update the <summary> text for
T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult,
P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult.Data,
and
T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData
to refer to "personnel" (or "personnel member") locations and response data
instead of "unit" to accurately describe the PersonnelLocation model in the
generated docs.
Comment on lines
+7671
to
7679
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.UserId"> | ||
| <summary> | ||
| Sorting weight for the user | ||
| UnitId of the apparatus that the state is being set for | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.Personnel.PersonnelInfoResultData.UdfValues"> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.Type"> | ||
| <summary> | ||
| User Defined Field values for this personnel record | ||
| The UnitStateType of the Unit | ||
| </summary> |
Contributor
There was a problem hiding this comment.
Correct SavePersonnelStaffingInput property descriptions.
Line [7673] and Line [7678] still refer to unit/apparatus semantics (UnitId, UnitStateType) instead of personnel staffing. The docs should describe user staffing fields to match the model.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml` around lines 7671 - 7679,
Update the XML documentation for the SavePersonnelStaffingInput properties to
describe personnel staffing rather than units: change the summary for member
P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.UserId
to state this is the Id of the user/personnel whose staffing is being set, and
change the summary for member
P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.Type
to describe the staffing type/role (e.g., PersonnelStaffingType or equivalent)
being applied to that user; ensure the text matches the model semantics for
personnel staffing.
Comment on lines
+7711
to
7714
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingsInput"> | ||
| <summary> | ||
| GPS Altitude of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationInput.AltitudeAccuracy"> | ||
| <summary> | ||
| GPS Altitude Accuracy of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationInput.Speed"> | ||
| <summary> | ||
| GPS Speed of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationInput.Heading"> | ||
| <summary> | ||
| GPS Heading of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult"> | ||
| <summary> | ||
| A unit location in the Resgrid system | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResult.Data"> | ||
| <summary> | ||
| Response Data | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData"> | ||
| <summary> | ||
| The information about a specific unit's location | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.UserId"> | ||
| <summary> | ||
| Id of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.Timestamp"> | ||
| <summary> | ||
| The Timestamp for the location in UTC | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.Latitude"> | ||
| <summary> | ||
| GPS Latitude of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.Longitude"> | ||
| <summary> | ||
| GPS Longitude of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.Accuracy"> | ||
| <summary> | ||
| GPS Latitude\Longitude Accuracy of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.Altitude"> | ||
| <summary> | ||
| GPS Altitude of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.AltitudeAccuracy"> | ||
| <summary> | ||
| GPS Altitude Accuracy of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.Speed"> | ||
| <summary> | ||
| GPS Speed of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelLocation.PersonnelLocationResultData.Heading"> | ||
| <summary> | ||
| GPS Heading of the Person | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResult"> | ||
| <summary> | ||
| The result of getting the current staffing for a user | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResult.Data"> | ||
| <summary> | ||
| Response Data | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResultData"> | ||
| <summary> | ||
| Information about a User staffing | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResultData.UserId"> | ||
| <summary> | ||
| The UserId GUID/UUID for the user status being return | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResultData.DepartmentId"> | ||
| <summary> | ||
| DepartmentId of the deparment the user belongs to | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResultData.StaffingType"> | ||
| <summary> | ||
| The current staffing type for the user | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResultData.TimestampUtc"> | ||
| <summary> | ||
| The timestamp of the last staffing. This is converted UTC version of the timestamp. | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResultData.Timestamp"> | ||
| <summary> | ||
| The timestamp of the last staffing. This is converted UTC to the departments, or users, TimeZone. | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.GetCurrentStaffingResultData.Note"> | ||
| <summary> | ||
| Note for this staffing | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput"> | ||
| <summary> | ||
| Saves (sets) and Personnel Staffing in the system, for a single user | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.UserId"> | ||
| <summary> | ||
| UnitId of the apparatus that the state is being set for | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.Type"> | ||
| <summary> | ||
| The UnitStateType of the Unit | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.TimestampUtc"> | ||
| <summary> | ||
| The timestamp of the status event in UTC | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.Timestamp"> | ||
| <summary> | ||
| The timestamp of the status event in the local time of the device | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.Note"> | ||
| <summary> | ||
| User provided note for this event | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingInput.EventId"> | ||
| <summary> | ||
| The event id used for queuing on mobile applications | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingResult"> | ||
| <summary> | ||
| Depicts a result after saving a person status | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingResult.Id"> | ||
| <summary> | ||
| Response Data | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.PersonnelStaffing.SavePersonnelStaffingsInput"> | ||
| <summary> | ||
| Saves (sets) and Personnel Status in the system, for a single user | ||
| Saves (sets) and Personnel Status in the system, for a single user | ||
| </summary> |
Contributor
There was a problem hiding this comment.
SavePersonnelStaffingsInput summary contradicts the model intent.
Line [7713] says “Personnel Status” and “single user”, but this type is plural staffing input. The summary should reflect bulk staffing updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml` around lines 7711 - 7714,
Update the XML summary for the type SavePersonnelStaffingsInput to reflect that
it handles bulk staffing updates rather than a single user; replace the current
text mentioning “Personnel Status” and “single user” with a concise description
such as “Saves or sets personnel staffing statuses in the system for multiple
users” so the summary matches the plural intent of the model.
Comment on lines
+8034
to
+8057
| <member name="T:Resgrid.Web.Services.Models.v4.Personnel.GetAllPersonnelInfosResult"> | ||
| <summary> | ||
| Result containing all the data required to populate the New Call form | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.Personnel.GetAllPersonnelInfosResult.Data"> | ||
| <summary> | ||
| Response Data | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.Personnel.GetPersonnelFilterOptionsResult"> | ||
| <summary> | ||
| Result that contains all the options available to filter personnel against compatible Resgrid APIs | ||
| </summary> | ||
| </member> | ||
| <member name="P:Resgrid.Web.Services.Models.v4.Personnel.GetPersonnelFilterOptionsResult.Data"> | ||
| <summary> | ||
| Response Data | ||
| </summary> | ||
| </member> | ||
| <member name="T:Resgrid.Web.Services.Models.v4.Personnel.PersonnelInfoResult"> | ||
| <summary> | ||
| Result containing all the data required to populate the New Call form | ||
| </summary> |
Contributor
There was a problem hiding this comment.
Replace copy-pasted “New Call form” summaries in personnel result wrappers.
Line [8036] and Line [8056] describe personnel result wrappers as “populate the New Call form,” which is inaccurate for personnel endpoints and degrades API doc quality.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml` around lines 8034 - 8057,
The XML docs mistakenly describe personnel result wrappers as "populate the New
Call form"; update the <summary> text for the types
Resgrid.Web.Services.Models.v4.Personnel.GetAllPersonnelInfosResult,
Resgrid.Web.Services.Models.v4.Personnel.GetPersonnelFilterOptionsResult, and
Resgrid.Web.Services.Models.v4.Personnel.PersonnelInfoResult to accurate
personnel-related descriptions (e.g., "Result containing personnel data" or
"Result containing personnel information for API responses") while leaving the
existing Data property summaries as-is.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Web/Resgrid.Web.Services/Controllers/TwilioController.cs (1)
1109-1152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed here instead of failing the Twilio webhook.
This helper is the graceful-degradation path for dispatch playback, but only the timeout case returns
false. IfGetShortenedAudioUrlAsync, reverse geocoding, orGetPromptUrlAsyncthrows any other transient exception,/VoiceCallreturns 500 and the caller never reaches thePleaseWaitForDispatchretry flow.Suggested fix
private async System.Threading.Tasks.Task<bool> TryAppendDispatchPlaybackAsync(VoiceResponse response, Call call) { - if (call.Attachments != null) - { - var audio = call.Attachments.FirstOrDefault(x => x.CallAttachmentType == (int)CallAttachmentTypes.DispatchAudio); - - if (audio != null) - { - var url = await _callsService.GetShortenedAudioUrlAsync(call.CallId, audio.CallAttachmentId); - if (!string.IsNullOrWhiteSpace(url) && Uri.TryCreate(url, UriKind.Absolute, out var audioUri)) - { - response.Append(new Play - { - Url = audioUri - }); - return true; - } - } - } - - var address = await ResolveCallAddressAsync(call); - var ttsLanguage = await GetDepartmentTtsLanguageAsync(call.DepartmentId); - var dispatchText = BuildDispatchPrompt(call, address); - - using var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(3)); - using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource( - timeoutCts.Token, - HttpContext?.RequestAborted ?? CancellationToken.None); - - try - { - var url = await _twilioVoiceResponseService.GetPromptUrlAsync(dispatchText, ttsLanguage, linkedCts.Token); - response.Append(new Play { Url = url }); - return true; - } - catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested) - { - return false; - } + try + { + if (call.Attachments != null) + { + var audio = call.Attachments.FirstOrDefault(x => x.CallAttachmentType == (int)CallAttachmentTypes.DispatchAudio); + + if (audio != null) + { + var url = await _callsService.GetShortenedAudioUrlAsync(call.CallId, audio.CallAttachmentId); + if (!string.IsNullOrWhiteSpace(url) && Uri.TryCreate(url, UriKind.Absolute, out var audioUri)) + { + response.Append(new Play { Url = audioUri }); + return true; + } + } + } + + var address = await ResolveCallAddressAsync(call); + var ttsLanguage = await GetDepartmentTtsLanguageAsync(call.DepartmentId); + var dispatchText = BuildDispatchPrompt(call, address); + + using var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(3)); + using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource( + timeoutCts.Token, + HttpContext?.RequestAborted ?? CancellationToken.None); + + try + { + var url = await _twilioVoiceResponseService.GetPromptUrlAsync(dispatchText, ttsLanguage, linkedCts.Token); + response.Append(new Play { Url = url }); + return true; + } + catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested) + { + return false; + } + } + catch (OperationCanceledException) when ((HttpContext?.RequestAborted ?? CancellationToken.None).IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + Logging.LogException(ex); + return false; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Services/Controllers/TwilioController.cs` around lines 1109 - 1152, TryAppendDispatchPlaybackAsync currently only returns false on a timeout, causing any other exceptions from GetShortenedAudioUrlAsync, ResolveCallAddressAsync, GetDepartmentTtsLanguageAsync, BuildDispatchPrompt, or GetPromptUrlAsync to bubble up and make the Twilio webhook fail; wrap the sections that call those methods in appropriate try/catch(s) (around the audio URL retrieval block and the TTS/address generation block) and on non-fatal/transient exceptions catch and log the error (use the controller logger) and return false so the caller can fall back to the PleaseWaitForDispatch/redirect flow instead of returning 500. Ensure you still preserve the existing timeout-specific OperationCanceledException handling for GetPromptUrlAsync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Web/Resgrid.Web.Services/Twilio/TwilioVoiceResponseService.cs`:
- Around line 187-208: PreWarmPromptAsync and GetPromptUrlAsync currently pass
raw text into the single-chunk path and can fail for inputs that
AppendPromptAsync handles via chunking; update these methods to either (a) reuse
the same normalization/chunking routine used by AppendPromptAsync (call the
shared "normalize/chunk" function before calling GetOrCreatePromptUrlAsync and
then handle/aggregate multiple chunks the same way AppendPromptAsync does), or
(b) explicitly reject multi-chunk input up front by running the chunker and
throwing an ArgumentException if it returns more than one chunk; apply this
change to both PreWarmPromptAsync and GetPromptUrlAsync (referencing
AppendPromptAsync, the shared normalization/chunking helper, and
GetOrCreatePromptUrlAsync).
In `@Web/Resgrid.Web.Tts/Services/TtsService.cs`:
- Around line 95-103: Phase 2 currently skips any voice whose effective model
equals defaultProfile.Voice (inside the foreach over distinctVoices using
_audioProcessingService.GetEffectiveSynthesisProfile), but if
_options.PreGeneratedPrompts was empty then phase 1 never warmed the
defaultProfile; update the logic to track whether phase 1 actually warmed the
default model (e.g. set a flag when any prompt warms defaultProfile) and only
skip in the phase-2 loop when that flag is true; ensure the check around
defaultProfile.Voice in the foreach only continues when the phase1-warmed flag
indicates the default was warmed so the default model still gets warmed in phase
2 when phase 1 did nothing.
In `@Web/Resgrid.Web/Areas/User/Models/Calls/UpdateCallView.cs`:
- Around line 40-41: The ScheduleDispatchDate property uses
[DisplayFormat(DataFormatString = "{0:MM/dd/yyyy HH:mm}")] but lacks
ApplyFormatInEditMode, so the edit input value is rendered in culture-specific
format and breaks the datetime picker; update the attribute on
ScheduleDispatchDate in UpdateCallView (and add the same attribute to the
ScheduleDispatchDate property in NewCallView for consistency) to include
ApplyFormatInEditMode = true so the value is formatted for edit-mode inputs to
match the picker’s 'MM/dd/yyyy HH:mm' (24-hour) format.
---
Outside diff comments:
In `@Web/Resgrid.Web.Services/Controllers/TwilioController.cs`:
- Around line 1109-1152: TryAppendDispatchPlaybackAsync currently only returns
false on a timeout, causing any other exceptions from GetShortenedAudioUrlAsync,
ResolveCallAddressAsync, GetDepartmentTtsLanguageAsync, BuildDispatchPrompt, or
GetPromptUrlAsync to bubble up and make the Twilio webhook fail; wrap the
sections that call those methods in appropriate try/catch(s) (around the audio
URL retrieval block and the TTS/address generation block) and on
non-fatal/transient exceptions catch and log the error (use the controller
logger) and return false so the caller can fall back to the
PleaseWaitForDispatch/redirect flow instead of returning 500. Ensure you still
preserve the existing timeout-specific OperationCanceledException handling for
GetPromptUrlAsync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c78fa09-a69a-425c-ad25-d0a45836e819
📒 Files selected for processing (8)
Web/Resgrid.Web.Services/Controllers/TwilioController.csWeb/Resgrid.Web.Services/Twilio/TwilioVoiceResponseService.csWeb/Resgrid.Web.Tts/Services/TtsService.csWeb/Resgrid.Web/Areas/User/Controllers/DispatchController.csWeb/Resgrid.Web/Areas/User/Models/Calls/UpdateCallView.csWeb/Resgrid.Web/Areas/User/Models/Dispatch/CallListJson.csWeb/Resgrid.Web/Areas/User/Views/Dispatch/UpdateCall.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js
🚧 Files skipped from review as they are similar to previous changes (2)
- Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.scheduledcalls.js
- Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs
Member
Author
|
Approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests