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(workflow-engine): add AssignedToConditionHandler #82956

Merged
merged 5 commits into from
Jan 7, 2025

Conversation

ameliahsu
Copy link
Member

add AssignedToConditionHandler

@ameliahsu ameliahsu requested review from cathteng and a team January 6, 2025 21:22
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 6, 2025
Comment on lines 36 to 38
for assignee in assignees:
if assignee.team and assignee.team_id == target_id:
return True
Copy link
Member

Choose a reason for hiding this comment

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

i think we could refactor this slightly

Suggested change
for assignee in assignees:
if assignee.team and assignee.team_id == target_id:
return True
return any(assignee.team and assignee.team_id == target_id for assignee in assignees)

and similarly for the chunk below

assignee_list: Sequence[GroupAssignee] | None = cache.get(cache_key)
if assignee_list is None:
assignee_list = list(group.assignee_set.all())
cache.set(cache_key, assignee_list, 60)
Copy link
Member

Choose a reason for hiding this comment

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

is 60 in seconds? is that a bit short 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷🏼

Copy link
Member

Choose a reason for hiding this comment

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

the original PR doesn't have much context 😭 oh well. at least we're keeping it the same

Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #82956       +/-   ##
===========================================
+ Coverage   45.27%   87.56%   +42.29%     
===========================================
  Files        9407     9423       +16     
  Lines      535958   536696      +738     
  Branches    21119    21106       -13     
===========================================
+ Hits       242632   469965   +227333     
+ Misses     292966    66366   -226600     
- Partials      360      365        +5     

@ameliahsu ameliahsu merged commit e42bb06 into master Jan 7, 2025
49 checks passed
@ameliahsu ameliahsu deleted the mia/aci/assigned-to branch January 7, 2025 21:01
andrewshie-sentry pushed a commit that referenced this pull request Jan 22, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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