Skip to content

fix(daily-usage): Fix from and to dates in daily usages service #3717

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

Merged
merged 2 commits into from
May 26, 2025

Conversation

ivannovosad
Copy link
Contributor

Description

This PR fixes a bug that calculates usages and pre-fills daily_usages table:

  • dates from to were not charges dates
  • the serializer that is used was not grouping charge filters

Copy link

@Copilot Copilot AI left a 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 fixes the date range used when populating daily_usages to correctly use charge dates and adjusts the serializer to group and aggregate filter metrics.

  • Switched from_datetime/to_datetime to charges_from_datetime/charges_to_datetime in the fill service.
  • Refactored ChargeUsageSerializer#filters to group fees by their filter attributes and sum units, amounts, and event counts.
  • Updated the serializer spec to expect aggregated totals over three identical fee entries.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
spec/serializers/v1/customers/charge_usage_serializer_spec.rb Updated test setup to build three fee entries and check summed values.
app/services/daily_usages/fill_from_invoice_service.rb Changed date fields to use charge-specific datetime attributes.
app/serializers/v1/customers/charge_usage_serializer.rb Replaced sort-by map logic with grouping and aggregation.
Comments suppressed due to low confidence (1)

spec/serializers/v1/customers/charge_usage_serializer_spec.rb:91

  • The spec currently only verifies grouping for identical filters. Consider adding a test case with multiple distinct charge_filter values to ensure the grouping and aggregation logic handles varied inputs correctly.
expect(result["charges"].first["filters"].first).to include(

@ivannovosad ivannovosad force-pushed the fix-daily-usages-dates branch from 0d5a21c to 73cbb72 Compare May 26, 2025 12:17
@ivannovosad ivannovosad merged commit cf6ee86 into main May 26, 2025
14 checks passed
@ivannovosad ivannovosad deleted the fix-daily-usages-dates branch May 26, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants