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

fix(aci): rename DataConditionHandler type and filter_group #87232

Merged
merged 1 commit into from
Mar 18, 2025

Conversation

ameliahsu
Copy link
Member

@ameliahsu ameliahsu commented Mar 17, 2025

Renaming type to group and filter_group to subgroup.

  • type was a bit confusing because it can be confused with the data condition type (ex. age_comparison or first_seen_event) instead of the handler type (ex. workflow_trigger or action_filter)
  • The name filter_group could have implied filtering conditions or data, rather than grouping in the UI. This attribute (now named subgroup) is used to create sub-categories (ex. "filter by frequency") Screenshot 2025-03-17 at 2 40 38 PM

Note that these are just used for the UI so that we can filter/sort the registry.

@ameliahsu ameliahsu requested a review from a team as a code owner March 17, 2025 21:35
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 17, 2025
Copy link

codecov bot commented Mar 17, 2025

Codecov Report

Attention: Patch coverage is 92.98246% with 4 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...try/workflow_engine/processors/delayed_workflow.py 60.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #87232       +/-   ##
===========================================
+ Coverage   33.18%   87.73%   +54.55%     
===========================================
  Files        8341     9864     +1523     
  Lines      464758   558746    +93988     
  Branches    22040    22040               
===========================================
+ Hits       154210   490201   +335991     
+ Misses     310117    68114   -242003     
  Partials      431      431               

Copy link
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

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

lgtm 🎉 thanks for the cleanup!

@ameliahsu ameliahsu merged commit 6fc0640 into master Mar 18, 2025
48 checks passed
@ameliahsu ameliahsu deleted the mia/dc-handlers/rename branch March 18, 2025 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants