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

feat(feature-activation): fix criteria validation #708

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Jul 18, 2023

Motivation

Due to the way validations were performed on the Criteria class, some tests were flaky and behaving in unexpected ways. This PR changes the way Criteria attributes are validated to a more robust solution.

Acceptance Criteria

  • Implement the ValidatedCriteria class and move all Criteria validation to it
  • Implement the Criteria.to_validated() method
  • Update FeatureSettings validation to use the new Criteria validation structure

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco self-assigned this Jul 18, 2023
@glevco glevco marked this pull request as ready for review July 18, 2023 01:08
@glevco glevco requested review from jansegre and msbrogli as code owners July 18, 2023 01:08
@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #708 (454ca26) into master (779494c) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #708      +/-   ##
==========================================
- Coverage   84.00%   83.99%   -0.02%     
==========================================
  Files         246      246              
  Lines       20517    20523       +6     
  Branches     2769     2770       +1     
==========================================
+ Hits        17236    17238       +2     
- Misses       2707     2709       +2     
- Partials      574      576       +2     
Impacted Files Coverage Δ
hathor/feature_activation/model/criteria.py 100.00% <100.00%> (ø)
hathor/feature_activation/settings.py 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

jansegre
jansegre previously approved these changes Jul 18, 2023
@glevco glevco force-pushed the fix/feature-activation/criteria-validation branch from 4d1be8e to 454ca26 Compare July 18, 2023 16:11
@glevco glevco merged commit 7bee591 into master Jul 19, 2023
@glevco glevco deleted the fix/feature-activation/criteria-validation branch July 19, 2023 17:44
@jansegre jansegre mentioned this pull request Jul 24, 2023
2 tasks
This was referenced Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants