fix(release): read Excel start dates, refuse impossible times and repeated columns, keep typed escapes, no suggestions for punctuation

apps/extension/src/omnibox.ts

@@ -53,9 +53,16 @@ function escapeXml(text: string): string {
     .replace(/[&<>"']/g, (character) => `&#${character.charCodeAt(0)};`)
 }
 
-/** `encodeURIComponent`, except a lone surrogate (which it refuses) is dropped. */
+/**
+ * A typed value encoded for the path as the address bar would send it: raw characters are
+ * escaped, an escape already typed (`a%2Fb`) is kept rather than escaped again, and a lone
+ * surrogate (which `encodeURIComponent` refuses) is dropped.
+ */
 function encodeSegment(segment: string): string {
-  return encodeURIComponent(segment.replace(LONE_SURROGATE, ''))
+  return encodeURIComponent(segment.replace(LONE_SURROGATE, '')).replace(
+    /%25([0-9A-Fa-f]{2})/g,
+    '%$1'
+  )
 }
 
 /**

apps/sim/app/o/[organizationId]/people/people-import-modal.tsx

@@ -14,76 +14,18 @@ import {
   useScrollEdges,
 } from '@sim/emcn'
 import { getErrorMessage } from '@sim/utils/errors'
-import { PEOPLE_IMPORT_MAX_ROWS, type PeopleImportResult } from '@/lib/api/contracts/people'
-import { assertOoxmlPreviewWithinLimits } from '@/lib/file-parsers/ooxml-preview-guard'
+import type { PeopleImportResult } from '@/lib/api/contracts/people'
 import { saveBlob } from '@/lib/uploads/client/download'
+import {
+  type PeopleSheet,
+  readPeopleSheet,
+} from '@/app/o/[organizationId]/people/read-people-sheet'
 import { useImportPeople } from '@/hooks/queries/people'
 
-const MAX_FILE_BYTES = 5 * 1024 * 1024
 const ACCEPTED_FILES = '.csv,.tsv,.xlsx,.xls'
 const TEMPLATE_CSV =
   'email,title,department,location,start_date,manager_email\nada@example.com,Engineer,Platform,Berlin,2024-01-31,grace@example.com\n'
 
-interface PeopleSheet {
-  fileName: string
-  headers: string[]
-  /** Each row's cells, in `headers` order. */
-  rows: string[][]
-}
-
-/**
- * The first sheet's named columns and its rows of cells, one per sheet row (blank ones included) so the
- * row numbers in the preview match the spreadsheet. Delimited text is read verbatim so the server
- * interprets dates the same way for every file. A spreadsheet date cell is read from its stored
- * value, not the text its locale displays, so `31/01/2024` and `1/31/24` both arrive as
- * `2024-01-31`.
- */
-async function readPeopleSheet(file: File): Promise<PeopleSheet> {
-  if (file.size > MAX_FILE_BYTES) throw new Error('Choose a file under 5 MB')
-  const data = new Uint8Array(await file.arrayBuffer())
-  if (/\.xlsx$/i.test(file.name)) await assertOoxmlPreviewWithinLimits(data)
-  const XLSX = await import('xlsx')
-  const workbook = XLSX.read(data, {
-    type: 'array',
-    raw: /\.(csv|tsv)$/i.test(file.name),
-    dense: true,
-  })
-  const sheet = workbook.Sheets[workbook.SheetNames[0] ?? '']
-  if (!sheet?.['!ref']) throw new Error('The file has no rows')
-  const range = XLSX.utils.decode_range(sheet['!ref'])
-  const grid = sheet['!data'] ?? []
-  const text = (row: number, column: number): string => {
-    const cell = grid[row]?.[column]
-    if (!cell || cell.v === undefined || cell.v === null) return ''
-    if (cell.t === 'n' && XLSX.SSF.is_date(cell.z ?? '')) {
-      const date = XLSX.SSF.parse_date_code(Number(cell.v))
-      return `${date.y}-${String(date.m).padStart(2, '0')}-${String(date.d).padStart(2, '0')}`
-    }
-    return String(cell.w ?? cell.v).trim()
-  }
-  const columns: { header: string; index: number }[] = []
-  for (let index = range.s.c; index <= range.e.c; index++) {
-    const header = text(range.s.r, index)
-    if (!header) continue
-    if (columns.some((column) => column.header === header)) {
-      throw new Error(`The ${header} column appears more than once`)
-    }
-    columns.push({ header, index })
-  }
-  if (!columns.length) throw new Error('The first row should name the columns')
-  const rowCount = range.e.r - range.s.r
-  if (rowCount > PEOPLE_IMPORT_MAX_ROWS) {
-    throw new Error(`Import at most ${PEOPLE_IMPORT_MAX_ROWS} rows at a time`)
-  }
-  return {
-    fileName: file.name,
-    headers: columns.map(({ header }) => header),
-    rows: Array.from({ length: rowCount }, (_, offset) =>
-      columns.map(({ index }) => text(range.s.r + 1 + offset, index))
-    ),
-  }
-}
-
 function downloadTemplate() {
   saveBlob(new Blob([TEMPLATE_CSV], { type: 'text/csv' }), 'people-template.csv')
 }

apps/sim/app/o/[organizationId]/people/read-people-sheet.test.ts

@@ -0,0 +1,90 @@
+import { describe, expect, it } from 'vitest'
+import * as XLSX from 'xlsx'
+import { readPeopleSheet } from '@/app/o/[organizationId]/people/read-people-sheet'
+
+/** January 31, 2024 as a date serial in each of Excel's two date systems. */
+const SERIAL_1900 = 45322
+const SERIAL_1904 = SERIAL_1900 - 1462
+
+function workbookFile(rows: unknown[][], { format = 'm/d/yy', date1904 = false } = {}): File {
+  const sheet = XLSX.utils.aoa_to_sheet(rows, { dense: false })
+  for (const [address, cell] of Object.entries(sheet)) {
+    if (!address.startsWith('!') && (cell as XLSX.CellObject).t === 'n') {
+      ;(cell as XLSX.CellObject).z = format
+    }
+  }
+  const workbook = XLSX.utils.book_new()
+  XLSX.utils.book_append_sheet(workbook, sheet, 'People')
+  workbook.Workbook = { WBProps: { date1904 } }
+  const bytes = XLSX.write(workbook, { type: 'array', bookType: 'xlsx' })
+  return new File([bytes], 'people.xlsx')
+}
+
+describe('readPeopleSheet', () => {
+  it.each(['m/d/yy', 'dd/mm/yyyy', 'yyyy-mm-dd'])(
+    'reads a date cell formatted %s as its calendar date, not its displayed text',
+    async (format) => {
+      const sheet = await readPeopleSheet(
+        workbookFile(
+          [
+            ['email', 'start date'],
+            ['ada@example.com', SERIAL_1900],
+          ],
+          { format }
+        )
+      )
+      expect(sheet.rows).toEqual([['ada@example.com', '2024-01-31']])
+    }
+  )
+
+  it('reads dates from a workbook saved in the 1904 date system', async () => {
+    const sheet = await readPeopleSheet(
+      workbookFile(
+        [
+          ['email', 'start date'],
+          ['ada@example.com', SERIAL_1904],
+        ],
+        { date1904: true }
+      )
+    )
+    expect(sheet.rows).toEqual([['ada@example.com', '2024-01-31']])
+  })
+
+  it('passes an out-of-range date serial through as text for the server to refuse', async () => {
+    const sheet = await readPeopleSheet(
+      workbookFile([
+        ['email', 'start date'],
+        ['ada@example.com', -5],
+      ])
+    )
+    expect(sheet.rows[0]?.[0]).toBe('ada@example.com')
+    expect(sheet.rows[0]?.[1]).not.toMatch(/^\d{4}-\d{2}-\d{2}$/)
+  })
+
+  it('keeps blank rows in place so issue row numbers match the sheet', async () => {
+    const sheet = await readPeopleSheet(
+      workbookFile([
+        ['email', 'title'],
+        ['ada@example.com', 'Engineer'],
+        [],
+        ['bo@example.com', 'Lead'],
+      ])
+    )
+    expect(sheet.rows).toEqual([
+      ['ada@example.com', 'Engineer'],
+      ['', ''],
+      ['bo@example.com', 'Lead'],
+    ])
+  })
+
+  it('refuses a header row that repeats a column name', async () => {
+    await expect(
+      readPeopleSheet(
+        workbookFile([
+          ['email', 'title', 'title'],
+          ['ada@example.com', 'a', 'b'],
+        ])
+      )
+    ).rejects.toThrow('The title column appears more than once')
+  })
+})

apps/sim/app/o/[organizationId]/people/read-people-sheet.ts

@@ -0,0 +1,70 @@
+import { PEOPLE_IMPORT_MAX_ROWS } from '@/lib/api/contracts/people'
+import { assertOoxmlPreviewWithinLimits } from '@/lib/file-parsers/ooxml-preview-guard'
+
+const MAX_FILE_BYTES = 5 * 1024 * 1024
+
+export interface PeopleSheet {
+  fileName: string
+  headers: string[]
+  /** Each row's cells, in `headers` order. */
+  rows: string[][]
+}
+
+/**
+ * The first sheet's named columns and its rows of cells, one per sheet row (blank ones included) so the
+ * row numbers in the preview match the spreadsheet. Delimited text is read verbatim so the server
+ * interprets dates the same way for every file. A spreadsheet date cell is read from its stored
+ * value, not the text its locale displays, so `31/01/2024` and `1/31/24` both arrive as
+ * `2024-01-31`.
+ */
+export async function readPeopleSheet(file: File): Promise<PeopleSheet> {
+  if (file.size > MAX_FILE_BYTES) throw new Error('Choose a file under 5 MB')
+  const data = new Uint8Array(await file.arrayBuffer())
+  if (/\.xlsx$/i.test(file.name)) await assertOoxmlPreviewWithinLimits(data)
+  const XLSX = await import('xlsx')
+  const workbook = XLSX.read(data, {
+    type: 'array',
+    raw: /\.(csv|tsv)$/i.test(file.name),
+    dense: true,
+    // Keeps each cell's number format, which is how a date cell is told from a number.
+    cellNF: true,
+  })
+  const sheet = workbook.Sheets[workbook.SheetNames[0] ?? '']
+  // Workbooks saved in the 1904 date system count their date serials from a different epoch.
+  const dateSystem = { date1904: Boolean(workbook.Workbook?.WBProps?.date1904) }
+  if (!sheet?.['!ref']) throw new Error('The file has no rows')
+  const range = XLSX.utils.decode_range(sheet['!ref'])
+  const grid = sheet['!data'] ?? []
+  const text = (row: number, column: number): string => {
+    const cell = grid[row]?.[column]
+    if (!cell || cell.v === undefined || cell.v === null) return ''
+    if (cell.t === 'n' && XLSX.SSF.is_date(cell.z ?? '')) {
+      // A serial outside Excel's calendar has no date; its text goes on for the server to refuse.
+      const date = XLSX.SSF.parse_date_code(Number(cell.v), dateSystem)
+      if (date)
+        return `${date.y}-${String(date.m).padStart(2, '0')}-${String(date.d).padStart(2, '0')}`
+    }
+    return String(cell.w ?? cell.v).trim()
+  }
+  const columns: { header: string; index: number }[] = []
+  for (let index = range.s.c; index <= range.e.c; index++) {
+    const header = text(range.s.r, index)
+    if (!header) continue
+    if (columns.some((column) => column.header === header)) {
+      throw new Error(`The ${header} column appears more than once`)
+    }
+    columns.push({ header, index })
+  }
+  if (!columns.length) throw new Error('The first row should name the columns')
+  const rowCount = range.e.r - range.s.r
+  if (rowCount > PEOPLE_IMPORT_MAX_ROWS) {
+    throw new Error(`Import at most ${PEOPLE_IMPORT_MAX_ROWS} rows at a time`)
+  }
+  return {
+    fileName: file.name,
+    headers: columns.map(({ header }) => header),
+    rows: Array.from({ length: rowCount }, (_, offset) =>
+      columns.map(({ index }) => text(range.s.r + 1 + offset, index))
+    ),
+  }
+}

apps/sim/lib/people/import.test.ts

@@ -65,6 +65,13 @@ describe('matchPeopleColumns', () => {
     })
   })
 
