Skip to content

Commit

Permalink
John conroy/search qa fixes CAT-1040 CAT-1041 CAT-1042 CAT-1043 CAT-1…
Browse files Browse the repository at this point in the history
…038 CAT-1039 (#3623)

* Fix sort bug

* Fix sort tile donors and samples

* Fix bar order

* Fix date range facet

* Fix calendar styling

* Add changelog

* Fix search note skeleton

* Fix disabled color

---------

Co-authored-by: John Conroy <[email protected]>
  • Loading branch information
john-conroy and john-conroy authored Nov 26, 2024
1 parent 1bd007c commit 21767b5
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 55 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG-search-qa-fixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Fix bug where tile dropdown on search pages was not displaying the selected field.
- Fix bug with date facet validation.
- Improve date picker styles.
- Fix order of search menus.
48 changes: 22 additions & 26 deletions context/app/static/js/components/search/Facets/DateRangeFacet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ function DatePickerComponent({
minDate,
maxDate,
...rest
}: DatePickerProps<Date> & Required<Pick<DatePickerProps<Date>, 'minDate' | 'maxDate'>>) {
}: DatePickerProps<Date> & Partial<Pick<DatePickerProps<Date>, 'minDate' | 'maxDate'>>) {
const [error, setError] = useState<DateValidationError | null>(null);

const errorMessage = useMemo(() => {
switch (error) {
case 'maxDate':
return `Please select a date of ${format(maxDate, 'MMMM yyyy')} or before.`;
return maxDate ? `Please select a date of ${format(maxDate, 'MMMM yyyy')} or before.` : null;

case 'minDate': {
return `Please select a date of ${format(minDate, 'MMMM yyyy')} or after.`;
return minDate ? `Please select a date of ${format(minDate, 'MMMM yyyy')} or after.` : null;
}

case 'invalidDate': {
Expand Down Expand Up @@ -55,19 +55,25 @@ function DatePickerComponent({
textField: {
helperText: errorMessage,
},
popper: {
sx: (theme) => ({
'.MuiDateCalendar-root': {
height: 'auto',
padding: theme.spacing(1.25),
paddingBottom: theme.spacing(2.5),
},
'.MuiPickersMonth-monthButton.Mui-disabled, .MuiPickersYear-yearButton.Mui-disabled': {
color: theme.palette.text.disabled,
},
}),
},
}}
{...rest}
/>
);
}

function DateRangeFacet({
field,
min,
max,
aggMin,
aggMax,
}: DateRangeFacetProps & { min: number; max: number; aggMin: number; aggMax: number }) {
function DateRangeFacet({ field, min, max }: DateRangeFacetProps & { min: number; max: number }) {
const filterDate = useSearchStore((state) => state.filterDate);
const analyticsCategory = useSearchStore((state) => state.analyticsCategory);

Expand All @@ -90,12 +96,12 @@ function DateRangeFacet({

const newMin = value.getTime();
setValues([newMin, values[1]]);
if (newMin >= aggMin && newMin <= values[1]) {
if (newMin <= values[1]) {
filterDate({ field, min: newMin, max });
}
}
},
[filterDate, max, field, setValues, values, aggMin, analyticsCategory],
[filterDate, max, field, setValues, values, analyticsCategory],
);

const filterMax = useCallback(
Expand All @@ -109,12 +115,12 @@ function DateRangeFacet({

const newMax = value.getTime();
setValues([values[0], newMax]);
if (newMax >= values[0] && newMax <= aggMax) {
if (newMax >= values[0]) {
filterDate({ field, min, max: newMax });
}
}
},
[filterDate, min, field, setValues, values, aggMax, analyticsCategory],
[filterDate, min, field, setValues, values, analyticsCategory],
);

return (
Expand All @@ -124,7 +130,6 @@ function DateRangeFacet({
label="Start"
value={new Date(values[0])}
onAccept={filterMin}
minDate={new Date(aggMin)}
maxDate={new Date(values[1])}
/>
<DatePickerComponent
Expand All @@ -133,7 +138,7 @@ function DateRangeFacet({
views={['month', 'year']}
onAccept={filterMax}
minDate={new Date(values[0])}
maxDate={new Date(aggMax)}
maxDate={new Date()}
/>
</Stack>
</FacetAccordion>
Expand Down Expand Up @@ -165,16 +170,7 @@ function DateRangeFacetGuard({ field, ...rest }: DateRangeFacetProps) {
return null;
}

return (
<DateRangeFacet
field={field}
min={min ?? aggMin.value}
max={max ?? aggMax.value}
aggMin={aggMin.value}
aggMax={aggMax.value}
{...rest}
/>
);
return <DateRangeFacet field={field} min={min ?? aggMin.value} max={max ?? aggMax.value} {...rest} />;
}

export default DateRangeFacetGuard;
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ function FilterChips() {
});
}

const hasValues = filterHasValues({ filter: v, facet: facetConfig });
const hasValues = filterHasValues({ filter: v });

if (isExistsFilter(v) && isExistsFacet(facetConfig) && hasValues) {
return (
Expand Down
13 changes: 4 additions & 9 deletions context/app/static/js/components/search/Results/ResultsTiles.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, { useCallback } from 'react';
import { useShallow } from 'zustand/react/shallow';
import { SearchHit } from '@elastic/elasticsearch/lib/api/types';
import Box from '@mui/material/Box';
import MenuItem from '@mui/material/MenuItem';
Expand All @@ -20,14 +19,10 @@ import { useSearchStore } from '../store';
import { useGetFieldLabel } from '../fieldConfigurations';

function TilesSortSelect() {
const { sortField, setSortField, sourceFields, analyticsCategory } = useSearchStore(
useShallow((state) => ({
sortField: state.sortField,
setSortField: state.setSortField,
sourceFields: state.sourceFields,
analyticsCategory: state.analyticsCategory,
})),
);
const sortField = useSearchStore((state) => state.sortField);
const setSortField = useSearchStore((state) => state.setSortField);
const sourceFields = useSearchStore((state) => state.sourceFields);
const analyticsCategory = useSearchStore((state) => state.analyticsCategory);

const { table: tableFields } = sourceFields;

Expand Down
2 changes: 1 addition & 1 deletion context/app/static/js/components/search/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ function Bar({ type }: TypeProps) {
</Box>
<MetadataMenu type={type} />
<WorkspacesDropdownMenu type={type} />
<BulkDownloadButtonFromSearch type={type} />
{view === 'tile' && <TilesSortSelect />}
<BulkDownloadButtonFromSearch type={type} />
<DefaultSearchViewSwitch />
</Stack>
);
Expand Down
7 changes: 5 additions & 2 deletions context/app/static/js/components/search/SearchNote.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Typography from '@mui/material/Typography';
import { InternalLink } from 'js/shared-styles/Links';
import useEntityData from 'js/hooks/useEntityData';
import Skeleton from '@mui/material/Skeleton';
import { Filter, isTermFilter, useSearchStore } from './store';
import { Filter, filterHasValues, isTermFilter, useSearchStore } from './store';

interface MessageProps {
label: string;
Expand Down Expand Up @@ -54,11 +54,14 @@ const paramNotes = [

function SearchNote() {
const filters = useSearchStore((state) => state.filters);
const notesToDisplay = paramNotes.filter(({ filter }) => filters?.[filter]);
const notesToDisplay = paramNotes.filter(
({ filter }) => filters?.[filter] && filterHasValues({ filter: filters?.[filter] }),
);

if (notesToDisplay.length === 0) {
return null;
}

return (
<Typography component="h2">
{notesToDisplay.map(({ filter, label, Component }) => (
Expand Down
12 changes: 6 additions & 6 deletions context/app/static/js/components/search/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export function isExistsFilter<V = Set<string>>(filter: Filter<V>): filter is Ex
return filter.type === 'EXISTS';
}

export function filterHasValues({ filter, facet }: { filter: Filter; facet: Facet }) {
export function filterHasValues({ filter }: { filter: Filter }) {
if (isTermFilter(filter)) {
return filter.values.size;
}
Expand All @@ -259,15 +259,15 @@ export function filterHasValues({ filter, facet }: { filter: Filter; facet: Face
return Object.keys(filter.values).length;
}

if (isRangeFilter(filter) && isRangeFacet(facet)) {
if (isRangeFilter(filter)) {
return !(filter.values.min === undefined && filter.values.max === undefined);
}

if (isDateFilter(filter) && isDateFacet(facet)) {
if (isDateFilter(filter)) {
return filter.values.min && filter.values.max;
}

if (isExistsFilter(filter) && isExistsFacet(facet)) {
if (isExistsFilter(filter)) {
return filter.values;
}
return false;
Expand All @@ -285,10 +285,10 @@ export function buildSearchLink({
}

function replaceURLSearchParams(state: SearchStoreState) {
const { search, sortField, filters, facets } = state;
const { search, sortField, filters } = state;

const filtersWithValues = Object.fromEntries(
Object.entries(filters).filter(([k, v]) => filterHasValues({ filter: v, facet: facets[k] })),
Object.entries(filters).filter(([, v]) => filterHasValues({ filter: v })),
);

const urlState = {
Expand Down
8 changes: 4 additions & 4 deletions context/app/static/js/components/search/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ export function buildQuery({
const facetConfig = facets[field];

if (isTermFilter(filter)) {
if (filterHasValues({ filter, facet: facetConfig })) {
if (filterHasValues({ filter })) {
draft[portalField] = esb.termsQuery(portalField, [...filter.values]);
}
}

if (isRangeFilter(filter) || isDateFilter(filter)) {
if (filterHasValues({ filter, facet: facetConfig })) {
if (filterHasValues({ filter })) {
if (filter.values.min && filter.values.max) {
// TODO: consider using zod in filterHasValues for validation.
draft[portalField] = esb.rangeQuery(portalField).gte(filter.values.min).lte(filter.values.max);
Expand All @@ -113,7 +113,7 @@ export function buildQuery({
}

if (isHierarchicalFilter(filter) && isHierarchicalFacet(facetConfig)) {
if (filterHasValues({ filter, facet: facetConfig })) {
if (filterHasValues({ filter })) {
const childPortalField = getESField({ field: facetConfig.childField, mappings });

draft[portalField] = esb.termsQuery(portalField, Object.keys(filter.values));
Expand All @@ -126,7 +126,7 @@ export function buildQuery({
}

if (isExistsFilter(filter) && isExistsFacet(facetConfig)) {
if (!(filterHasValues({ filter, facet: facetConfig }) && facetConfig?.invert)) {
if (!(filterHasValues({ filter }) && facetConfig?.invert)) {
draft[portalField] = esb.existsQuery(field);
}
}
Expand Down
17 changes: 11 additions & 6 deletions context/app/static/js/pages/search/S.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ import { useAppContext } from 'js/components/Contexts';
const sharedConfig = {
searchFields: ['all_text', 'description'],
// TODO: figure out how to make assertion unnecessary.
sortField: { field: 'last_modified_timestamp', direction: 'desc' as const },
size: 18,
};

const sharedTileFields = [
'hubmap_id',
'uuid',
'last_modified_timestamp',
'created_timestamp',
'published_timestamp',
'entity_type',
'descendant_counts.entity_type',
];
Expand Down Expand Up @@ -65,6 +66,7 @@ const donorFacetGroups = {

const donorConfig = {
...sharedConfig,
sortField: { field: 'last_modified_timestamp', direction: 'desc' as const },
sourceFields: {
table: [
'hubmap_id',
Expand Down Expand Up @@ -107,6 +109,7 @@ const sampleFacetGroups = {

const sampleConfig = {
...sharedConfig,
sortField: { field: 'created_timestamp', direction: 'desc' as const },
sourceFields: {
table: ['hubmap_id', 'group_name', 'sample_category', 'origin_samples_unique_mapped_organs', 'created_timestamp'],
tile: [...sharedTileFields, 'origin_samples_unique_mapped_organs'],
Expand Down Expand Up @@ -183,11 +186,13 @@ function buildDatasetConfig(isHubmapUser: boolean) {
],
tile: [...sharedTileFields, 'thumbnail_file.file_uuid', 'origin_samples_unique_mapped_organs'],
},
sortField: {
field: 'published_timestamp',
direction: 'desc' as const,
secondaryField: { field: 'mapped_status', direction: 'desc' as const },
},
sortField: isHubmapUser
? { field: 'last_modified_timestamp', direction: 'desc' as const }
: {
field: 'published_timestamp',
direction: 'desc' as const,
secondaryField: { field: 'mapped_status', direction: 'desc' as const },
},
facets: buildDatasetFacetGroups(isHubmapUser),
...buildDefaultQuery('Dataset'),
// TODO: figure out how to make assertion unnecessary.
Expand Down

0 comments on commit 21767b5

Please sign in to comment.