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.

Sign up for GitHub

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

Jump to bottom

Fix Side Drawer Advance Filters (CRASM-967) #740

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

Fix Side Drawer Advance Filters (CRASM-967) #740

wants to merge 20 commits into from
+192 −91

Conversation

hawkishpolicy
Copy link
Collaborator

@hawkishpolicy hawkishpolicy commented Dec 12, 2024 •
edited
Loading

Applies a more user friendly and consistent display, plus increased functionality, to the checkbox controlled filters in the Advanced Filters section of the Side Drawer.

🗣 Description

  • Applied ascending sort to the following Advanced Filters:
    • Root Domain(s)
    • Ports
    • CVE
    • Severity
  • Added grouping and sorting logic similar to that of VulnerabilityBarChart to Severity Filters:
    • Null, undefined, empty string, and N/A values are placed into the "N/A" category.
    • Special characters, numbers, and Other are placed into the "Other" category.
    • All categories are now case insensitive.
  • Fixed issue with npm run syncdb (old TS backend):
    • searchSync was nested inside of syncdb -- -d populate.
    • This was preventing local db changes from syncing.
    • Issue is resolved.
  • Added aggregations to:
    • Returned result from /search.
    • SearchResponse (Base Model).
    • These are needed to form the facet filters used in the Filter Drawer.
  • Ensured Elastic Search returns accurate number of results based on the Severity Level filter passed in:
    • All versions of Null, N/A, undefined are returned with the N/A filter.
    • Case insensitive versions of Low, Medium, High, and Critical are returned with their respective filter.
    • Everything else, i.e. special characters, numbers, or some other severity level is returned with Other.
  • Ensured cve's were included in the vulnerabilities object used in searchSync.
  • Added a few random cve's modeled after real ones to be used for sample data in local db.

💭 Motivation and context

🧪 Testing

  • Tested locally.

📷 Screenshots (if appropriate)

Screenshot 2024-12-12 at 10 22 11 AM Screenshot 2024-12-12 at 10 22 32 AM Screenshot 2024-12-12 at 10 22 44 AM Screenshot 2024-12-12 at 10 22 55 AM

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

✅ Pre-merge checklist

  • Revert dependencies to default branches.
  • Finalize version.

✅ Post-merge checklist

  • Create a release.

@hawkishpolicy
Edited DrawerInterior.tsx to fix filter sort
186bbdc 
- Added sort to portFacet.
- Added sort to cveFacet.
- Severity levels are now sorted in order of Low, Medium, High, Critical.
@hawkishpolicy
Edited severityFacet logic and syncdb script
938777f 
- npm run syncdb was not syncing domains on run due to it being nested inside syncdb -- -d populate.
-- Thus changes to local db were not being reflected in the search index.
- Edited severityFacet logic to only show severity filters if there are severity filters to show.
- Added a sort to display severity filters in the following order: N/A, Low, Medium, High, Critical, Other.
@hawkishpolicy
Edited Severity Facet Filter in DrawerInterior.tsx
48a5907 
- Added data validation logic to group irregular severity values into appropriate categories.
- Null, empty string, N/A, and undefined values are grouped into the "N/A" category.
- Special characters, numbers, and Other are grouped into the "Other" category.
- All categories are case insensitive.
- Edited vulnerability.severity agg in buildRequest.ts to inclues "null" values.
- Increased aggregation size to 50 to include all severity values.
@hawkishpolicy
Cleaned up code
7e9723f 
- Removed console.logs.
- Removed commented out code.
@hawkishpolicy hawkishpolicy self-assigned this Dec 12, 2024
hawkishpolicy and others added 11 commits December 12, 2024 10:31
@hawkishpolicy
Added sort logic to root domain filter
443ef42
@hawkishpolicy
Updated backend snapshots
eadf409
@hawkishpolicy
Commented out args in searchSync test
563f4c6
@hawkishpolicy
Merge remote-tracking branch 'origin/develop' into Fix-Sort-of-Side-D…
b856c97 
…rawer-Filters
Fixes search-sync test
537da7e
@hawkishpolicy
Removed console.logs and commented out code
86bb333
@hawkishpolicy
Refined if check for N/A severity level values
73b1a8e 
- Now includes permutations of NUll, N/A, empty string, and undefined values.
@hawkishpolicy
Merge remote-tracking branch 'origin/develop' into Fix-Sort-of-Side-D…
2f9db50 
…rawer-Filters
@hawkishpolicy
empty commit
6718c12
@hawkishpolicy
Updated search endpoint to use Elasti Search aggs
f2da131 
- Added "aggregations" to the search endpoint result.
- Updated SearchResponse Base Model to include aggregations.
@hawkishpolicy
Merge remote-tracking branch 'origin/develop' into Fix-Sort-of-Side-D…
dd7bce4 
…rawer-Filters
@hawkishpolicy
Edited elastic search helper functions
7a40058 
- severity filter values are forced to be lowercase.
- N/A value returns Null, N/A, None, undefined, and empty strings.
- Other returns all other values not included in N/A or regular severity level values.
@hawkishpolicy
Merge remote-tracking branch 'origin/develop' into Fix-Sort-of-Side-D…
504ad0a 
…rawer-Filters
@hawkishpolicy hawkishpolicy changed the title Fix sort of Side Drawer Advance Filters (CRASM-967) Fix Side Drawer Advance Filters (CRASM-967) Jan 24, 2025
@hawkishpolicy hawkishpolicy marked this pull request as ready for review January 24, 2025 18:25
@hawkishpolicy
Restored CVE as a facet filter
f02d9cb 
- Vulnerabilities.cve were not included in the searchSync handler that updates Elastic Search. They are now.
- Added some random CVE's for sample data population to aid in testing.
@hawkishpolicy
Removed accidental trivial conditional
a63f1f8
aloftus23
aloftus23 approved these changes Jan 29, 2025
View reviewed changes
Copy link
Contributor

@aloftus23 aloftus23 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

chrtorres
chrtorres approved these changes Jan 29, 2025
View reviewed changes
Copy link
Collaborator

@chrtorres chrtorres left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrtorres chrtorres chrtorres approved these changes

@aloftus23 aloftus23 aloftus23 approved these changes

@cduhn17 cduhn17 Awaiting requested review from cduhn17 cduhn17 is a code owner

@Matthew-Grayson Matthew-Grayson Awaiting requested review from Matthew-Grayson Matthew-Grayson is a code owner

@nickviola nickviola Awaiting requested review from nickviola nickviola is a code owner

@rapidray12 rapidray12 Awaiting requested review from rapidray12 rapidray12 is a code owner

@schmelz21 schmelz21 Awaiting requested review from schmelz21 schmelz21 is a code owner

Assignees

@hawkishpolicy hawkishpolicy

Labels
backend frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hawkishpolicy @aloftus23 @chrtorres