Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe PR adds a moderation workflow across storage, services, APIs, controllers, and chat interfaces. It also updates notification matching, localization, validation, message metadata, database migrations, and several isolated controller and template behaviors. ChangesModeration platform
Supporting behavior updates
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Reporter
participant ModerationController
participant ModerationService
participant ModerationRepository
participant Moderator
Reporter->>ModerationController: submit report
ModerationController->>ModerationService: flag content
ModerationService->>ModerationRepository: store request and report
ModerationRepository-->>ModerationService: return moderation data
ModerationService-->>Reporter: return report status
Moderator->>ModerationController: search or complete request
ModerationController->>ModerationService: authorize and complete
ModerationService->>ModerationRepository: store action and status
ModerationService-->>Moderator: return completed request
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.14.0)Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chat.cssConfigurationError: Could not find "stylelint-config-sass-guidelines". Do you need to install the package or use the "configBasedir" option? 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Core/Resgrid.Services/NotificationService.cs`:
- Around line 507-511: Update the single-select comparisons in the notification
matching logic around the existing before/current data branches to test
normalized values for exact equality with "-1", not substring containment. Apply
this consistently to the early wildcard match and both beforeAny/currentAny
assignments, including the corresponding branches around the additional affected
locations, so values such as "-10" are not treated as the wildcard.
🪄 Autofix
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 Plus
Run ID: 9c388c78-29fd-42a9-9a14-01652d4b0775
⛔ Files ignored due to path filters (1)
Tests/Resgrid.Tests/Services/NotificationServiceTests.csis excluded by!**/Tests/**
📒 Files selected for processing (2)
Core/Resgrid.Services/NotificationService.csWeb/Resgrid.Web/wwwroot/js/app/internal/notifications/resgrid.notifications.addNotification.js
|
|
||
| if ((currentAny || currentState.State == int.Parse(setting.CurrentData)) && | ||
| (beforeAny || beforeState.State == int.Parse(setting.BeforeData))) | ||
| if ((currentAny || currentState.State == int.Parse(currentData)) && |
There was a problem hiding this comment.
Unsafe string conversion: int.Parse(currentData) is used without TryParse validation for user/IO input. Prefer int.TryParse and validate culture/format where applicable.
Kody rule violation: Use TryParse for string conversions
Prompt for LLM
File Core/Resgrid.Services/NotificationService.cs:
Line 521:
Unsafe string conversion: `int.Parse(currentData)` is used without `TryParse` validation for user/IO input. Prefer `int.TryParse` and validate culture/format where applicable.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (setting.BeforeData.Contains("-1") && setting.CurrentData.Contains("-1")) | ||
| // Empty Before/Current data means "Any": the post-Telerik UI posts "" for the | ||
| // default Any option, so settings saved that way must still match every change. | ||
| var beforeData = String.IsNullOrWhiteSpace(setting.BeforeData) ? "-1" : setting.BeforeData; |
There was a problem hiding this comment.
Magic string literal "-1" is scattered across NotificationService.cs (lines 505–565) and resgrid.notifications.addNotification.js (lines 132–146) as a sentinel meaning "Any" without a named constant. Define a class-level constant private const string AnySelection = "-1"; and replace all inline occurrences.
Kody rule violation: Centralize string constants
Prompt for LLM
File Core/Resgrid.Services/NotificationService.cs:
Line 504:
Magic string literal `"-1"` is scattered across `NotificationService.cs` (lines 505–565) and `resgrid.notifications.addNotification.js` (lines 132–146) as a sentinel meaning "Any" without a named constant. Define a class-level constant `private const string AnySelection = "-1";` and replace all inline occurrences.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (setting.BeforeData.Contains("-1") && setting.CurrentData.Contains("-1")) | ||
| // Empty Before/Current data means "Any": the post-Telerik UI posts "" for the | ||
| // default Any option, so settings saved that way must still match every change. | ||
| var beforeData = String.IsNullOrWhiteSpace(setting.BeforeData) ? "-1" : setting.BeforeData; |
There was a problem hiding this comment.
Triplicated logic: the normalization-plus-state-comparison sequence is duplicated verbatim across the UnitStatusChanged, PersonnelStaffingChanged, and PersonnelStatusChanged cases. Extract a single generic helper such as ValidateStateChangeAsync<TState>(setting, Func<int, Task<TState>> getCurrent, Func<TState, Task<TState>> getBefore, Func<TState,int> stateSelector) and call it from each case.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Core/Resgrid.Services/NotificationService.cs:
Line 504:
Triplicated logic: the normalization-plus-state-comparison sequence is duplicated verbatim across the `UnitStatusChanged`, `PersonnelStaffingChanged`, and `PersonnelStatusChanged` cases. Extract a single generic helper such as `ValidateStateChangeAsync<TState>(setting, Func<int, Task<TState>> getCurrent, Func<TState, Task<TState>> getBefore, Func<TState,int> stateSelector)` and call it from each case.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| DepartmentId = 1, | ||
| MessageId = "123456", | ||
| Data = new NotificationItem() { StateId = 3, DepartmentId = 1, PreviousStateId = 2 }.SerializeProto(), |
There was a problem hiding this comment.
Magic numbers StateId=3 and PreviousStateId=2 lack self-documenting domain meaning. Use enum casts such as StateId=(int)UnitStateTypes.Responding, consistent with existing BeforeData casts in the same test.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File Tests/Resgrid.Tests/Services/NotificationServiceTests.cs:
Line 771:
Magic numbers `StateId=3` and `PreviousStateId=2` lack self-documenting domain meaning. Use enum casts such as `StateId=(int)UnitStateTypes.Responding`, consistent with existing `BeforeData` casts in the same test.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| $('#beforeStateControl').empty().append('<select id="Notification_BeforeData" name="Notification.BeforeData" style="width:100%"></select>'); | ||
| $('#currentStateControl').empty().append('<select id="Notification_CurrentData" name="Notification.CurrentData" style="width:100%"></select>'); | ||
| var url = resgrid.absoluteBaseUrl + '/User/CustomStatuses/GetUnitStatusesLevelsForDepartmentCombined?includeAny=True'; | ||
| var url = resgrid.absoluteBaseUrl + '/User/CustomStatuses/GetUnitStatusesLevelsForDepartmentCombined?includeAny=False'; |
There was a problem hiding this comment.
String concatenation using + violates the team template literals rule. Replace with a template literal to improve readability and reduce error-proneness.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File Web/Resgrid.Web/wwwroot/js/app/internal/notifications/resgrid.notifications.addNotification.js:
Line 132:
String concatenation using `+` violates the team template literals rule. Replace with a template literal to improve readability and reduce error-proneness.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var $sel = $(selector).empty().append('<option value="">-- Any --</option>'); | ||
| // "Any" must post "-1", not "" — the notification engine treats the value as a | ||
| // state id and an empty string used to make the setting never match. | ||
| var $sel = $(selector).empty().append('<option value="-1">-- Any --</option>'); |
There was a problem hiding this comment.
var declaration of $sel violates Rule [37] and risks function-scoping pitfalls. Use const since $sel is never reassigned.
Kody rule violation: Always use const and let
Prompt for LLM
File Web/Resgrid.Web/wwwroot/js/app/internal/notifications/resgrid.notifications.addNotification.js:
Line 119:
`var` declaration of `$sel` violates Rule [37] and risks function-scoping pitfalls. Use `const` since `$sel` is never reassigned.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| public async Task<ActionResult<GetCallResult>> GetCall(string callId, [FromQuery] string departmentId = null) | ||
| { | ||
| if (String.IsNullOrWhiteSpace(callId)) | ||
| if (!int.TryParse(callId, NumberStyles.Integer, CultureInfo.InvariantCulture, out int parsedCallId)) |
| var result = new EditCallResult(); | ||
|
|
||
| var canDoOperation = await _authorizationService.CanUserEditCallAsync(UserId, int.Parse(editCallInput.Id)); | ||
| if (editCallInput == null || !ModelState.IsValid || |
|
|
||
| var canDoOperation = await _authorizationService.CanUserEditCallAsync(UserId, int.Parse(editCallInput.Id)); | ||
| if (editCallInput == null || !ModelState.IsValid || | ||
| !int.TryParse(editCallInput.Id, NumberStyles.Integer, CultureInfo.InvariantCulture, out int callId)) |
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(StatusCodes.Status401Unauthorized)] | ||
| public async Task<ActionResult<ModerationActionResult>> Flag([FromBody] FlagModerationInput input, |
| public async Task<ActionResult<ModerationActionResult>> Flag([FromBody] FlagModerationInput input, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| if (!ModelState.IsValid || input == null) |
|
|
||
| /// <summary>Completes a scoped request with no action or by removing the live content.</summary> | ||
| [HttpPost("Complete")] | ||
| public async Task<ActionResult<ModerationActionResult>> Complete(string requestId, |
| public async Task<ActionResult<ModerationActionResult>> Complete(string requestId, | ||
| [FromBody] CompleteModerationInput input, CancellationToken cancellationToken) | ||
| { | ||
| if (!ModelState.IsValid || input == null || string.IsNullOrWhiteSpace(requestId)) |
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs (1)
1461-1486: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
FlagMessagedoes not handle the exceptionsFlagAsyncthrows.
ModerationService.FlagAsyncthrows in cases this endpoint can reach:
InvalidOperationExceptionwhen the chat message is deleted.CheckMessageChannelAccessAsyncdoes not inspectDeletedOn, so flagging a tombstoned message reachesLoadEvidenceAsyncand throws.ArgumentOutOfRangeExceptionwheninput.Reasonis outside theModerationReasonrange.FlagMessageInput.Reasonis a plainint.UnauthorizedAccessExceptionfromLoadEvidenceAsync.None are caught, so each returns 500.
ModerationController.Flagcatches all three and maps them to 400 or 401. Mirror that handling here.The cast
(ModerationReason)input.Reasonalso couples two independently declared enums. The values align today. Map them explicitly so a future change to either enum fails at compile time instead of silently mislabelling a report.🛠️ Proposed fix
var result = new ChatActionResult(); - var flag = await _moderationService.FlagAsync(DepartmentId, UserId, - ModerationItemType.ChatMessage, messageId, (ModerationReason)input.Reason, input.Note, - BuildModerationContext("Reporter"), cancellationToken); + ModerationReport flag; + + try + { + flag = await _moderationService.FlagAsync(DepartmentId, UserId, + ModerationItemType.ChatMessage, messageId, (ModerationReason)input.Reason, input.Note, + BuildModerationContext("Reporter"), cancellationToken); + } + catch (UnauthorizedAccessException) + { + return Unauthorized(); + } + catch (ArgumentException ex) + { + return BadRequest(ex.Message); + } + catch (InvalidOperationException ex) + { + return BadRequest(ex.Message); + } result.Success = flag != null;
ArgumentOutOfRangeExceptionderives fromArgumentException, so theArgumentExceptioncatch covers the invalid-reason case.🤖 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/v4/ChatController.cs` around lines 1461 - 1486, Update FlagMessage to explicitly map input.Reason to the corresponding ModerationReason value instead of directly casting between enums, and wrap FlagAsync in exception handling matching ModerationController.Flag: map ArgumentException and InvalidOperationException to BadRequest, UnauthorizedAccessException to Unauthorized, and preserve the existing success response for successful flags.Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.ts (1)
341-345: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCopy
IsModeratedfor deleted thread replies.The channel-message branch copies moderation state from
HubDeletedPayload, but the thread-reply branch does not. A moderator-deleted reply remainsIsModerated: falsein local state. Apply the samepayload.IsModerated ?? payload.DeletedByModeratorvalue in this branch.🤖 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/Apps/src/components/chat/chatStore.ts` around lines 341 - 345, Update the thread-reply handling in the loop over state.threadMessagesByRoot to include IsModerated from payload.IsModerated ?? payload.DeletedByModerator when calling upsertThreadMessage, alongside DeletedOn and Body. Preserve the existing reply lookup and early return behavior.Web/Resgrid.Web/Areas/User/Controllers/MessagesController.cs (1)
37-56: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the required dependency-resolution pattern.
These changes add constructor injection for new dependencies. Resolve the dependencies with
Bootstrapper.GetKernel().Resolve<T>()in each constructor instead.
Web/Resgrid.Web/Areas/User/Controllers/MessagesController.cs#L37-L56: ResolveIModerationServiceand the moderation localizer through the required service locator.Web/Resgrid.Web/Areas/User/Controllers/ModerationController.cs#L13-L18: ResolveIDepartmentGroupsServicethrough the required service locator.As per coding guidelines, use
Bootstrapper.GetKernel().Resolve<T>()to resolve dependencies explicitly in constructors rather than constructor injection.🤖 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/MessagesController.cs` around lines 37 - 56, Replace constructor injection with explicit service-locator resolution in MessagesController: remove the IModerationService and moderation localizer parameters and initialize both fields via Bootstrapper.GetKernel().Resolve<T>(). In Web/Resgrid.Web/Areas/User/Controllers/ModerationController.cs lines 13-18, resolve IDepartmentGroupsService through Bootstrapper.GetKernel().Resolve<T>() instead of injecting it; update each constructor accordingly.Source: Coding guidelines
🧹 Nitpick comments (8)
Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs (1)
45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
IChatModerationServicedependency fromChatController.
_chatModerationServiceand its constructor parameter are no longer referenced byChatController; removing them reduces unnecessary injected dependencies and the unused registration can be cleaned up separately if this is its only remaining direct dependency.🤖 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/v4/ChatController.cs` around lines 45 - 46, Remove the unused IChatModerationService field and its constructor parameter from ChatController, and update constructor assignments and calls accordingly while preserving the existing IModerationService dependency.Source: Coding guidelines
Web/Resgrid.Web.Services/Models/v4/Moderation/ModerationApiModels.cs (1)
78-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive moderation ranges from the enums.
FlagAsyncandCompleteRequestAsyncuseEnum.IsDefined/explicit checks forModerationItemType,ModerationReason, and the acceptedModerationDispositionvalues, while the API input uses literalRangeconstraints. If a future enum value is added, validation can block it at the controller before the service rejects it. Bind these properties to their enum types or validate withEnum.IsDefinedso the enum remains the source of truth.Note limits are not an issue here: both migration definitions use
text/int.MaxValue.🤖 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/Models/v4/Moderation/ModerationApiModels.cs` around lines 78 - 101, Replace the literal Range constraints on FlagModerationInput.ItemType, FlagModerationInput.Reason, and CompleteModerationInput.Disposition with enum-based validation using their corresponding moderation enums, preferably by changing the properties to those enum types or applying Enum.IsDefined validation. Preserve the existing note and ItemId validation.Core/Resgrid.Services/ChatMessageService.cs (1)
316-324: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRemove the duplicate moderator flag from the deletion event payload.
DeletedByModeratornow carries the same rawasModeratorvalue asIsModerated, so moderators deleting their own messages publish both fields as true. The chat client derives the display state fromIsModerated, so keep that field and removeDeletedByModeratorfrom the event and type.🤖 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 `@Core/Resgrid.Services/ChatMessageService.cs` around lines 316 - 324, Update the deletion event payload in the message deletion flow around PublishEvent to remove DeletedByModerator = asModerator while retaining message.IsModerated. Remove the corresponding DeletedByModerator property from the event payload type and update any affected consumers to use IsModerated only.Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx (2)
246-246: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConfirm the destructive action before it runs.
The
RemoveContentbutton callscomplete(request, 2)on the first click. The moderation service removes the live content for that disposition, and the table offers no undo. Add a confirmation step, so a mis-click does not delete a message, a call note, or a call image.🤖 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/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx` at line 246, Update the RemoveContent button in ModerationRequestsTable so it asks the user for confirmation before invoking complete(request, 2). Only call complete after confirmation is accepted; preserve the existing isBusy disabled state and behavior for other moderation actions.
71-75: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winIndex the personnel list once.
personNameruns a linearpeople.findfor every content author, every report, and every audit action. With a page of 100 requests and a large personnel roster, the render performs thousands of scans. Build aMaponce and read from it.♻️ Proposed refactor
+ const peopleById = useMemo( + () => new Map(people.map((item) => [item.userId, item.name])), + [people], + ); + const personName = useCallback((userId?: string | null) => { if (!userId) return moderationText('SystemOrUnknown'); - const person = people.find((item) => item.userId === userId); - return person ? `${person.name} (${userId})` : userId; - }, [people]); + const name = peopleById.get(userId); + return name ? `${name} (${userId})` : userId; + }, [peopleById]);🤖 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/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx` around lines 71 - 75, Update the personnel lookup used by personName to build a Map keyed by userId once per people change, then read entries from that Map instead of calling people.find for each author, report, or audit action. Preserve the existing SystemOrUnknown fallback and display formatting for found and missing users.Repositories/Resgrid.Repositories.DataRepository/ModerationRepositories.cs (2)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the no-op ternary.
Both branches return
"r", and every generated statement already hard-codes theralias. Replace the variable with the literal at line 153.♻️ Proposed cleanup
- var requestAlias = postgres ? "r" : "r"; var filters = new List<string>();Then use the literal alias in the PostgreSQL statement:
-FROM {_sqlConfiguration.SchemaName}.moderationrequests {requestAlias} +FROM {_sqlConfiguration.SchemaName}.moderationrequests r🤖 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 `@Repositories/Resgrid.Repositories.DataRepository/ModerationRepositories.cs` at line 69, Remove the no-op requestAlias assignment in the moderation repository and replace its usage in the PostgreSQL statement around the generated query with the literal “r” alias. Preserve the existing SQL behavior and remove the now-unused variable.
180-193: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the
_unitOfWorknull handling consistent.Line 185 tests
_unitOfWork?.Connection, which states that_unitOfWorkcan be null. Line 183 then reads_unitOfWork.Transactioninside the delegate, and line 192 calls_unitOfWork.CreateOrGetConnection(). If_unitOfWorkwere ever null, the delegate throws aNullReferenceExceptionwhen it runs, not at line 185. The same mix exists at lines 244/246 and 298/300.The constructors require the dependency, so remove the null-conditional operator, or guard the whole method.
Also applies to: 243-253, 297-307
🤖 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 `@Repositories/Resgrid.Repositories.DataRepository/ModerationRepositories.cs` around lines 180 - 193, Make null handling consistent in QueryAsync and the corresponding methods around the later query blocks: since constructors require _unitOfWork, remove the null-conditional checks and use _unitOfWork.Connection directly, or consistently guard the entire method before accessing _unitOfWork.Transaction and CreateOrGetConnection(). Apply the same correction to all three affected query methods.Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatFormat.ts (1)
192-192: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the formatter out of the function.
formatRelativeDayruns once per rendered message and once per moderation or export table row. Each call with a one-day-old date constructs a newIntl.RelativeTimeFormat. Create the formatter once at module scope and reuse it.Note that the output is now lowercase in English, for example "yesterday" instead of the previous "Yesterday". Confirm that reads correctly in the cells that show it.
♻️ Proposed change
+const relativeDayFormatter = new Intl.RelativeTimeFormat(undefined, { numeric: 'auto' }); + export function formatRelativeDay(iso: string | null | undefined): string {if (diffDays === 1) { - return new Intl.RelativeTimeFormat(undefined, { numeric: 'auto' }).format(-1, 'day'); + return relativeDayFormatter.format(-1, 'day'); }🤖 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/Apps/src/components/chat/chatFormat.ts` at line 192, Hoist the Intl.RelativeTimeFormat instance used by formatRelativeDay to module scope and reuse it for each one-day-old date instead of constructing it per call. Preserve the relative-day output, and verify the resulting lowercase English text such as “yesterday” reads correctly in message, moderation, and export table cells.
🤖 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 `@Core/Resgrid.Model/Repositories/IChatRepositories.cs`:
- Around line 147-148: Update the delete flow that calls TombstoneAsync so it
derives one effective moderator/sender flag and reuses it for the TombstoneAsync
isModerated argument, message.IsModerated, and delete-event audit type;
alternatively block moderator self-deletion consistently. Ensure persisted
tombstone state and audit history reflect the same actor classification.
In `@Core/Resgrid.Services/ChatMessageService.cs`:
- Around line 303-313: Make the edit-history type and IsModerated assignment in
DeleteMessageAsync use the same moderator-deletion condition, including the
isSender case consistently. Derive both from one shared condition so
moderator-authored deletions cannot produce SenderDelete while setting
IsModerated to true.
In `@Core/Resgrid.Services/ModerationService.cs`:
- Around line 519-542: The HydrateAsync loop performs per-request report and
action queries, causing excessive sequential database round trips. Add batch
repository lookup methods accepting the collected ModerationRequestId values,
call each once, group the returned reports and actions by request ID in memory,
and use those groups while preserving ApplyGroupScope and null-scope behavior.
- Around line 338-343: Update LoadEvidenceAsync to retain and load every
attachment returned by GetMetadataByMessageIdsAsync instead of selecting only
FirstOrDefault, and pass the complete attachment collection through the evidence
model so RemoveLiveContentAsync preserves all attachments in the audit trail. If
the surrounding API cannot support multiple attachments, document the enforced
single-attachment limitation in LoadEvidenceAsync instead.
- Around line 272-282: Make CompleteRequestAsync persist content removal and the
moderation request status transition atomically, so RemoveLiveContentAsync
cannot leave destructive changes committed when
_moderationRequestRepository.UpdateAsync fails. Use the existing
transaction/unit-of-work mechanism around both operations; preserve the current
failure behavior and only finalize the transaction after the removal and request
update succeed.
- Around line 110-124: Update both insert exception handlers in the moderation
request flow, including the blocks around InsertAsync and the reporter lookup,
to catch OperationCanceledException separately and rethrow it so cancellation
propagates. Restrict the general handler to non-cancellation exceptions, log
each swallowed insert failure with Resgrid.Framework.Logging.LogException, then
retain the existing concurrent-row recovery and rethrow behavior. Add the
required Resgrid.Framework import.
- Around line 654-689: Update NotifyReportersAsync to verify the result returned
by SaveMessageAsync before passing it to SendMessageAsync; skip sending when the
saved message is null so notification failure cannot throw. Move the
per-recipient profile lookup, message persistence, and send/enqueue work out of
CompleteRequestAsync’s synchronous completion path by dispatching the
notification fan-out through the existing background queue mechanism, while
preserving recipient filtering and cancellation behavior.
In `@Providers/Resgrid.Providers.Migrations/Migrations/M0112_AddModeration.cs`:
- Around line 177-179: Replace manual metadata JSON string concatenation in the
migration’s notification metadata construction, including the logic around
n.Source, n.Latitude, n.Longitude, and the lines handling names, with SQL
Server’s JSON API such as FOR JSON or JSON_OBJECT. Ensure all values are
serialized with valid JSON escaping and numeric formatting, while preserving the
existing metadata fields and fallback values.
- Around line 118-127: Update M0112_AddModeration.cs at
Providers/Resgrid.Providers.Migrations/Migrations/M0112_AddModeration.cs:118-127
and M0112_AddModerationPg.cs at
Providers/Resgrid.Providers.MigrationsPg/Migrations/M0112_AddModerationPg.cs:118-127
by guarding each foreign-key creation with a constraint-existence check, and
invoke ImportLegacyFlags() only when ModerationRequests/moderationrequests
contains no rows. Apply the equivalent checks and condition in both migration
implementations.
In `@Repositories/Resgrid.Repositories.DataRepository/ModerationRepositories.cs`:
- Around line 41-45: Update the SQL built by GetByItemAsync to replace SELECT *
with the same explicit non-blob moderation request column list used by
SearchAsync and ModerationActionRepository.GetByRequestAsync. Exclude
OriginalContent and any other evidence blob columns while preserving the
existing PostgreSQL and non-PostgreSQL table and filter syntax.
In `@Web/Resgrid.Web.Services/Controllers/v4/ModerationController.cs`:
- Around line 190-206: Update DownloadEvidence to accept a CancellationToken and
pass it through to the moderation service calls, matching Flag and Complete.
Handle UnauthorizedAccessException from RecordEvidenceAccessAsync by returning
Unauthorized instead of allowing a 500, while preserving the existing
no-disclosure behavior. Also revise the method summary to describe general
evidence or retained content rather than only image evidence.
In `@Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatPageElement.tsx`:
- Around line 59-65: Update openFlag to ignore stale getMyModerationRequest
responses when the selected ChatMessageId changes, using a request token or
matching the response to the current target before calling setFlagStatus. Apply
the same guard to success and failure handlers so an earlier lookup cannot alter
the second message’s dialog state.
In
`@Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx`:
- Line 208: Update the conditional rendering for request.CallId and
report.ReporterGroupId to explicitly check that each value is neither null nor
undefined, preventing a numeric zero from rendering as text while preserving
rendering for valid zero and nonzero identifiers.
- Around line 77-86: Update ModerationRequestsTable’s pagination flow so
moderators can access results beyond the hard-coded first 100 records: add page
state and previous/next controls that update search.page, while keeping pageSize
within the repository’s 200-row cap. Reuse the existing ActionsTab pagination
behavior and ensure controls reflect whether another page is available based on
the returned result count.
- Line 270: Replace the dynamic moderationText key construction for
action.ActorRole in ModerationRequestsTable with an explicit
role-to-localization-key map and a defined fallback for unmapped roles,
following the existing ITEM_LABEL_KEYS and ACTION_LABEL_KEYS pattern. Preserve
the UnknownRole behavior when ActorRole is absent and ensure persisted values
such as LegacyImport do not render as raw localization keys.
In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs`:
- Line 74: Remove the IModerationService parameter from the DispatchController
constructor and initialize the existing _moderationService field by resolving
IModerationService through
Bootstrapper.GetKernel().Resolve<IModerationService>() inside the constructor.
Update all affected constructor call sites while preserving the controller’s
existing behavior.
- Around line 1792-1793: Update the call-note handling in DispatchController to
load the current reporter’s moderation requests once before the note loop,
instead of awaiting GetReporterRequestAsync for each note. Build a set of
flagged CallNote item IDs from that result, then assign note.IsFlagged by
checking callNote.CallNoteId against the set while preserving the existing
department, user, and moderation item-type filters.
---
Outside diff comments:
In `@Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs`:
- Around line 1461-1486: Update FlagMessage to explicitly map input.Reason to
the corresponding ModerationReason value instead of directly casting between
enums, and wrap FlagAsync in exception handling matching
ModerationController.Flag: map ArgumentException and InvalidOperationException
to BadRequest, UnauthorizedAccessException to Unauthorized, and preserve the
existing success response for successful flags.
In `@Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.ts`:
- Around line 341-345: Update the thread-reply handling in the loop over
state.threadMessagesByRoot to include IsModerated from payload.IsModerated ??
payload.DeletedByModerator when calling upsertThreadMessage, alongside DeletedOn
and Body. Preserve the existing reply lookup and early return behavior.
In `@Web/Resgrid.Web/Areas/User/Controllers/MessagesController.cs`:
- Around line 37-56: Replace constructor injection with explicit service-locator
resolution in MessagesController: remove the IModerationService and moderation
localizer parameters and initialize both fields via
Bootstrapper.GetKernel().Resolve<T>(). In
Web/Resgrid.Web/Areas/User/Controllers/ModerationController.cs lines 13-18,
resolve IDepartmentGroupsService through Bootstrapper.GetKernel().Resolve<T>()
instead of injecting it; update each constructor accordingly.
---
Nitpick comments:
In `@Core/Resgrid.Services/ChatMessageService.cs`:
- Around line 316-324: Update the deletion event payload in the message deletion
flow around PublishEvent to remove DeletedByModerator = asModerator while
retaining message.IsModerated. Remove the corresponding DeletedByModerator
property from the event payload type and update any affected consumers to use
IsModerated only.
In `@Repositories/Resgrid.Repositories.DataRepository/ModerationRepositories.cs`:
- Line 69: Remove the no-op requestAlias assignment in the moderation repository
and replace its usage in the PostgreSQL statement around the generated query
with the literal “r” alias. Preserve the existing SQL behavior and remove the
now-unused variable.
- Around line 180-193: Make null handling consistent in QueryAsync and the
corresponding methods around the later query blocks: since constructors require
_unitOfWork, remove the null-conditional checks and use _unitOfWork.Connection
directly, or consistently guard the entire method before accessing
_unitOfWork.Transaction and CreateOrGetConnection(). Apply the same correction
to all three affected query methods.
In `@Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs`:
- Around line 45-46: Remove the unused IChatModerationService field and its
constructor parameter from ChatController, and update constructor assignments
and calls accordingly while preserving the existing IModerationService
dependency.
In `@Web/Resgrid.Web.Services/Models/v4/Moderation/ModerationApiModels.cs`:
- Around line 78-101: Replace the literal Range constraints on
FlagModerationInput.ItemType, FlagModerationInput.Reason, and
CompleteModerationInput.Disposition with enum-based validation using their
corresponding moderation enums, preferably by changing the properties to those
enum types or applying Enum.IsDefined validation. Preserve the existing note and
ItemId validation.
In `@Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatFormat.ts`:
- Line 192: Hoist the Intl.RelativeTimeFormat instance used by formatRelativeDay
to module scope and reuse it for each one-day-old date instead of constructing
it per call. Preserve the relative-day output, and verify the resulting
lowercase English text such as “yesterday” reads correctly in message,
moderation, and export table cells.
In
`@Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx`:
- Line 246: Update the RemoveContent button in ModerationRequestsTable so it
asks the user for confirmation before invoking complete(request, 2). Only call
complete after confirmation is accepted; preserve the existing isBusy disabled
state and behavior for other moderation actions.
- Around line 71-75: Update the personnel lookup used by personName to build a
Map keyed by userId once per people change, then read entries from that Map
instead of calling people.find for each author, report, or audit action.
Preserve the existing SystemOrUnknown fallback and display formatting for found
and missing users.
🪄 Autofix
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 Plus
Run ID: 5f4c2e9b-e2eb-408b-8004-393cdc24a872
⛔ Files ignored due to path filters (36)
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/Moderation/Moderation.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Moderation/Moderation.uk.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.uk.resxis excluded by!**/*.resxTests/Resgrid.Tests/Chatbot/ChatbotDeptConfigAndSessionTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Localization/ModerationLocalizationTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Models/FormAutomationTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/MessageServiceInboxTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/ModerationServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/NotificationServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/CallsControllerTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/MessagesControllerTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/User/SubscriptionControllerTests.csis excluded by!**/Tests/**
📒 Files selected for processing (69)
Core/Resgrid.Localization/Areas/User/Moderation/Moderation.csCore/Resgrid.Model/AuditLogTypes.csCore/Resgrid.Model/Chat/ChatMessage.csCore/Resgrid.Model/ChatbotDepartmentConfig.csCore/Resgrid.Model/FormAutomation.csCore/Resgrid.Model/Message.csCore/Resgrid.Model/Moderation/Moderation.csCore/Resgrid.Model/Repositories/IChatRepositories.csCore/Resgrid.Model/Repositories/IModerationRepositories.csCore/Resgrid.Model/Services/IModerationService.csCore/Resgrid.Services/AuditService.csCore/Resgrid.Services/ChatMessageService.csCore/Resgrid.Services/MessageService.csCore/Resgrid.Services/ModerationService.csCore/Resgrid.Services/NotificationService.csCore/Resgrid.Services/Resgrid.Services.csprojCore/Resgrid.Services/ServicesModule.csProviders/Resgrid.Providers.Migrations/Migrations/M0111_CascadeCommunicationTestDeletes.csProviders/Resgrid.Providers.Migrations/Migrations/M0112_AddModeration.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0111_CascadeCommunicationTestDeletesPg.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0112_AddModerationPg.csRepositories/Resgrid.Repositories.DataRepository/ChatRepositories.csRepositories/Resgrid.Repositories.DataRepository/ModerationRepositories.csRepositories/Resgrid.Repositories.DataRepository/Modules/ApiDataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/DataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/NonWebDataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.csWeb/Resgrid.Web.Services/Controllers/v4/CallsController.csWeb/Resgrid.Web.Services/Controllers/v4/ChatController.csWeb/Resgrid.Web.Services/Controllers/v4/MessagesController.csWeb/Resgrid.Web.Services/Controllers/v4/ModerationController.csWeb/Resgrid.Web.Services/Models/v4/Calls/EditCallInput.csWeb/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.csWeb/Resgrid.Web.Services/Models/v4/Moderation/ModerationApiModels.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xmlWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatPageElement.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/FlagDialog.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/MessageBubble.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/chat.cssWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/chatFormat.tsWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.tsWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ActionsTab.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ExportsTab.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ModerationRequestsTable.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ReportsTab.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/SettingsTab.tsxWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderationApi.tsWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/moderationI18n.tsWeb/Resgrid.Web/Areas/User/Apps/src/components/chat/types.tsWeb/Resgrid.Web/Areas/User/Apps/src/elements.tsWeb/Resgrid.Web/Areas/User/Controllers/ChatController.csWeb/Resgrid.Web/Areas/User/Controllers/DispatchController.csWeb/Resgrid.Web/Areas/User/Controllers/MessagesController.csWeb/Resgrid.Web/Areas/User/Controllers/ModerationController.csWeb/Resgrid.Web/Areas/User/Controllers/SubscriptionController.csWeb/Resgrid.Web/Areas/User/Models/Dispatch/FlagCallImageView.csWeb/Resgrid.Web/Areas/User/Models/Dispatch/FlagCallNoteView.csWeb/Resgrid.Web/Areas/User/Models/Messages/ViewMessageView.csWeb/Resgrid.Web/Areas/User/Views/Chat/Moderation.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/FlagCallImage.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/FlagCallNote.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Messages/ViewMessage.cshtmlWeb/Resgrid.Web/Areas/User/Views/Moderation/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/Shared/_Navigation.cshtmlWeb/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/templates/resgrid.templates.newtemplate.js
🚧 Files skipped from review as they are similar to previous changes (1)
- Core/Resgrid.Services/NotificationService.cs
| /// <summary>Targeted tombstone (body/metadata cleared, deletion and moderation state stamped) guarded by DeletedOn IS NULL.</summary> | ||
| Task<bool> TombstoneAsync(string chatMessageId, DateTime deletedOn, string deletedByUserId, bool isModerated, CancellationToken cancellationToken); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\bDeleteMessageAsync\s*\(|\bTombstoneAsync\s*\(' --glob '*.cs'Repository: Resgrid/Core
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git ls-files matching relevant names =="
git ls-files | rg '(IChatRepositories\.cs|ChatMessageService\.cs|ChatMessage\.cs|ChatMessageRepository.*\.cs)' || true
echo "== all TombstoneAsync/DeleteMessageAsync occurrences across repo =="
rg -n -C 6 '\b(TombstoneAsync|DeleteMessageAsync)\s*\(' --glob '*.cs' . || true
echo "== file sizes =="
while IFS= read -r f; do
[ -n "$f" ] && wc -l "$f"
done < <(git ls-files | rg '(IChatRepositories\.cs|ChatMessageService\.cs|ChatMessage\.cs|ChatMessageRepository.*\.cs)' || true)Repository: Resgrid/Core
Length of output: 12019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ChatMessage model =="
sed -n '1,220p' Core/Resgrid.Model/Chat/ChatMessage.cs
echo "== DeleteMessageAsync implementation =="
sed -n '260,325p' Core/Resgrid.Services/ChatMessageService.cs
echo "== ModeratorDeleteMessageAsync implementation =="
sed -n '90,130p' Core/Resgrid.Services/ChatModerationService.cs
echo "== repository tombstone implementation =="
sed -n '1475,1535p' Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs
echo "== ChatController DeleteMessage context =="
sed -n '840,885p' Web/Resgrid.Web.Services/Controllers/v4/ChatController.csRepository: Resgrid/Core
Length of output: 14773
Keep the tombstone actor state consistent with audit history.
ChatMessage.IsModerated means a moderator applied the tombstone, but DeleteMessageAsync records one audit type from asModerator && !isSender and a different persisted state from raw asModerator. Use the same effective sender/moderator flag for TombstoneAsync(), message.IsModerated, and delete-events, or prevent this code path from running when a moderator deletes their own message.
🤖 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 `@Core/Resgrid.Model/Repositories/IChatRepositories.cs` around lines 147 - 148,
Update the delete flow that calls TombstoneAsync so it derives one effective
moderator/sender flag and reuses it for the TombstoneAsync isModerated argument,
message.IsModerated, and delete-event audit type; alternatively block moderator
self-deletion consistently. Ensure persisted tombstone state and audit history
reflect the same actor classification.
| await SaveEditHistoryAsync(message, asModerator && !isSender ? ChatMessageEditType.ModeratorDelete : ChatMessageEditType.SenderDelete, byUserId, cancellationToken); | ||
|
|
||
| var deletedOn = DateTime.UtcNow; | ||
| if (!await _chatMessageRepository.TombstoneAsync(chatMessageId, deletedOn, byUserId, cancellationToken)) | ||
| if (!await _chatMessageRepository.TombstoneAsync(chatMessageId, deletedOn, byUserId, asModerator, cancellationToken)) | ||
| return false; | ||
|
|
||
| message.Body = null; | ||
| message.MetadataJson = null; | ||
| message.DeletedOn = deletedOn; | ||
| message.DeletedByUserId = byUserId; | ||
| message.IsModerated = asModerator; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
IsModerated and the edit-history type disagree for a moderator's own message.
Line 303 records ChatMessageEditType.ModeratorDelete only when asModerator && !isSender. Line 313 sets IsModerated = asModerator with no isSender term.
ModerationService.RemoveLiveContentAsync always calls DeleteMessageAsync(..., asModerator: true, ...). If the moderator authored the reported message, isSender is true. The edit history then records SenderDelete while the message is stored with IsModerated = true. A report that counts moderator deletions from edit history will not match the IsModerated flag.
Pick one derivation and use it in both places.
🤖 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 `@Core/Resgrid.Services/ChatMessageService.cs` around lines 303 - 313, Make the
edit-history type and IsModerated assignment in DeleteMessageAsync use the same
moderator-deletion condition, including the isSender case consistently. Derive
both from one shared condition so moderator-authored deletions cannot produce
SenderDelete while setting IsModerated to true.
| try | ||
| { | ||
| request = await _moderationRequestRepository.InsertAsync(request, cancellationToken); | ||
| } | ||
| catch | ||
| { | ||
| // The unique department/type/item index is the race backstop. If another reporter won | ||
| // the insert, join that request; otherwise preserve the original failure. | ||
| var concurrent = await _moderationRequestRepository.GetByItemAsync(departmentId, (int)itemType, itemId); | ||
| if (concurrent == null) | ||
| throw; | ||
|
|
||
| request = concurrent; | ||
| createdRequest = false; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Log the swallowed exception and do not catch cancellation.
Both catch blocks discard the original exception without logging it. If GetByItemAsync or GetByRequestAndReporterAsync then returns a row, the real insert failure is lost, so a schema error, a timeout, or a permission error looks like a normal concurrent-insert race. The coding guidelines require Resgrid.Framework.Logging.LogException when catching exceptions.
A bare catch also catches OperationCanceledException from cancellationToken. A cancelled request then follows the race-recovery path instead of propagating cancellation.
Restrict the catch and log the swallowed failure.
🛠️ Proposed fix for both catch blocks
try
{
request = await _moderationRequestRepository.InsertAsync(request, cancellationToken);
}
- catch
+ catch (Exception ex) when (ex is not OperationCanceledException)
{
// The unique department/type/item index is the race backstop. If another reporter won
// the insert, join that request; otherwise preserve the original failure.
var concurrent = await _moderationRequestRepository.GetByItemAsync(departmentId, (int)itemType, itemId);
if (concurrent == null)
throw;
+ Logging.LogException(ex, "Moderation request insert lost the unique-index race; joining the existing request.");
request = concurrent;
createdRequest = false;
} try
{
report = await _moderationReportRepository.InsertAsync(report, cancellationToken);
}
- catch
+ catch (Exception ex) when (ex is not OperationCanceledException)
{
// A unique request/reporter index prevents duplicate reports under concurrent submissions.
var concurrent = await _moderationReportRepository.GetByRequestAndReporterAsync(
request.ModerationRequestId, reportedByUserId);
if (concurrent == null)
throw;
+ Logging.LogException(ex, "Moderation report insert lost the unique-index race; returning the existing report.");
return concurrent;
}Add using Resgrid.Framework; for the Logging static class.
Also applies to: 163-176
🤖 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 `@Core/Resgrid.Services/ModerationService.cs` around lines 110 - 124, Update
both insert exception handlers in the moderation request flow, including the
blocks around InsertAsync and the reporter lookup, to catch
OperationCanceledException separately and rethrow it so cancellation propagates.
Restrict the general handler to non-cancellation exceptions, log each swallowed
insert failure with Resgrid.Framework.Logging.LogException, then retain the
existing concurrent-row recovery and rethrow behavior. Add the required
Resgrid.Framework import.
Source: Coding guidelines
| if (disposition == ModerationDisposition.ContentRemoved && !await RemoveLiveContentAsync(request, completedByUserId, cancellationToken)) | ||
| throw new InvalidOperationException(ModerationResources.GetCurrent("ContentCouldNotBeRemoved")); | ||
|
|
||
| var previousStatus = request.Status; | ||
| request.Status = (int)ModerationRequestStatus.Completed; | ||
| request.Disposition = (int)disposition; | ||
| request.CompletedByUserId = completedByUserId; | ||
| request.CompletedOn = DateTime.UtcNow; | ||
| request.ModifiedOn = request.CompletedOn.Value; | ||
| request.AdminNote = adminNote; | ||
| request = await _moderationRequestRepository.UpdateAsync(request, cancellationToken); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Content removal is destructive and runs before the status update.
RemoveLiveContentAsync performs irreversible writes. For CallImage it sets attachment.Data = null and attachment.Size = 0. For Message it overwrites Subject and Body. For ChatMessage it tombstones the message.
The request status update on Line 282 is a separate write. If UpdateAsync fails, the live content is already destroyed while the request stays Pending. A moderator can then retry CompleteRequestAsync with NoAction, and the request records NoAction for content that no longer exists.
Wrap the removal and the status update in one unit of work, or record the removal before mutating live content so a failed status update is recoverable.
🤖 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 `@Core/Resgrid.Services/ModerationService.cs` around lines 272 - 282, Make
CompleteRequestAsync persist content removal and the moderation request status
transition atomically, so RemoveLiveContentAsync cannot leave destructive
changes committed when _moderationRequestRepository.UpdateAsync fails. Use the
existing transaction/unit-of-work mechanism around both operations; preserve the
current failure behavior and only finalize the transaction after the removal and
request update succeed.
| ChatAttachment attachment = null; | ||
| var attachmentMetadata = await _chatAttachmentRepository.GetMetadataByMessageIdsAsync(new[] { itemId }); | ||
| var firstAttachment = attachmentMetadata?.FirstOrDefault(); | ||
| if (firstAttachment != null) | ||
| attachment = await _chatAttachmentRepository.GetByIdAsync(firstAttachment.ChatAttachmentId); | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Only the first attachment is captured as evidence.
LoadEvidenceAsync reads the attachment metadata list for the chat message and keeps FirstOrDefault(). If the reported message carries more than one attachment, the remaining attachments are never captured. RemoveLiveContentAsync still tombstones the whole message, so the uncaptured attachments are lost from the audit trail.
Capture every attachment, or document the single-attachment limit in the method comment.
🤖 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 `@Core/Resgrid.Services/ModerationService.cs` around lines 338 - 343, Update
LoadEvidenceAsync to retain and load every attachment returned by
GetMetadataByMessageIdsAsync instead of selecting only FirstOrDefault, and pass
the complete attachment collection through the evidence model so
RemoveLiveContentAsync preserves all attachments in the audit trail. If the
surrounding API cannot support multiple attachments, document the enforced
single-attachment limitation in LoadEvidenceAsync instead.
| const search = useMemo<ModerationSearch>(() => ({ | ||
| status: status < 0 ? undefined : status, | ||
| itemType: itemType < 0 ? undefined : itemType, | ||
| contentAuthorUserId: contentAuthorUserId.trim() || undefined, | ||
| reportedByUserId: reportedByUserId.trim() || undefined, | ||
| from: from ? new Date(`${from}T00:00:00`).toISOString() : undefined, | ||
| to: to ? new Date(`${to}T23:59:59.999`).toISOString() : undefined, | ||
| page: 1, | ||
| pageSize: 100, | ||
| }), [contentAuthorUserId, from, itemType, reportedByUserId, status, to]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add pagination, or show that results are truncated.
search hard-codes page: 1 and pageSize: 100, and the interface has no control that changes either value. The repository caps a page at 200 rows and applies LIMIT/OFFSET. A department with more than 100 matching requests therefore has moderation items that no moderator can reach through this table. ActionsTab already implements previous and next controls for the same reason.
Add page controls, or display a message when the returned count reaches the page size.
Also applies to: 172-172
🤖 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/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx`
around lines 77 - 86, Update ModerationRequestsTable’s pagination flow so
moderators can access results beyond the hard-coded first 100 records: add page
state and previous/next controls that update search.page, while keeping pageSize
within the repository’s 200-row cap. Reuse the existing ActionsTab pagination
behavior and ensure controls reflect whether another page is available based on
the returned result count.
| <td> | ||
| <strong>{moderationText(ITEM_LABEL_KEYS[request.ItemType] ?? 'UnknownContentType')}</strong> | ||
| <div className="rgchat-convo__sub">{moderationText('IdFormat', request.ItemId)}</div> | ||
| {request.CallId && <div className="rgchat-convo__sub">{moderationText('CallFormat', request.CallId)}</div>} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against the falsy zero render.
{request.CallId && ...} and {report.ReporterGroupId && ...} render the literal 0 when the value is 0, because React renders the number 0 as a text node. Compare against null and undefined instead.
🐛 Proposed fix
- {request.CallId && <div className="rgchat-convo__sub">{moderationText('CallFormat', request.CallId)}</div>}
+ {request.CallId != null && <div className="rgchat-convo__sub">{moderationText('CallFormat', request.CallId)}</div>}- {report.ReporterGroupId && <span className="rgchat-convo__sub"> · {moderationText('GroupFormat', report.ReporterGroupId)}</span>}
+ {report.ReporterGroupId != null && <span className="rgchat-convo__sub"> · {moderationText('GroupFormat', report.ReporterGroupId)}</span>}Also applies to: 228-228
🤖 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/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx`
at line 208, Update the conditional rendering for request.CallId and
report.ReporterGroupId to explicitly check that each value is neither null nor
undefined, preventing a numeric zero from rendering as text while preserving
rendering for valid zero and nonzero identifiers.
| <strong>{moderationText(ACTION_LABEL_KEYS[action.ActionType] ?? 'Action')}</strong> {moderationText('By')} {personName(action.PerformedByUserId)} | ||
| <div>{action.Note || moderationText('NoNote')}</div> | ||
| <div className="rgchat-convo__sub"> | ||
| {formatTimestamp(action.PerformedOn)} · {action.ActorRole ? moderationText(`Actor${action.ActorRole}`) : moderationText('UnknownRole')} · {action.IpAddress || moderationText('NoIp')} · {moderationText('TraceFormat', action.TraceId || moderationText('NotAvailable'))} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not build a localization key from stored server data.
moderationText(Actor${action.ActorRole}) derives the key from the persisted ActorRole string. moderationText returns the key itself when the entry is missing, so an unmapped role renders a raw identifier. The SQL Server and PostgreSQL migrations both write ActorRole = 'LegacyImport' for every imported action, which produces the key ActorLegacyImport.
Use an explicit map with a fallback, like ITEM_LABEL_KEYS and ACTION_LABEL_KEYS do for the numeric enums.
🤖 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/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx`
at line 270, Replace the dynamic moderationText key construction for
action.ActorRole in ModerationRequestsTable with an explicit
role-to-localization-key map and a defined fallback for unmapped roles,
following the existing ITEM_LABEL_KEYS and ACTION_LABEL_KEYS pattern. Preserve
the UnknownRole behavior when ActorRole is absent and ensure persisted values
such as LegacyImport do not render as raw localization keys.
| private readonly ICheckInTimerService _checkInTimerService; | ||
| private readonly IWeatherAlertService _weatherAlertService; | ||
| private readonly ICallDispatchStatusService _callDispatchStatusService; | ||
| private readonly IModerationService _moderationService; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Resolve IModerationService through the required service locator.
Line 87 adds constructor injection for IModerationService. Remove this parameter. Resolve the service in the constructor with Bootstrapper.GetKernel().Resolve<IModerationService>().
As per coding guidelines, use Service Locator pattern via Bootstrapper.GetKernel().Resolve<T>() to resolve dependencies explicitly in constructors, rather than constructor injection.
Also applies to: 87-87, 118-118
🤖 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` at line 74,
Remove the IModerationService parameter from the DispatchController constructor
and initialize the existing _moderationService field by resolving
IModerationService through
Bootstrapper.GetKernel().Resolve<IModerationService>() inside the constructor.
Update all affected constructor call sites while preserving the controller’s
existing behavior.
Source: Coding guidelines
| note.IsFlagged = await _moderationService.GetReporterRequestAsync(DepartmentId, UserId, | ||
| ModerationItemType.CallNote, callNote.CallNoteId.ToString(CultureInfo.InvariantCulture)) != null; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid one moderation lookup per call note.
The loop awaits GetReporterRequestAsync once for each call note. A call with N notes now causes N sequential moderation lookups. Load the current reporter’s requests in one service call before the loop. Build an item-ID set for IsFlagged.
🤖 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
1792 - 1793, Update the call-note handling in DispatchController to load the
current reporter’s moderation requests once before the note loop, instead of
awaiting GetReporterRequestAsync for each note. Build a set of flagged CallNote
item IDs from that result, then assign note.IsFlagged by checking
callNote.CallNoteId against the set while preserving the existing department,
user, and moderation item-type filters.
|
|
||
| return resourceSet.Cast<DictionaryEntry>() | ||
| .Where(x => x.Key is string && x.Value is string) | ||
| .ToDictionary(x => (string)x.Key, x => (string)x.Value!); |
There was a problem hiding this comment.
Unsafe type casting violates team rule. Use the as operator or pattern matching for safe casts and guard null results before usage.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File Core/Resgrid.Localization/Areas/User/Moderation/Moderation.cs:
Line 49:
Unsafe type casting violates team rule. Use the `as` operator or pattern matching for safe casts and guard null results before usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| var cultureInfo = GetSupportedCulture(culture); | ||
| var value = ResourceManager.GetString(key, cultureInfo) | ||
| ?? ResourceManager.GetString(key, CultureInfo.GetCultureInfo("en")) |
There was a problem hiding this comment.
Magic string "en" for the default culture is repeated across multiple locations, risking inconsistency during changes. Define a private constant like private const string DefaultCulture = "en"; in ModerationResources and reference it.
Kody rule violation: Centralize string constants
Prompt for LLM
File Core/Resgrid.Localization/Areas/User/Moderation/Moderation.cs:
Line 28:
Magic string `"en"` for the default culture is repeated across multiple locations, risking inconsistency during changes. Define a private constant like `private const string DefaultCulture = "en";` in `ModerationResources` and reference it.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| } | ||
|
|
||
| private async Task<List<ModerationRequest>> HydrateAsync(IEnumerable<ModerationRequest> requests, |
There was a problem hiding this comment.
Query amplification in HydrateAsync issues two sequential database round-trips (GetByRequestAsync for reports and actions) per request, causing up to 400 queries on a 200-item page. Batch-load reports and actions using WHERE ModerationRequestId IN (...) and group them in memory.
var requestIds = result.Select(x => x.ModerationRequestId).ToList();
var allReports = await _moderationReportRepository.GetByRequestIdsAsync(requestIds);
var allActions = await _moderationActionRepository.GetByRequestIdsAsync(requestIds);
var reportsByRequest = allReports.ToLookup(x => x.ModerationRequestId);
var actionsByRequest = allActions.ToLookup(x => x.ModerationRequestId);
foreach (var request in result)
{
var reports = reportsByRequest[request.ModerationRequestId].ToList();
var actions = actionsByRequest[request.ModerationRequestId].ToList();
if (visibleGroupIds == null)
{
request.Reports = reports;
request.Actions = actions;
}
else
{
ApplyGroupScope(request, reports, actions, visibleGroupIds, viewerUserId);
}
}Prompt for LLM
File Core/Resgrid.Services/ModerationService.cs:
Line 519:
Query amplification in `HydrateAsync` issues two sequential database round-trips (`GetByRequestAsync` for reports and actions) per request, causing up to 400 queries on a 200-item page. Batch-load reports and actions using `WHERE ModerationRequestId IN (...)` and group them in memory.
Suggested Code:
var requestIds = result.Select(x => x.ModerationRequestId).ToList();
var allReports = await _moderationReportRepository.GetByRequestIdsAsync(requestIds);
var allActions = await _moderationActionRepository.GetByRequestIdsAsync(requestIds);
var reportsByRequest = allReports.ToLookup(x => x.ModerationRequestId);
var actionsByRequest = allActions.ToLookup(x => x.ModerationRequestId);
foreach (var request in result)
{
var reports = reportsByRequest[request.ModerationRequestId].ToList();
var actions = actionsByRequest[request.ModerationRequestId].ToList();
if (visibleGroupIds == null)
{
request.Reports = reports;
request.Actions = actions;
}
else
{
ApplyGroupScope(request, reports, actions, visibleGroupIds, viewerUserId);
}
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| } | ||
|
|
||
| private async Task<List<ModerationRequest>> HydrateAsync(IEnumerable<ModerationRequest> requests, |
There was a problem hiding this comment.
O(N) query amplification in HydrateAsync issues two database round-trips (GetByRequestAsync for reports and actions) per ModerationRequest, causing up to 400 sequential queries on a 200-item page. Replace the loop with two batch queries keyed on request IDs (WHERE ModerationRequestId IN @ids) and group the results client-side.
// Batch-load all reports and actions for the page in two queries, then group client-side:
// var ids = result.Select(r => r.ModerationRequestId).ToList();
// var allReports = (await _moderationReportRepository.GetByRequestIdsAsync(ids))
// .GroupBy(r => r.ModerationRequestId).ToDictionary(g => g.Key, g => g.ToList());
// var allActions = (await _moderationActionRepository.GetByRequestIdsAsync(ids))
// .GroupBy(a => a.ModerationRequestId).ToDictionary(g => g.Key, g => g.ToList());
// then iterate result and pull reports/actions from the dictionaries (falling back to empty lists).Prompt for LLM
File Core/Resgrid.Services/ModerationService.cs:
Line 519:
O(N) query amplification in `HydrateAsync` issues two database round-trips (`GetByRequestAsync` for reports and actions) per `ModerationRequest`, causing up to 400 sequential queries on a 200-item page. Replace the loop with two batch queries keyed on request IDs (`WHERE ModerationRequestId IN @ids`) and group the results client-side.
Suggested Code:
// Batch-load all reports and actions for the page in two queries, then group client-side:
// var ids = result.Select(r => r.ModerationRequestId).ToList();
// var allReports = (await _moderationReportRepository.GetByRequestIdsAsync(ids))
// .GroupBy(r => r.ModerationRequestId).ToDictionary(g => g.Key, g => g.ToList());
// var allActions = (await _moderationActionRepository.GetByRequestIdsAsync(ids))
// .GroupBy(a => a.ModerationRequestId).ToDictionary(g => g.Key, g => g.ToList());
// then iterate result and pull reports/actions from the dictionaries (falling back to empty lists).
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| request.CompletedOn = null; | ||
| request.AdminNote = null; | ||
| request.ModifiedOn = DateTime.UtcNow; | ||
| await _moderationRequestRepository.UpdateAsync(request, cancellationToken); |
There was a problem hiding this comment.
Database inconsistency risk arises when the request-reopen block performs three separate writes (UpdateAsync, InsertAsync, SaveAuditLogAsync) without a wrapping transaction. Wrap the entire sequence in a transaction or unit-of-work to ensure atomic commits.
Kody rule violation: Handle transaction rollbacks properly
Prompt for LLM
File Core/Resgrid.Services/ModerationService.cs:
Line 141:
Database inconsistency risk arises when the request-reopen block performs three separate writes (`UpdateAsync`, `InsertAsync`, `SaveAuditLogAsync`) without a wrapping transaction. Wrap the entire sequence in a transaction or unit-of-work to ensure atomic commits.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .WithColumn("ContentAuthorUserId").AsString(450).Nullable() | ||
| .WithColumn("ContentAuthorUnitId").AsInt32().Nullable() | ||
| .WithColumn("ContentCreatedOn").AsDateTime2().Nullable() | ||
| .WithColumn("OriginalSubject").AsString(int.MaxValue).Nullable() |
There was a problem hiding this comment.
Inefficient row storage occurs because OriginalSubject uses NVARCHAR(MAX), preventing SQL Server optimization for bounded subjects. Use a bounded length like AsString(512).
Kody rule violation: Optimize string column types
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0112_AddModeration.cs:
Line 28:
Inefficient row storage occurs because `OriginalSubject` uses `NVARCHAR(MAX)`, preventing SQL Server optimization for bounded subjects. Use a bounded length like `AsString(512)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Create.ForeignKey("fk_communicationtestruns_communicationtests") | ||
| .FromTable("communicationtestruns").ForeignColumn("communicationtestid") | ||
| .ToTable("communicationtests").PrimaryColumn("communicationtestid") | ||
| .OnDelete(Rule.Cascade); |
There was a problem hiding this comment.
Locking and downtime risk occurs when adding a new FK constraint, as it takes ACCESS EXCLUSIVE locks while validating existing rows. Use the PostgreSQL online pattern by adding the constraint NOT VALID first, then executing VALIDATE CONSTRAINT in a later transaction.
Kody rule violation: Block risky database migrations (locking ops, downtime risk)
Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0111_CascadeCommunicationTestDeletesPg.cs:
Line 16 to 19:
Locking and downtime risk occurs when adding a new FK constraint, as it takes `ACCESS EXCLUSIVE` locks while validating existing rows. Use the PostgreSQL online pattern by adding the constraint `NOT VALID` first, then executing `VALIDATE CONSTRAINT` in a later transaction.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| MIN(m.senderuserid::text), MIN(m.senderunitid), MIN(m.senton), | ||
| COALESCE(MIN(m.body), (SELECT e.priorbody FROM chatmessageedits e | ||
| WHERE e.chatmessageid = f.chatmessageid ORDER BY e.editedon DESC LIMIT 1)), | ||
| (SELECT ca.filename FROM chatattachments ca |
There was a problem hiding this comment.
Tripled per-row I/O occurs due to three separate correlated subqueries to chatattachments for the same chatmessageid. Replace them with a single LEFT JOIN LATERAL to select the required fields in one pass.
Kody rule violation: Optimize database queries with JOINs
Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0112_AddModerationPg.cs:
Line 141:
Tripled per-row I/O occurs due to three separate correlated subqueries to `chatattachments` for the same `chatmessageid`. Replace them with a single `LEFT JOIN LATERAL` to select the required fields in one pass.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| } | ||
|
|
||
| var extra = filters.Count > 0 ? " AND " + string.Join(" AND ", filters) : string.Empty; |
There was a problem hiding this comment.
Error-prone string concatenation using + violates team rule. Use template literals to improve readability.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/ModerationRepositories.cs:
Line 144:
Error-prone string concatenation using `+` violates team rule. Use template literals to improve readability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| foreach (var pair in english) | ||
| { | ||
| var expected = Regex.Matches(pair.Value, @"\{\d+\}").Select(x => x.Value).OrderBy(x => x); |
There was a problem hiding this comment.
Regular expression denial of service (ReDoS) vulnerability violates team rule. Define a timeout when using regex on untrusted input to prevent DoS attacks.
Kody rule violation: Specify Timeout for Regular Expressions
Prompt for LLM
File Tests/Resgrid.Tests/Localization/ModerationLocalizationTests.cs:
Line 118:
Regular expression denial of service (ReDoS) vulnerability violates team rule. Define a timeout when using regex on untrusted input to prevent DoS attacks.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| foreach (var pair in english) | ||
| { | ||
| var expected = Regex.Matches(pair.Value, @"\{\d+\}").Select(x => x.Value).OrderBy(x => x); |
There was a problem hiding this comment.
Wasted CPU cycles occur because Regex.Matches is called with a constant pattern inside a foreach loop, forcing recompilation on every iteration. Declare a private static readonly Regex with RegexOptions.Compiled at the class level and reuse it.
Kody rule violation: Cache expensive operations outside loops
Prompt for LLM
File Tests/Resgrid.Tests/Localization/ModerationLocalizationTests.cs:
Line 118:
Wasted CPU cycles occur because `Regex.Matches` is called with a constant pattern inside a `foreach` loop, forcing recompilation on every iteration. Declare a `private static readonly Regex` with `RegexOptions.Compiled` at the class level and reuse it.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .GetColumns(new SqlServerConfiguration(), ignoreProperties: automation.IgnoredProperties) | ||
| .ToList(); | ||
|
|
||
| automation.IdType.Should().Be(1); |
There was a problem hiding this comment.
Opaque magic number 1 represents an IdType value without a named constant, making the test brittle and hard to read. Reference the named enum or constant, such as automation.IdType.Should().Be((int)IdType.String), for self-documenting assertions.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File Tests/Resgrid.Tests/Models/FormAutomationTests.cs:
Line 28:
Opaque magic number `1` represents an `IdType` value without a named constant, making the test brittle and hard to read. Reference the named enum or constant, such as `automation.IdType.Should().Be((int)IdType.String)`, for self-documenting assertions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| var response = await _controller.GetCall(callId); | ||
|
|
||
| response.Result.Should().BeOfType<BadRequestResult>(); |
There was a problem hiding this comment.
Blocking async methods with .Result or .Wait() can cause deadlocks and violates team rule. Use await instead for proper asynchronous execution.
Kody rule violation: Avoid Blocking Calls to Async Methods
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/CallsControllerTests.cs:
Line 63:
Blocking async methods with `.Result` or `.Wait()` can cause deadlocks and violates team rule. Use `await` instead for proper asynchronous execution.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| var response = await _controller.GetCall(callId); | ||
|
|
||
| response.Result.Should().BeOfType<BadRequestResult>(); |
There was a problem hiding this comment.
Blocking async operation violates team rule. Await Tasks instead of blocking with .Result or .Wait(), and prefer async/await end-to-end.
Kody rule violation: Await async operations properly
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/CallsControllerTests.cs:
Line 63:
Blocking async operation violates team rule. Await Tasks instead of blocking with `.Result` or `.Wait()`, and prefer `async/await` end-to-end.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var message = await _chatMessageService.GetMessageByIdAsync(attachment.ChatMessageId); | ||
| if (message == null || message.DeletedOn.HasValue) | ||
| return NotFound(); |
There was a problem hiding this comment.
Duplicated domain logic for the deleted-message check violates the DRY principle across GetAttachment and GetAttachmentThumbnail. Extract a helper method like EnsureMessageNotDeletedAsync(ChatMessageId) to handle the rule in one location.
Kody rule violation: Extract duplicated business logic
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:
Line 1282 to 1284:
Duplicated domain logic for the deleted-message check violates the DRY principle across `GetAttachment` and `GetAttachmentThumbnail`. Extract a helper method like `EnsureMessageNotDeletedAsync(ChatMessageId)` to handle the rule in one location.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!Enum.IsDefined(typeof(ModerationItemType), itemType) || string.IsNullOrWhiteSpace(itemId)) | ||
| return BadRequest(); | ||
|
|
||
| var request = await _moderationService.GetReporterRequestAsync(DepartmentId, UserId, |
There was a problem hiding this comment.
Unhandled domain exceptions from GetReporterRequestAsync return generic 500 errors instead of appropriate 401/400 responses. Wrap the external service call in a try/catch block to map UnauthorizedAccessException or ArgumentException to the correct HTTP responses.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ModerationController.cs:
Line 78:
Unhandled domain exceptions from `GetReporterRequestAsync` return generic 500 errors instead of appropriate 401/400 responses. Wrap the external service call in a `try/catch` block to map `UnauthorizedAccessException` or `ArgumentException` to the correct HTTP responses.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| int? itemType = null, string contentAuthorUserId = null, string reportedByUserId = null, | ||
| DateTime? from = null, DateTime? to = null, int page = 1, int pageSize = 50) | ||
| { | ||
| if (!await _moderationService.CanModerateAsync(DepartmentId, UserId)) |
There was a problem hiding this comment.
Unnecessary database round-trips occur when CanModerateAsync executes before validating enum inputs, allowing invalid query parameters to hit the database. Move the Enum.IsDefined checks above the CanModerateAsync call to return BadRequest first.
Kody rule violation: Order validations before database queries
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ModerationController.cs:
Line 98:
Unnecessary database round-trips occur when `CanModerateAsync` executes before validating enum inputs, allowing invalid query parameters to hit the database. Move the `Enum.IsDefined` checks above the `CanModerateAsync` call to return `BadRequest` first.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Page = Math.Max(page, 1), | ||
| PageSize = requests.Count, | ||
| Status = requests.Count > 0 ? ResponseHelper.Success : ResponseHelper.NotFound |
There was a problem hiding this comment.
Incorrect pagination metadata occurs when GetRequests assigns PageSize = requests.Count instead of the requested page size, returning partial counts on the last page. Set PageSize = Math.Max(pageSize, 1) to report the correct requested page size.
Page = Math.Max(page, 1),
PageSize = Math.Max(pageSize, 1),
Status = requests.Count > 0 ? ResponseHelper.Success : ResponseHelper.NotFoundPrompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ModerationController.cs:
Line 122 to 124:
Incorrect pagination metadata occurs when `GetRequests` assigns `PageSize = requests.Count` instead of the requested page size, returning partial counts on the last page. Set `PageSize = Math.Max(pageSize, 1)` to report the correct requested page size.
Suggested Code:
Page = Math.Max(page, 1),
PageSize = Math.Max(pageSize, 1),
Status = requests.Count > 0 ? ResponseHelper.Success : ResponseHelper.NotFound
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// <summary> | ||
| /// Whether the tombstone represents a moderation action | ||
| /// </summary> | ||
| public bool IsModerated { get; set; } |
There was a problem hiding this comment.
Uninitialized auto-property IsModerated relies on implicit language defaults, violating rule requirements. Initialize the property explicitly with a sensible default value, such as public bool IsModerated { get; set; } = false;.
Kody rule violation: Initialize properties with default values
Prompt for LLM
File Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:
Line 540:
Uninitialized auto-property `IsModerated` relies on implicit language defaults, violating rule requirements. Initialize the property explicitly with a sensible default value, such as `public bool IsModerated { get; set; } = false;`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| export default function ChatModerationElement({ departmentAdmin = false }: ChatModerationElementProps) { | ||
| const [tab, setTab] = useState<ModTab>('requests'); | ||
| const sharedTabs: { key: ModTab; label: string }[] = [ | ||
| { key: 'requests', label: moderationText('TabRequests') }, |
There was a problem hiding this comment.
Repeated raw string literal 'requests' for the finite ModTab set introduces typo risks and hides intent. Introduce a const object like const ModTabKey = { Requests: 'requests', ... } as const; to reference these values safely.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsx:
Line 20:
Repeated raw string literal `'requests'` for the finite `ModTab` set introduces typo risks and hides intent. Introduce a const object like `const ModTabKey = { Requests: 'requests', ... } as const;` to reference these values safely.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { key: 'settings', label: 'Settings' }, | ||
| { key: 'exports', label: 'Exports' }, | ||
| ]; | ||
| type ModTab = 'requests' | 'reports' | 'actions' | 'settings' | 'exports'; |
There was a problem hiding this comment.
Maintenance drift risk arises because the ModTab union type duplicates string literals used in the sharedTabs and departmentTabs arrays. Declare a const tuple to derive both the type and runtime arrays from a single source of truth.
Kody rule violation: Derive TypeScript types from validation schemas
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsx:
Line 10:
Maintenance drift risk arises because the `ModTab` union type duplicates string literals used in the `sharedTabs` and `departmentTabs` arrays. Declare a `const` tuple to derive both the type and runtime arrays from a single source of truth.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| export default function ChatModerationElement(_props: ChatModerationElementProps) { | ||
| const [tab, setTab] = useState<ModTab>('flags'); | ||
| export default function ChatModerationElement({ departmentAdmin = false }: ChatModerationElementProps) { |
There was a problem hiding this comment.
Reduced refactor safety and discoverability occur because ChatModerationElement uses a default export. Use a named export instead (export function ChatModerationElement(...)) and update importers accordingly.
Kody rule violation: Avoid default exports
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsx:
Line 17:
Reduced refactor safety and discoverability occur because `ChatModerationElement` uses a default export. Use a named export instead (`export function ChatModerationElement(...)`) and update importers accordingly.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <FlagDialog | ||
| existingRequest={flagStatus} | ||
| statusLoading={flagStatus === undefined} | ||
| onClose={() => setFlagTarget(null)} |
There was a problem hiding this comment.
Performance degradation caused by using .bind() or inline arrow functions in JSX props violates team rule. Move function definitions outside the render method to prevent creating new functions on every render.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatPageElement.tsx:
Line 205:
Performance degradation caused by using `.bind()` or inline arrow functions in JSX props violates team rule. Move function definitions outside the render method to prevent creating new functions on every render.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| setFlagStatus(undefined); | ||
| getMyModerationRequest(0, message.ChatMessageId) | ||
| .then(setFlagStatus) | ||
| .catch(() => setFlagStatus(null)); |
There was a problem hiding this comment.
Silent error swallowing violates team rules when the .catch handler discards rejections without logging context. Log the error with identifying details using logger.error inside the .catch handler to ensure failures are visible during debugging.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatPageElement.tsx:
Line 64:
Silent error swallowing violates team rules when the `.catch` handler discards rejections without logging context. Log the error with identifying details using `logger.error` inside the `.catch` handler to ensure failures are visible during debugging.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <div>{summarize(request.OriginalText)}</div> | ||
| {request.OriginalFileName && <div className="rgchat-convo__sub">{request.OriginalFileName}</div>} | ||
| {request.HasOriginalContent && ( | ||
| <button type="button" className="rgchat-thread-link" onClick={() => void downloadModerationEvidence(request)}> |
There was a problem hiding this comment.
Unhandled promise rejection occurs when downloadModerationEvidence(request) is invoked with void and no error guard. Wrap the async operation in a try/catch block or chain a .catch() handler.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx:
Line 215:
Unhandled promise rejection occurs when `downloadModerationEvidence(request)` is invoked with `void` and no error guard. Wrap the async operation in a `try/catch` block or chain a `.catch()` handler.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <> | ||
| <label> | ||
| <span>{moderationText('AddedByUserId')}</span> | ||
| <input className="rgchat-input" list="rg-moderation-authors" value={contentAuthorUserId} onChange={(event) => setContentAuthorUserId(event.target.value)} /> |
There was a problem hiding this comment.
Excessive network requests occur because the onChange handler fires getModerationRequests on every keystroke without debouncing. Debounce the state update or the load effect to ensure a burst of keystrokes issues only one request.
Kody rule violation: Debounce or throttle user input that triggers work
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ModerationRequestsTable.tsx:
Line 150:
Excessive network requests occur because the `onChange` handler fires `getModerationRequests` on every keystroke without debouncing. Debounce the state update or the `load` effect to ensure a burst of keystrokes issues only one request.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| } catch (error) { | ||
| console.error('Failed to save chat settings.', error); | ||
| console.error(moderationText('FailedSaveSettings'), error); |
There was a problem hiding this comment.
Unstructured error logging violates team rules by using plain strings instead of structured fields. Replace console.error with a structured logger call that includes the operation name and relevant identifiers.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/SettingsTab.tsx:
Line 57:
Unstructured error logging violates team rules by using plain strings instead of structured fields. Replace `console.error` with a structured logger call that includes the operation name and relevant identifiers.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| document.body.appendChild(anchor); | ||
| anchor.click(); | ||
| anchor.remove(); | ||
| setTimeout(() => URL.revokeObjectURL(objectUrl), 4000); |
There was a problem hiding this comment.
Memory leak risk occurs when a setTimeout ID is not stored for cleanup, allowing the callback to fire even if the component unmounts. Store the timer ID in a variable and clear it via clearTimeout(timerId) in the component's teardown path.
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderationApi.ts:
Line 133:
Memory leak risk occurs when a `setTimeout` ID is not stored for cleanup, allowing the callback to fire even if the component unmounts. Store the timer ID in a variable and clear it via `clearTimeout(timerId)` in the component's teardown path.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| note.IsFlagged = await _moderationService.GetReporterRequestAsync(DepartmentId, UserId, | ||
| ModerationItemType.CallNote, callNote.CallNoteId.ToString(CultureInfo.InvariantCulture)) != null; |
There was a problem hiding this comment.
N+1 query pattern degrades performance as GetReporterRequestAsync is called per note inside the GetCallNotes loop. Add a batch method to IModerationService and call it once before the loop to retrieve all moderation statuses in a single query.
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs:
Line 1792 to 1793:
N+1 query pattern degrades performance as `GetReporterRequestAsync` is called per note inside the `GetCallNotes` loop. Add a batch method to `IModerationService` and call it once before the loop to retrieve all moderation statuses in a single query.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| note.IsFlagged = await _moderationService.GetReporterRequestAsync(DepartmentId, UserId, | ||
| ModerationItemType.CallNote, callNote.CallNoteId.ToString(CultureInfo.InvariantCulture)) != null; |
There was a problem hiding this comment.
N+1 query pattern in the foreach (var callNote in call.CallNotes) loop calls GetReporterRequestAsync per note, executing 2N database round-trips per request. Add a batch method to IModerationService and call it once before the loop to retrieve all flagged note IDs.
// Resolve all flagged note IDs in one query before the loop:
var flaggedNoteIds = await _moderationService.GetReporterItemIdsAsync(DepartmentId, UserId,
ModerationItemType.CallNote, call.CallNotes.Select(n => n.CallNoteId.ToString(CultureInfo.InvariantCulture)));
// Inside the loop:
note.IsFlagged = flaggedNoteIds.Contains(callNote.CallNoteId.ToString(CultureInfo.InvariantCulture));Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs:
Line 1792 to 1793:
N+1 query pattern in the `foreach (var callNote in call.CallNotes)` loop calls `GetReporterRequestAsync` per note, executing 2N database round-trips per request. Add a batch method to `IModerationService` and call it once before the loop to retrieve all flagged note IDs.
Suggested Code:
// Resolve all flagged note IDs in one query before the loop:
var flaggedNoteIds = await _moderationService.GetReporterItemIdsAsync(DepartmentId, UserId,
ModerationItemType.CallNote, call.CallNotes.Select(n => n.CallNoteId.ToString(CultureInfo.InvariantCulture)));
// Inside the loop:
note.IsFlagged = flaggedNoteIds.Contains(callNote.CallNoteId.ToString(CultureInfo.InvariantCulture));
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var moderationRequest = await _moderationService.GetReporterRequestAsync(DepartmentId, UserId, | ||
| ModerationItemType.CallImage, attachment.CallAttachmentId.ToString(CultureInfo.InvariantCulture)); | ||
| var ownReport = moderationRequest?.Reports?.FirstOrDefault(); | ||
| model.IsFlagged = moderationRequest != null; | ||
| model.FlagNote = ownReport?.Note; | ||
| model.ModerationStatus = moderationRequest?.Status; | ||
| model.ModerationAdminNote = moderationRequest?.AdminNote; |
There was a problem hiding this comment.
Duplicated mapping logic for CallImage and CallNote increases maintenance burden and risks inconsistency. Extract a helper method like MapModerationFieldsAsync(IFlagViewModel model, ModerationItemType itemType, string itemId) to handle the fetch and property mapping for both actions.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs:
Line 1582 to 1588:
Duplicated mapping logic for CallImage and CallNote increases maintenance burden and risks inconsistency. Extract a helper method like `MapModerationFieldsAsync(IFlagViewModel model, ModerationItemType itemType, string itemId)` to handle the fetch and property mapping for both actions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <div class="wrapper wrapper-content"> | ||
| <div class="ibox float-e-margins"> | ||
| <div class="ibox-content"> | ||
| <rg-chat-moderation departmentadmin="@ClaimsAuthorizationHelper.IsUserDepartmentAdmin().ToString().ToLowerInvariant()"></rg-chat-moderation> |
There was a problem hiding this comment.
Thick UI violation occurs because authorization logic (ClaimsAuthorizationHelper.IsUserDepartmentAdmin()) is invoked directly in the Razor view. Move this logic to the controller, compute the flag, and pass it via ViewBag.IsDepartmentAdmin to keep the view thin.
Kody rule violation: Separate UI logic from business logic
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Moderation/Index.cshtml:
Line 25:
Thick UI violation occurs because authorization logic (`ClaimsAuthorizationHelper.IsUserDepartmentAdmin()`) is invoked directly in the Razor view. Move this logic to the controller, compute the flag, and pass it via `ViewBag.IsDepartmentAdmin` to keep the view thin.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <div class="wrapper wrapper-content"> | ||
| <div class="ibox float-e-margins"> | ||
| <div class="ibox-content"> | ||
| <rg-chat-moderation departmentadmin="@ClaimsAuthorizationHelper.IsUserDepartmentAdmin().ToString().ToLowerInvariant()"></rg-chat-moderation> |
There was a problem hiding this comment.
Unclear naming convention violates team rules because the custom element rg-chat-moderation uses an abbreviation. Rename it to a full-word format like resgrid-chat-moderation to improve clarity and maintainability.
Kody rule violation: Full-word component names
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Moderation/Index.cshtml:
Line 25:
Unclear naming convention violates team rules because the custom element `rg-chat-moderation` uses an abbreviation. Rename it to a full-word format like `resgrid-chat-moderation` to improve clarity and maintainability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
3 participants
PR Description: RG-T129 Notification System Bug Fixes
Summary
This PR fixes several bugs in the notification system that prevented notifications from firing correctly and caused runtime errors under certain conditions.
Changes
Bug Fix: Incorrect Event Type in Group Lookup
In
GetGroupForEventAsync, the code block that looks up personnel staffing data was incorrectly matched againstPersonnelStatusChangedinstead ofPersonnelStaffingChanged. This meant group-based notifications for staffing changes would never resolve the correct department group.Bug Fix: Empty BeforeData/CurrentData Causing Notifications to Never Fire
The notification validation logic previously returned
falsewheneverBeforeDataorCurrentDatawas null or empty. Since the UI's "Any" option was posting an empty string, notifications saved with default "Any" settings would never trigger. The validation now treats empty/null values as"-1"(the system's "Any" sentinel), allowing these notifications to process as intended.Bug Fix: NullReferenceException When No Previous State Exists
For
UnitStatusChanged,PersonnelStaffingChanged, andPersonnelStatusChangedevents, when a "before" state was required but no prior state existed (e.g., the very first state change), the code would throw a null reference exception. Null checks were added so the notification is safely skipped (returnsfalse) instead of crashing. A missing null check oncurrentStatewas also added forPersonnelStatusChanged.Bug Fix: UI Dropdown Posting Incorrect "Any" Value
The client-side dropdown initialization was changed to post
"-1"for the "Any" option instead of an empty string, aligning with the notification engine's expected value format. The API calls for populating these dropdowns were updated to stop requesting an "Any" entry from the server (includeAny=False), since it is now provided client-side.Test Coverage
New unit tests were added covering:
Summary by CodeRabbit
New Features
Bug Fixes