Merge pull request #84 from simonsobs/odds-and-ends

Odds and ends

Author: jernestmyers

src/App.tsx

@@ -9,6 +9,7 @@ import { SearchResults } from './components/SearchResults';
 import { ErrorBoundary } from './components/ErrorBoundary';
 import { ErrorBoundaryWrapper } from './components/ErrorBoundaryWrapper';
 import { Footer } from './Footer';
+import { IS_CROSSMATCH_ENABLED } from './configs/constants';
 
 function App() {
   const location = useLocation();
@@ -35,8 +36,14 @@ function App() {
           <Route index element={null} />
           <Route path="/search/*" element={<SearchResults />} />
           <Route path="/source/:id" element={<Source />} />
-          <Route path="/unassigned/" element={<UnassignedSourcesTable />} />
-          <Route path="/unassigned/:id" element={<UnassignedSource />} />
+          {/* Gated like the Cross Matcher nav button, so a disabled deployment can't reach these
+          pages by URL either; they fall through to PageNotFound instead */}
+          {IS_CROSSMATCH_ENABLED && (
+            <>
+              <Route path="/unassigned/" element={<UnassignedSourcesTable />} />
+              <Route path="/unassigned/:id" element={<UnassignedSource />} />
+            </>
+          )}
           <Route path="*" element={<PageNotFound />} />
         </Routes>
       </ErrorBoundaryWrapper>

src/components/NearbySourcesSection.tsx

@@ -52,11 +52,8 @@ export function NearbySourcesSection({
                   {s.name}
                 </Link>
                 <p className="small-text">
-                  {angularSeparationDeg(
-                    sourceRa,
-                    sourceDec,
-                    s.ra,
-                    s.dec
+                  {(
+                    angularSeparationDeg(sourceRa, sourceDec, s.ra, s.dec) * 60
                   ).toFixed(2)}{' '}
                   arcmin
                 </p>

src/components/RangeInput.tsx

@@ -51,6 +51,7 @@ export function RangeInput({
         onChange={handleChange}
         onMouseUp={handleCommit}
         onTouchEnd={handleCommit}
+        onKeyUp={handleCommit}
       />
     </div>
   );

src/components/SearchResults.tsx

@@ -84,23 +84,27 @@ export function SearchResults() {
             </h3>
           )}
           <Table
+            className="themed-table"
             data={results}
             columns={
               [
                 {
                   header: 'Source ID',
                   // Link to individual sources from the table
                   cell: ({ row }) => (
-                    <Link to={`/source/${row.original.source_id}`}>
+                    <Link
+                      className="text-so-blue"
+                      to={`/source/${row.original.source_id}`}
+                    >
                       {row.original.source_id}
                     </Link>
                   ),
                   size: 50,
                   accessorFn: (row) => row.source_id,
                 },
                 {
-                  header: 'ra',
-                  // accessorFn: (row) => row.ra.toFixed(1),
+                  header: 'RA (deg)',
+                  accessorFn: (row) => row.ra,
                   cell: ({ row }) => (
                     <span title={String(row.original.ra)}>
                       {row.original.ra.toFixed(3)}
@@ -109,8 +113,8 @@ export function SearchResults() {
                   size: 125,
                 },
                 {
-                  header: 'dec',
-                  // accessorFn: (row) => row.dec.toFixed(1),
+                  header: 'Dec (deg)',
+                  accessorFn: (row) => row.dec,
                   cell: ({ row }) => (
                     <span title={String(row.original.dec)}>
                       {row.original.dec.toFixed(3)}

src/components/Sources.tsx

@@ -111,26 +111,30 @@ export function UnassignedSource() {
         STATUS_TO_EXCLUDE_SIMBAD_SEARCH.includes(data.source.status)
       )
         return { data: [], queryTimestamp: undefined };
-      let matches: SimbadMatch[] = [];
+      // Any failure here (network error, non-2xx status, or an unexpected response shape) is
+      // surfaced in the SIMBAD card below rather than shown as an empty match list, which would
+      // read as "nothing nearby" and could lead a reviewer to wrongly rule out an external match.
       const getSimbadHits = await fetch(
         SIMBAD_BASE_URL +
           '/cone' +
           `?ra=${toSimbadRa(data.source.ra)}&dec=${data.source.dec}&sr=${UNASSIGNED_BEAM_RADIUS_ARCMIN / 60}&verb=2&maxrec=10&responseformat=json&order_by=nb_ref&order_dir=desc`
       );
-      try {
-        const hitsJson = (await getSimbadHits.json()) as SimbadConeResponse;
-        matches = hitsJson.data.map((row) => ({
-          identifier: row[1] ?? 'SIMBAD object',
-          objectType: row[4] ?? null,
-          ra: row[2] ?? null,
-          dec: row[3] ?? null,
-          separationArcmin: row[0] != null ? row[0] * 60 : null,
-        }));
-      } catch {
-        console.error(
-          'An error occurred when querying for nearby SIMBAD sources.'
+      if (!getSimbadHits.ok) {
+        throw new Error(
+          `SIMBAD returned ${getSimbadHits.status} ${getSimbadHits.statusText}`
         );
       }
+      const hitsJson = (await getSimbadHits.json()) as SimbadConeResponse;
+      if (!Array.isArray(hitsJson?.data)) {
+        throw new Error('SIMBAD returned an unexpected response.');
+      }
+      const matches: SimbadMatch[] = hitsJson.data.map((row) => ({
+        identifier: row[1] ?? 'SIMBAD object',
+        objectType: row[4] ?? null,
+        ra: row[2] ?? null,
+        dec: row[3] ?? null,
+        separationArcmin: row[0] != null ? row[0] * 60 : null,
+      }));
       return {
         // ISO 8601, not a locale-formatted string: this doubles as the `queried_at` sent to the
         // server's external_evidence.queried_at (a datetime field) if the reviewer accepts one
@@ -244,10 +248,6 @@ export function UnassignedSource() {
     throw new Error(error.message);
   }
 
-  if (simbadError) {
-    throw new Error(simbadError.message);
-  }
-
   return (
     <div className="unassigned-source-page-container">
       <ConfirmDialog
@@ -423,6 +423,11 @@ export function UnassignedSource() {
         >
           {isSimbadQueryActive ? (
             <p>Loading...</p>
+          ) : simbadError ? (
+            <p className="small-text">
+              The SIMBAD search failed, so potential matches couldn&rsquo;t be
+              checked. Reload the page to try again.
+            </p>
           ) : (
             <>
               <p className="small-text">

src/components/UnassignedSource.tsx

@@ -132,12 +132,13 @@ export function UnassignedSourcesTable() {
       <p className="small-text">
         {tableData.length}{' '}
         {statusFilter === INCLUDE_ALL_STATUSES
-          ? 'detected'
-          : formatSourceStatus(statusFilter).toLowerCase()}{' '}
-        sources awaiting review
+          ? 'sources'
+          : statusFilter === 'unmatched'
+            ? 'sources awaiting review'
+            : `${formatSourceStatus(statusFilter).toLowerCase()} sources`}
       </p>
       <Table
-        className="unassigned-sources-table"
+        className="unassigned-sources-table themed-table"
         data={tableData}
         columns={columns}
       />

src/components/UnassignedSourcesTable.tsx

@@ -31,4 +31,17 @@ describe('App', () => {
       screen.getByRole('heading', { name: /page not found/i })
     ).toBeInTheDocument();
   });
+
+  it('renders the not-found page for cross-match routes when cross-matching is disabled', () => {
+    // VITE_CROSSMATCH_ENABLED isn't set in the test environment
+    render(
+      <MemoryRouter initialEntries={['/unassigned/some-source-id']}>
+        <App />
+      </MemoryRouter>
+    );
+
+    expect(
+      screen.getByRole('heading', { name: /page not found/i })
+    ).toBeInTheDocument();
+  });
 });

tests/App.test.tsx

@@ -100,4 +100,27 @@ describe('RangeInput', () => {
     expect(onFinalChange).toHaveBeenCalledTimes(1);
     expect(onFinalChange).toHaveBeenCalledWith(9);
   });
+
+  it('calls onFinalChange after a keyboard change', () => {
+    const onFinalChange = vi.fn();
+
+    render(
+      <RangeInput
+        min={0}
+        max={10}
+        defaultValue={5}
+        onFinalChange={onFinalChange}
+        label="Radius"
+      />
+    );
+
+    const slider = screen.getByRole('slider');
+    // The browser fires change on keydown, then keyup once the key is released
+    fireEvent.change(slider, { target: { value: '6' } });
+    fireEvent.keyUp(slider, { key: 'ArrowRight' });
+    vi.advanceTimersByTime(100);
+
+    expect(onFinalChange).toHaveBeenCalledTimes(1);
+    expect(onFinalChange).toHaveBeenCalledWith(6);
+  });
 });