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

Add search to table component #2684

Merged
merged 17 commits into from
Jun 13, 2024
Merged
3 changes: 2 additions & 1 deletion assets/js/common/Table/Table.stories.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React, { useState } from 'react';
import { createStringSortingPredicate } from './sorting';

import Table from '.';

import { createStringSortingPredicate } from './sorting';

export default {
/* 👇 The title prop is optional.
* See https://storybook.js.org/docs/react/configure/overview#configure-story-loading
Expand Down
4 changes: 4 additions & 0 deletions assets/js/lib/filter/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const regularizeString = (str) => str.normalize().trim().toLowerCase();

export const foundStringNaive = (str = '', substring = '') =>
janvhs marked this conversation as resolved.
Show resolved Hide resolved
regularizeString(str).includes(regularizeString(substring));
40 changes: 40 additions & 0 deletions assets/js/lib/filter/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { faker } from '@faker-js/faker';

import { foundStringNaive } from '.';

describe('search', () => {
it('should always match with an empty search string', () => {
expect(foundStringNaive('', '')).toBe(true);
expect(foundStringNaive(faker.word.words(10), '')).toBe(true);
});

it('should match strings case in an insensitive fashion', () => {
const original = faker.word.words(1);
const upper = original.toUpperCase();
const lower = original.toLowerCase();

expect(foundStringNaive(original, upper)).toBe(true);
expect(foundStringNaive(original, lower)).toBe(true);
});

it('should match substrings', () => {
const original = faker.word.words(1);
const sub = original.substring(original.length / 2);

expect(foundStringNaive(original, sub)).toBe(true);
});

it('should not match not included words', () => {
const words = faker.word.words(2).split(' ');

expect(foundStringNaive(words[0], words[1])).toBe(false);
expect(foundStringNaive('', words[1])).toBe(false);
});

it('should match unicode in different forms', () => {
const name1 = '\u0041\u006d\u00e9\u006c\u0069\u0065';
const name2 = '\u0041\u006d\u0065\u0301\u006c\u0069\u0065';

expect(foundStringNaive(name1, name2)).toBe(true);
});
});
5 changes: 5 additions & 0 deletions assets/js/lib/test-utils/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,8 @@ export async function recordSaga(saga, initialAction, state = {}) {

return dispatched;
}

export const inspect = (val) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but I used it for debugging long call chains a lot. I thought it might be useful to have it in the test utils

console.dir(val); // eslint-disable-line no-console
return val;
};
23 changes: 10 additions & 13 deletions assets/js/pages/HostRelevantPatches/HostRelevantPatchesPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import Input from '@common/Input';
import Select from '@common/Select';
import Button from '@common/Button';

import { foundStringNaive } from '@lib/filter';

const advisoryTypesFromPatches = (patches) =>
Array.from(new Set(patches.map(({ advisory_type }) => advisory_type))).sort();

Expand All @@ -15,14 +17,6 @@ const filterPatchesByAdvisoryType = (patches, advisoryType) =>
advisoryType === 'all' ? true : advisory_type === advisoryType
);

// TODO(janvhs): Fuzzy, case insensitive search, input delay?
const filterPatchesBySynopsis = (patches, synopsis) =>
patches.filter(({ advisory_synopsis }) =>
synopsis.trim() === ''
? true
: advisory_synopsis.trim().startsWith(synopsis.trim())
);

function HostRelevantPatches({ hostName, onNavigate, patches }) {
const advisoryTypes = ['all'].concat(advisoryTypesFromPatches(patches));

Expand All @@ -32,12 +26,15 @@ function HostRelevantPatches({ hostName, onNavigate, patches }) {
const [displayedPatches, setDisplayedPatches] = useState(patches);

useEffect(() => {
setDisplayedPatches(
filterPatchesBySynopsis(
filterPatchesByAdvisoryType(patches, displayedAdvisories),
search
)
const filteredByAdvisoryKind = filterPatchesByAdvisoryType(
janvhs marked this conversation as resolved.
Show resolved Hide resolved
patches,
displayedAdvisories
);
const searchResult = filteredByAdvisoryKind.filter(
({ advisory_synopsis }) =>
advisory_synopsis ? foundStringNaive(advisory_synopsis, search) : false
);
setDisplayedPatches(searchResult);
}, [patches, displayedAdvisories, search]);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,5 +113,24 @@ describe('HostRelevantPatchesPage', () => {
expect(screen.getByText(patch.advisory_synopsis)).toBeVisible();
});
});

it('should filter patch by content', async () => {
const user = userEvent.setup();

const patches = relevantPatchFactory.buildList(8);
const searchTerm = patches[0].advisory_synopsis;

const { container } = render(
<HostRelevantPatchesPage patches={patches} />
);

const searchInput = screen.getByRole('textbox');
await user.click(searchInput);
await user.type(searchInput, searchTerm);

const tableRows = container.querySelectorAll('tbody > tr');

expect(tableRows.length).toBe(1);
});
});
});
43 changes: 43 additions & 0 deletions assets/js/pages/UpgradablePackagesPage/UpgradablePackages.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import React, { useState } from 'react';
import { EOS_SEARCH } from 'eos-icons-react';

import UpgradablePackagesList from '@common/UpgradablePackagesList';
import PageHeader from '@common/PageHeader';
import Input from '@common/Input';
import { foundStringNaive } from '@lib/filter';

export default function UpgradablePackages({
hostName,
upgradablePackages,
}) {
const [search, setSearch] = useState('');

const displayedPackages = upgradablePackages.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

nice 😎

({ name, patches }) =>
foundStringNaive(name, search) ||
patches
.map(({ advisory }) => foundStringNaive(advisory, search))
.includes(true)
);

return (
<>
<div className="flex flex-wrap">
<div className="flex w-2/3 h-auto overflow-ellipsis break-words">
<PageHeader>
Upgradable packages: <span className="font-bold">{hostName}</span>
</PageHeader>
</div>
<div className="flex w-1/3 justify-end">
<Input
className="flex"
onChange={(e) => setSearch(e.target.value)}
placeholder="Search by Name or Patch"
prefix={<EOS_SEARCH size="l" />}
/>
</div>
</div>
<UpgradablePackagesList upgradablePackages={displayedPackages} />
</>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';

import { upgradablePackageFactory } from '@lib/test-utils/factories/upgradablePackage';
import { hostFactory } from '@lib/test-utils/factories/hosts';

import UpgradablePackages from './UpgradablePackages';

export default {
title: 'Layouts/UpgradablePackages',
components: UpgradablePackages,
argTypes: {},
render: (args) => <UpgradablePackages {...args} />,
};

export const Default = {
args: {
hostName: hostFactory.build().hostname,
upgradablePackages: upgradablePackageFactory.buildList(5),
},
};
42 changes: 42 additions & 0 deletions assets/js/pages/UpgradablePackagesPage/UpgradablePackages.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import React from 'react';
import { screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';

import { renderWithRouter as render } from '@lib/test-utils';
import { upgradablePackageFactory } from '@lib/test-utils/factories/upgradablePackage';

import UpgradablePackages from './UpgradablePackages';

describe('UpgradablePackages', () => {
it('shows all packages by default', () => {
const packages = upgradablePackageFactory.buildList(8);

const { container } = render(
<UpgradablePackages upgradablePackages={packages} />
);

const tableRows = container.querySelectorAll('tbody > tr');

expect(tableRows.length).toBe(8);
});

it('should filter package by its name', async () => {
const user = userEvent.setup();

const packages = upgradablePackageFactory.buildList(8);
const searchTerm = packages[0].name;

const { container } = render(
<UpgradablePackages upgradablePackages={packages} />
);

const searchInput = screen.getByRole('textbox');
await user.click(searchInput);
await user.type(searchInput, searchTerm);

const tableRows = container.querySelectorAll('tbody > tr');

expect(tableRows.length).toBe(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ import {
fetchUpgradablePackagesPatches,
} from '@state/softwareUpdates';

import PageHeader from '@common/PageHeader';
import BackButton from '@common/BackButton';
import UpgradablePackagesList from '@common/UpgradablePackagesList';
import UpgradablePackages from './UpgradablePackages';
janvhs marked this conversation as resolved.
Show resolved Hide resolved

function UpgradablePackagesPage() {
const { hostID } = useParams();
Expand Down Expand Up @@ -43,10 +42,10 @@ function UpgradablePackagesPage() {
return (
<>
<BackButton url={`/hosts/${hostID}`}>Back to Host Details</BackButton>
<PageHeader>
Upgradable packages: <span className="font-bold">{hostname}</span>
</PageHeader>
<UpgradablePackagesList upgradablePackages={upgradablePackages} />
<UpgradablePackages
hostName={hostname}
upgradablePackages={upgradablePackages}
/>
</>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import { upgradablePackageFactory } from '@lib/test-utils/factories/upgradablePackage';
import { patchForPackageFactory } from '@lib/test-utils/factories/relevantPatches';

import UpgradablePackagesPage from './UpgradablePackagesPage';
import UpgradablePackagesPage from '.';

describe('UpgradablePackagesPage', () => {
it('should render correctly', () => {
Expand Down
2 changes: 1 addition & 1 deletion assets/js/pages/UpgradablePackagesPage/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import UpgradablePackagesPage from './UpgradablePackagesPage';

export default UpgradablePackagesPage;
export default UpgradablePackagesPage
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should fail the lint step?

Copy link
Contributor

Choose a reason for hiding this comment

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

grrr reactions for out prettier check, why did it green light this? :0

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Prettier should be really mad for this, I don't why it doesn't happen

Loading