refactor: improve gsheet import UX

This commit is contained in:
Carlos Valente
2026-05-03 18:17:17 +02:00
committed by GitHub
parent 7c1d2f4554
commit ba1c3235b3
12 changed files with 182 additions and 60 deletions
@@ -15,23 +15,25 @@ import Input from '../../../../../common/components/input/input/Input';
import Tag from '../../../../../common/components/tag/Tag';
import { openLink } from '../../../../../common/utils/linkUtils';
import * as Panel from '../../../panel-utils/PanelUtils';
import { extractSheetId, getPersistedSheetId, persistSheetId } from './gsheetUtils';
import style from './SourcesPanel.module.scss';
interface GSheetSetupProps {
onCancel: () => void;
onSheetLoaded: (sheetId: string, options: SpreadsheetWorksheetOptions) => void;
closedByUser: boolean;
}
export default function GSheetSetup(props: GSheetSetupProps) {
const { onCancel, onSheetLoaded } = props;
const { onCancel, onSheetLoaded, closedByUser } = props;
const [file, setFile] = useState<File | null>(null);
const [sheetId, setSheetId] = useState('');
const [sheetId, setSheetId] = useState(getPersistedSheetId);
const [authenticationStatus, setAuthenticationStatus] = useState<AuthenticationStatus>('not_authenticated');
const [authKey, setAuthKey] = useState<string | null>(null);
const [loading, setLoading] = useState<'' | 'cancel' | 'connect' | 'authenticate' | 'load-sheet'>('');
const [authLink, setAuthLink] = useState('');
const [loading, setLoading] = useState<'' | 'cancel' | 'connect' | 'authenticate' | 'load-sheet'>('');
const [authError, setAuthError] = useState('');
const [worksheetError, setWorksheetError] = useState('');
const pollTimeoutRef = useRef<number | null>(null);
@@ -62,6 +64,7 @@ export default function GSheetSetup(props: GSheetSetupProps) {
const loadWorksheetOptions = useCallback(
async (nextSheetId: string) => {
const worksheetOptions = await getWorksheetOptions(nextSheetId);
persistSheetId(nextSheetId);
onSheetLoaded(nextSheetId, worksheetOptions);
setWorksheetError('');
},
@@ -71,6 +74,7 @@ export default function GSheetSetup(props: GSheetSetupProps) {
const pollUntilAuthenticated = useCallback(
async (attempts: number = 0) => {
clearPollTimeout();
if (closedByUser) return;
try {
const result = await verifyAuthenticationStatus();
@@ -82,13 +86,16 @@ export default function GSheetSetup(props: GSheetSetupProps) {
pollTimeoutRef.current = window.setTimeout(() => {
pollUntilAuthenticated(attempts + 1);
}, 2000);
} else {
setLoading('');
return; // Keep authKey for next poll
}
// Polling timed out
setAuthKey(null);
setLoading('');
return;
}
if (result.authenticated === 'authenticated') {
if (result.authenticated === 'authenticated' && result.sheetId) {
setLoading('load-sheet');
try {
await loadWorksheetOptions(result.sheetId);
} catch (error) {
@@ -96,13 +103,15 @@ export default function GSheetSetup(props: GSheetSetupProps) {
}
}
setAuthKey(null);
setLoading('');
} catch (error) {
setAuthError(maybeAxiosError(error));
setAuthKey(null);
setLoading('');
}
},
[clearPollTimeout, loadWorksheetOptions],
[clearPollTimeout, loadWorksheetOptions, closedByUser],
);
/** check if the current session has been authenticated */
@@ -150,7 +159,7 @@ export default function GSheetSetup(props: GSheetSetupProps) {
};
/**
* Requests connection to google auth
* Requests a device code from Google. The user can copy it before opening the browser.
*/
const handleConnect = async () => {
if (!file) return;
@@ -171,7 +180,7 @@ export default function GSheetSetup(props: GSheetSetupProps) {
};
/**
* Open google auth
* Opens the Google verification page and starts polling for completion.
*/
const handleAuthenticate = () => {
setLoading('authenticate');
@@ -180,7 +189,6 @@ export default function GSheetSetup(props: GSheetSetupProps) {
clearPollTimeout();
clearAuthFallbackTimeout();
// open link and schedule a check for when the user focuses again
openLink(authLink);
authFallbackTimeoutRef.current = window.setTimeout(() => {
if (document.hasFocus()) {
@@ -269,18 +277,19 @@ export default function GSheetSetup(props: GSheetSetupProps) {
</Panel.ListGroup>
)}
<Panel.ListGroup className={style.setupBlock}>
<Panel.Description>Enter ID of sheet to synchronise</Panel.Description>
<Panel.Description>Enter ID of sheet to synchronize</Panel.Description>
<Panel.Error>{worksheetError}</Panel.Error>
<Input
fluid
value={sheetId}
placeholder='Sheet ID'
placeholder='Sheet ID or Google Sheets URL'
onChange={(event) => {
setWorksheetError('');
setSheetId(event.target.value);
setSheetId(extractSheetId(event.target.value));
}}
disabled={isLoading || canAuthenticate}
/>
<div className={style.setupHint}>Paste a Google Sheets URL or the sheet ID from the URL bar.</div>
</Panel.ListGroup>
{isAuthenticated ? (
<Panel.ListGroup className={style.setupBlock}>
@@ -304,18 +313,16 @@ export default function GSheetSetup(props: GSheetSetupProps) {
</Panel.ListGroup>
) : (
<Panel.ListGroup className={style.setupBlock}>
<Panel.Description>Authenticate this Ontime session with Google</Panel.Description>
<Panel.Description>Copy the device code, then authenticate with Google</Panel.Description>
<Panel.InlineElements wrap='wrap' className={style.setupActions}>
{isAuthenticating && <span>Authenticating...</span>}
<CopyTag copyValue={authKey ?? ''} disabled={!canAuthenticate}>
{authKey ? authKey : 'Upload files to generate Auth Key'}
</CopyTag>
<Button onClick={handleAuthenticate} disabled={!canAuthenticate}>
<CopyTag copyValue={authKey ?? ''}>{authKey}</CopyTag>
<Button onClick={handleAuthenticate} disabled={isLoading} loading={loading === 'authenticate'}>
<IoShieldCheckmarkOutline />
Authenticate
</Button>
</Panel.InlineElements>
<div className={style.setupHint}>Open the browser prompt, complete the code flow, then come back here.</div>
<div className={style.setupHint}>Copy the code, then open the browser prompt to complete the flow.</div>
</Panel.ListGroup>
)}
</Panel.Section>
@@ -37,12 +37,15 @@ type ActiveSource =
kind: 'excel';
worksheetNames: string[];
initialWorksheetMetadata: SpreadsheetWorksheetMetadata | null;
closedByUser: boolean;
}
| {
kind: 'gsheet';
sheetId: string;
worksheetNames: string[];
initialWorksheetMetadata: SpreadsheetWorksheetMetadata | null;
title: string;
closedByUser: boolean;
};
export default function SourcesPanel() {
@@ -73,6 +76,7 @@ export default function SourcesPanel() {
kind: 'excel',
worksheetNames: worksheetOptions.worksheets,
initialWorksheetMetadata: worksheetOptions.metadata,
closedByUser: false,
});
setImportFlow('excel');
setHasFile('done');
@@ -106,7 +110,13 @@ export default function SourcesPanel() {
};
const cancelImportFlow = () => {
resetFlow();
if (activeSource && activeSource.kind === 'gsheet') {
// Return to GSheetSetup so the user can change the sheet ID or revoke auth
setActiveSource({ ...activeSource, closedByUser: true });
setError('');
} else {
resetFlow();
}
};
const handleFinished = () => {
@@ -177,15 +187,22 @@ export default function SourcesPanel() {
sheetId,
worksheetNames: worksheetOptions.worksheets,
initialWorksheetMetadata: worksheetOptions.metadata,
title: worksheetOptions.title ?? '',
closedByUser: false,
});
}, []);
const closedByUser = activeSource?.closedByUser ?? false;
const isGSheetFlow = importFlow === 'gsheet';
const showInput = importFlow === 'none';
const showCompleted = importFlow === 'finished';
const showAuth = isGSheetFlow && activeSource === null;
const showImportWorkspace = activeSource !== null;
const importModalTitle = activeSource?.kind === 'excel' ? 'Import spreadsheet' : 'Synchronise with Google Sheet';
const showImportWorkspace = activeSource !== null && !closedByUser;
const importModalTitle = (() => {
if (!activeSource) return '';
if (activeSource.kind === 'excel') return 'Import spreadsheet';
return activeSource.title ? `Sync: ${activeSource.title}` : 'Synchronize with Google Sheet';
})();
const sourceKey = (() => {
if (!activeSource) return null;
if (activeSource.kind === 'excel') return 'excel';
@@ -195,7 +212,7 @@ export default function SourcesPanel() {
return (
<Panel.Section>
<Panel.Card>
<Panel.SubHeader>Synchronise your rundown with an external source</Panel.SubHeader>
<Panel.SubHeader>Synchronize your rundown with an external source</Panel.SubHeader>
{error && <Panel.Error>{error}</Panel.Error>}
{showInput && (
<div className={style.introStack}>
@@ -233,7 +250,7 @@ export default function SourcesPanel() {
</section>
<section className={style.sourceCard}>
<div className={style.sourceHeader}>
<h4 className={style.sourceTitle}>Synchronise with Google</h4>
<h4 className={style.sourceTitle}>Synchronize with Google</h4>
</div>
<p className={style.sourceDescription}>
Connect a Google account once, then load any sheet by ID and keep the import flow inside Ontime.
@@ -241,7 +258,7 @@ export default function SourcesPanel() {
<div className={style.sourceMeta}>Requires Google OAuth client credentials</div>
<Button variant='primary' size='large' fluid onClick={openGSheetFlow} disabled={hasFile !== 'none'}>
<IoCloudOutline />
Synchronise with Google
Synchronize with Google
</Button>
</section>
</div>
@@ -257,7 +274,9 @@ export default function SourcesPanel() {
</Button>
</div>
)}
{showAuth && <GSheetSetup onCancel={cancelGSheetFlow} onSheetLoaded={handleSheetLoaded} />}
{isGSheetFlow && (
<GSheetSetup onCancel={cancelGSheetFlow} onSheetLoaded={handleSheetLoaded} closedByUser={closedByUser} />
)}
<Modal
isOpen={showImportWorkspace}
title={importModalTitle}
@@ -0,0 +1,45 @@
import { describe, expect, it } from 'vitest';
import { extractSheetId } from '../gsheetUtils';
describe('extractSheetId()', () => {
it('extracts the ID from a full Google Sheets URL', () => {
const url = 'https://docs.google.com/spreadsheets/d/1aBcDeFgHiJkLmNoPqRsTuVwXyZ/edit#gid=0';
expect(extractSheetId(url)).toBe('1aBcDeFgHiJkLmNoPqRsTuVwXyZ');
});
it('extracts the ID when the URL has no trailing path', () => {
expect(extractSheetId('https://docs.google.com/spreadsheets/d/abc123')).toBe('abc123');
});
it('handles IDs with hyphens and underscores', () => {
const url = 'https://docs.google.com/spreadsheets/d/1a-B_c2/edit';
expect(extractSheetId(url)).toBe('1a-B_c2');
});
it('returns a raw sheet ID unchanged', () => {
expect(extractSheetId('1aBcDeFgHiJkLmNoPqRsTuVwXyZ')).toBe('1aBcDeFgHiJkLmNoPqRsTuVwXyZ');
});
it('trims whitespace from the input', () => {
expect(extractSheetId(' 1aBcDeFg ')).toBe('1aBcDeFg');
});
it('trims whitespace from a pasted URL', () => {
const url = ' https://docs.google.com/spreadsheets/d/1aBcDeFg/edit ';
expect(extractSheetId(url)).toBe('1aBcDeFg');
});
it('strips query params from a raw ID', () => {
expect(extractSheetId('1aBcDeFgHiJkLmNoPqRsTuVwXyZ?edit=1')).toBe('1aBcDeFgHiJkLmNoPqRsTuVwXyZ');
});
it('strips fragment from a raw ID', () => {
expect(extractSheetId('1aBcDeFg#gid=0')).toBe('1aBcDeFg');
});
it('returns empty string for empty input', () => {
expect(extractSheetId('')).toBe('');
expect(extractSheetId(' ')).toBe('');
});
});
@@ -0,0 +1,29 @@
import { makeStageKey } from '../../../../../common/utils/localStorage';
// Matches the document ID segment from a Google Sheets URL:
// https://docs.google.com/spreadsheets/d/{ID}/edit#gid=0
// ^^^^ captured group
// IDs consist of alphanumeric chars, hyphens, and underscores.
const sheetIdPattern = /\/spreadsheets\/d\/([a-zA-Z0-9_-]+)/;
const lastSheetIdKey = makeStageKey('gsheet:lastSheetId');
/**
* Extracts a Google Sheet ID from a full URL, or returns the raw input if it doesn't match.
*/
export function extractSheetId(input: string): string {
const trimmed = input.trim();
const match = trimmed.match(sheetIdPattern);
if (match) return match[1];
// Strip query params (?...) and fragments (#...) in case the user
// pasted a partial URL or an ID with trailing junk like "abc123?edit=1"
return trimmed.split(/[?#]/)[0];
}
export function getPersistedSheetId(): string {
return localStorage.getItem(lastSheetIdKey) ?? '';
}
export function persistSheetId(sheetId: string) {
localStorage.setItem(lastSheetIdKey, sheetId);
}
@@ -16,7 +16,7 @@ export function getWarningText(warning: MappingWarning): string {
case 'invalid-name':
return 'Column cannot be converted into an Ontime field name';
case 'name-collision':
return 'Column name resolves to a duplicate column';
return 'Column name resolves to a duplicate Ontime field';
default:
return '';
}
@@ -50,7 +50,7 @@ export default function SheetImportMappingPane({
return (
<section className={style.mappingPane}>
<Panel.InlineElements align='apart' className={style.mappingPaneHeader}>
<span className={style.mappingPaneTitle}>Fields</span>
<span className={style.mappingPaneTitle}>Column mapping</span>
<Panel.InlineElements relation='inner' className={style.mappingPaneActions}>
<Button className={style.addColumnTrigger} onClick={handleAddCustomField} disabled={isBusy}>
<IoAdd />
@@ -58,7 +58,7 @@ describe('getImportWarnings()', () => {
expect(warnings['custom.0.importName']).toStrictEqual({ kind: 'invalid-name' });
});
it('warns when a custom header resolves to an existing Ontime field name', () => {
it('warns when a custom header resolves to a built-in or duplicate imported field name', () => {
const values = createDefaultFormValues();
values.custom = [
{ ontimeName: '', importName: 'Artist-1' },
@@ -67,12 +67,20 @@ describe('getImportWarnings()', () => {
{ ontimeName: '', importName: 'Presenter-1' },
];
const warnings = getImportWarnings(values, ['Artist-1', 'Artist/1', 'Title!', 'Presenter-1'], ['Presenter 1']);
const warnings = getImportWarnings(values, ['Artist-1', 'Artist/1', 'Title!', 'Presenter-1']);
expect(warnings['custom.0.importName']).toBeUndefined();
expect(warnings['custom.1.importName']).toStrictEqual({ kind: 'name-collision' });
expect(warnings['custom.2.importName']).toStrictEqual({ kind: 'name-collision' });
expect(warnings['custom.3.importName']).toStrictEqual({ kind: 'name-collision' });
expect(warnings['custom.3.importName']).toBeUndefined();
});
it('does not give false warnings for pre-existing custom fields', () => {
const values = createDefaultFormValues();
values.custom = [{ ontimeName: '', importName: 'Video Info' }];
const warnings = getImportWarnings(values, ['Video Info']);
expect(warnings['custom.0.importName']).toBeUndefined();
});
});
@@ -127,26 +127,26 @@ function isPersistedFormValues(obj: unknown): obj is ImportFormValues {
);
}
export function getPersistedImportState(sourceKey: string): ImportFormValues {
export function getPersistedImportState(sourceKey: string): { values: ImportFormValues; isPersisted: boolean } {
const storageKey = getImportMapKey(sourceKey);
try {
const raw = localStorage.getItem(storageKey);
if (!raw) {
return createDefaultFormValues();
return { values: createDefaultFormValues(), isPersisted: false };
}
const parsed: unknown = JSON.parse(raw);
if (isPersistedFormValues(parsed)) {
return parsed;
return { values: parsed, isPersisted: true };
}
// Invalid schema - delete malformed data
localStorage.removeItem(storageKey);
return createDefaultFormValues();
return { values: createDefaultFormValues(), isPersisted: false };
} catch {
// Parse error - delete corrupted data
localStorage.removeItem(storageKey);
return createDefaultFormValues();
return { values: createDefaultFormValues(), isPersisted: false };
}
}
@@ -156,11 +156,9 @@ export function getPersistedImportState(sourceKey: string): ImportFormValues {
export function getImportWarnings(
values: ImportFormValues,
detectedSpreadsheetColumns: string[],
existingCustomFieldLabels: string[] = [],
): Record<string, MappingWarning | undefined> {
const normalisedHeaders = new Set(detectedSpreadsheetColumns.map(normaliseColumnName).filter(Boolean));
const builtInLabels = new Set(builtInFieldDefs.map((def) => def.label.toLowerCase()));
const existingLabels = new Set(existingCustomFieldLabels.map((label) => label.trim().toLowerCase()).filter(Boolean));
const seenColumns = new Set<string>();
const seenDerivedLabels = new Set<string>();
const warnings: Record<string, MappingWarning | undefined> = {};
@@ -200,11 +198,7 @@ export function getImportWarnings(
warnings[key] = { kind: 'missing' };
} else {
const normalisedDerivedLabel = sanitisedLabel.toLowerCase();
if (
builtInLabels.has(normalisedDerivedLabel) ||
existingLabels.has(normalisedDerivedLabel) ||
seenDerivedLabels.has(normalisedDerivedLabel)
) {
if (builtInLabels.has(normalisedDerivedLabel) || seenDerivedLabels.has(normalisedDerivedLabel)) {
warnings[key] = { kind: 'name-collision' };
}
}
@@ -5,7 +5,6 @@ import { useCallback, useEffect, useMemo, useReducer, useRef } from 'react';
import { useFieldArray, useForm } from 'react-hook-form';
import { maybeAxiosError } from '../../../../../../common/api/utils';
import useCustomFields from '../../../../../../common/hooks-query/useCustomFields';
import { formatDuration } from '../../../../../../common/utils/time';
import {
type ImportFormValues,
@@ -118,10 +117,15 @@ export function useSheetImportForm({
onApply,
onExport,
}: UseSheetImportFormProps) {
const initialFormValues = useMemo(() => {
const persisted = getPersistedImportState(sourceKey);
const worksheet = getPreferredWorksheet(worksheetNames, persisted.worksheet, initialMetadata?.worksheet ?? '');
return { ...persisted, worksheet };
const { initialFormValues, skipAutoPreview } = useMemo(() => {
const { values, isPersisted } = getPersistedImportState(sourceKey);
const worksheet = getPreferredWorksheet(worksheetNames, values.worksheet, initialMetadata?.worksheet ?? '');
// Skip auto-preview when restoring a persisted mapping that was saved for a different worksheet.
const hasWorksheetChanged = values.worksheet !== worksheet;
return {
initialFormValues: { ...values, worksheet },
skipAutoPreview: isPersisted && hasWorksheetChanged,
};
}, [initialMetadata?.worksheet, sourceKey, worksheetNames]);
const {
@@ -139,7 +143,6 @@ export function useSheetImportForm({
const { fields, append, remove } = useFieldArray({ control, name: 'custom' });
const values = watch();
const { data: existingCustomFields } = useCustomFields();
// --- Worksheet metadata via react-query ---
const queryClient = useQueryClient();
@@ -167,16 +170,14 @@ export function useSheetImportForm({
const columnLabels = buildColumnLabels(values);
const [state, dispatch] = useReducer(importReducer, initialImportState);
const existingCustomFieldLabels = useMemo(
() => Object.values(existingCustomFields).map((field) => field.label),
[existingCustomFields],
);
const warnings = getImportWarnings(values, headers, existingCustomFieldLabels);
const warnings = getImportWarnings(values, headers);
const warningCount = Object.values(warnings).filter(Boolean).length;
const previewRef = useRef<SpreadsheetPreviewResponse | null>(null);
const autoPreviewFiredRef = useRef(false);
// Rehydrate the form from persisted/default state whenever the source context changes.
useEffect(() => {
autoPreviewFiredRef.current = false;
reset(initialFormValues);
dispatch({ type: 'reset' });
}, [initialFormValues, reset]);
@@ -225,6 +226,16 @@ export function useSheetImportForm({
[previewImport],
);
// Auto-preview on mount once metadata is ready.
useEffect(() => {
if (autoPreviewFiredRef.current) return;
if (skipAutoPreview) return;
if (!isValid || headers.length === 0) return;
autoPreviewFiredRef.current = true;
handleSubmit(handlePreview)();
}, [skipAutoPreview, isValid, headers, handleSubmit, handlePreview]);
const handleApply = useCallback(async () => {
if (!state.preview) return;
@@ -223,10 +223,15 @@ export function hasAuth(): { authenticated: AuthenticationStatus; sheetId: strin
return { authenticated: currentAuthClient ? 'authenticated' : 'not_authenticated', sheetId: currentSheetId };
}
type VerifySheetResult = {
worksheets: string[];
title: string;
};
/**
* Validates that a spreadsheet exists and returns its worksheet titles without reading cell data.
*/
async function verifySheet(sheetId = currentSheetId, authClient = currentAuthClient): Promise<string[]> {
async function verifySheet(sheetId = currentSheetId, authClient = currentAuthClient): Promise<VerifySheetResult> {
if (!sheetId || !authClient) {
throw new Error('Missing sheet ID or authentication');
}
@@ -247,7 +252,9 @@ async function verifySheet(sheetId = currentSheetId, authClient = currentAuthCli
if (worksheets.length === 0) {
throw new Error('No worksheets found');
}
return worksheets;
const title = spreadsheets.data.properties?.title ?? '';
return { worksheets, title };
} catch (error) {
// attempt to catch errors caused by importing xlsx
catchCommonImportXlsxError(error);
@@ -287,17 +294,18 @@ export async function handleInitialConnection(
*/
export async function getWorksheetOptions(
sheetId: string,
): Promise<{ worksheets: string[]; metadata: SpreadsheetWorksheetMetadata | null }> {
): Promise<{ worksheets: string[]; metadata: SpreadsheetWorksheetMetadata | null; title: string }> {
if (!currentAuthClient) {
throw new Error('Not authenticated');
}
currentSheetId = sheetId;
const worksheets = await verifySheet(sheetId);
const { worksheets, title } = await verifySheet(sheetId);
return {
worksheets,
metadata: null,
title,
};
}
@@ -16,7 +16,7 @@ test('imports spreadsheet and applies imported rundown to editor', async ({ page
await page.getByRole('button', { name: 'Toggle settings' }).click();
await page.getByRole('button', { name: 'Project settings' }).click();
await page.getByRole('button', { name: 'Import spreadsheet' }).first().click();
await expect(page.getByText('Synchronise your rundown with an external source')).toBeVisible();
await expect(page.getByText('Synchronize your rundown with an external source')).toBeVisible();
// upload the spreadsheet
const fileChooserPromise = page.waitForEvent('filechooser');
@@ -10,6 +10,7 @@ export type SpreadsheetWorksheetMetadata = {
export type SpreadsheetWorksheetOptions = {
worksheets: string[];
metadata: SpreadsheetWorksheetMetadata | null;
title?: string;
};
export type SpreadsheetPreviewResponse = {