Skip to content

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

Merged
ckbedwell merged 1 commit into
mainfrom
fix/execution-count-inactive-checks
Nov 27, 2024
Merged

fix: filter out disabled checks for execution count total#999
ckbedwell merged 1 commit into
mainfrom
fix/execution-count-inactive-checks

Conversation

@ckbedwell

Copy link
Copy Markdown
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 VikaCep and w1kman November 25, 2024 16:46
},
]);

expect(result.current.data).toBe(EXECUTIONS);

Copy link
Copy Markdown
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
Copy Markdown
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 😃

@VikaCep VikaCep left a comment

Copy link
Copy Markdown
Contributor

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
@ckbedwell ckbedwell deleted the fix/execution-count-inactive-checks branch November 27, 2024 09:53
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