From 49a5f14892840e074eb371418174da3870c69bf3 Mon Sep 17 00:00:00 2001 From: Stalgia Grigg Date: Tue, 29 Oct 2024 13:49:35 -0700 Subject: [PATCH 1/7] SortableIssuesTable first pass --- .../components/SortableIssuesTable/index.jsx | 182 ++++++++++++++++++ .../components/TestPlanVersionsPage/index.jsx | 58 +----- .../common/SortableTableHeader/index.js | 34 ++-- 3 files changed, 201 insertions(+), 73 deletions(-) create mode 100644 client/components/SortableIssuesTable/index.jsx diff --git a/client/components/SortableIssuesTable/index.jsx b/client/components/SortableIssuesTable/index.jsx new file mode 100644 index 000000000..914304fb6 --- /dev/null +++ b/client/components/SortableIssuesTable/index.jsx @@ -0,0 +1,182 @@ +import React, { useMemo, useState } from 'react'; +import { ThemeTable, ThemeTableUnavailable } from '../common/ThemeTable'; +import { dates } from 'shared'; +import { NoneText } from '../TestPlanVersionsPage'; +import SortableTableHeader, { + TABLE_SORT_ORDERS +} from '../common/SortableTableHeader'; +import FilterButtons from '../common/FilterButtons'; +import { IssuePropType } from '../common/proptypes'; +import PropTypes from 'prop-types'; + +const FILTER_OPTIONS = { + OPEN: 'Open', + CLOSED: 'Closed', + ALL: 'All' +}; + +const SORT_FIELDS = { + AUTHOR: 'author', + TITLE: 'title', + STATUS: 'status', + AT: 'at', + CREATED_AT: 'createdAt', + CLOSED_AT: 'closedAt' +}; + +const SortableIssuesTable = ({ issues }) => { + const [activeSort, setActiveSort] = useState(SORT_FIELDS.STATUS); + const [sortOrder, setSortOrder] = useState(TABLE_SORT_ORDERS.ASC); + const [activeFilter, setActiveFilter] = useState('OPEN'); + + // Helper function to get sortable value from issue + const getSortableValue = (issue, sortField) => { + switch (sortField) { + case SORT_FIELDS.AUTHOR: + return issue.author; + case SORT_FIELDS.TITLE: + return issue.title; + case SORT_FIELDS.AT: + return issue.at?.name ?? ''; + case SORT_FIELDS.CREATED_AT: + return new Date(issue.createdAt); + case SORT_FIELDS.CLOSED_AT: + return issue.closedAt ? new Date(issue.closedAt) : new Date(0); + default: + return ''; + } + }; + + // Helper function to compare two issues by status + const compareByStatus = (a, b) => { + if (a.isOpen !== b.isOpen) { + if (sortOrder === TABLE_SORT_ORDERS.ASC) { + return a.isOpen ? 1 : -1; // Closed first for ascending + } + return a.isOpen ? -1 : 1; // Open first for descending + } + // If status is the same, sort by date created (newest first) + return new Date(b.createdAt) - new Date(a.createdAt); + }; + + // Helper function to compare two values based on sort order + const compareValues = (aValue, bValue) => { + return sortOrder === TABLE_SORT_ORDERS.ASC + ? aValue < bValue + ? -1 + : 1 + : aValue > bValue + ? -1 + : 1; + }; + + const sortedAndFilteredIssues = useMemo(() => { + // Filter issues + const filtered = + activeFilter === 'ALL' + ? issues + : issues.filter(issue => issue.isOpen === (activeFilter === 'OPEN')); + + // Sort issues + return filtered.sort((a, b) => { + // Special handling for status sorting + if (activeSort === SORT_FIELDS.STATUS) { + return compareByStatus(a, b); + } + + // Normal sorting for other fields + const aValue = getSortableValue(a, activeSort); + const bValue = getSortableValue(b, activeSort); + return compareValues(aValue, bValue); + }); + }, [issues, activeSort, sortOrder, activeFilter]); + + const handleSort = column => newSortOrder => { + setActiveSort(column); + setSortOrder(newSortOrder); + }; + + const renderTableHeader = () => ( + + + {[ + { field: SORT_FIELDS.AUTHOR, title: 'Author' }, + { field: SORT_FIELDS.TITLE, title: 'Issue' }, + { field: SORT_FIELDS.STATUS, title: 'Status' }, + { field: SORT_FIELDS.AT, title: 'AT' }, + { field: SORT_FIELDS.CREATED_AT, title: 'Created On' }, + { field: SORT_FIELDS.CLOSED_AT, title: 'Closed On' } + ].map(({ field, title }) => ( + + ))} + + + ); + + const renderTableBody = () => ( + + {sortedAndFilteredIssues.map(issue => ( + + + + {issue.author} + + + + + {issue.title} + + + {issue.isOpen ? 'Open' : 'Closed'} + {issue.at?.name ?? 'AT not specified'} + {dates.convertDateToString(issue.createdAt, 'MMM D, YYYY')} + + {!issue.closedAt ? ( + N/A + ) : ( + dates.convertDateToString(issue.closedAt, 'MMM D, YYYY') + )} + + + ))} + + ); + + return ( + <> +

GitHub Issues

+ + {!sortedAndFilteredIssues.length ? ( + + No GitHub Issues + + ) : ( + + {renderTableHeader()} + {renderTableBody()} + + )} + + ); +}; + +SortableIssuesTable.propTypes = { + issues: PropTypes.arrayOf(IssuePropType).isRequired +}; + +export default SortableIssuesTable; diff --git a/client/components/TestPlanVersionsPage/index.jsx b/client/components/TestPlanVersionsPage/index.jsx index 0dce14740..e64c62940 100644 --- a/client/components/TestPlanVersionsPage/index.jsx +++ b/client/components/TestPlanVersionsPage/index.jsx @@ -7,7 +7,6 @@ import { Helmet } from 'react-helmet'; import { Container } from 'react-bootstrap'; import { ThemeTable, - ThemeTableUnavailable, ThemeTableHeaderH3 as UnstyledThemeTableHeader } from '../common/ThemeTable'; import VersionString from '../common/VersionString'; @@ -22,6 +21,7 @@ import { import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import DisclosureComponentUnstyled from '../common/DisclosureComponent'; import useForceUpdate from '../../hooks/useForceUpdate'; +import SortableIssuesTable from '../SortableIssuesTable'; const DisclosureContainer = styled.div` .timeline-for-version-table { @@ -40,7 +40,7 @@ const DisclosureComponent = styled(DisclosureComponentUnstyled)` } `; -const NoneText = styled.span` +export const NoneText = styled.span` font-style: italic; color: #6a7989; `; @@ -390,59 +390,7 @@ const TestPlanVersionsPage = () => { )} - GitHub Issues - {!issues.length ? ( - - No GitHub Issues - - ) : ( - - - - Author - Issue - Status - AT - Created On - Closed On - - - - {issues.map(issue => { - return ( - - - - {issue.author} - - - - - {issue.title} - - - {issue.isOpen ? 'Open' : 'Closed'} - {issue.at?.name ?? 'AT not specified'} - - {dates.convertDateToString(issue.createdAt, 'MMM D, YYYY')} - - - {!issue.closedAt ? ( - N/A - ) : ( - dates.convertDateToString(issue.closedAt, 'MMM D, YYYY') - )} - - - ); - })} - - - )} + Timeline for All Versions diff --git a/client/components/common/SortableTableHeader/index.js b/client/components/common/SortableTableHeader/index.js index c1fb828a2..f006c0f8d 100644 --- a/client/components/common/SortableTableHeader/index.js +++ b/client/components/common/SortableTableHeader/index.js @@ -10,40 +10,38 @@ import { import { useAriaLiveRegion } from '../../providers/AriaLiveRegionProvider'; const SortableTableHeaderWrapper = styled.th` - position: relative; - padding: 0; + background: #e9ebee; + padding: 0 !important; + height: 100%; `; const SortableTableHeaderButton = styled(Button)` - background: #e9ebee; + background: transparent; border: none; color: black; font-size: 1rem; - padding: 0; font-weight: 700; text-align: left; - position: absolute; - top: 0; - bottom: 0; - left: 0; - right: 0; + width: 100%; + min-height: 100%; + display: flex; justify-content: space-between; align-items: flex-end; - padding: 8px 12px; + padding: 0.5rem; border-radius: 0; - z-index: 0; - display: flex; + margin: 0; + + // Force the button to stretch + position: relative; + height: stretch; + height: -webkit-fill-available; + height: -moz-available; + &:hover, &:focus { - background: #e9ebee; - z-index: 1; color: #0b60ab; background-color: var(--bs-table-hover-bg); } - - &:hover { - border: none; - } `; const InactiveIcon = styled(FontAwesomeIcon)` From 6462d02e7435d58c44d16b148073a5b348f53b94 Mon Sep 17 00:00:00 2001 From: Stalgia Grigg Date: Tue, 29 Oct 2024 14:06:51 -0700 Subject: [PATCH 2/7] Templated issues table header --- client/components/SortableIssuesTable/index.jsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/client/components/SortableIssuesTable/index.jsx b/client/components/SortableIssuesTable/index.jsx index 914304fb6..a5f56cff2 100644 --- a/client/components/SortableIssuesTable/index.jsx +++ b/client/components/SortableIssuesTable/index.jsx @@ -29,6 +29,12 @@ const SortableIssuesTable = ({ issues }) => { const [sortOrder, setSortOrder] = useState(TABLE_SORT_ORDERS.ASC); const [activeFilter, setActiveFilter] = useState('OPEN'); + const issueStats = useMemo(() => { + const openIssues = issues.filter(issue => issue.isOpen).length; + const closedIssues = issues.length - openIssues; + return { openIssues, closedIssues }; + }, [issues]); + // Helper function to get sortable value from issue const getSortableValue = (issue, sortField) => { switch (sortField) { @@ -47,7 +53,6 @@ const SortableIssuesTable = ({ issues }) => { } }; - // Helper function to compare two issues by status const compareByStatus = (a, b) => { if (a.isOpen !== b.isOpen) { if (sortOrder === TABLE_SORT_ORDERS.ASC) { @@ -59,7 +64,6 @@ const SortableIssuesTable = ({ issues }) => { return new Date(b.createdAt) - new Date(a.createdAt); }; - // Helper function to compare two values based on sort order const compareValues = (aValue, bValue) => { return sortOrder === TABLE_SORT_ORDERS.ASC ? aValue < bValue @@ -153,7 +157,10 @@ const SortableIssuesTable = ({ issues }) => { return ( <> -

GitHub Issues

+

+ GitHub Issues ({issueStats.openIssues} open, {issueStats.closedIssues} +  closed) +

Date: Tue, 29 Oct 2024 15:05:37 -0700 Subject: [PATCH 3/7] e2e tests for SortableIssuesTable --- .../components/SortableIssuesTable/index.jsx | 21 ++- .../components/common/FilterButtons/index.jsx | 1 + client/tests/e2e/TestPlanVersions.e2e.test.js | 128 ++++++++++++++++++ 3 files changed, 144 insertions(+), 6 deletions(-) diff --git a/client/components/SortableIssuesTable/index.jsx b/client/components/SortableIssuesTable/index.jsx index a5f56cff2..0b31b6477 100644 --- a/client/components/SortableIssuesTable/index.jsx +++ b/client/components/SortableIssuesTable/index.jsx @@ -56,9 +56,9 @@ const SortableIssuesTable = ({ issues }) => { const compareByStatus = (a, b) => { if (a.isOpen !== b.isOpen) { if (sortOrder === TABLE_SORT_ORDERS.ASC) { - return a.isOpen ? 1 : -1; // Closed first for ascending + return a.isOpen ? -1 : 1; // Open first for ascending } - return a.isOpen ? -1 : 1; // Open first for descending + return a.isOpen ? 1 : -1; // Closed first for descending } // If status is the same, sort by date created (newest first) return new Date(b.createdAt) - new Date(a.createdAt); @@ -116,6 +116,7 @@ const SortableIssuesTable = ({ issues }) => { title={title} active={activeSort === field} onSort={handleSort(field)} + data-test={`sort-${field.toLowerCase()}`} /> ))} @@ -125,7 +126,11 @@ const SortableIssuesTable = ({ issues }) => { const renderTableBody = () => ( {sortedAndFilteredIssues.map(issue => ( - + { {issue.title} - {issue.isOpen ? 'Open' : 'Closed'} + {issue.isOpen ? 'Open' : 'Closed'} {issue.at?.name ?? 'AT not specified'} {dates.convertDateToString(issue.createdAt, 'MMM D, YYYY')} @@ -162,7 +167,7 @@ const SortableIssuesTable = ({ issues }) => {  closed) { No GitHub Issues ) : ( - + {renderTableHeader()} {renderTableBody()} diff --git a/client/components/common/FilterButtons/index.jsx b/client/components/common/FilterButtons/index.jsx index d9b35ea3c..84a9d9c7d 100644 --- a/client/components/common/FilterButtons/index.jsx +++ b/client/components/common/FilterButtons/index.jsx @@ -42,6 +42,7 @@ const FilterButtons = ({ return (
  • { ); }); }); + +describe('Issues table interactions', () => { + const TEST_URL = '/data-management/alert'; + + it('displays correct issue counts in heading', async () => { + await getPage({ role: false, url: TEST_URL }, async page => { + await page.waitForSelector('[data-test="issues-table"]'); + const headingText = await page.$eval( + 'h2#github-issues', + el => el.textContent + ); + expect(headingText.replace(/\s+/g, ' ').trim()).toBe( + 'GitHub Issues (1 open, 1 closed)' + ); + }); + }); + + it('filters issues correctly', async () => { + await getPage({ role: false, url: TEST_URL }, async page => { + await page.waitForSelector('[data-test="issues-table"]'); + + // Check initial state (Open filter should be active by default) + const isOpenPressed = await page.$eval('[data-test="filter-open"]', el => + el.getAttribute('aria-pressed') + ); + expect(isOpenPressed).toBe('true'); + + // Verify only open issues are shown initially + let visibleIssues = await page.$$eval( + '[data-test="issue-row"]', + rows => rows.length + ); + let firstRowStatus = await page.$eval('[data-test="issue-row"]', row => + row.getAttribute('data-status') + ); + expect(visibleIssues).toBe(1); + expect(firstRowStatus).toBe('open'); + + // Click "Closed" filter + await page.click('[data-test="filter-closed"]'); + + // Verify only closed issues are shown + visibleIssues = await page.$$eval( + '[data-test="issue-row"]', + rows => rows.length + ); + firstRowStatus = await page.$eval('[data-test="issue-row"]', row => + row.getAttribute('data-status') + ); + expect(visibleIssues).toBe(1); + expect(firstRowStatus).toBe('closed'); + + // Click "All" filter + await page.click('[data-test="filter-all"]'); + + // Verify all issues are shown + visibleIssues = await page.$$eval( + '[data-test="issue-row"]', + rows => rows.length + ); + expect(visibleIssues).toBe(2); + }); + }); + + it('sorts issues by different columns', async () => { + await getPage({ role: false, url: TEST_URL }, async page => { + await page.waitForSelector('[data-test="issues-table"]'); + + // Click "All" filter first so we can see all issues + await page.click('[data-test="filter-all"]'); + await page.waitForSelector('[data-test="issue-row"]'); + + // Initial sort is by Status (ascending) + let firstRowStatus = await page.$eval( + '[data-test="issue-row"]:first-child [data-test="issue-status"]', + el => el.textContent.trim() + ); + expect(firstRowStatus).toBe('Open'); + + // Click Status header to reverse sort + await page.click('th[role="columnheader"] button::-p-text(Status)'); + + // Wait for the table to update after sorting + await page.waitForSelector('[data-test="issue-row"]'); + + firstRowStatus = await page.$eval( + '[data-test="issue-row"]:first-child [data-test="issue-status"]', + el => el.textContent.trim() + ); + expect(firstRowStatus).toBe('Closed'); + + // Test sorting by Author + await page.click('th[role="columnheader"] button::-p-text(Author)'); + + // Wait for the table to update after sorting + await page.waitForSelector('[data-test="issue-row"]'); + + const firstRowAuthor = await page.$eval( + '[data-test="issue-row"]:first-child td:first-child', + el => el.textContent.trim() + ); + expect(firstRowAuthor).toBe('alflennik'); + }); + }); + + it('maintains filter state when sorting changes', async () => { + await getPage({ role: false, url: TEST_URL }, async page => { + await page.waitForSelector('[data-test="issues-table"]'); + + // Select "Open" filter (should already be selected by default) + await page.click('[data-test="filter-open"]'); + + // Sort by Author + await page.click('th[role="columnheader"] button::-p-text(Author)'); + + // Verify only open issues are still shown + const visibleIssues = await page.$$eval( + '[data-test="issue-row"]', + rows => ({ + length: rows.length, + status: rows[0].getAttribute('data-status') + }) + ); + expect(visibleIssues.length).toBe(1); + expect(visibleIssues.status).toBe('open'); + }); + }); +}); From 9cab0114c3f78d59bea138550aa50f474b0d10ea Mon Sep 17 00:00:00 2001 From: Stalgia Grigg Date: Tue, 29 Oct 2024 15:08:07 -0700 Subject: [PATCH 4/7] Update snapshots --- .../e2e/snapshots/saved/_data-management.html | 17 +++++---- .../saved/_data-management_meter.html | 35 ++++++++++++++++++- .../e2e/snapshots/saved/_test-review_8.html | 4 +++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/client/tests/e2e/snapshots/saved/_data-management.html b/client/tests/e2e/snapshots/saved/_data-management.html index 31435f1b7..ed12eebac 100644 --- a/client/tests/e2e/snapshots/saved/_data-management.html +++ b/client/tests/e2e/snapshots/saved/_data-management.html @@ -367,6 +367,7 @@

    Test Plans Status Summary

  • +
    • +
  • + +
    • +
  • + +
    • +
    No GitHub Issues
    diff --git a/client/tests/e2e/snapshots/saved/_test-review_8.html b/client/tests/e2e/snapshots/saved/_test-review_8.html index 03df550a9..7b5fbeb8b 100644 --- a/client/tests/e2e/snapshots/saved/_test-review_8.html +++ b/client/tests/e2e/snapshots/saved/_test-review_8.html @@ -133,6 +133,7 @@

    Tests