mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 01:38:56 -04:00
[8.16] [Security Solution] [KB Management] Fix sorting by name in the kb entries table (#209141) (#210524)
# Backport This will backport the following commits from `main` to `8.16`: - [[Security Solution] [KB Management] Fix sorting by name in the kb entries table (#209141)](https://github.com/elastic/kibana/pull/209141) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kenneth Kreindler","email":"42113355+KDKHD@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-02-11T10:18:02Z","message":"[Security Solution] [KB Management] Fix sorting by name in the kb entries table (#209141)\n\nFixes https://github.com/elastic/kibana/issues/199253\r\n\r\n## Summary\r\n\r\nFixes the issue where KB entries in knowledge base management were not\r\nsorting correctly by name. Previously, entries with names starting with\r\nuppercase letters appeared before those with lowercase names, instead of\r\nfollowing a case-insensitive alphabetical order. This update ensures\r\nproper sorting regardless of letter case.\r\n\r\nBefore:\r\n<img width=\"3120\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1657fce4-abba-4672-aaa8-8c5c6c660c98\"\r\n/>\r\n\r\n\r\nAfter:\r\n<img width=\"3120\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/10d1f3fc-f6db-4b5b-b67b-8b8ae7250dd4\"\r\n/>\r\n\r\n\r\n\r\nHow to test:\r\n- See instructions in the issue. Create uppercase and lowercase KB\r\nentries (e.g. A,a,b,B,c,C. Create the entries in that order).\r\n- Reorder the table based on name and check the ordering makes sense.\r\n\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\r\n- [x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [X] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] If a plugin configuration key changed, check if it needs to be\r\nallowlisted in the cloud and added to the [docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n- [x] This was checked for breaking HTTP API changes, and any breaking\r\nchanges have been approved by the breaking-change committee. The\r\n`release_note:breaking` label should be applied in these situations.\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n### Identify risks\r\n\r\nDoes this PR introduce any risks? For example, consider risks like hard\r\nto test bugs, performance regression, potential of data loss.\r\n\r\nDescribe the risk, its severity, and mitigation for each identified\r\nrisk. Invite stakeholders and evaluate how to proceed before merging.\r\n\r\n- [ ] [See some risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n- [ ] ...\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"a1356ffb9f10b0958553ddab2b7d8332fc495f43","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:prev-major","Team:Security Generative AI","v9.1.0"],"title":"[Security Solution] [KB Management] Fix sorting by name in the kb entries table","number":209141,"url":"https://github.com/elastic/kibana/pull/209141","mergeCommit":{"message":"[Security Solution] [KB Management] Fix sorting by name in the kb entries table (#209141)\n\nFixes https://github.com/elastic/kibana/issues/199253\r\n\r\n## Summary\r\n\r\nFixes the issue where KB entries in knowledge base management were not\r\nsorting correctly by name. Previously, entries with names starting with\r\nuppercase letters appeared before those with lowercase names, instead of\r\nfollowing a case-insensitive alphabetical order. This update ensures\r\nproper sorting regardless of letter case.\r\n\r\nBefore:\r\n<img width=\"3120\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1657fce4-abba-4672-aaa8-8c5c6c660c98\"\r\n/>\r\n\r\n\r\nAfter:\r\n<img width=\"3120\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/10d1f3fc-f6db-4b5b-b67b-8b8ae7250dd4\"\r\n/>\r\n\r\n\r\n\r\nHow to test:\r\n- See instructions in the issue. Create uppercase and lowercase KB\r\nentries (e.g. A,a,b,B,c,C. Create the entries in that order).\r\n- Reorder the table based on name and check the ordering makes sense.\r\n\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\r\n- [x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [X] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] If a plugin configuration key changed, check if it needs to be\r\nallowlisted in the cloud and added to the [docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n- [x] This was checked for breaking HTTP API changes, and any breaking\r\nchanges have been approved by the breaking-change committee. The\r\n`release_note:breaking` label should be applied in these situations.\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n### Identify risks\r\n\r\nDoes this PR introduce any risks? For example, consider risks like hard\r\nto test bugs, performance regression, potential of data loss.\r\n\r\nDescribe the risk, its severity, and mitigation for each identified\r\nrisk. Invite stakeholders and evaluate how to proceed before merging.\r\n\r\n- [ ] [See some risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n- [ ] ...\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"a1356ffb9f10b0958553ddab2b7d8332fc495f43"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209141","number":209141,"mergeCommit":{"message":"[Security Solution] [KB Management] Fix sorting by name in the kb entries table (#209141)\n\nFixes https://github.com/elastic/kibana/issues/199253\r\n\r\n## Summary\r\n\r\nFixes the issue where KB entries in knowledge base management were not\r\nsorting correctly by name. Previously, entries with names starting with\r\nuppercase letters appeared before those with lowercase names, instead of\r\nfollowing a case-insensitive alphabetical order. This update ensures\r\nproper sorting regardless of letter case.\r\n\r\nBefore:\r\n<img width=\"3120\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1657fce4-abba-4672-aaa8-8c5c6c660c98\"\r\n/>\r\n\r\n\r\nAfter:\r\n<img width=\"3120\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/10d1f3fc-f6db-4b5b-b67b-8b8ae7250dd4\"\r\n/>\r\n\r\n\r\n\r\nHow to test:\r\n- See instructions in the issue. Create uppercase and lowercase KB\r\nentries (e.g. A,a,b,B,c,C. Create the entries in that order).\r\n- Reorder the table based on name and check the ordering makes sense.\r\n\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\r\n- [x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [X] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] If a plugin configuration key changed, check if it needs to be\r\nallowlisted in the cloud and added to the [docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n- [x] This was checked for breaking HTTP API changes, and any breaking\r\nchanges have been approved by the breaking-change committee. The\r\n`release_note:breaking` label should be applied in these situations.\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n### Identify risks\r\n\r\nDoes this PR introduce any risks? For example, consider risks like hard\r\nto test bugs, performance regression, potential of data loss.\r\n\r\nDescribe the risk, its severity, and mitigation for each identified\r\nrisk. Invite stakeholders and evaluate how to proceed before merging.\r\n\r\n- [ ] [See some risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n- [ ] ...\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"a1356ffb9f10b0958553ddab2b7d8332fc495f43"}}]}] BACKPORT--> --------- Co-authored-by: Kenneth Kreindler <42113355+KDKHD@users.noreply.github.com>
This commit is contained in:
parent
320b497b68
commit
020e9f51c5
2 changed files with 109 additions and 1 deletions
|
@ -224,6 +224,114 @@ describe('KnowledgeBaseSettingsManagement', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('renders entries in correct order', async () => {
|
||||
(useKnowledgeBaseEntries as jest.Mock).mockReturnValue({
|
||||
data: {
|
||||
data: [
|
||||
{
|
||||
id: '1',
|
||||
createdAt: '2024-10-21T18:54:14.773Z',
|
||||
createdBy: 'u_user_id_1',
|
||||
updatedAt: '2024-10-23T17:33:15.933Z',
|
||||
updatedBy: 'u_user_id_1',
|
||||
users: [{ name: 'Test User 1' }],
|
||||
name: 'A',
|
||||
namespace: 'default',
|
||||
type: 'document',
|
||||
kbResource: 'user',
|
||||
source: 'user',
|
||||
text: 'Very nice text',
|
||||
},
|
||||
{
|
||||
id: '2',
|
||||
createdAt: '2024-10-25T09:55:56.596Z',
|
||||
createdBy: 'u_user_id_2',
|
||||
updatedAt: '2024-10-25T09:55:56.596Z',
|
||||
updatedBy: 'u_user_id_2',
|
||||
users: [],
|
||||
name: 'b',
|
||||
namespace: 'default',
|
||||
type: 'index',
|
||||
index: 'index-1',
|
||||
field: 'semantic_field1',
|
||||
description: 'Test description',
|
||||
queryDescription: 'Test query instruction',
|
||||
},
|
||||
{
|
||||
id: '3',
|
||||
createdAt: '2024-10-25T09:55:56.596Z',
|
||||
createdBy: 'u_user_id_2',
|
||||
updatedAt: '2024-10-25T09:55:56.596Z',
|
||||
updatedBy: 'u_user_id_2',
|
||||
users: [],
|
||||
name: 'B',
|
||||
namespace: 'default',
|
||||
type: 'index',
|
||||
index: 'index-1',
|
||||
field: 'semantic_field1',
|
||||
description: 'Test description',
|
||||
queryDescription: 'Test query instruction',
|
||||
},
|
||||
{
|
||||
id: '4',
|
||||
createdAt: '2024-10-25T09:55:56.596Z',
|
||||
createdBy: 'u_user_id_1',
|
||||
updatedAt: '2024-10-25T09:55:56.596Z',
|
||||
updatedBy: 'u_user_id_1',
|
||||
users: [{ name: 'Test User 1' }],
|
||||
name: 'a',
|
||||
namespace: 'default',
|
||||
type: 'index',
|
||||
index: 'index-2',
|
||||
field: 'semantic_field2',
|
||||
description: 'Test description',
|
||||
queryDescription: 'Test query instruction',
|
||||
},
|
||||
],
|
||||
},
|
||||
isFetching: false,
|
||||
refetch: jest.fn(),
|
||||
});
|
||||
|
||||
render(<KnowledgeBaseSettingsManagement dataViews={mockDataViews} />, {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('knowledge-base-entries-table')).toBeInTheDocument();
|
||||
expect(screen.getByText('A')).toBeInTheDocument();
|
||||
expect(screen.getByText('B')).toBeInTheDocument();
|
||||
expect(screen.getByText('a')).toBeInTheDocument();
|
||||
expect(screen.getByText('b')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Order ascending
|
||||
await userEvent.click(screen.getByText('Name'));
|
||||
|
||||
await waitFor(() => {
|
||||
// Upper case letters should come before lower case letters
|
||||
expect(screen.getByText('A').compareDocumentPosition(screen.getByText('a'))).toBe(4);
|
||||
expect(screen.getByText('B').compareDocumentPosition(screen.getByText('b'))).toBe(4);
|
||||
|
||||
expect(screen.getByText('A').compareDocumentPosition(screen.getByText('B'))).toBe(4);
|
||||
expect(screen.getByText('a').compareDocumentPosition(screen.getByText('B'))).toBe(4);
|
||||
expect(screen.getByText('a').compareDocumentPosition(screen.getByText('b'))).toBe(4);
|
||||
});
|
||||
|
||||
// Order decending
|
||||
await userEvent.click(screen.getByText('Name'));
|
||||
|
||||
await waitFor(() => {
|
||||
// Lower case letters should come before upper case letters
|
||||
expect(screen.getByText('A').compareDocumentPosition(screen.getByText('a'))).toBe(2);
|
||||
expect(screen.getByText('B').compareDocumentPosition(screen.getByText('b'))).toBe(2);
|
||||
|
||||
expect(screen.getByText('A').compareDocumentPosition(screen.getByText('B'))).toBe(2);
|
||||
expect(screen.getByText('a').compareDocumentPosition(screen.getByText('B'))).toBe(2);
|
||||
expect(screen.getByText('a').compareDocumentPosition(screen.getByText('b'))).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
it('opens the flyout when add document button is clicked', async () => {
|
||||
const openFlyoutMock = jest.fn();
|
||||
(useFlyoutModalVisibility as jest.Mock).mockReturnValue({
|
||||
|
|
|
@ -168,7 +168,7 @@ export const useKnowledgeBaseTable = () => {
|
|||
render: (entry: KnowledgeBaseEntryResponse) => (
|
||||
<NameColumn entry={entry} existingIndices={existingIndices} />
|
||||
),
|
||||
sortable: ({ name }: KnowledgeBaseEntryResponse) => name,
|
||||
sortable: ({ name }: KnowledgeBaseEntryResponse) => name.toLocaleLowerCase() + name, // Ensures that the sorting is case-insensitive and that the sorting is stable
|
||||
width: '30%',
|
||||
},
|
||||
{
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue