Skip to content

Fix: Show data for the last day of the month for usage graphs #1999

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 1, 2025
Merged

Fix: Show data for the last day of the month for usage graphs #1999

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/webapp/app/utils/graphs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ type Options<R> = {
export function createTimeSeriesData<R>({ startDate, endDate, window = "DAY", data }: Options<R>) {
const outputData: Array<{ date: Date; value?: R }> = [];
const periodLength = periodLengthMs(window);
const periodCount = Math.floor((endDate.getTime() - startDate.getTime()) / periodLength);
const periodCount = Math.round((endDate.getTime() - startDate.getTime()) / periodLength);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Logic improved to include last day of the month

The change from Math.floor to Math.round fixes the issue where the last day of the month might be cut off in usage graphs. This correctly aligns with the PR objective.

Previously, if there was a partial period at the end of the date range, it would always be truncated. Now, if that partial period is at least half complete, it will be included in the output data.

You might want to verify this behavior with a simple test to ensure it works as expected:

🏁 Script executed:

#!/bin/bash
# Find any existing tests for this function
rg "createTimeSeriesData" --type ts --type tsx

Length of output: 74

🏁 Script executed:

#!/bin/bash
# Search for createTimeSeriesData definition across TS/TSX files
rg --context 2 "function createTimeSeriesData" -g '*.ts' -g '*.tsx'

# Search for any test files referencing createTimeSeriesData
rg --context 2 "createTimeSeriesData" -g '*.spec.ts' -g '*.test.ts'

Length of output: 507

🏁 Script executed:

#!/bin/bash
# Retrieve relevant code around periodCount in createTimeSeriesData
rg --context 5 "const periodCount" -g 'apps/webapp/app/utils/graphs.ts'

# Locate periodLengthMs function definition to understand periodLength calculation
rg --context 5 "function periodLengthMs" -g 'apps/webapp/app/utils/graphs.ts'

Length of output: 1544

Add unit tests for the updated rounding logic

We switched from Math.floor to Math.round so that any partial period ≥50% is now included in the output. However, there are no existing tests for createTimeSeriesData, so we can’t automatically verify the new behavior. Please add a test suite covering boundary conditions:

• File: apps/webapp/app/utils/graphs.ts

const periodCount = Math.round((endDate.getTime() - startDate.getTime()) / periodLength);

• Scenarios to cover:

  • Exact multiples of a period (e.g. 24h → 1 day)
  • Partial period just under 50% (e.g. 11h for a 24h window → expect 0 days)
  • Partial period just over 50% (e.g. 13h for a 24h window → expect 1 day)
  • Each window type (“MINUTE”, “HOUR”, “DAY”)
  • Very short ranges (<50% of one period)

Adding these will ensure the rounding change behaves correctly across all cases.


for (let i = 0; i < periodCount; i++) {
const periodStart = new Date(startDate);
Expand Down