Skip to content

[NoQA] [HR Import] Rename and parameterize the sync results modal #91285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
grgia merged 6 commits into from
May 21, 2026
Merged

[NoQA] [HR Import] Rename and parameterize the sync results modal #91285

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
13b81ae
[HR Import] Rename and parameterize the sync results modal
mhawryluk May 21, 2026
be8e00d
Fix modal close animation
mhawryluk May 21, 2026
ce001bb
Implement review suggestions
mhawryluk May 21, 2026
27c8d45
Implement AI reviewer's suggestion
mhawryluk May 21, 2026
8973c5e
Remove unnecessary comment
mhawryluk May 21, 2026
0eecf30
Move providerDisplayName to HRSyncResultsModal
mhawryluk May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 28 additions & 21 deletions src/components/GustoSyncResultsModal.tsx → src/components/HRSyncResultsModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {useMemoizedLazyExpensifyIcons, useMemoizedLazyIllustrations} from '@hook
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import type {GustoSyncResult} from '@libs/API/GustoSyncResult';
import type {HrSyncResult} from '@libs/API/HrSyncResult';
import CONST from '@src/CONST';
import Button from './Button';
import FixedFooter from './FixedFooter';
Expand All @@ -16,46 +16,53 @@ import PressableWithoutFeedback from './Pressable/PressableWithoutFeedback';
import ScrollView from './ScrollView';
import Text from './Text';

