Skip to content

fix: filter out disabled checks for execution count total #999

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

ckbedwell
Copy link
Contributor

Addresses the FE portion of: https://github.com/grafana/support-escalations/issues/13498

Problem

We aren't filtering out disabled checks when calculating a user's execution count.

Solution

Add a filter to remove disabled checks when checking monthly execution counts.

@ckbedwell ckbedwell requested a review from a team as a code owner November 25, 2024 16:45
@ckbedwell ckbedwell requested review from w1kman and VikaCep November 25, 2024 16:46
},
]);

expect(result.current.data).toBe(EXECUTIONS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we certain EXECUTIONS contains the correct value here? Shouldn't we use a mocked value instead to validate it is returning the results we want in a more explicit way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about hard-coding it but the problem is if we ever do update the calculation in getChecksPerMonth (which is likely as it doesn't match how backend calculate it) this test breaks for no real reason.

The focus of this test is checking whether the execution count matches what we would expect for the amount of enabled tests rather than checking if the precise execution count is correct. You helpfully added those tests so that's why I'm not concerned with this test doubling up and checking them, too 😃

Copy link
Contributor

@VikaCep VikaCep left a comment

Choose a reason for hiding this comment

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

💯🚀✅

@ckbedwell ckbedwell merged commit 5383102 into main Nov 27, 2024
5 checks passed
@ckbedwell ckbedwell deleted the fix/execution-count-inactive-checks branch November 27, 2024 09:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants