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

Update payloads_not_blocked to host_not_blocked in report_generator.py #67

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

ameliav
Copy link
Contributor

@ameliav ameliav commented Nov 2, 2023

πŸ—£ Description

Update payloads_not_blocked to host_not_blocked in report_generator.py

πŸ’­ Motivation and context

"We expected the total payloads executed value to match the total number of Not Blocked's in the Host Protection Column of the payloads." Found in issue #65

πŸ§ͺ Testing

I created the report a few times with different input values to determine that the result matched.

πŸ“· Screenshots

The screenshot show that it is only capturing the total amount from host protection.
image

βœ… Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All future TODOs are captured in issues, which are referenced
    in code comments.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

βœ… Pre-merge checklist

  • Revert dependencies to default branches.
  • Finalize version.

βœ… Post-merge checklist

  • Create a release.

@ameliav ameliav linked an issue Nov 2, 2023 that may be closed by this pull request
@ameliav ameliav self-assigned this Nov 2, 2023
@schmelz21 schmelz21 added the bug This issue or pull request addresses broken functionality label Nov 2, 2023
@ameliav ameliav marked this pull request as ready for review November 2, 2023 15:43
@ameliav ameliav requested review from jsf9k and dav3r November 2, 2023 15:44
@coveralls
Copy link

coveralls commented Nov 2, 2023

Pull Request Test Coverage Report for Build 6734918259

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 6549233012: 0.0%
Covered Lines: 0
Relevant Lines: 0

πŸ’› - Coveralls

Copy link
Contributor

@schmelz21 schmelz21 left a comment

Choose a reason for hiding this comment

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

Approved.
@JCantu248 or @nickviola are you able to take a quick look?

@schmelz21
Copy link
Contributor

FYI - as of now current report backlog = 8

@schmelz21
Copy link
Contributor

@dav3r or @jsf9k - either one of you able to take a look?
It is a minor calculation change, and we have some reports on hold until we update. Thanks.

Copy link
Collaborator

@nickviola nickviola left a comment

Choose a reason for hiding this comment

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

LGTM

@ameliav ameliav merged commit 1f49d01 into develop Nov 2, 2023
44 checks passed
@ameliav ameliav deleted the 65-change-logic-behind-of-payloads-executed branch November 2, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue or pull request addresses broken functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Logic Behind # of Payloads Executed
5 participants