type GustoSyncResultsModalProps = ModalProps & {
/** Sync result returned by the completed Gusto sync job */
result: GustoSyncResult;
type HRSyncResultsModalProps = ModalProps & {
/** Sync result returned by the completed HR sync job */
Comment thread
mhawryluk marked this conversation as resolved.
result: HrSyncResult;

/** Human-readable display name for the HR provider (e.g. "Gusto") */
providerDisplayName: string;
};

function GustoSyncResultsModal({result, closeModal}: GustoSyncResultsModalProps) {
function HRSyncResultsModal({result, providerDisplayName, closeModal}: HRSyncResultsModalProps) {
const {translate} = useLocalize();
const theme = useTheme();
const styles = useThemeStyles();
const icons = useMemoizedLazyExpensifyIcons(['DownArrow']);
const illustrations = useMemoizedLazyIllustrations(['SyncUsers']);
const [isSkippedSectionExpanded, setIsSkippedSectionExpanded] = useState(false);
const [isVisible, setIsVisible] = useState(true);

const addedCount = result.addedEmployeesCount ?? 0;
const removedCount = result.removedEmployeesCount ?? 0;
const skippedCount = result.skippedEmployees?.length ?? 0;
const closeResultsModal = () => closeModal();

// Starts the exit animation; closeModal (passed via onModalHide) is called once the animation finishes.
Comment thread
mhawryluk marked this conversation as resolved.
Outdated
const hideModal = () => setIsVisible(false);
Comment thread
mhawryluk marked this conversation as resolved.

const renderResultSummary = (label: string, count: number) => (
<View style={[styles.mb6]}>
<Text style={[styles.textSupporting, styles.mb1]}>{label}</Text>
<Text style={[styles.textNormalThemeText, styles.textStrong]}>{translate('workspace.hr.gusto.syncResults.employeeCount', {count})}</Text>
<Text style={[styles.textNormalThemeText, styles.textStrong]}>{translate('workspace.hr.syncResults.employeeCount', {count})}</Text>
</View>
);

return (
<Modal
type={CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED}
isVisible
onClose={closeResultsModal}
isVisible={isVisible}
onClose={hideModal}
onModalHide={closeModal}
shouldHandleNavigationBack
enableEdgeToEdgeBottomSafeAreaPadding
>
<View
testID="GustoSyncResultsModal"
testID="HRSyncResultsModal"
style={[styles.flex1, styles.appBG]}
>
<HeaderWithBackButton
title={translate('workspace.hr.gusto.syncResults.title')}
onBackButtonPress={closeResultsModal}
title={translate('workspace.hr.syncResults.title', providerDisplayName)}
onBackButtonPress={hideModal}
/>
<ScrollView contentContainerStyle={[styles.flexGrow1, styles.ph5, styles.pb8]}>
<View style={[styles.alignItemsCenter, styles.mt4, styles.mb4, styles.pRelative]}>
Expand All @@ -65,19 +72,19 @@ function GustoSyncResultsModal({result, closeModal}: GustoSyncResultsModalProps)
height={68}
/>
</View>
<Text style={[styles.textHeadlineH1, styles.mb8]}>{translate('workspace.hr.gusto.syncResults.successTitle')}</Text>
{renderResultSummary(translate('workspace.hr.gusto.syncResults.added'), addedCount)}
{renderResultSummary(translate('workspace.hr.gusto.syncResults.removed'), removedCount)}
<Text style={[styles.textHeadlineH1, styles.mb8]}>{translate('workspace.hr.syncResults.successTitle', providerDisplayName)}</Text>
Comment thread
mhawryluk marked this conversation as resolved.
{renderResultSummary(translate('workspace.hr.syncResults.added'), addedCount)}
{renderResultSummary(translate('workspace.hr.syncResults.removed'), removedCount)}
<PressableWithoutFeedback
accessibilityLabel={translate('workspace.hr.gusto.syncResults.skipped')}
sentryLabel="GustoSyncResultsModal-SkippedEmployees"
accessibilityLabel={translate('workspace.hr.syncResults.skipped')}
sentryLabel="HRSyncResultsModal-SkippedEmployees"
role={CONST.ROLE.BUTTON}
onPress={() => setIsSkippedSectionExpanded((isExpanded) => !isExpanded)}
style={[styles.flexRow, styles.justifyContentBetween, styles.alignItemsCenter]}
>
<View>
<Text style={[styles.textSupporting, styles.mb1]}>{translate('workspace.hr.gusto.syncResults.skipped')}</Text>
<Text style={[styles.textNormalThemeText, styles.textStrong]}>{translate('workspace.hr.gusto.syncResults.employeeCount', {count: skippedCount})}</Text>
<Text style={[styles.textSupporting, styles.mb1]}>{translate('workspace.hr.syncResults.skipped')}</Text>
<Text style={[styles.textNormalThemeText, styles.textStrong]}>{translate('workspace.hr.syncResults.employeeCount', {count: skippedCount})}</Text>
</View>
<Icon
src={icons.DownArrow}
Expand All @@ -101,12 +108,12 @@ function GustoSyncResultsModal({result, closeModal}: GustoSyncResultsModalProps)
large
success
text={translate('common.buttonConfirm')}
onPress={closeResultsModal}
onPress={hideModal}
/>
</FixedFooter>
</View>
</Modal>
);
}

export default GustoSyncResultsModal;
export default HRSyncResultsModal;
45 changes: 0 additions & 45 deletions src/hooks/useGustoSyncResultsModal.ts
View file
Open in desktop

This file was deleted.

61 changes: 61 additions & 0 deletions src/hooks/useHRSyncResultsModal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import {useEffect} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import type {TupleToUnion} from 'type-fest';
import HRSyncResultsModal from '@components/HRSyncResultsModal';
import {useModal} from '@components/Modal/Global/ModalContext';
import {getConnectedHRProvider} from '@libs/PolicyUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import {policyConnectionsSelector} from '@src/selectors/Policy';
import type {PolicyConnectionSyncProgress} from '@src/types/onyx/Policy';
import useOnyx from './useOnyx';
import usePrevious from './usePrevious';

/**
* Watches an HR provider's sync progress and automatically opens the `HRSyncResultsModal`
* when the sync transitions to the `JOB_DONE` stage with a result payload.
*/
function useHRSyncResultsModal(policyID: string, connectionSyncProgress: OnyxEntry<PolicyConnectionSyncProgress>, isFocused: boolean) {
Comment thread
mhawryluk marked this conversation as resolved.
const modal = useModal();
const previousSyncProgress = usePrevious(connectionSyncProgress);
const [policyConnections] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {selector: policyConnectionsSelector});
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ PERF-11 (docs)

The policyConnectionsSelector extracts the entire Connections object (which contains data for QBO, Xero, NetSuite, Sage Intacct, QBD, Certinia, Gusto, Zenefits, and Merge HR) without narrowing. Because the hook uses a selector, Onyx will run deepEqual on this large object on every policy update, which is expensive and yields no re-render savings since most of the data is unused.

The hook only needs to determine which HR provider is connected and its display name. Replace the broad selector with one that computes and returns just the providerDisplayName string (a primitive), so deepEqual is trivially cheap:

const [providerDisplayName] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
    selector: (policy) => {
        const hrProvider = getConnectedHRProvider(policy);
        return hrProvider?.displayName
            ?? CONST.POLICY.CONNECTIONS.NAME_USER_FRIENDLY[connectionName as keyof typeof CONST.POLICY.CONNECTIONS.NAME_USER_FRIENDLY]
            ?? connectionName;
    },
});

