Skip to content
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

misc(filters-contracts): Define contracts to validate query objects filters #2650

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ancorcruz
Copy link
Contributor

Context

Currently query objects receive a filters hash, the filters are not validated and used to build the SQL query agains Lago DB.

Also, developers need to read the query object implementation just to be aware of the available filters.

Description

This is a PoC introduces contracts to validate query objects filters, validating the filter provides a following benefits;

  1. Ensure each query object filters are correctly validated before executing the query.

  2. Decouple filter validation logic form the query objects.

  3. Makes explicit to developers which filters are available without having to dig into the query object implementation details.

dry-validation is added as dependency.

Bear in mind, this first contract is meant as an example an covers a simple scenario, there are other query objects with many and more complex filters available.

Validate query objects filters received by against a contract, this will
provide a few benefits;

1. Ensure each query object filters are correctly validated before
   excuting the query.

2. Decouple filter validation logic form the query objects.

3. Makes explicit to developers which filters are available without
   having to dig into the query object implementation details.
@ancorcruz ancorcruz self-assigned this Oct 2, 2024
Copy link
Collaborator

@vincent-pochet vincent-pochet left a comment

Choose a reason for hiding this comment

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

Really cool! I like how it fit with the Result pattern 👍

@@ -22,6 +22,19 @@ def initialize(organization:, pagination: DEFAULT_PAGINATION_PARAMS, filters: {}

attr_reader :organization, :pagination_params, :filters, :search_term, :order

def validate_filters
contract_class_name = "#{self.class.name}FiltersContract"
contract = Object.const_get(contract_class_name).new
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a thought, do you think we could find a way to pass or assign the Contract class to the query class, to avoid doing some magic to load it?

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