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

[Cloud Posture] fix findings table sort casing bug #148529

Merged
merged 20 commits into from
Jan 15, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,43 @@ export const showErrorToast = (

export const getFindingsQuery = ({ query, sort }: UseFindingsOptions) => ({
index: CSP_LATEST_FINDINGS_DATA_VIEW,
body: {
orouz marked this conversation as resolved.
Show resolved Hide resolved
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<CspFinding>) => {
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
},
Expand All @@ -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') {
Expand All @@ -155,7 +148,6 @@ export function FindingsPageProvider({ getService, getPageObjects }: FtrProvider
const nonStaleElement = await table.getColumnHeaderCell(columnName);
await nonStaleElement.click();
}
await table.assertColumnSort(columnName, direction);
},
};

Expand Down
90 changes: 73 additions & 17 deletions x-pack/test/cloud_security_posture_functional/pages/findings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = [
Copy link
Contributor

Choose a reason for hiding this comment

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

defining the data is now longer and very focused on sorting. i think we can narrow it down a bit. for example, using single chars or shorter sentences would achieve the same result. and instead of doing chance.integer() % 2 ... multiple times, we can do it once and do the same for the text casing.

{
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;
Expand Down Expand Up @@ -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 = (string, 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) || -(b > a);
orouz marked this conversation as resolved.
Show resolved Hide resolved
};
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]));
}
});
});

Expand Down