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

Fix inverted data management status sort order, always sort ascending by name on columns other than name #790

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
21 changes: 12 additions & 9 deletions client/components/DataManagement/filterSortHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,23 +165,26 @@ export const useDataManagementTableFiltering = (
export const useDataManagementTableSorting = (
testPlans,
testPlanVersions,
ats
ats,
initialSortDirection = TABLE_SORT_ORDERS.ASC
) => {
const [activeSort, setActiveSort] = useState({
key: DATA_MANAGEMENT_TABLE_SORT_OPTIONS.PHASE,
direction: TABLE_SORT_ORDERS.ASC
direction: initialSortDirection
});

const { derivedOverallPhaseByTestPlanId } =
useDerivedOverallPhaseByTestPlanId(testPlans, testPlanVersions);

const sortedTestPlans = useMemo(() => {
// Ascending and descending interpreted differently for statuses
// (ascending = earlier phase first, descending = later phase first)
const phaseOrder = {
NOT_STARTED: -1,
RD: 0,
DRAFT: 1,
CANDIDATE: 2,
RECOMMENDED: 3
NOT_STARTED: 4,
RD: 3,
DRAFT: 2,
CANDIDATE: 1,
RECOMMENDED: 0
};
const directionMod =
activeSort.direction === TABLE_SORT_ORDERS.ASC ? -1 : 1;
Expand All @@ -192,7 +195,7 @@ export const useDataManagementTableSorting = (
const sortByAts = (a, b) => {
const countA = ats.length; // Stubs based on current rendering in DataManagementRow
const countB = ats.length;
if (countA === countB) return sortByName(a, b);
if (countA === countB) return sortByName(a, b, -1);
return directionMod * (countA - countB);
};

Expand All @@ -202,7 +205,7 @@ export const useDataManagementTableSorting = (
const testPlanVersionOverallB =
derivedOverallPhaseByTestPlanId[b.id] ?? 'NOT_STARTED';
if (testPlanVersionOverallA === testPlanVersionOverallB) {
return sortByName(a, b, directionMod);
return sortByName(a, b, -1);
}
return (
directionMod *
Expand Down
12 changes: 10 additions & 2 deletions client/components/DataManagement/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import ManageTestQueue from '../ManageTestQueue';
import DataManagementRow from '@components/DataManagement/DataManagementRow';
import './DataManagement.css';
import { evaluateAuth } from '@client/utils/evaluateAuth';
import SortableTableHeader from '../common/SortableTableHeader';
import SortableTableHeader, {
TABLE_SORT_ORDERS
} from '../common/SortableTableHeader';
import FilterButtons from '../common/FilterButtons';
import {
useDataManagementTableFiltering,
Expand Down Expand Up @@ -61,7 +63,12 @@ const DataManagement = () => {
);

const { sortedTestPlans, updateSort, activeSort } =
useDataManagementTableSorting(filteredTestPlans, testPlanVersions, ats);
useDataManagementTableSorting(
filteredTestPlans,
testPlanVersions,
ats,
TABLE_SORT_ORDERS.DESC
);

if (error) {
return (
Expand Down Expand Up @@ -198,6 +205,7 @@ const DataManagement = () => {
direction
})
}
initialSortDirection={TABLE_SORT_ORDERS.DESC}
/>
<th>R&D Version</th>
<th>Draft Review</th>
Expand Down
18 changes: 13 additions & 5 deletions client/components/common/SortableTableHeader/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,14 @@ export const TABLE_SORT_ORDERS = {
DESC: 'DESCENDING'
};

const SortableTableHeader = ({ title, active, onSort = () => {} }) => {
const [currentSortOrder, setCurrentSortOrder] = useState(
TABLE_SORT_ORDERS.ASC
);
const SortableTableHeader = ({
title,
active,
onSort = () => {},
initialSortDirection = TABLE_SORT_ORDERS.ASC
}) => {
const [currentSortOrder, setCurrentSortOrder] =
useState(initialSortDirection);

const { setMessage } = useAriaLiveRegion();

Expand Down Expand Up @@ -129,7 +133,11 @@ const SortableTableHeader = ({ title, active, onSort = () => {} }) => {
SortableTableHeader.propTypes = {
title: PropTypes.string.isRequired,
active: PropTypes.bool.isRequired,
onSort: PropTypes.func.isRequired
onSort: PropTypes.func.isRequired,
initialSortDirection: PropTypes.oneOf([
TABLE_SORT_ORDERS.ASC,
TABLE_SORT_ORDERS.DESC
])
};

export default SortableTableHeader;
7 changes: 6 additions & 1 deletion client/tests/DataManagement.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,12 @@ const ats = []; // ATs are stubbed until this model is defined
describe('useDataManagementTableSorting hook', () => {
it('sorts by phase by default', () => {
const { result } = renderHook(() =>
useDataManagementTableSorting(testPlans, testPlanVersions, ats)
useDataManagementTableSorting(
testPlans,
testPlanVersions,
ats,
TABLE_SORT_ORDERS.DESC
)
);
expect(result.current.sortedTestPlans).toEqual([
testPlans[3], // RECOMMENDED
Expand Down