+  it('refuses two columns that name the same field, rather than ignoring one', () => {
+    expect(matchPeopleColumns(['Email', 'e-mail', 'Title'])).toEqual({
+      ok: false,
+      message: 'The Email and e-mail columns name the same field; keep one',
+    })
+  })
+
   it('refuses a file without an email column, which is how people are matched', () => {
     expect(matchPeopleColumns(['name', 'title'])).toEqual({
       ok: false,
@@ -215,6 +222,7 @@ describe('planPeopleImport', () => {
     ['2021/3/4', '2021-03-04'],
     ['3/4/2021', '2021-03-04'],
     ['2021-03-04T09:00:00Z', '2021-03-04'],
+    ['2021-03-04T09:00:00.123+02:00', '2021-03-04'],
     ['2021-03-04 09:00', '2021-03-04'],
   ])('reads the start date %s', (value, startDate) => {
     const result = plan(
@@ -224,22 +232,30 @@ describe('planPeopleImport', () => {
     expect(result.changes).toEqual([{ memberId: 'm-ada', profile: profile({ startDate }) }])
   })
 
-  it.each(['2021-02-30', '13/45/2021', '04.03.2021', 'March 4', '1/2/21', '2021-03-04Tsoon'])(
-    'skips the row with the unreadable start date %s instead of guessing',
-    (value) => {
-      const result = plan(
-        ['email', 'start date'],
-        [{ email: 'ada@example.com', 'start date': value }]
-      )
-      expect(result.changes).toEqual([])
-      expect(result.issues).toEqual([
-        expect.objectContaining({
-          severity: 'skipped',
-          message: 'Use a start date like 2024-01-31 or 1/31/2024',
-        }),
-      ])
-    }
-  )
+  it.each([
+    '2021-02-30',
+    '13/45/2021',
+    '04.03.2021',
+    'March 4',
+    '1/2/21',
+    '2021-03-04Tsoon',
+    '2021-03-04T99:99',
+    '2021-03-04T24:00',
+    '2021-03-04T09:00+99:99',
+    '2021-03/04',
+  ])('skips the row with the unreadable start date %s instead of guessing', (value) => {
+    const result = plan(
+      ['email', 'start date'],
+      [{ email: 'ada@example.com', 'start date': value }]
+    )
+    expect(result.changes).toEqual([])
+    expect(result.issues).toEqual([
+      expect.objectContaining({
+        severity: 'skipped',
+        message: 'Use a start date like 2024-01-31 or 1/31/2024',
+      }),
+    ])
+  })
 
   it('leaves the manager as it was when the manager is not a member, and says so', () => {
     const result = plan(

apps/sim/lib/people/import.ts

@@ -56,10 +56,35 @@ function findColumn(headers: readonly string[], aliases: readonly string[]): str
   return undefined
 }
 
+const RECOGNIZED_KEYS = new Set<string>([
+  ...Object.values(COLUMN_ALIASES).flat(),
+  ...LOCATION_PARTS,
+])
+
+/** Two headers that spell the same recognized column differently (`Email`, `e-mail`), if any. */
+function repeatedColumn(headers: readonly string[]): [string, string] | null {
+  const seen = new Map<string, string>()
+  for (const header of headers) {
+    const key = headerKey(header)
+    if (!RECOGNIZED_KEYS.has(key)) continue
+    const earlier = seen.get(key)
+    if (earlier !== undefined) return [earlier, header]
+    seen.set(key, header)
+  }
+  return null
+}
+
 /** Which header holds each field, or why the file can't be imported. */
 export function matchPeopleColumns(
   headers: readonly string[]
 ): { ok: true; columns: PeopleColumns } | { ok: false; message: string } {
+  const repeated = repeatedColumn(headers)
+  if (repeated) {
+    return {
+      ok: false,
+      message: `The ${repeated[0]} and ${repeated[1]} columns name the same field; keep one`,
+    }
+  }
   const email = findColumn(headers, COLUMN_ALIASES.email)
   if (!email) {
     return { ok: false, message: 'Add an email column so each row can be matched to a member' }

apps/sim/lib/people/validation.ts

@@ -2,9 +2,12 @@ import { z } from 'zod'
 
 export const PROFILE_TEXT_MAX_LENGTH = 200
 
-/** A date, optionally with a time of day and offset, which are dropped. */
+/**
+ * A date with one separator throughout, optionally followed by a real time of day and offset,
+ * which are checked and then dropped.
+ */
 const ISO_DATE =
-  /^(\d{4})[-/](\d{1,2})[-/](\d{1,2})(?:[T ]\d{2}:\d{2}(?::\d{2}(?:\.\d+)?)?(?:Z|[+-]\d{2}:?\d{2})?)?$/
+  /^(\d{4})([-/])(\d{1,2})\2(\d{1,2})(?:[T ](?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|[+-](?:[01]\d|2[0-3]):?[0-5]\d)?)?$/
 const US_DATE = /^(\d{1,2})\/(\d{1,2})\/(\d{4})$/
 
 function calendarDate(year: number, month: number, day: number): string | null {
@@ -22,7 +25,7 @@ function calendarDate(year: number, month: number, day: number): string | null {
  */
 export function parseStartDate(value: string): string | null {
   const iso = ISO_DATE.exec(value.trim())
-  if (iso) return calendarDate(Number(iso[1]), Number(iso[2]), Number(iso[3]))
+  if (iso) return calendarDate(Number(iso[1]), Number(iso[3]), Number(iso[4]))
   const us = US_DATE.exec(value.trim())
   if (us) return calendarDate(Number(us[3]), Number(us[1]), Number(us[2]))
   return null