Reviewed at: ce001bb | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.


const connectionName = connectionSyncProgress?.connectionName;
const providerDisplayName =
getConnectedHRProvider({connections: policyConnections})?.displayName ??
Copy link
Copy Markdown
Contributor

@grgia grgia May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be in a useeffect?

Copy link
Copy Markdown
Contributor Author

@mhawryluk mhawryluk May 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could. but actually I moved it to the useOnyx selector per AI reviewer's suggestion

CONST.POLICY.CONNECTIONS.NAME_USER_FRIENDLY[connectionName as keyof typeof CONST.POLICY.CONNECTIONS.NAME_USER_FRIENDLY] ??
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Derive provider name from sync event, not connected policy

Use of getConnectedHRProvider({connections: policyConnections}) can label the modal with the wrong provider when connectionSyncProgress.connectionName and current policyConnections diverge (for example during reconnect/disconnect transitions or stale Onyx updates). In that case a Zenefits/Merge sync completion can render a Gusto title because getConnectedHRProvider picks the first connected provider, ignoring the sync event's connectionName; the modal title should be derived from the sync event connection being completed.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

@mhawryluk mhawryluk May 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it would be better to have the merge hr integration slug in policyConnections, so that we ensure there is no mismatch, but I'm not sure if the backend will send it, and we can get that information from policy data, so that is what we do. and I don't think the mismatch is plausible in this scenario and having consistent Onyx data between policy and policyConnections is enough to make the modal render correct data

connectionName;

useEffect(() => {
const syncResult = connectionSyncProgress?.result;
const isHRSyncDoneWithResult =
CONST.POLICY.CONNECTIONS.HR_CONNECTION_NAMES.includes(connectionName as TupleToUnion<typeof CONST.POLICY.CONNECTIONS.HR_CONNECTION_NAMES>) &&
connectionSyncProgress?.stageInProgress === CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.JOB_DONE &&
!!syncResult;
const didTransitionToJobDone = previousSyncProgress?.connectionName === connectionName && previousSyncProgress?.stageInProgress !== CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.JOB_DONE;
const didHRSyncComplete = isFocused && isHRSyncDoneWithResult && didTransitionToJobDone;

if (!didHRSyncComplete || !syncResult || !connectionName) {
return;
}

modal.showModal({
component: HRSyncResultsModal,
props: {result: syncResult, providerDisplayName},
id: `${connectionName}-sync-results-${policyID}`,
});
}, [
connectionName,
connectionSyncProgress?.result,
connectionSyncProgress?.stageInProgress,
connectionSyncProgress?.timestamp,
isFocused,
providerDisplayName,
policyID,
previousSyncProgress?.connectionName,
previousSyncProgress?.stageInProgress,
modal,
]);
}

