Skip to content

Commit 4764c74

Browse files
asizikovCopilot
andcommitted
fix: require explicit valid seat counts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3a96213 commit 4764c74

4 files changed

Lines changed: 50 additions & 42 deletions

File tree

src/components/SeatCountConfirmation.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { useMemo, useState } from 'react'
22
import type { ChangeEvent } from 'react'
33
import { PeopleIcon, ArrowRightIcon } from '@primer/octicons-react'
44
import { ValidationPopover } from './InfoTip'
5-
import { getSeatReductionError, parseSeatCountInput } from '../utils/seatCounts'
5+
import { getSeatCountInputError, parseSeatCountInput } from '../utils/seatCounts'
66

77
export type SeatCountConfirmationProps = {
88
fileName: string | null
@@ -24,22 +24,22 @@ export function SeatCountConfirmation({
2424
const [businessDraft, setBusinessDraft] = useState<string>(String(defaultBusinessSeats))
2525
const [enterpriseDraft, setEnterpriseDraft] = useState<string>(String(defaultEnterpriseSeats))
2626

27-
const businessError = useMemo(() => getSeatReductionError(businessDraft, defaultBusinessSeats), [businessDraft, defaultBusinessSeats])
28-
const enterpriseError = useMemo(() => getSeatReductionError(enterpriseDraft, defaultEnterpriseSeats), [enterpriseDraft, defaultEnterpriseSeats])
27+
const businessError = useMemo(() => getSeatCountInputError(businessDraft, defaultBusinessSeats), [businessDraft, defaultBusinessSeats])
28+
const enterpriseError = useMemo(() => getSeatCountInputError(enterpriseDraft, defaultEnterpriseSeats), [enterpriseDraft, defaultEnterpriseSeats])
2929
const hasError = Boolean(businessError || enterpriseError)
30-
const normalizedBusinessSeats = parseSeatCountInput(businessDraft, defaultBusinessSeats)
31-
const normalizedEnterpriseSeats = parseSeatCountInput(enterpriseDraft, defaultEnterpriseSeats)
32-
const hasAddedSeats = normalizedBusinessSeats > defaultBusinessSeats || normalizedEnterpriseSeats > defaultEnterpriseSeats
30+
const parsedBusinessSeats = parseSeatCountInput(businessDraft)
31+
const parsedEnterpriseSeats = parseSeatCountInput(enterpriseDraft)
32+
const hasAddedSeats = (parsedBusinessSeats ?? defaultBusinessSeats) > defaultBusinessSeats || (parsedEnterpriseSeats ?? defaultEnterpriseSeats) > defaultEnterpriseSeats
3333
const canApply = !hasError && !isApplying
3434

3535
const onBusinessChange = (event: ChangeEvent<HTMLInputElement>) => setBusinessDraft(event.target.value)
3636
const onEnterpriseChange = (event: ChangeEvent<HTMLInputElement>) => setEnterpriseDraft(event.target.value)
3737

3838
const handleApply = () => {
39-
if (!canApply) return
39+
if (!canApply || parsedBusinessSeats === null || parsedEnterpriseSeats === null) return
4040
onConfirm({
41-
business: normalizedBusinessSeats,
42-
enterprise: normalizedEnterpriseSeats,
41+
business: parsedBusinessSeats,
42+
enterprise: parsedEnterpriseSeats,
4343
})
4444
}
4545

src/utils/seatCounts.test.ts

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,26 @@
11
import { describe, expect, it } from 'vitest'
2-
import { getSeatReductionError, parseSeatCountInput } from './seatCounts'
2+
import { getSeatCountInputError, parseSeatCountInput } from './seatCounts'
33

44
describe('seat count helpers', () => {
5-
it('normalizes seat count input to the historical minimum', () => {
6-
expect(parseSeatCountInput('', 10)).toBe(10)
7-
expect(parseSeatCountInput(' ', 10)).toBe(10)
8-
expect(parseSeatCountInput(' 12.9 ', 10)).toBe(12)
9-
expect(parseSeatCountInput('12.9', 10)).toBe(12)
10-
expect(parseSeatCountInput('not-a-number', 10)).toBe(10)
11-
expect(parseSeatCountInput('8', 10)).toBe(10)
5+
it('parses only explicit whole-number seat counts', () => {
6+
expect(parseSeatCountInput('10')).toBe(10)
7+
expect(parseSeatCountInput(' 12 ')).toBe(12)
8+
expect(parseSeatCountInput('')).toBeNull()
9+
expect(parseSeatCountInput(' ')).toBeNull()
10+
expect(parseSeatCountInput('12.9')).toBeNull()
11+
expect(parseSeatCountInput('not-a-number')).toBeNull()
1212
})
1313

14-
it('reports only reductions below the historical count', () => {
15-
expect(getSeatReductionError('', 10)).toBeNull()
16-
expect(getSeatReductionError(' ', 10)).toBeNull()
17-
expect(getSeatReductionError('10', 10)).toBeNull()
18-
expect(getSeatReductionError(' 12 ', 10)).toBeNull()
19-
expect(getSeatReductionError('12', 10)).toBeNull()
20-
expect(getSeatReductionError('8', 10)).toBe('Cannot go below 10 because that count comes from historical report data.')
14+
it('requires a whole-number seat count at or above the historical count', () => {
15+
const invalidInputError = 'Enter a whole number greater than or equal to 10 because that count comes from historical report data.'
16+
17+
expect(getSeatCountInputError('', 10)).toBe(invalidInputError)
18+
expect(getSeatCountInputError(' ', 10)).toBe(invalidInputError)
19+
expect(getSeatCountInputError('12.9', 10)).toBe(invalidInputError)
20+
expect(getSeatCountInputError('not-a-number', 10)).toBe(invalidInputError)
21+
expect(getSeatCountInputError('10', 10)).toBeNull()
22+
expect(getSeatCountInputError(' 12 ', 10)).toBeNull()
23+
expect(getSeatCountInputError('12', 10)).toBeNull()
24+
expect(getSeatCountInputError('8', 10)).toBe('Cannot go below 10 because that count comes from historical report data.')
2125
})
2226
})

src/utils/seatCounts.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,23 @@ export function normalizeSeatCount(value: number, minimum: number): number {
66
return Math.max(minimum, Math.floor(value))
77
}
88

9-
export function parseSeatCountInput(raw: string, minimum: number): number {
9+
export function parseSeatCountInput(raw: string): number | null {
1010
const trimmed = raw.trim()
11-
if (trimmed === '') return minimum
11+
if (trimmed === '') return null
12+
13+
const parsed = Number(trimmed)
14+
if (!Number.isFinite(parsed) || !Number.isInteger(parsed)) return null
1215

13-
return normalizeSeatCount(Number(trimmed), minimum)
16+
return parsed
1417
}
1518

16-
export function getSeatReductionError(value: string, minimum: number): string | null {
17-
const trimmed = value.trim()
18-
if (trimmed === '') return null
19+
export function getSeatCountInputError(value: string, minimum: number): string | null {
20+
const parsed = parseSeatCountInput(value)
21+
if (parsed === null) {
22+
return `Enter a whole number greater than or equal to ${minimum.toLocaleString()} because that count comes from historical report data.`
23+
}
1924

20-
const parsed = Number(trimmed)
21-
if (!Number.isFinite(parsed) || Math.floor(parsed) >= minimum) return null
25+
if (parsed >= minimum) return null
2226

2327
return `Cannot go below ${minimum.toLocaleString()} because that count comes from historical report data.`
2428
}

src/views/UsersView.tsx

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import { calculateSavingsDifference } from '../utils/billingComparison'
1212
import { InfoTip, ValidationPopover } from '../components/InfoTip'
1313
import { formatAic, formatDifference } from '../utils/format'
14-
import { getSeatReductionError, parseSeatCountInput } from '../utils/seatCounts'
14+
import { getSeatCountInputError, parseSeatCountInput } from '../utils/seatCounts'
1515
import { Trie } from '../utils/trie'
1616
import { th, thBase, thNum, td, tdNum, sortBtn } from '../components/ui/tableStyles'
1717

@@ -75,13 +75,15 @@ export function UsersView({ users, seatOverrides = {}, onSeatOverridesChange, on
7575
const savedEnterprise = seatOverrides.enterprise ?? defaultEnterprise
7676
const hasSavedOverride = seatOverrides.business !== undefined || seatOverrides.enterprise !== undefined
7777

78-
const displayBusiness = isEditing ? (draftBusiness !== '' ? parseInt(draftBusiness, 10) || 0 : savedBusiness) : savedBusiness
79-
const displayEnterprise = isEditing ? (draftEnterprise !== '' ? parseInt(draftEnterprise, 10) || 0 : savedEnterprise) : savedEnterprise
78+
const parsedDraftBusiness = parseSeatCountInput(draftBusiness)
79+
const parsedDraftEnterprise = parseSeatCountInput(draftEnterprise)
80+
const displayBusiness = isEditing ? (parsedDraftBusiness ?? savedBusiness) : savedBusiness
81+
const displayEnterprise = isEditing ? (parsedDraftEnterprise ?? savedEnterprise) : savedEnterprise
8082
const businessSeatError = isEditing
81-
? getSeatReductionError(draftBusiness, defaultBusiness)
83+
? getSeatCountInputError(draftBusiness, defaultBusiness)
8284
: null
8385
const enterpriseSeatError = isEditing
84-
? getSeatReductionError(draftEnterprise, defaultEnterprise)
86+
? getSeatCountInputError(draftEnterprise, defaultEnterprise)
8587
: null
8688
const hasSeatValidationError = Boolean(businessSeatError || enterpriseSeatError)
8789

@@ -106,13 +108,11 @@ export function UsersView({ users, seatOverrides = {}, onSeatOverridesChange, on
106108
}
107109

108110
const handleSave = () => {
109-
if (hasSeatValidationError) return
111+
if (hasSeatValidationError || parsedDraftBusiness === null || parsedDraftEnterprise === null) return
110112

111-
const normalizedBusiness = parseSeatCountInput(draftBusiness, defaultBusiness)
112-
const normalizedEnterprise = parseSeatCountInput(draftEnterprise, defaultEnterprise)
113113
const newOverrides: SeatOverrides = {}
114-
if (normalizedBusiness !== defaultBusiness) newOverrides.business = normalizedBusiness
115-
if (normalizedEnterprise !== defaultEnterprise) newOverrides.enterprise = normalizedEnterprise
114+
if (parsedDraftBusiness !== defaultBusiness) newOverrides.business = parsedDraftBusiness
115+
if (parsedDraftEnterprise !== defaultEnterprise) newOverrides.enterprise = parsedDraftEnterprise
116116
onSeatOverridesChange?.(newOverrides)
117117
setIsEditing(false)
118118
}

0 commit comments

Comments
 (0)