Merge pull request #962 from nowcommunity/codex/investigate-and-fix-crown-type-column-bug-pzfegy

Fix crown type column ids in species table.

The updateLogNavigation.cy.ts test did not pass, but it is likely due to another issue.

frontend/src/components/CrossSearch/CrossSearchTable.tsx

@@ -1,6 +1,6 @@
 import { useMemo } from 'react'
 import { type MRT_ColumnDef } from 'material-react-table'
-import { CrossSearch } from '@/shared/types'
+import { CrossSearch, formatDevelopmentalCrownType } from '@/shared/types'
 import { TableView } from '../TableView/TableView'
 import { useGetAllCrossSearchQuery, useGetAllCrossSearchLocalitiesQuery } from '@/redux/crossSearchReducer'
 import { usePageContext } from '../Page'
@@ -522,7 +522,7 @@ export const CrossSearchTable = ({ selectorFn }: { selectorFn?: (newObject: Cros
       },
       {
         id: 'crowntype',
-        accessorFn: row => row.crowntype || '',
+        accessorFn: formatDevelopmentalCrownType,
         header: 'Developmental Crown Type',
         size: 20,
         filterFn: 'contains',

frontend/src/components/Species/SpeciesTable.tsx

@@ -1,7 +1,7 @@
 import { useMemo, useState } from 'react'
 import { type MRT_ColumnDef, type MRT_FilterFn } from 'material-react-table'
 import { useGetAllSpeciesQuery } from '../../redux/speciesReducer'
-import { Species } from '@/shared/types'
+import { Species, formatDevelopmentalCrownType, formatFunctionalCrownType } from '@/shared/types'
 import { TableView } from '../TableView/TableView'
 import { SynonymsModal } from './SynonymsModal'
 
@@ -188,16 +188,16 @@ export const SpeciesTable = ({ selectorFn }: { selectorFn?: (id: Species) => voi
         filterFn: 'contains',
       },
       {
-        id: 'crowntype',
-        accessorFn: row => row.crowntype || '',
-        header: 'Crown Type',
+        id: 'developmental_crown_type',
+        accessorFn: formatDevelopmentalCrownType,
+        header: 'Developmental Crown Type',
         size: 10,
         filterFn: 'contains',
       },
       {
-        id: 'crowntype',
-        accessorFn: row => row.crowntype || '',
-        header: 'Developmental Crown Type',
+        id: 'functional_crown_type',
+        accessorFn: formatFunctionalCrownType,
+        header: 'Functional Crown Type',
         size: 10,
         filterFn: 'contains',
       },
@@ -396,7 +396,8 @@ export const SpeciesTable = ({ selectorFn }: { selectorFn?: (id: Species) => voi
     tshm: false,
     tht: false,
     horizodonty: false,
-    crowntype: false,
+    developmental_crown_type: false,
+    functional_crown_type: false,
     cusp_shape: false,
     cusp_count_buccal: false,
     cusp_count_lingual: false,

frontend/src/shared/types/util.ts

@@ -29,3 +29,45 @@ export type RowState = 'new' | 'removed' | 'cancelled' | 'clean'
 // Use this for fields that include array that has to be edited by EditableTable.
 // For example see LocalityDetails: museums field
 export type Editable<T> = T & { rowState?: RowState }
+
+type CrownSegment = string | number | null | undefined
+
+const mapCrownSegment = (segment: CrownSegment): string => {
+  if (segment === null || segment === undefined || segment === '') {
+    return '-'
+  }
+
+  return String(segment)
+}
+
+type DevelopmentalCrownTypeSource = {
+  cusp_shape?: CrownSegment
+  cusp_count_buccal?: CrownSegment
+  cusp_count_lingual?: CrownSegment
+  loph_count_lon?: CrownSegment
+  loph_count_trs?: CrownSegment
+}
+
+export const formatDevelopmentalCrownType = <T extends DevelopmentalCrownTypeSource>(source: T): string => {
+  return [
+    source.cusp_shape,
+    source.cusp_count_buccal,
+    source.cusp_count_lingual,
+    source.loph_count_lon,
+    source.loph_count_trs,
+  ]
+    .map(mapCrownSegment)
+    .join('')
+}
+
+type FunctionalCrownTypeSource = {
+  fct_al?: CrownSegment
+  fct_ol?: CrownSegment
+  fct_sf?: CrownSegment
+  fct_ot?: CrownSegment
+  fct_cm?: CrownSegment
+}
+
+export const formatFunctionalCrownType = <T extends FunctionalCrownTypeSource>(source: T): string => {
+  return [source.fct_al, source.fct_ol, source.fct_sf, source.fct_ot, source.fct_cm].map(mapCrownSegment).join('')
+}

frontend/src/tests/util/formatDevelopmentalCrownType.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from '@jest/globals'
+import { formatDevelopmentalCrownType } from '../../shared/types/util'
+
+describe('formatDevelopmentalCrownType', () => {
+  it('returns hyphen placeholders when all segments are empty', () => {
+    expect(
+      formatDevelopmentalCrownType({
+        cusp_shape: null,
+        cusp_count_buccal: null,
+        cusp_count_lingual: null,
+        loph_count_lon: null,
+        loph_count_trs: null,
+      })
+    ).toBe('-----')
+  })
+
+  it('converts numeric values to strings and preserves characters', () => {
+    expect(
+      formatDevelopmentalCrownType({
+        cusp_shape: 'tri',
+        cusp_count_buccal: 2,
+        cusp_count_lingual: 3,
+        loph_count_lon: 4,
+        loph_count_trs: 5,
+      })
+    ).toBe('tri2345')
+  })
+
+  it('treats empty strings as missing data', () => {
+    expect(
+      formatDevelopmentalCrownType({
+        cusp_shape: 'bun',
+        cusp_count_buccal: '',
+        cusp_count_lingual: 1,
+        loph_count_lon: undefined,
+        loph_count_trs: 'x',
+      })
+    ).toBe('bun-1-x')
+  })
+})

frontend/src/tests/util/formatFunctionalCrownType.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from '@jest/globals'
+import { formatFunctionalCrownType } from '../../shared/types/util'
+
+describe('formatFunctionalCrownType', () => {
+  it('returns hyphen placeholders when all segments are empty', () => {
+    expect(
+      formatFunctionalCrownType({
+        fct_al: null,
+        fct_ol: null,
+        fct_sf: null,
+        fct_ot: null,
+        fct_cm: null,
+      })
+    ).toBe('-----')
+  })
+
+  it('converts numeric values to strings and preserves characters', () => {
+    expect(
+      formatFunctionalCrownType({
+        fct_al: 1,
+        fct_ol: 0,
+        fct_sf: 'S',
+        fct_ot: 2,
+        fct_cm: 3,
+      })
+    ).toBe('10S23')
+  })
+
+  it('treats empty strings as missing data', () => {
+    expect(
+      formatFunctionalCrownType({
+        fct_al: 'A',
+        fct_ol: '',
+        fct_sf: null,
+        fct_ot: 'T',
+        fct_cm: undefined,
+      })
+    ).toBe('A--T-')
+  })
+})

frontend/tests/components/CrossSearchTable.test.tsx

@@ -0,0 +1,128 @@
+import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'
+import { render, waitFor } from '@testing-library/react'
+
+type CrossSearchRow = {
+  lid_now_loc: number
+  species_id_com_species: number
+  appr_num_spm: number
+  cusp_shape?: string | number | null
+  cusp_count_buccal?: number | null
+  cusp_count_lingual?: number | null
+  loph_count_lon?: number | null
+  loph_count_trs?: number | null
+  [key: string]: unknown
+}
+
+type MockedColumn = {
+  id?: string
+  accessorKey?: string
+  accessorFn?: (row: CrossSearchRow) => unknown
+}
+
+type MockedTableViewProps = {
+  data: CrossSearchRow[] | undefined
+  columns?: MockedColumn[]
+}
+
+const mockTableView = jest.fn((props: MockedTableViewProps) => {
+  capturedProps = props
+  return null
+})
+
+const mockUseGetAllCrossSearchQuery = jest.fn()
+const mockUseGetAllCrossSearchLocalitiesQuery = jest.fn()
+const mockUsePageContext = jest.fn()
+
+jest.mock('../../src/components/TableView/TableView', () => ({
+  TableView: (props: MockedTableViewProps) => mockTableView(props),
+}))
+
+jest.mock('../../src/redux/crossSearchReducer', () => ({
+  useGetAllCrossSearchQuery: (...args: unknown[]) => mockUseGetAllCrossSearchQuery(...args),
+  useGetAllCrossSearchLocalitiesQuery: (...args: unknown[]) => mockUseGetAllCrossSearchLocalitiesQuery(...args),
+}))
+
+jest.mock('../../src/components/Map/LocalitiesMap', () => ({
+  LocalitiesMap: () => null,
+}))
+
+jest.mock('../../src/components/Page', () => ({
+  usePageContext: (...args: unknown[]) => mockUsePageContext(...args),
+}))
+
+let capturedProps: MockedTableViewProps | undefined
+
+const createCrossSearch = (overrides: Partial<CrossSearchRow> = {}): CrossSearchRow => {
+  return {
+    species_id_com_species: 0,
+    lid_now_loc: 0,
+    appr_num_spm: 0,
+    ...overrides,
+  }
+}
+
+describe('CrossSearchTable column configuration', () => {
+  beforeEach(async () => {
+    capturedProps = undefined
+    mockUsePageContext.mockReturnValue({
+      sqlLimit: 25,
+      sqlOffset: 0,
+      sqlColumnFilters: [],
+      sqlOrderBy: [],
+    })
+    mockUseGetAllCrossSearchQuery.mockReturnValue({ data: [createCrossSearch()], isFetching: false })
+    mockUseGetAllCrossSearchLocalitiesQuery.mockReturnValue({ data: [], isFetching: false })
+
+    const { CrossSearchTable } = await import('../../src/components/CrossSearch/CrossSearchTable')
+    render(<CrossSearchTable />)
+    await waitFor(() => {
+      expect(mockTableView).toHaveBeenCalled()
+    })
+  })
+
+  afterEach(() => {
+    jest.clearAllMocks()
+    capturedProps = undefined
+  })
+
+  it('provides unique column identifiers', () => {
+    expect(capturedProps?.columns).toBeDefined()
+    const identifiers = (capturedProps?.columns ?? [])
+      .map(column => {
+        if (!column) {
+          return undefined
+        }
+
+        if (typeof column.id === 'string') {
+          return column.id
+        }
+
+        if (typeof column.accessorKey === 'string') {
+          return column.accessorKey
+        }
+
+        return undefined
+      })
+      .filter((identifier): identifier is string => Boolean(identifier))
+
+    expect(new Set(identifiers).size).toBe(identifiers.length)
+  })
+
+  it('formats developmental crown type values using the shared formatter', () => {
+    const developmentalColumn = (capturedProps?.columns ?? []).find(column => column?.id === 'crowntype')
+    expect(developmentalColumn).toBeDefined()
+
+    const accessorFn = developmentalColumn?.accessorFn
+    expect(typeof accessorFn).toBe('function')
+
+    const sample = createCrossSearch({
+      cusp_shape: 'S',
+      cusp_count_buccal: 1,
+      cusp_count_lingual: 2,
+      loph_count_lon: 3,
+      loph_count_trs: 4,
+    })
+
+    expect(accessorFn?.(sample)).toBe('S1234')
+  })
+})

frontend/tests/components/SpeciesTable.test.tsx

@@ -1,11 +1,18 @@
 import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'
-import { renderToStaticMarkup } from 'react-dom/server'
+import { render, waitFor } from '@testing-library/react'
 import type { Species } from '../../src/shared/types'
 
 type SynonymFilter = (row: { original: Species }, columnId: string, filterValue: unknown) => boolean
 
+type MockedColumn = {
+  id?: string
+  accessorKey?: string
+  accessorFn?: (row: Species) => unknown
+}
+
 type MockedTableViewProps = {
   data: Species[] | undefined
+  columns?: MockedColumn[]
   filterFns?: Record<string, SynonymFilter>
 }
 
@@ -102,7 +109,10 @@ describe('SpeciesTable synonym filtering', () => {
     mockUseGetAllSpeciesQuery.mockReturnValue({ data: [regularSpecies, speciesWithSynonyms], isFetching: false })
 
     const { SpeciesTable } = await import('../../src/components/Species/SpeciesTable')
-    renderToStaticMarkup(<SpeciesTable />)
+    render(<SpeciesTable />)
+    await waitFor(() => {
+      expect(mockTableView).toHaveBeenCalled()
+    })
   })
 
   afterEach(() => {
@@ -141,4 +151,63 @@ describe('SpeciesTable synonym filtering', () => {
     expect(speciesFilter!(row as { original: Species }, 'species_name', 'fami')).toBe(true)
     expect(speciesFilter!(row as { original: Species }, 'species_name', 'absent')).toBe(false)
   })
+
+  it('provides unique column identifiers', () => {
+    expect(capturedProps?.columns).toBeDefined()
+    const identifiers = (capturedProps?.columns ?? [])
+      .map(column => {
+        if (!column) {
+          return undefined
+        }
+
+        if (typeof column.id === 'string') {
+          return column.id
+        }
+
+        if (typeof column.accessorKey === 'string') {
+          return column.accessorKey
+        }
+
+        return undefined
+      })
+      .filter((identifier): identifier is string => Boolean(identifier))
+
+    expect(new Set(identifiers).size).toBe(identifiers.length)
+  })
+
+  it('formats developmental crown type values using the shared formatter', () => {
+    const developmentalColumn = (capturedProps?.columns ?? []).find(column => column?.id === 'developmental_crown_type')
+    expect(developmentalColumn).toBeDefined()
+
+    const accessorFn = developmentalColumn?.accessorFn
+    expect(typeof accessorFn).toBe('function')
+
+    const sample = createSpecies({
+      cusp_shape: 'B',
+      cusp_count_buccal: '2',
+      cusp_count_lingual: '3',
+      loph_count_lon: '4',
+      loph_count_trs: '5',
+    })
+
+    expect(accessorFn?.(sample)).toBe('B2345')
+  })
+
+  it('formats functional crown type values using the shared formatter', () => {
+    const functionalColumn = (capturedProps?.columns ?? []).find(column => column?.id === 'functional_crown_type')
+    expect(functionalColumn).toBeDefined()
+
+    const accessorFn = functionalColumn?.accessorFn
+    expect(typeof accessorFn).toBe('function')
+
+    const sample = createSpecies({
+      fct_al: 'A',
+      fct_ol: '0',
+      fct_sf: 'S',
+      fct_ot: '',
+      fct_cm: null,
+    })
+
+    expect(accessorFn?.(sample)).toBe('A0S--')
+  })
 })