UX improvements to roster search (EDFIAL-689)

app/fe/src/app/Pages/Jobs/MatchReview/components.tsx

@@ -100,36 +100,78 @@ export const SearchField = ({
   label,
   value,
   placeholder,
+  edited,
+  problem,
   onChange,
   onEnter,
 }: {
   label: string;
   value: string;
   placeholder?: string;
+  /** Whether the value differs from the file's. */
+  edited: boolean;
+  /** Why the value can't be searched, shown when the reviewer isn't typing in the field. */
+  problem?: string;
   onChange: (value: string) => void;
   onEnter: () => void;
-}) => (
-  <FormControl>
-    <FormLabel
-      textStyle="body"
-      fontSize="0.75rem"
-      fontWeight="normal"
-      color="blue.50"
-      opacity="0.7"
-      marginBottom="100"
-    >
-      {label}
-    </FormLabel>
-    <Input
-      size="sm"
-      autoComplete="off"
-      value={value}
-      placeholder={placeholder}
-      onChange={(event) => onChange(event.target.value)}
-      onKeyDown={(event) => event.key === 'Enter' && onEnter()}
-    />
-  </FormControl>
-);
+}) => {
+  const [focused, setFocused] = useState(false);
+  // Pressing Enter shows the problem mid-typing; typing again hides it.
+  const [entered, setEntered] = useState(false);
+  return (
+    <FormControl position="relative">
+      <FormLabel
+        display="flex"
+        gap="200"
+        textStyle="body"
+        fontSize="0.75rem"
+        fontWeight="normal"
+        color="blue.50"
+        marginBottom="100"
+      >
+        <Box as="span" opacity="0.7">
+          {label}
+        </Box>
+        {edited && (
+          <Box as="span" color="blue.100" fontWeight="bold">
+            edited
+          </Box>
+        )}
+      </FormLabel>
+      <Input
+        size="sm"
+        autoComplete="off"
+        value={value}
+        placeholder={placeholder}
+        onChange={(event) => {
+          setEntered(false);
+          onChange(event.target.value);
+        }}
+        onKeyDown={(event) => {
+          if (event.key !== 'Enter') return;
+          setEntered(true);
+          onEnter();
+        }}
+        onFocus={() => setFocused(true)}
+        onBlur={() => setFocused(false)}
+      />
+      {problem && (!focused || entered) && (
+        // Out of the flow, so the row's other fields keep their height; it
+        // sits in the gap above the Search button.
+        <Box
+          position="absolute"
+          top="100%"
+          left="200"
+          fontSize="0.7rem"
+          color="pink.100"
+          marginTop="100"
+        >
+          {problem}
+        </Box>
+      )}
+    </FormControl>
+  );
+};
 
 /**
  * When no suggestion fits, or there were none: two ways forward, set apart
@@ -168,7 +210,7 @@ export const NoSuggestionFits = ({
     {children}
     <OptionBox
       title="Search the roster"
-      detail="Look them up with corrected details, and any IDs you have for them. Using a result matches them and moves on."
+      detail="Look the student up in your SIS. Add or correct their ID, and fix or clear any name or date of birth that differs."
     >
       {search}
     </OptionBox>
@@ -179,7 +221,7 @@ export const NoSuggestionFits = ({
     </HStack>
     <OptionBox
       title="Exclude this record from the job"
-      detail="For junk data, or a student who isn't in the roster. Their records won't be reprocessed. You can undo this."
+      detail="For a record that isn't a real student, or a student who isn't in the roster. This record won't be reprocessed. You can undo this."
       tone="caution"
     >
       <SecondaryButton

app/fe/src/app/Pages/Jobs/MatchReview/search.test.ts

@@ -0,0 +1,85 @@
+import { describe, expect, it } from '@jest/globals';
+import {
+  changedFields,
+  isSameSearch,
+  searchBlock,
+  searchDetailsFrom,
+  SearchTerms,
+  termsFrom,
+} from './search';
+
+const terms = (overrides: Partial<SearchTerms> = {}): SearchTerms => ({
+  first_name: 'Ana',
+  last_name: 'Lopez',
+  birth_date: '2012-04-01',
+  student_ids: '123, 456',
+  ...overrides,
+});
+
+describe('changedFields', () => {
+  it('ignores surrounding spaces and the order and repeats of IDs', () => {
+    expect(changedFields(terms(), terms({ first_name: ' Ana ', student_ids: '456 123' }))).toEqual(
+      []
+    );
+    expect(isSameSearch(terms(), terms({ student_ids: '456,123, 456' }))).toBe(true);
+  });
+
+  it('names each changed, added or cleared field', () => {
+    expect(
+      changedFields(
+        terms({ birth_date: '' }),
+        terms({ last_name: 'López', student_ids: '123, 456, 789' })
+      )
+    ).toEqual(['last_name', 'birth_date', 'student_ids']);
+  });
+});
+
+describe('searchBlock', () => {
+  const file = terms();
+  const edit = terms({ last_name: 'López' });
+
+  it("blocks the file's values, already searched for the suggestions", () => {
+    expect(searchBlock(terms({ first_name: ' Ana ' }), file, null)).toBe('file');
+  });
+
+  it('blocks the search whose results are shown, but not an earlier one', () => {
+    expect(searchBlock(edit, file, edit)).toBe('shown');
+    expect(searchBlock(edit, file, terms({ student_ids: '789' }))).toBeNull();
+  });
+
+  it('allows any edit before a search has returned results', () => {
+    expect(searchBlock(edit, file, null)).toBeNull();
+  });
+
+  it('blocks a typed date that is not a real YYYY-MM-DD date', () => {
+    expect(searchBlock(terms({ birth_date: '2012-02-30' }), file, null)).toBe('bad-date');
+    expect(searchBlock(terms({ birth_date: '4/1/2012' }), file, null)).toBe('bad-date');
+  });
+
+  // An unedited form holds the file's values, so a bad date in the file is
+  // in the form too. Editing another field wouldn't enable Search, so the
+  // date is the reason to give.
+  it("blocks a bad date from the file as a bad date, ahead of the file's values", () => {
+    expect(searchBlock(file, file, null)).toBe('file');
+    const fileWithBadDate = terms({ birth_date: '2012-4-1' });
+    expect(searchBlock(fileWithBadDate, fileWithBadDate, null)).toBe('bad-date');
+  });
+
+  it('allows a blank date', () => {
+    expect(searchBlock(terms({ birth_date: ' ' }), file, null)).toBeNull();
+  });
+});
+
+// Some files repeat the same ID across columns.
+describe('repeated IDs', () => {
+  it('appear once in the form', () => {
+    expect(termsFrom({ student_ids: ['123', '123', '456', '123'] }).student_ids).toBe('123, 456');
+  });
+
+  it('are sent once', () => {
+    expect(searchDetailsFrom(terms({ student_ids: '123, 123 456' })).student_ids).toEqual([
+      '123',
+      '456',
+    ]);
+  });
+});

app/fe/src/app/Pages/Jobs/MatchReview/search.ts

@@ -15,19 +15,65 @@ export const termsFrom = (file: StudentInputDetailsJson): SearchTerms => ({
   first_name: valueText(file.first_name) ?? '',
   last_name: valueText(file.last_name) ?? '',
   birth_date: valueText(file.birth_date) ?? '',
-  student_ids: Array.isArray(file.student_ids) ? file.student_ids.map(String).join(', ') : '',
+  student_ids: Array.isArray(file.student_ids)
+    ? [...new Set(file.student_ids.map(String))].join(', ')
+    : '',
 });
 
 /** The search route's body: IDRS's input, with blank fields left out. */
 export const searchDetailsFrom = (terms: SearchTerms): StudentSearchDetailsDto => {
-  const ids = terms.student_ids
-    .split(/[\s,]+/)
-    .map((id) => id.trim())
-    .filter(Boolean);
+  const ids = [
+    ...new Set(
+      terms.student_ids
+        .split(/[\s,]+/)
+        .map((id) => id.trim())
+        .filter(Boolean)
+    ),
+  ];
   return {
     first_name: terms.first_name.trim() || null,
     last_name: terms.last_name.trim() || null,
     birth_date: terms.birth_date.trim() || null,
     student_ids: ids.length ? ids : null,
   };
 };