export default useHRSyncResultsModal;
22 changes: 11 additions & 11 deletions src/languages/de.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7189,19 +7189,19 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
}
}
},
syncResults: {
title: (provider: string) => `${provider}-Synchronisierung abgeschlossen`,
successTitle: (provider: string) => `Ihre ${provider}-Verbindung wurde erfolgreich synchronisiert!`,
added: 'Hinzugefügt',
removed: 'Entfernt',
skipped: 'Übersprungen',
employeeCount: () => ({
one: '1 Mitarbeiter',
other: (count: number) => `${count} Mitarbeitende`,
}),
},
gusto: {
title: 'Gusto',
syncResults: {
title: 'Gusto-Synchronisierungsergebnisse',
successTitle: 'Ihre Gusto-Verbindung wurde erfolgreich synchronisiert!',
added: 'Hinzugefügt',
removed: 'Entfernt',
skipped: 'Übersprungen',
employeeCount: () => ({
one: '1 Mitarbeiter',
other: (count: number) => `${count} Mitarbeitende`,
}),
},
},
zenefits: {
title: 'TriNet',
Expand Down
22 changes: 11 additions & 11 deletions src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6493,19 +6493,19 @@ const translations = {
}
}
},
syncResults: {
title: (provider: string) => `${provider} sync complete`,
successTitle: (provider: string) => `Successfully synced your ${provider} connection!`,
added: 'Added',
removed: 'Removed',
skipped: 'Skipped',
employeeCount: () => ({
one: '1 employee',
other: (count: number) => `${count} employees`,
}),
},
gusto: {
title: 'Gusto',
syncResults: {
title: 'Gusto sync results',
successTitle: 'Successfully synced your Gusto connection!',
added: 'Added',
removed: 'Removed',
skipped: 'Skipped',
employeeCount: () => ({
one: '1 employee',
other: (count: number) => `${count} employees`,
}),
},
},
zenefits: {
title: 'TriNet',
Expand Down
22 changes: 11 additions & 11 deletions src/languages/es.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6306,19 +6306,19 @@ ${amount} para ${merchant} - ${date}`,
}
}
},
syncResults: {
title: (provider: string) => `Sincronización de ${provider} completada`,
successTitle: (provider: string) => `¡Se sincronizó correctamente tu conexión de ${provider}!`,
added: 'Añadido',
removed: 'Eliminado',
skipped: 'Omitido',
employeeCount: () => ({
one: '1 empleado',
other: (count: number) => `${count} empleados`,
}),
},
gusto: {
title: 'Gusto',
syncResults: {
title: 'Resultados de la sincronización de Gusto',
successTitle: '¡Se sincronizó correctamente tu conexión con Gusto!',
added: 'Añadido',
removed: 'Eliminado',
skipped: 'Omitido',
employeeCount: () => ({
one: '1 empleado',
other: (count: number) => `${count} empleados`,
}),
},
},
zenefits: {
title: 'TriNet',
Expand Down
22 changes: 11 additions & 11 deletions src/languages/fr.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7218,19 +7218,19 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
}
}
},
syncResults: {
title: (provider: string) => `Synchronisation ${provider} terminée`,
Comment thread
mhawryluk marked this conversation as resolved.
successTitle: (provider: string) => `Connexion ${provider} synchronisée avec succès !`,
added: 'Ajouté',
removed: 'Supprimé',
skipped: 'Ignoré',
employeeCount: () => ({
one: '1 employé',
other: (count: number) => `${count} employés`,
}),
},
gusto: {
title: 'Gusto',
syncResults: {
title: 'Résultats de la synchronisation Gusto',
successTitle: 'Connexion Gusto synchronisée avec succès !',
added: 'Ajouté',
removed: 'Supprimé',
skipped: 'Ignoré',
employeeCount: () => ({
one: '1 employé',
other: (count: number) => `${count} employés`,
}),
},
},
zenefits: {
title: 'TriNet',
Expand Down
22 changes: 11 additions & 11 deletions src/languages/it.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7176,19 +7176,19 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`,
}
}
},
syncResults: {
title: (provider: string) => `Sincronizzazione ${provider} completata`,
successTitle: (provider: string) => `Connessione ${provider} sincronizzata correttamente!`,
added: 'Aggiunto',
removed: 'Rimosso',
skipped: 'Saltato',
employeeCount: () => ({
one: '1 dipendente',
other: (count: number) => `${count} dipendenti`,
}),
},
gusto: {
title: 'Gusto',
syncResults: {
title: 'Risultati sincronizzazione Gusto',
successTitle: 'Connessione a Gusto sincronizzata con successo!',
added: 'Aggiunto',
removed: 'Rimosso',
skipped: 'Saltato',
employeeCount: () => ({
one: '1 dipendente',
other: (count: number) => `${count} dipendenti`,
}),
},
},
zenefits: {
title: 'TriNet',
Expand Down
Loading
Loading