LNK-4924: Added Search field to Report Dashboard and Facility View Re… - #1515
Merged
Conversation
…port pages. Added deleted facilities to the facilities displayed in the search fields if show deleted is checked
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds a creation date filter capability across the reporting stack. The backend propagates a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… LNK-4924-Add-SearchFieldsReportScreen # Conflicts: # Web/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.html
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
DotNet/Tenant/Controllers/FacilityController.cs (1)
149-164:⚠️ Potential issue | 🟡 MinorMissing input sanitization on
searchparameter.The
searchquery parameter should be sanitized before use, consistent with other endpoints in this controller (e.g.,GetFacilitiessanitizesfacilityId,facilityName,sortBy). As per coding guidelines: "TheHtmlInputSanitizerclass'sSanitize()andSanitizeAndRemove()methods should be used when dealing withstringquery parameters from REST requests."Proposed fix
public async Task<IActionResult> GetFacilityList([FromQuery] string? search, bool includeDeleted = false) { try { + search = search?.Sanitize(); FacilitySearchModel searchModel = new FacilitySearchModel(); if (!string.IsNullOrEmpty(search)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DotNet/Tenant/Controllers/FacilityController.cs` around lines 149 - 164, GetFacilityList reads the raw search query into FacilitySearchModel without sanitizing; before assigning to searchModel.FacilityName use the HtmlInputSanitizer (call Sanitize or SanitizeAndRemove as appropriate) on the incoming search string to strip unsafe input, then set FacilityName and FacilityNameContains and proceed to call _facilityQueries.SearchAsync; ensure the same sanitizer is used as in other controller methods (e.g., GetFacilities) so the input is consistently cleaned before constructing the FacilitySearchModel.DotNet/Report/Controllers/ReportScheduleController.cs (1)
241-269:⚠️ Potential issue | 🟡 MinorDocument the new
createDatequery parameter.The XML comments stop at
pageNumber, so this filter will be exposed without any description of what day semantics it uses. Please add a<param name="createDate">entry here. As per coding guidelines, "Implement Swagger/OpenAPI documentation for every API."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DotNet/Report/Controllers/ReportScheduleController.cs` around lines 241 - 269, Add an XML <param name="createDate"> entry to the Search method's documentation in ReportScheduleController (the Search(...) action) describing the new createDate query parameter as an optional filter that matches schedules created on the specified date; clarify the day semantics (treats the value as a date-only filter matching any timestamp on that calendar day, inclusive of the day's start and end, and specify whether it uses UTC or server/local time per project convention). Ensure the new <param> follows the same style as the other parameters.
🧹 Nitpick comments (2)
Web/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.html (1)
91-141: Pick one filtering interaction model.These selects/date pickers already call
applyFilters()on every change, so the Apply button is redundant and each intermediate tweak fires a new request. If this screen should follow the manual pattern used elsewhere, remove the per-control change handlers and let the explicit Apply action own reloads. Based on learnings, the team prefers manual filtering with an explicitapplyFilters()action rather than per-control change handlers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.html` around lines 91 - 141, Remove the per-control automatic submissions and rely solely on the explicit Apply button: delete all (selectionChange)="applyFilters()" on the mat-selects and all (dateChange)="applyFilters()" on the date inputs so statusFilter, frequencyFilter, createDateFilter, reportStartDateFilter and reportEndDateFilter only update via ngModel; keep the Apply button wired to applyFilters() and Clear button to clearFilters()/hasActiveFilters() so manual filtering behavior is preserved.Web/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.html (1)
45-92: Pick one filtering interaction model.These selects/date pickers already call
applyFilters()on every change, so the Apply button is redundant and each intermediate tweak fires a new request. If this screen is meant to use explicit filtering, remove the per-control change handlers and let the Apply action own reloads. Based on learnings, the team prefers manual filtering with an explicitapplyFilters()action rather than per-control change handlers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.html` around lines 45 - 92, The template currently calls applyFilters() on each control change ((selectionChange) and (dateChange)), causing immediate reloads; since we want an explicit/manual filter model, remove the per-control change handlers from the status/frequency mat-selects and the createDate/start/end date inputs so they only update bound properties (statusFilter, frequencyFilter, createDateFilter, reportStartDateFilter, reportEndDateFilter) and rely on the existing Apply button's (click)="applyFilters()" to trigger filtering; leave clearFilters()/hasActiveFilters() behavior as-is for the Clear button.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DotNet/Report/Business/Managers/ReportScheduledManager.cs`:
- Around line 234-239: The current filter builds day bounds from a server
timestamp (createDate.Value.Date) which causes timezone off-by-one errors for
ReportSchedule.CreateDate; update the predicate in ReportScheduledManager (the
block using createDate, dayStart, dayEnd and predicate.And) to use explicit UTC
date bounds (e.g., interpret the incoming createDate as a date-only in a
documented timezone, convert to UTC start-of-day and end-of-day offsets, and
compare CreateDate converted to UTC) or switch to a date-only comparison
(DateOnly) so comparisons are timezone-safe; also add XUnit tests covering
createDate == null and the boundary cases (exactly at start and end-of-day in
both server and non-server timezones) so each branch is covered.
In
`@Web/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.ts`:
- Around line 155-156: The reportEndDateFilter is sent raw to the API while the
backend treats it as inclusive, causing same-day results to be dropped; before
the API call in ReportsDashboardComponent, normalize reportEndDateFilter by
converting it to an end-of-day timestamp (set time to 23:59:59.999) or
alternatively advance it by one day and send that as an exclusive upper bound,
then pass that normalized value instead of this.reportEndDateFilter (keep
reportStartDateFilter unchanged); update the place where these values are
assembled (the code around the this.reportStartDateFilter /
this.reportEndDateFilter usage) to perform this normalization.
- Around line 120-126: The facility textbox and filter logic are out of sync:
typed edits to facilityInputControl never update selectedFacilityId so requests
keep using a stale ID or none; fix by syncing the ID with control edits and
selection events — when the user selects a suggestion set selectedFacilityId to
that facilityId (handle the option selection/selection event), and on
valueChanges (in filteredFacilities pipeline or a separate subscriber) clear
selectedFacilityId whenever the typed text no longer equals the selected
facilityName; alternatively change the Apply/Enter filter logic to derive the
requested facility filter directly from facilityInputControl.value instead of
selectedFacilityId. Update the code referencing filteredFacilities,
facilityInputControl, selectedFacilityId and
tenantService.autocompleteFacilities (and repeat the same fix for the other
occurrences mentioned) so the textbox and selected ID stay consistent.
In
`@Web/Admin.UI/src/app/components/tenant/acquisition-log/acquisition-log-view/acquisition-log-view.component.ts`:
- Around line 394-405: The onIncludeDeletedChange method calls applyFilters()
immediately after subscribing to tenantService.getAllFacilities, causing a race
where filters run before facilities are updated; move the applyFilters() call
into the subscription handlers so filters are applied after facilities are
loaded (inside the next callback after setting facilityFilterOptions and
possibly resetting selectedFacilityFilter) and also call applyFilters() in the
error callback to ensure consistent behavior when the request fails.
In
`@Web/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.ts`:
- Around line 186-187: The reportEndDateFilter is sent as-is but backend treats
ReportEndDate as inclusive, so normalize reportEndDateFilter before the API call
in facility-view.component.ts (where reportStartDateFilter and
reportEndDateFilter are passed) by converting it to an end-of-day timestamp (set
time to 23:59:59.999) or by converting it to an exclusive next-day bound (add
one day and use exclusive comparison) so reports on the selected day are not
dropped; update the code that constructs the request payload to use the
normalizedReportEndDate instead of raw reportEndDateFilter.
---
Outside diff comments:
In `@DotNet/Report/Controllers/ReportScheduleController.cs`:
- Around line 241-269: Add an XML <param name="createDate"> entry to the Search
method's documentation in ReportScheduleController (the Search(...) action)
describing the new createDate query parameter as an optional filter that matches
schedules created on the specified date; clarify the day semantics (treats the
value as a date-only filter matching any timestamp on that calendar day,
inclusive of the day's start and end, and specify whether it uses UTC or
server/local time per project convention). Ensure the new <param> follows the
same style as the other parameters.
In `@DotNet/Tenant/Controllers/FacilityController.cs`:
- Around line 149-164: GetFacilityList reads the raw search query into
FacilitySearchModel without sanitizing; before assigning to
searchModel.FacilityName use the HtmlInputSanitizer (call Sanitize or
SanitizeAndRemove as appropriate) on the incoming search string to strip unsafe
input, then set FacilityName and FacilityNameContains and proceed to call
_facilityQueries.SearchAsync; ensure the same sanitizer is used as in other
controller methods (e.g., GetFacilities) so the input is consistently cleaned
before constructing the FacilitySearchModel.
---
Nitpick comments:
In
`@Web/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.html`:
- Around line 45-92: The template currently calls applyFilters() on each control
change ((selectionChange) and (dateChange)), causing immediate reloads; since we
want an explicit/manual filter model, remove the per-control change handlers
from the status/frequency mat-selects and the createDate/start/end date inputs
so they only update bound properties (statusFilter, frequencyFilter,
createDateFilter, reportStartDateFilter, reportEndDateFilter) and rely on the
existing Apply button's (click)="applyFilters()" to trigger filtering; leave
clearFilters()/hasActiveFilters() behavior as-is for the Clear button.
In
`@Web/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.html`:
- Around line 91-141: Remove the per-control automatic submissions and rely
solely on the explicit Apply button: delete all
(selectionChange)="applyFilters()" on the mat-selects and all
(dateChange)="applyFilters()" on the date inputs so statusFilter,
frequencyFilter, createDateFilter, reportStartDateFilter and reportEndDateFilter
only update via ngModel; keep the Apply button wired to applyFilters() and Clear
button to clearFilters()/hasActiveFilters() so manual filtering behavior is
preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a7a1709-95a7-4adf-9341-12d754ca02c5
📒 Files selected for processing (16)
DotNet/Admin.BFF/Application/Clients/ReportService.csDotNet/Admin.BFF/Presentation/Endpoints/Aggregation/Handlers/Report/GetReportSummaries.csDotNet/Report/Business/Managers/ReportScheduledManager.csDotNet/Report/Controllers/ReportScheduleController.csDotNet/Tenant/Controllers/FacilityController.csWeb/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.htmlWeb/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.scssWeb/Admin.UI/src/app/components/reports/reports-dashboard/reports-dashboard.component.tsWeb/Admin.UI/src/app/components/tenant/acquisition-log/acquisition-log-view/acquisition-log-view.component.htmlWeb/Admin.UI/src/app/components/tenant/acquisition-log/acquisition-log-view/acquisition-log-view.component.tsWeb/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.htmlWeb/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.scssWeb/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.tsWeb/Admin.UI/src/app/interfaces/report/report-schedule.interface.tsWeb/Admin.UI/src/app/services/gateway/report/report.service.tsWeb/Admin.UI/src/app/services/gateway/tenant/tenant.service.ts
amphillipsLGC
approved these changes
Mar 24, 2026
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠️ Description of Changes
Added Search field to Report Dashboard and Facility View Report pages.
Added deleted facilities to the facilities displayed in the search fields if show deleted is checked
🧪 Testing Performed
Tested locally
🧑🔬 Unit Testing
📓 Documentation Updated
Please update any relevant sections in the project documentation that were impacted by the changes in the PR.
Summary by CodeRabbit
Release Notes
New Features
Style