From 47789daeeb5c0ae84cdfb3b412072d952e702e84 Mon Sep 17 00:00:00 2001 From: ofiriro3 Date: Sun, 15 Jan 2023 18:16:25 +0200 Subject: [PATCH] [Cloud Posture] fix findings table sort casing bug (#148529) --- .../latest_findings/use_latest_findings.ts | 40 +++++++-- .../page_objects/findings_page.ts | 20 ++--- .../pages/findings.ts | 90 +++++++++++++++---- 3 files changed, 113 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/cloud_security_posture/public/pages/findings/latest_findings/use_latest_findings.ts b/x-pack/plugins/cloud_security_posture/public/pages/findings/latest_findings/use_latest_findings.ts index b938c202014d8..a10758da90c77 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/findings/latest_findings/use_latest_findings.ts +++ b/x-pack/plugins/cloud_security_posture/public/pages/findings/latest_findings/use_latest_findings.ts @@ -55,15 +55,43 @@ export const showErrorToast = ( export const getFindingsQuery = ({ query, sort }: UseFindingsOptions) => ({ index: CSP_LATEST_FINDINGS_DATA_VIEW, - body: { - query, - sort: [{ [sort.field]: sort.direction }], - size: MAX_FINDINGS_TO_LOAD, - aggs: getFindingsCountAggQuery(), - }, + query, + sort: getSortField(sort), + size: MAX_FINDINGS_TO_LOAD, + aggs: getFindingsCountAggQuery(), ignore_unavailable: false, }); +/** + * By default, ES will sort keyword fields in case-sensitive format, the + * following fields are required to have a case-insensitive sorting. + */ +const fieldsRequiredSortingByPainlessScript = [ + 'rule.section', + 'resource.name', + 'resource.sub_type', +]; + +/** + * Generates Painless sorting if the given field is matched or returns default sorting + * This painless script will sort the field in case-insensitive manner + */ +const getSortField = ({ field, direction }: Sort) => { + if (fieldsRequiredSortingByPainlessScript.includes(field)) { + return { + _script: { + type: 'string', + order: direction, + script: { + source: `doc["${field}"].value.toLowerCase()`, + lang: 'painless', + }, + }, + }; + } + return { [field]: direction }; +}; + export const useLatestFindings = (options: UseFindingsOptions) => { const { data, diff --git a/x-pack/test/cloud_security_posture_functional/page_objects/findings_page.ts b/x-pack/test/cloud_security_posture_functional/page_objects/findings_page.ts index 5c3477e521176..70c90e0cf323f 100644 --- a/x-pack/test/cloud_security_posture_functional/page_objects/findings_page.ts +++ b/x-pack/test/cloud_security_posture_functional/page_objects/findings_page.ts @@ -111,11 +111,13 @@ export function FindingsPageProvider({ getService, getPageObjects }: FtrProvider }, getColumnValues: async (columnName: string) => { + const elementsWithNoFilterCell = ['CIS Section', '@timestamp']; const tableElement = await table.getElement(); const columnIndex = await table.getColumnIndex(columnName); - const columnCells = await tableElement.findAllByCssSelector( - `tbody tr td:nth-child(${columnIndex}) div[data-test-subj="filter_cell_value"]` - ); + const selector = elementsWithNoFilterCell.includes(columnName) + ? `tbody tr td:nth-child(${columnIndex})` + : `tbody tr td:nth-child(${columnIndex}) div[data-test-subj="filter_cell_value"]`; + const columnCells = await tableElement.findAllByCssSelector(selector); return await Promise.all(columnCells.map((cell) => cell.getVisibleText())); }, @@ -125,16 +127,7 @@ export function FindingsPageProvider({ getService, getPageObjects }: FtrProvider return values.includes(value); }, - assertColumnSort: async (columnName: string, direction: 'asc' | 'desc') => { - const values = (await table.getColumnValues(columnName)).filter(Boolean); - expect(values).to.not.be.empty(); - const sorted = values - .slice() - .sort((a, b) => (direction === 'asc' ? a.localeCompare(b) : b.localeCompare(a))); - values.forEach((value, i) => expect(value).to.be(sorted[i])); - }, - - toggleColumnSortOrFail: async (columnName: string, direction: 'asc' | 'desc') => { + toggleColumnSort: async (columnName: string, direction: 'asc' | 'desc') => { const element = await table.getColumnHeaderCell(columnName); const currentSort = await element.getAttribute('aria-sort'); if (currentSort === 'none') { @@ -155,7 +148,6 @@ export function FindingsPageProvider({ getService, getPageObjects }: FtrProvider const nonStaleElement = await table.getColumnHeaderCell(columnName); await nonStaleElement.click(); } - await table.assertColumnSort(columnName, direction); }, }; diff --git a/x-pack/test/cloud_security_posture_functional/pages/findings.ts b/x-pack/test/cloud_security_posture_functional/pages/findings.ts index ef631e3896cb2..3cfbc0234388a 100644 --- a/x-pack/test/cloud_security_posture_functional/pages/findings.ts +++ b/x-pack/test/cloud_security_posture_functional/pages/findings.ts @@ -6,6 +6,7 @@ */ import expect from '@kbn/expect'; +import Chance from 'chance'; import type { FtrProviderContext } from '../ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -14,19 +15,48 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const filterBar = getService('filterBar'); const retry = getService('retry'); const pageObjects = getPageObjects(['common', 'findings']); - - const data = Array.from({ length: 2 }, (_, id) => { - return { - resource: { id, name: `Resource ${id}` }, - result: { evaluation: id === 0 ? 'passed' : 'failed' }, + const chance = new Chance(); + + // We need to use a dataset for the tests to run + // We intentionally make some fields start with a capital letter to test that the query bar is case-insensitive/case-sensitive + const data = [ + { + resource: { id: chance.guid(), name: `kubelet`, sub_type: 'lower case sub type' }, + result: { evaluation: chance.integer() % 2 === 0 ? 'passed' : 'failed' }, rule: { - name: `Rule ${id}`, - section: 'Kubelet', - tags: ['Kubernetes'], - type: 'process', + name: 'Upper case rule name', + section: 'Upper case section', }, - }; - }); + cluster_id: 'Upper case cluster id', + }, + { + resource: { id: chance.guid(), name: `Pod`, sub_type: 'Upper case sub type' }, + result: { evaluation: chance.integer() % 2 === 0 ? 'passed' : 'failed' }, + rule: { + name: 'lower case rule name', + section: 'Another upper case section', + }, + cluster_id: 'Another Upper case cluster id', + }, + { + resource: { id: chance.guid(), name: `process`, sub_type: 'another lower case type' }, + result: { evaluation: 'passed' }, + rule: { + name: 'Another upper case rule name', + section: 'lower case section', + }, + cluster_id: 'lower case cluster id', + }, + { + resource: { id: chance.guid(), name: `process`, sub_type: 'Upper case type again' }, + result: { evaluation: 'failed' }, + rule: { + name: 'some lower case rule name', + section: 'another lower case section', + }, + cluster_id: 'another lower case cluster id', + }, + ]; const ruleName1 = data[0].rule.name; const ruleName2 = data[1].rule.name; @@ -102,12 +132,38 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { }); describe('Table Sort', () => { - it('sorts by rule name', async () => { - await table.toggleColumnSortOrFail('Rule', 'asc'); - }); - - it('sorts by resource name', async () => { - await table.toggleColumnSortOrFail('Resource Name', 'desc'); + type SortingMethod = (a: string, b: string) => number; + type SortDirection = 'asc' | 'desc'; + // Sort by lexical order will sort by the first character of the string (case-sensitive) + const compareStringByLexicographicOrder = (a: string, b: string) => { + return a > b ? 1 : b > a ? -1 : 0; + }; + const sortByAlphabeticalOrder = (a: string, b: string) => { + return a.localeCompare(b); + }; + + it('sorts by a column, should be case sensitive/insensitive depending on the column', async () => { + type TestCase = [string, SortDirection, SortingMethod]; + const testCases: TestCase[] = [ + ['CIS Section', 'asc', sortByAlphabeticalOrder], + ['CIS Section', 'desc', sortByAlphabeticalOrder], + ['Cluster ID', 'asc', compareStringByLexicographicOrder], + ['Cluster ID', 'desc', compareStringByLexicographicOrder], + ['Resource Name', 'asc', sortByAlphabeticalOrder], + ['Resource Name', 'desc', sortByAlphabeticalOrder], + ['Resource Type', 'asc', sortByAlphabeticalOrder], + ['Resource Type', 'desc', sortByAlphabeticalOrder], + ]; + for (const [columnName, dir, sortingMethod] of testCases) { + await table.toggleColumnSort(columnName, dir); + const values = (await table.getColumnValues(columnName)).filter(Boolean); + expect(values).to.not.be.empty(); + + const sorted = values + .slice() + .sort((a, b) => (dir === 'asc' ? sortingMethod(a, b) : sortingMethod(b, a))); + values.forEach((value, i) => expect(value).to.be(sorted[i])); + } }); });