-
-
Notifications
You must be signed in to change notification settings - Fork 76
CU-868d85dv8 Fixing contact insert and adding delete message inbox api. #230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR expands dispatch settings to include unit-level options, updates the Contacts table schema for longer/nullable fields, and introduces a new API endpoint to delete inbox messages.
- Add new “Unit Dispatch” options in UI, view model, controller, and settings service
- Create Contact table migration (M0035) to alter string columns to nullable with max length
- Implement
DeleteMessageendpoint inInboxControllerand corresponding Novu provider support
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| Workers/Resgrid.Workers.Framework/Logic/SystemQueueLogic.cs | Added exception logging and removed an empty else block |
| Workers/Resgrid.Workers.Framework/Logic/CallBroadcast.cs | Fetched new dispatch flags (currently unused) and added an unused import |
| Web/Resgrid.Web/Areas/User/Views/Department/DispatchSettings.cshtml | Switched hardcoded text to localization and added checkboxes for unit dispatch settings |
| Web/Resgrid.Web/Areas/User/Models/Departments/DispatchSettingsView.cs | Added properties for the new unit dispatch settings |
| Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs | Persist and load new dispatch settings, renamed regions for clarity |
| Web/Resgrid.Web.Services/Resgrid.Web.Services.xml | Documented new InboxController.DeleteMessage API |
| Web/Resgrid.Web.Services/Controllers/v4/InboxController.cs | Added DeleteMessage endpoint for inbox API |
| Providers/Resgrid.Providers.Migrations/Migrations/M0035_UpdatingContactsTableAgain.cs | Migration to alter Contacts table columns to MaxValue and nullable |
| Providers/Resgrid.Providers.Messaging/NovuProvider.cs | Added DeleteMessage to provider, updated notification payload formatting |
| Core/Resgrid.Services/PushService.cs | Commented out old push registration logic, added exception-safe calls for notifications |
| Core/Resgrid.Services/DepartmentSettingsService.cs | Added methods to read new unit dispatch settings |
| Core/Resgrid.Model/Services/IDepartmentSettingsService.cs | Exposed new unit dispatch getters in the interface |
| Core/Resgrid.Model/Providers/Models/INovuProvider.cs | Added DeleteMessage contract |
| Core/Resgrid.Model/DepartmentSettingTypes.cs | Introduced enum values for new dispatch settings |
| Core/Resgrid.Localization/Areas/User/Department/Department.resx (and .es/.en) | Added resource keys for new dispatch settings and updated schema metadata |
| .github/copilot-instructions.md | Added repository-specific coding guidelines |
Comments suppressed due to low confidence (2)
Web/Resgrid.Web.Services/Resgrid.Web.Services.xml:3556
- The
<returns>tag forDeleteMessageis empty; consider documenting the HTTP status codes or return values.
<returns></returns>
Web/Resgrid.Web.Services/Controllers/v4/InboxController.cs:43
- [nitpick] Consider using a RESTful route like
[HttpDelete("{messageId}")]so themessageIdis part of the path rather than a query parameter.
[HttpDelete("DeleteMessage")]
| using SharpKml.Dom; | ||
|
|
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The SharpKml.Dom import appears unused; please remove it to keep dependencies clean.
| using SharpKml.Dom; |
| if (_unitsService == null) | ||
| _unitsService = Bootstrapper.GetKernel().Resolve<IUnitsService>(); | ||
|
|
||
| bool alsoDispatchToAssignedPersonnel = await _departmentSettingsService.GetUnitDispatchAlsoDispatchToAssignedPersonnelAsync(cqi.Call.DepartmentId); |
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This setting value is retrieved but never used; implement logic to use alsoDispatchToAssignedPersonnel or remove the call.
| bool alsoDispatchToGroup = await _departmentSettingsService.GetUnitDispatchAlsoDispatchToGroupAsync(cqi.Call.DepartmentId); | ||
|
|
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This setting value is retrieved but never used; implement logic to use alsoDispatchToGroup or remove the call.
| bool alsoDispatchToGroup = await _departmentSettingsService.GetUnitDispatchAlsoDispatchToGroupAsync(cqi.Call.DepartmentId); |
|
|
||
| public override void Down() | ||
| { | ||
|
|
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Down method is empty—implement reverse operations or throw NotImplementedException to ensure clean rollbacks.
| throw new NotImplementedException("Rollback is not supported for this migration."); |
| { | ||
| try | ||
| { | ||
| using (var httpClient = new HttpClient()) |
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new HttpClient per call can exhaust sockets; consider reusing a singleton or injecting via IHttpClientFactory.
| }//, | ||
| //data = new | ||
| //{ | ||
| // title = title, | ||
| // message = body, | ||
| // eventCode = eventCode, | ||
| // type = type | ||
| //} |
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Remove commented-out payload code to keep the implementation clear and maintainable.
| }//, | |
| //data = new | |
| //{ | |
| // title = title, | |
| // message = body, | |
| // eventCode = eventCode, | |
| // type = type | |
| //} | |
| } |
| using DnsClient; | ||
|
|
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The DnsClient import is unused and can be removed to reduce clutter.
| using DnsClient; |
| <value /> | ||
| </data> | ||
| <data name="UnitDispatchAlsoDispatchToGroupLabel" xml:space="preserve"> | ||
| <value /> |
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This new resource key has an empty <value>—add default text to prevent missing UI labels.
| <value /> | |
| </data> | |
| <data name="UnitDispatchAlsoDispatchToGroupLabel" xml:space="preserve"> | |
| <value /> | |
| <value>Also dispatch to assigned personnel</value> | |
| </data> | |
| <data name="UnitDispatchAlsoDispatchToGroupLabel" xml:space="preserve"> | |
| <value>Also dispatch to group</value> |
| <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value> | ||
| </resheader> | ||
| <data name="DeleteDepartmentSettingsInfo" xml:space="preserve"> | ||
| <value /> |
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Spanish localization is missing entries for the new dispatch settings keys; add translations to match the English .resx.
| <value /> | |
| <value>Información sobre la configuración de eliminación del departamento</value> |
| <value>Unit Dispatch Settings</value> | ||
| </data> | ||
| <data name="UnitDispatchAlsoDispatchToAssignedPersonnelLabel" xml:space="preserve"> | ||
| <value>Also Dispatch To Assigned Personnel</value> |
Copilot
AI
Jul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] There are two spaces between "Assigned" and "Personnel"; reduce to a single space for consistency.
| <value>Also Dispatch To Assigned Personnel</value> | |
| <value>Also Dispatch To Assigned Personnel</value> |
|
Approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is approved.
No description provided.