[HR Import] Handle Merge daily sync limit error with popup#94889
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
mhawryluk
changed the title
mhawryluk
changed the title
jmusial
left a comment
jmusial
left a comment
There was a problem hiding this comment.
Overall looks good, couple comments.
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index c45021fe94f..8db77da07c9 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -7705,6 +7705,7 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>Verbunden. ${setupLink ? `<a href="${setupLink}">Einrichtung abschließen</a>` : 'Einrichtung abschließen'} zum Importieren von Mitarbeitenden.</muted-text-label>`,
groups: {title: 'Gruppen', description: 'Wählen Sie die Mitarbeitergruppen aus, die Sie mit diesem Workspace synchronisieren möchten'},
+ syncLimitReached: {title: 'Versuchen Sie es morgen noch einmal', prompt: 'Sie haben Ihr Synchronisierungslimit für heute erreicht.'},
},
},
emptyDomain: {
diff --git a/src/languages/es.ts b/src/languages/es.ts
index a1d8c166ea6..40f20dee9e0 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -6647,6 +6647,7 @@ ${amount} para ${merchant} - ${date}`,
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>Conectado. ${setupLink ? `<a href="${setupLink}">Completa la configuración</a>` : 'Completar configuración'} para importar empleados.</muted-text-label>`,
groups: {title: 'Grupos', description: 'Elige los grupos de empleados que te gustaría sincronizar con este espacio de trabajo'},
+ syncLimitReached: {title: 'Inténtalo de nuevo mañana', prompt: 'Has alcanzado tu límite de sincronización de hoy.'},
},
},
export: {
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index b6eded4cb85..6c7091e7109 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -7738,6 +7738,7 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>Connecté. ${setupLink ? `<a href="${setupLink}">Terminer la configuration</a>` : 'Terminer la configuration'} pour importer les employés.</muted-text-label>`,
groups: {title: 'Groupes', description: 'Choisissez les groupes d’employés que vous souhaitez synchroniser avec cet espace de travail'},
+ syncLimitReached: {title: 'Réessayez demain', prompt: "Vous avez atteint votre limite de synchronisation pour aujourd'hui."},
},
},
emptyDomain: {
diff --git a/src/languages/it.ts b/src/languages/it.ts
index efd84c1b63e..0e18e0e31ac 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -7690,6 +7690,7 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`,
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>Connesso. ${setupLink ? `<a href="${setupLink}">Completa la configurazione</a>` : 'Completa configurazione'} per importare i dipendenti.</muted-text-label>`,
groups: {title: 'Gruppi', description: 'Scegli i gruppi di dipendenti che vuoi sincronizzare con questo spazio di lavoro'},
+ syncLimitReached: {title: 'Riprova domani', prompt: 'Hai raggiunto il limite di sincronizzazioni per oggi.'},
},
},
emptyDomain: {
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index c28af32576a..47e432f33c0 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -7602,6 +7602,7 @@ ${reportName}`,
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>接続されました。従業員をインポートするには ${setupLink ? `<a href="${setupLink}">セットアップを完了</a>` : '設定を完了'} に接続してください。</muted-text-label>`,
groups: {title: 'グループ', description: 'このワークスペースと同期したい従業員グループを選択してください'},
+ syncLimitReached: {title: '明日もう一度お試しください', prompt: '本日の同期上限に達しました。'},
},
},
emptyDomain: {
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index fad31d4c17f..7f015b1798e 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -7668,6 +7668,7 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`,
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>Verbonden. ${setupLink ? `<a href="${setupLink}">Instellen voltooien</a>` : 'Configuratie voltooien'} om werknemers te importeren.</muted-text-label>`,
groups: {title: 'Groepen', description: 'Kies de groepen werknemers die je met deze workspace wilt synchroniseren'},
+ syncLimitReached: {title: 'Probeer het morgen opnieuw', prompt: 'Je hebt je synchronisatielimiet voor vandaag bereikt.'},
},
},
emptyDomain: {title: 'Verbeter je beveiliging met domeinen', subtitle: 'Vereis dat leden op je domein inloggen via single sign-on, beperk het aanmaken van werkruimtes en meer.'},
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 8f423f10c49..a827d2a2282 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -7657,6 +7657,7 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`,
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>Połączono. ${setupLink ? `<a href="${setupLink}">Zakończ konfigurację</a>` : 'Zakończ konfigurację'}, aby zaimportować pracowników.</muted-text-label>`,
groups: {title: 'Grupy', description: 'Wybierz grupy pracowników, które chcesz zsynchronizować z tą przestrzenią roboczą'},
+ syncLimitReached: {title: 'Spróbuj ponownie jutro', prompt: 'Osiągnąłeś dzienny limit synchronizacji.'},
},
},
emptyDomain: {
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 165371fdccc..5dce99214ec 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -7659,6 +7659,7 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`,
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>Conectado. ${setupLink ? `<a href="${setupLink}">Concluir configuração</a>` : 'Concluir configuração'} para importar funcionários.</muted-text-label>`,
groups: {title: 'Grupos', description: 'Escolha os grupos de funcionários que você gostaria de sincronizar com este workspace'},
+ syncLimitReached: {title: 'Tente novamente amanhã', prompt: 'Você atingiu o limite de sincronizações de hoje.'},
},
},
emptyDomain: {
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index f229588b3cc..763c6590851 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -7459,6 +7459,7 @@ ${reportName}`,
setupIncomplete: (setupLink: string | undefined) =>
`<muted-text-label>已连接。${setupLink ? `<a href="${setupLink}">完成设置</a>` : '完成设置'} 用于导入员工。</muted-text-label>`,
groups: {title: '群组', description: '选择要与此工作区同步的员工分组'},
+ syncLimitReached: {title: '请明天再试', prompt: '您已达到今日的同步上限。'},
},
},
emptyDomain: {title: '通过域名提升安全性', subtitle: '要求您域中的成员通过单点登录登录、限制工作区创建等。'},
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@bernhardoj Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
melvin-bot
Bot
requested review from
flaviadefaria
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3d725bf30
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| const windowStart = DateUtils.subtractMillisecondsFromDateTime(DateUtils.getDBTime(), CONST.MERGE_HR.MANUAL_SYNC_WINDOW_MS); | ||
| const syncsWithinWindow = manualSyncTimestamps.filter((timestamp) => timestamp > windowStart).length; |
There was a problem hiding this comment.
When this client does not already have two manualSyncTimestamps cached, this returns false and both sync entry points still call syncConnection; in the documented limit case the backend returns a 429 with empty onyxData, so syncMergeHR just applies its failureData and the user never sees the “try again tomorrow” popup. This also affects cases where another admin exhausts the workspace quota before this policy receives updated timestamps, so the 429 response itself needs to drive the modal rather than relying only on this local list.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
the scenario of calling syncConnection when the limit is reached is a very unlikely corner case. it can happen when some other admin scheduled a sync and then we click "Sync now" before we get the data pushed from backend. in that case we will likely show a generic error, which I think is fine
the error doesn't show up because the |
Hmm, could it be because my account is not on the beta 🤔? I'm currently hard-coding the |
actually, it was because the |
|
I investigated this and checked the backend logs. During that time, the @bernhardoj's sync calls were timing out on the backend and returning 500. Failed tries still count against the daily limit but add no manual sync timestamp, so after 2 tries 429 was received for the rest of the day. That is why Let's try again now, the backend issues should be gone. |
|
There is a generic error now when it fails, but I still get 429 with no onyxData (for manualSyncTimestamps). 
(still time out when I sync) |
I think it's intended, you should be getting |
oh, so the first two manual syncs today timed out and in that case the backend didn't update |
Yes, this is the case I think BE should always return the |
I believe |
|
Thanks for testing @bernhardoj, this was very helpful. Your testing found two real backend bugs:
Both are backend problems, not the frontend. Your change is fine. I will open the backend follow-ups. |
|
Please let me know once the BE is fixed |
|
@bernhardoj the backend is now fixed. Could you please retest? 🙏 |
|
@mhawryluk, now that the beta has been removed, I think we could remove the [No QA] prefix from the title and let the PR take the regular path |
mhawryluk
changed the title
|
Thanks, |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.mp4Android: mWeb Chromeandroid.mweb.mp4iOS: HybridAppios.mp4iOS: mWeb Safariios.mweb.mp4MacOS: Chrome / Safariweb.mp4 |
|
🚧 cristipaval has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@twisterdotcom @bernhardoj @cristipaval Is the Merge HR still under manually configured BETA? We only have 1 account previously setup which had already connected. Or we can now use any Expensifail account? We could potentially use new accounts like these |
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.4.33-0 🚀
|
|
🤖 No help site changes required. This PR adds a 2-per-day manual sync limit for Merge HR connections and shows a "Try again tomorrow" / "You've reached your sync limit for the day." modal when the limit is reached. The Merge HR providers are Workday, BambooHR, and HiBob (
Each article already states, in both the "Sync now" section and an FAQ:
That matches the modal copy shipped in this PR ( Because the help site already reflects this behavior accurately, I did not open a draft docs PR — there's nothing to change. If you'd like the articles to also mention the per-workspace enforcement (one admin hitting the limit blocks the others) or the Members > More > Sync with <provider> entry point, let me know and I'll draft a PR adding that. |
8 participants
Explanation of Change
Displays a confirm modal informing the user they cannot schedule a manual Merge HR synchronization due to reaching a daily limit and doesn't schedule the backend call in that case.
Fixed Issues
$ #91426
PROPOSAL: N/A
Tests
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-07-02.at.11.41.54.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Nagranie.z.ekranu.2026-06-30.o.10.04.11.mov
Nagranie.z.ekranu.2026-07-2.o.11.36.22.mov