CU-868frm4jk Removing v3 api, minor fixes. #249
Conversation
WalkthroughRemoves most v3 API controllers and their base; adds null/validation guards in core services; adds conditional geolocation with error logging in email-parsing; fixes shift-group aggregation upsert; tightens several v4 input/role validations; fixes contact address loading and a Geofence null-guard and text. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as v4 MessagesController
participant Svc as MessageService
Client->>API: POST /SendMessage { Title, Body, Recipients }
API->>API: Validate Title && Body (non-empty, non-whitespace)
alt invalid
API-->>Client: 400 BadRequest
else valid
API->>Svc: SaveMessage(...)
Svc-->>API: MessageId
API-->>Client: 201 Created
end
sequenceDiagram
autonumber
participant Parser as EmailParser
participant Tmpl as ResgridEmailTemplate
participant Geo as GeoLocationProvider
Parser->>Tmpl: Parse(data[])
alt data[3] (address) present
Tmpl->>Tmpl: c.Address = data[3]
Tmpl->>Geo: GetLatLonFromAddress(address)
alt geo success
Geo-->>Tmpl: Lat/Lon
Tmpl->>Tmpl: c.GeoLocationData = result
else exception/failure
Tmpl->>Tmpl: Log exception with email body context
end
else no address
Tmpl->>Tmpl: Skip geolocation
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
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 |
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Web/Resgrid.Web/Areas/User/Views/Groups/Geofence.cshtml (1)
74-84: Guard forModel.Groupis incompleteAdding
Model.Group == nullhere doesn’t actually prevent the null-reference crash you’re aiming to avoid. Earlier in the view (Line 42’sHiddenFor, Line 47’s@Model.Group.Name, etc.) we still dereferenceModel.Groupwithout a safety check, so if the controller hands us a null Group we’ll explode long before we reach this script block. Please either guaranteeModel.Groupis non-null before rendering the view, or wrap the form/UI in an@if (Model.Group != null)/elseso every access is guarded. Otherwise this change only masks the real problem.Web/Resgrid.Web/Areas/User/Controllers/ContactsController.cs (2)
498-505: Handle missing physical address records before reuseLine 498 rehydrates an existing physical address, but
_addressService.GetAddressByIdAsynccan legitimately returnnull(e.g., the address row was deleted while the FK still exists). The next line immediately dereferencesphysicalAddress, so we now crash with aNullReferenceExceptionon edit. Please keep the existingnew Address()fallback when no record is found.- if (contact.PhysicalAddressId.HasValue) - physicalAddress = await _addressService.GetAddressByIdAsync(contact.PhysicalAddressId.Value); + if (contact.PhysicalAddressId.HasValue) + { + var existingPhysical = await _addressService.GetAddressByIdAsync(contact.PhysicalAddressId.Value); + if (existingPhysical != null) + physicalAddress = existingPhysical; + }
518-525: Guard against null when loading the mailing addressSame issue as the physical address block: if the stored mailing address ID points to a record that no longer exists,
mailingAddressbecomesnulland the subsequent property assignments throw. Preserve the freshly constructedAddresswhen no row is returned.- if (contact.MailingAddressId.HasValue) - mailingAddress = await _addressService.GetAddressByIdAsync(contact.MailingAddressId.Value); + if (contact.MailingAddressId.HasValue) + { + var existingMailing = await _addressService.GetAddressByIdAsync(contact.MailingAddressId.Value); + if (existingMailing != null) + mailingAddress = existingMailing; + }Web/Resgrid.Web.Services/Controllers/v4/PersonnelController.cs (1)
188-194: Guard against nullactiveFiltersbefore iterating.We still enter the loop when
activeFiltercontains only whitespace (it’s not null,.Any()returns true), but in that branchactiveFiltersnever gets initialized and theforeachwill throw a NullReferenceException. Please switch the condition to check the array instead, e.g.:- if (activeFilter != null && activeFilter.Any()) + if (activeFilters != null && activeFilters.Any())That way we only iterate when the split actually produced entries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
Core/Resgrid.Services/AuthorizationService.cs(1 hunks)Core/Resgrid.Services/CallEmailTemplates/ResgridEmailTemplate.cs(2 hunks)Core/Resgrid.Services/ShiftsService.cs(1 hunks)Web/Resgrid.Web.Services/Controllers/v3/AuthController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/AvatarsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/BigBoardController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/CalendarController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/CallPrioritiesController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/CallsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/ChatController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/CommandAppController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/CommandsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/ConnectController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/CoreDataController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/DepartmentController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/DepartmentStatusController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/DevicesController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/DispatchAppController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/DispatchController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/FeedsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/GeoController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/HealthController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/LinksController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/MessagesController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/NotesController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/PersonnelController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/ProfileController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/ProtocolsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/SecurityController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/ShiftsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/StaffingController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/StaffingSchedulesController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/StationsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/StatusController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/UnitAppController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/UnitLocationController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/UnitStateController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/UnitsController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/V3AuthenticatedApiControllerBase.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v3/VoiceController.cs(0 hunks)Web/Resgrid.Web.Services/Controllers/v4/MessagesController.cs(1 hunks)Web/Resgrid.Web.Services/Controllers/v4/PersonnelController.cs(8 hunks)Web/Resgrid.Web.Services/Controllers/v4/UnitRolesController.cs(1 hunks)Web/Resgrid.Web.Services/Controllers/v4/UnitStatusController.cs(1 hunks)Web/Resgrid.Web.Services/Controllers/v4/UnitsController.cs(1 hunks)Web/Resgrid.Web/Areas/User/Controllers/ContactsController.cs(2 hunks)Web/Resgrid.Web/Areas/User/Views/Groups/Geofence.cshtml(2 hunks)
💤 Files with no reviewable changes (37)
- Web/Resgrid.Web.Services/Controllers/v3/DepartmentStatusController.cs
- Web/Resgrid.Web.Services/Controllers/v3/UnitLocationController.cs
- Web/Resgrid.Web.Services/Controllers/v3/BigBoardController.cs
- Web/Resgrid.Web.Services/Controllers/v3/MessagesController.cs
- Web/Resgrid.Web.Services/Controllers/v3/UnitAppController.cs
- Web/Resgrid.Web.Services/Controllers/v3/UnitStateController.cs
- Web/Resgrid.Web.Services/Controllers/v3/DevicesController.cs
- Web/Resgrid.Web.Services/Controllers/v3/StaffingController.cs
- Web/Resgrid.Web.Services/Controllers/v3/DispatchAppController.cs
- Web/Resgrid.Web.Services/Controllers/v3/DepartmentController.cs
- Web/Resgrid.Web.Services/Controllers/v3/PersonnelController.cs
- Web/Resgrid.Web.Services/Controllers/v3/AuthController.cs
- Web/Resgrid.Web.Services/Controllers/v3/CommandAppController.cs
- Web/Resgrid.Web.Services/Controllers/v3/HealthController.cs
- Web/Resgrid.Web.Services/Controllers/v3/ProfileController.cs
- Web/Resgrid.Web.Services/Controllers/v3/StaffingSchedulesController.cs
- Web/Resgrid.Web.Services/Controllers/v3/VoiceController.cs
- Web/Resgrid.Web.Services/Controllers/v3/UnitsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/AvatarsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/LinksController.cs
- Web/Resgrid.Web.Services/Controllers/v3/DispatchController.cs
- Web/Resgrid.Web.Services/Controllers/v3/CallPrioritiesController.cs
- Web/Resgrid.Web.Services/Controllers/v3/ChatController.cs
- Web/Resgrid.Web.Services/Controllers/v3/ProtocolsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/CalendarController.cs
- Web/Resgrid.Web.Services/Controllers/v3/StatusController.cs
- Web/Resgrid.Web.Services/Controllers/v3/NotesController.cs
- Web/Resgrid.Web.Services/Controllers/v3/CoreDataController.cs
- Web/Resgrid.Web.Services/Controllers/v3/CommandsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/SecurityController.cs
- Web/Resgrid.Web.Services/Controllers/v3/CallsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/FeedsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/GeoController.cs
- Web/Resgrid.Web.Services/Controllers/v3/StationsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/ShiftsController.cs
- Web/Resgrid.Web.Services/Controllers/v3/V3AuthenticatedApiControllerBase.cs
- Web/Resgrid.Web.Services/Controllers/v3/ConnectController.cs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: Prefer functional patterns and immutable data where appropriate in C#
Use modern C# features appropriately
Use meaningful, descriptive names for types, methods, and parameters; avoid unclear abbreviations
Separate state from behavior (e.g., use records for state and place operations in separate static classes)
Prefer pure methods over methods with side effects
Use extension methods appropriately for domain-specific operations
Design for testability; avoid hidden dependencies inside methods and prefer explicit, pure functions
Minimize constructor injection; keep the number of injected dependencies small
Prefer composition with interfaces to extend behavior
Files:
Web/Resgrid.Web.Services/Controllers/v4/UnitsController.csWeb/Resgrid.Web.Services/Controllers/v4/UnitRolesController.csCore/Resgrid.Services/AuthorizationService.csCore/Resgrid.Services/CallEmailTemplates/ResgridEmailTemplate.csWeb/Resgrid.Web.Services/Controllers/v4/UnitStatusController.csCore/Resgrid.Services/ShiftsService.csWeb/Resgrid.Web.Services/Controllers/v4/MessagesController.csWeb/Resgrid.Web/Areas/User/Controllers/ContactsController.csWeb/Resgrid.Web.Services/Controllers/v4/PersonnelController.cs
🧬 Code graph analysis (1)
Core/Resgrid.Services/CallEmailTemplates/ResgridEmailTemplate.cs (1)
Core/Resgrid.Framework/Logging.cs (2)
Logging(15-178)LogException(65-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
Web/Resgrid.Web/Areas/User/Views/Groups/Geofence.cshtml (1)
24-24: Breadcrumb casing fix acknowledgedThe copy edit from “GeoFence” to “Geofence” keeps the UI consistent with the rest of the product terminology. Nice catch.
Web/Resgrid.Web.Services/Controllers/v4/UnitsController.cs (1)
137-138: Guard prevents short filters from triggering substring errorsLine 137’s length/whitespace check cleanly skips empty segments, so the subsequent
Substring(0, 2)on Line 142 remains safe. Thanks for tightening this up.
Core/Resgrid.Services/CallEmailTemplates/ResgridEmailTemplate.cs
Outdated
Show resolved
Hide resolved
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit
New Features
Bug Fixes
Chores