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

Author: waleedlatif1

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

@@ -47,6 +47,8 @@ async function readPeopleSheet(file: File): Promise<PeopleSheet> {
     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] ?? '']
   if (!sheet?.['!ref']) throw new Error('The file has no rows')

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

apps/sim/lib/sim-links/application/use-cases.ts

@@ -20,11 +20,9 @@ import { requireSimLinksEnabled } from '@/lib/sim-links/feature-flag'
 import type { SimLinkAccess, SimLinkRecord, SimLinkViewer } from '@/lib/sim-links/repository'
 import * as repository from '@/lib/sim-links/repository'
 import {
-  normalizeSimLinkName,
-  parseTypedSimLink,
   type SimLinkFields,
   type SimLinkSort,
-  stripSimLinkPrefix,
+  typedSimLinkPrefix,
 } from '@/lib/sim-links/validation'
 import type { SortDirection } from '@/lib/url-state'
 
@@ -255,12 +253,6 @@ export const resolveSimLink = defineAuthorizedOrganizationUseCase({
   },
 })
 
-/** The name in what was typed after the `sim` keyword: `sim/jira/ABC-1?x` suggests from `jira`. */
-function typedNamePrefix(query: string): string {
-  const [path = ''] = stripSimLinkPrefix(query.trim()).split('?')
-  return normalizeSimLinkName(parseTypedSimLink(path.split('/')).name)
-}
-
 /**
  * Address-bar suggestions from the organization `sim/` opens in for this session, so every
  * suggestion resolves where it is followed. Nothing is suggested where Sim links are off.
@@ -269,7 +261,8 @@ export const suggestSimLinks = defineAuthorizedCredentialUserUseCase({
   operation: simLinkOperations.suggest,
   async execute({ principal, input }: { principal: SessionPrincipal; input: { query: string } }) {
     const organizationId = await resolveSimLinkOrganization(principal)
-    if (!organizationId) return []
-    return repository.suggestSimLinks(organizationId, typedNamePrefix(input.query))
+    const prefix = typedSimLinkPrefix(input.query)
+    if (!organizationId || prefix === null) return []
+    return repository.suggestSimLinks(organizationId, prefix)
   },
 })

apps/sim/lib/sim-links/destination.test.ts

@@ -1,6 +1,10 @@
 import { describe, expect, it } from 'vitest'
 import { resolveSimLinkTarget } from '@/lib/sim-links/destination'
-import { parseTypedSimLink, simLinkFieldsSchema } from '@/lib/sim-links/validation'
+import {
+  parseTypedSimLink,
+  simLinkFieldsSchema,
+  typedSimLinkPrefix,
+} from '@/lib/sim-links/validation'
 
 const fields = {
   name: 'docs',
@@ -163,6 +167,19 @@ describe('Sim link destinations', () => {
   })
 })
 
+describe('typedSimLinkPrefix', () => {
+  it.each([
+    ['', ''],
+    ['sim/Team-Docs/ABC-1?x=1', 'teamdocs'],
+  ])('reads the name prefix of %j', (typed, prefix) => {
+    expect(typedSimLinkPrefix(typed)).toBe(prefix)
+  })
+
+  it.each(['...', '--', '.:-'])('has no prefix for %j, which no link name can match', (typed) => {
+    expect(typedSimLinkPrefix(typed)).toBeNull()
+  })
+})
+
 describe('parseTypedSimLink', () => {
   it.each([
     [' onboarding', 'onboarding'],

apps/sim/lib/sim-links/validation.ts

@@ -20,7 +20,7 @@ export function normalizeSimLinkName(name: string): string {
 }
 
 /** Text as typed after `sim/`, whether or not the prefix itself was typed too. */
-export function stripSimLinkPrefix(input: string): string {
+function stripSimLinkPrefix(input: string): string {
   return input.replace(/^sim\//i, '')
 }
 
@@ -55,6 +55,18 @@ export function parseTypedSimLink(segments: readonly string[]): {
   return { name: name.replace(PASTED_TRAILING_PUNCTUATION, '').trimEnd() || name, values }
 }
 
+/**
+ * The normalized name prefix to suggest links from, out of what was typed after the `sim`
+ * keyword: `sim/jira/ABC-1?x` suggests from `jira`, and nothing typed browses every link. `null`
+ * when a name was typed that no link can match, such as only punctuation.
+ */
+export function typedSimLinkPrefix(query: string): string | null {
+  const [path = ''] = stripSimLinkPrefix(query.trim()).split('?')
+  const { name } = parseTypedSimLink(path.split('/'))
+  const prefix = normalizeSimLinkName(name)
+  return name && !prefix ? null : prefix
+}
+
 export function isVariableSimLinkUrl(url: string): boolean {
   return url.includes(SIM_LINK_VARIABLE)
 }