+
+/**
+ * The fields where two sets of terms send IDRS different searches, ignoring
+ * spacing and the order and repeats of IDs.
+ */
+export const changedFields = (a: SearchTerms, b: SearchTerms): (keyof SearchTerms)[] => {
+  const sent = (terms: SearchTerms) => {
+    const details = searchDetailsFrom(terms);
+    return { ...details, student_ids: details.student_ids?.slice().sort().join(' ') ?? null };
+  };
+  const [sentA, sentB] = [sent(a), sent(b)];
+  return (Object.keys(a) as (keyof SearchTerms)[]).filter((key) => sentA[key] !== sentB[key]);
+};
+
+export const isSameSearch = (a: SearchTerms, b: SearchTerms) => changedFields(a, b).length === 0;
+
+/** Whether a date is a real YYYY-MM-DD date, the only form the search route takes. */
+export const realDate = (value: string) => {
+  if (!/^\d{4}-\d{2}-\d{2}$/.test(value)) return false;
+  const date = new Date(`${value}T00:00:00Z`);
+  return !Number.isNaN(date.getTime()) && date.toISOString().slice(0, 10) === value;
+};
+
+/**
+ * Why the terms can't be searched, or null when they can. A search mustn't
+ * repeat what the reviewer already has: the file's values, which IDRS
+ * searched for the suggestions (with the file's school IDs, which the form
+ * leaves out), or the search whose results are shown.
+ */
+export const searchBlock = (
+  terms: SearchTerms,
+  fileTerms: SearchTerms,
+  shownTerms: SearchTerms | null
+): 'bad-date' | 'file' | 'shown' | null => {
+  const birthDate = terms.birth_date.trim();
+  if (birthDate && !realDate(birthDate)) return 'bad-date';
+  if (isSameSearch(terms, fileTerms)) return 'file';
+  if (shownTerms && isSameSearch(terms, shownTerms)) return 'shown';
+  return null;
+};