Skip to content

feat(pricing_group_keys): Add validation logic for all charge models #3666

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 3 commits into from
May 26, 2025

Conversation

vincent-pochet
Copy link
Collaborator

@vincent-pochet vincent-pochet commented May 16, 2025

Context

This PR is part of an epic to add grouped_by feature on all charge models and not only on the standard one like today.

Description

This PR is the first step:

  • It refactors the validation logic for Charge and ChargeFilter to share the logic
  • It moves the grouped_by validation logic from the Charges::Validators::StandardService to the Charges::Validators::BaseService so that it can be applied to all charge models
  • It adds the ability to send grouping keys via pricing_group_keys or grouped_by to ensure a smooth transition between the two keys.

For now grouped_by properties remains whitelisted only on the standard charge model. This will remain like this until the grouped_by logic is applied to all charge model

@vincent-pochet vincent-pochet force-pushed the feat-grouped-by branch 3 times, most recently from 6611a0c to 4676d4c Compare May 16, 2025 15:56
@vincent-pochet vincent-pochet changed the title feat(pricing_group_keys): Add validation logic for all pricing group keys feat(pricing_group_keys): Add validation logic for all charge models May 16, 2025
@vincent-pochet vincent-pochet merged commit 6efa551 into main May 26, 2025
14 checks passed
@vincent-pochet vincent-pochet deleted the feat-grouped-by branch May 26, 2025 07:48
vincent-pochet added a commit that referenced this pull request Jun 5, 2025
…3681)

## Context

This PR follows #3666 and is
part of an epic to add grouped_by feature on all charge models and not
only on the standard one like today.

## Description

This PR is the second step:
- It updates the GraphQL and the Rest APIs to accept and return the new
`pricing_group_keys` attribute
- It makes sure that the deprecated `grouped_by` attribute is still
accepted and returned
- It adapt the aggregation logic to use the `pricing_group_keys` or the
`grouped_by` attributes

For now the grouped aggregation remains remains whitelisted only on the
standard and dynamic charge model. This will remain like this until the
logic is applied to all charge model
vincent-pochet added a commit that referenced this pull request Jun 6, 2025
…ions (#3789)

## Context

This PR follows #3666 and
#3681 as part of the epic to add
grouped_by feature on all charge models and not only on the standard one
like today.

## Description

This PR is the second step:
- Adds the ability to inject a set of grouped_by values when fetching
events in an event store implementation (with_grouped_by_values)
- Takes advantage of this ability to change the implementation of all
"per event" aggregation logic making sure it always targets the right
pricing_group.


For now the grouped aggregation remains remains whitelisted only on the
standard and dynamic charge model. This will change in the next step of
this feature.
vincent-pochet added a commit that referenced this pull request Jun 17, 2025
## Context

This PR follows #3666,
#3681 and
#3789 as part of the epic to add
grouped_by feature on all charge models and not only on the standard one
like today.

## Description

This PR removes the restriction on charge model to allow
`pricing_group_keys` on every charge model
diegocharles pushed a commit that referenced this pull request Jul 11, 2025
## Context

This PR follows #3666,
#3681 and
#3789 as part of the epic to add
grouped_by feature on all charge models and not only on the standard one
like today.

## Description

This PR removes the restriction on charge model to allow
`pricing_group_keys` on every charge model
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