LNK-4955: Report Service Bug FIxes#1526
Merged
nvmLantana merged 5 commits intodevfrom Mar 31, 2026
Merged
Conversation
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 PR normalizes Changes
Sequence DiagramsequenceDiagram
participant Producer as DataAcquisition<br/>Producer
participant Kafka as Kafka Topic
participant Listener as ReportScheduled<br/>Listener
participant Helper as QuartzJobHelper
participant JobDataMap as JobDataMap<br/>Extensions
participant Scheduler as Quartz Scheduler
Producer->>Producer: Normalize ReportStartDate,<br/>ReportEndDate to UTC
Producer->>Kafka: Publish ScheduledReport<br/>(UTC dates)
Listener->>Listener: Receive Message &<br/>Normalize ReportEndDate to UTC
Listener->>Helper: ScheduleJob with jobData<br/>dictionary
Helper->>JobDataMap: PutObject for each<br/>key/value pair
JobDataMap->>JobDataMap: Serialize objects<br/>(Guid, DateTime,<br/>DateTimeOffset, etc.)
Helper->>Scheduler: Schedule job with<br/>DateTimeOffset trigger
Scheduler->>Scheduler: Execute at<br/>scheduled UTC time
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DotNet/Report/KafkaProducers/DataAcquisitionRequestedProducer.cs (1)
76-95:⚠️ Potential issue | 🟠 MajorAdd unit tests and fix the DateTime normalization logic.
Lines 76-82 use
DateTime.SpecifyKind()to mark non-UTC dates as UTC instead of properly converting them to UTC. This preserves the wrong instant: ifschedule.ReportStartDateisLocal, relabeling it as UTC without conversion causesScheduledReport.StartDateto be serialized with an incorrect instant on the Kafka payload.Additionally, per coding guidelines, the if/else branches introduced at lines 76-82 must have corresponding XUnit tests to validate both the UTC and non-UTC code paths.
🕒 Suggested fix
- var reportStartDateUtc = schedule.ReportStartDate.Kind == DateTimeKind.Utc - ? schedule.ReportStartDate - : DateTime.SpecifyKind(schedule.ReportStartDate, DateTimeKind.Utc); - - var reportEndDateUtc = schedule.ReportEndDate.Kind == DateTimeKind.Utc - ? schedule.ReportEndDate - : DateTime.SpecifyKind(schedule.ReportEndDate, DateTimeKind.Utc); + static DateTime NormalizeUtc(DateTime value) => value.Kind switch + { + DateTimeKind.Utc => value, + DateTimeKind.Local => value.ToUniversalTime(), + _ => DateTime.SpecifyKind(value, DateTimeKind.Utc) + }; + + var reportStartDateUtc = NormalizeUtc(schedule.ReportStartDate); + var reportEndDateUtc = NormalizeUtc(schedule.ReportEndDate);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DotNet/Report/KafkaProducers/DataAcquisitionRequestedProducer.cs` around lines 76 - 95, The DateTime normalization in DataAcquisitionRequestedProducer is wrong: instead of using DateTime.SpecifyKind to relabel non-UTC instants, convert schedule.ReportStartDate and schedule.ReportEndDate to UTC (e.g., call ToUniversalTime() or use DateTimeOffset conversion) before assigning ScheduledReport.StartDate and EndDate so the instant is preserved; update the logic in the DataAcquisitionRequestedProducer class replacing SpecifyKind branches with proper UTC conversion and ensure ScheduledReport.StartDate/EndDate are the converted values. Also add XUnit tests for both code paths: one test where schedule.ReportStartDate/ReportEndDate already have Kind==Utc and one where they are Local/Unspecified to verify the produced Kafka payload contains the correct UTC instants (assert on the serialized ScheduledReport.StartDate/EndDate or resulting message value). Ensure tests target the method that builds the darValue (or the producer method that serializes it) and include clear assertions for expected UTC values.
🤖 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/Listeners/ReportScheduledListener.cs`:
- Around line 186-194: The code incorrectly uses DateTime.SpecifyKind for all
cases; change the conversion so that when reportSchedule.ReportEndDate.Kind ==
DateTimeKind.Utc you use the value as-is, when Kind == DateTimeKind.Local
convert with reportSchedule.ReportEndDate.ToUniversalTime(), and when Kind ==
DateTimeKind.Unspecified use DateTime.SpecifyKind(..., DateTimeKind.Utc) to
treat it as UTC; ensure the variable reportEndDateUtc holds the correct UTC
instant and then pass new DateTimeOffset(reportEndDateUtc) into
ScheduleJob<EndOfReportPeriodJob>. Add XUnit tests that exercise ScheduleJob
invocation (or inspect the computed DateTimeOffset) for all three branches (Utc,
Local, Unspecified) to verify the scheduled time is correct.
In `@DotNet/Shared/Application/Extensions/JobDataMapExtensions.cs`:
- Around line 52-57: The current branch in JobDataMapExtensions (checking
targetType == typeof(string)) treats empty/whitespace strings as missing by
using string.IsNullOrWhiteSpace, which drops valid payloads; update the branch
in the PutObject/round-trip logic to preserve raw string values: if storedValue
is string storedString then return storedString (do not treat empty or
whitespace as default/missing). Adjust/remove the string.IsNullOrWhiteSpace
check and ensure other branches (the non-string path that handles
whitespace-as-missing) remain unchanged. Add an xUnit test verifying that empty
("") and whitespace (" ") strings round-trip correctly through
PutObject/GetObject.
---
Outside diff comments:
In `@DotNet/Report/KafkaProducers/DataAcquisitionRequestedProducer.cs`:
- Around line 76-95: The DateTime normalization in
DataAcquisitionRequestedProducer is wrong: instead of using DateTime.SpecifyKind
to relabel non-UTC instants, convert schedule.ReportStartDate and
schedule.ReportEndDate to UTC (e.g., call ToUniversalTime() or use
DateTimeOffset conversion) before assigning ScheduledReport.StartDate and
EndDate so the instant is preserved; update the logic in the
DataAcquisitionRequestedProducer class replacing SpecifyKind branches with
proper UTC conversion and ensure ScheduledReport.StartDate/EndDate are the
converted values. Also add XUnit tests for both code paths: one test where
schedule.ReportStartDate/ReportEndDate already have Kind==Utc and one where they
are Local/Unspecified to verify the produced Kafka payload contains the correct
UTC instants (assert on the serialized ScheduledReport.StartDate/EndDate or
resulting message value). Ensure tests target the method that builds the
darValue (or the producer method that serializes it) and include clear
assertions for expected UTC values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 001e79e3-77bb-4baa-9182-49f6d8a7f50c
📒 Files selected for processing (8)
DotNet/Report/KafkaProducers/DataAcquisitionRequestedProducer.csDotNet/Report/Listeners/ReportScheduledListener.csDotNet/Report/Program.csDotNet/Report/Report.csprojDotNet/ServiceTests/UnitTests/Shared/JobDataMapExtensionsTests.csDotNet/ServiceTests/UnitTests/Shared/QuartzJobHelperTests.csDotNet/Shared/Application/Extensions/JobDataMapExtensions.csDotNet/Shared/Application/Utilities/QuartzJobHelper.cs
smailliwcs
approved these changes
Mar 30, 2026
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
https://lantana.atlassian.net/browse/LNK-4955
This PR applies the Report scheduled-workflow bug fixes and supporting reliability tests.
Quartz Registration
UTC-safe scheduling for End of Report Period
UTC-safe serialization for Data Acquisition payload dates
Quartz JobDataMap reliability improvements
🧪 Testing Performed
Adhoc End to End
Report Sceduled End to End
In depth data validation.
JobDataMapExtensionsTests
QuartzJobHelperTests
🧑🔬 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
Bug Fixes
New Features
Tests