[NoQA] [HR Import] Show "Complete Setup" GBR in Workspaces tab icon#93591
Conversation
jmusial
left a comment
jmusial
left a comment
There was a problem hiding this comment.
Couple nits, but overall LGTM 🚀
|
@ZhenjaHorbach 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] |
|
cc: @bernhardoj @ikevin127 (Merge HR project) |
Reviewer Checklist
Screenshots/Videos
|
ikevin127
left a comment
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM
This [NoQA] PR adds a green indicator dot to the Workspaces tab when a policy has Merge HR in "complete setup" state (sync done, groups available, but setup not completed). It also refactors usePolicyIndicatorChecks to cleanly separate error (red) from info (green) policy statuses ✅
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #93445 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
@grgia can probably help review this tomorrow instead @luacmartins |
| const {policyErrorStatus, domainStatus, policyIDWithErrors} = usePolicyIndicatorChecks(); | ||
|
|
||
| const errorStatus = accountStatus ?? policyStatus ?? domainStatus; | ||
| const errorStatus = accountStatus ?? policyErrorStatus ?? domainStatus; |
There was a problem hiding this comment.
Do we no longer need the policyInfoStatus fallback?
There was a problem hiding this comment.
@mhawryluk ill hold merge on this, idk the answer
There was a problem hiding this comment.
useIndicatorStatus is used for the Debug Mode banner in the navigation tab bar. in this PR I only updated useWorkspacesTabIndicatorStatus as this is the one responsible for the dot in the workspace tab icon. so currently the new info status is not included in the debug mode thing. but maybe we should update it for consistency? since we include Account info status there already, though not all statuses are actually presented, cause some lack a configured message in getSettingsMessage. so if we wanted to include "Complete setup" in the debug mode tab view, I would also need a copy for the message, something like "HR integration requires setup" maybe? @grgia @twisterdotcom
There was a problem hiding this comment.
There was a problem hiding this comment.
Implemented the debug mode support. An actual, nonhypothetical screenshot below.
There was a problem hiding this comment.
there is an issue with navigation when clicking the View button from a different workspace to navigate to another one, the left part of the split does not get updated in the wide view. but this is an existing bug reproducable on staging
Nagranie.z.ekranu.2026-06-18.o.17.24.51.mov
Nagranie.z.ekranu.2026-06-18.o.17.15.06.mov
|
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 |
🦜 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 d600b72d4e7..bca80bd2169 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -9555,6 +9555,7 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
theresAProblemWithYourWallet: 'Es gibt ein Problem mit deinem Wallet',
theresAProblemWithYourWalletTerms: 'Es gibt ein Problem mit deinen Wallet-Bedingungen',
aBankAccountIsLocked: 'Ein Bankkonto ist gesperrt',
+ completeHrSetup: 'HR-Einrichtung abschließen',
},
},
emptySearchView: {
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 31e9b699cab..5e73b9358c0 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -9712,6 +9712,7 @@ ${amount} para ${merchant} - ${date}`,
theresAProblemWithYourWallet: 'Hay un problema con tu billetera',
theresAProblemWithYourWalletTerms: 'Hay un problema con los términos de tu billetera',
aBankAccountIsLocked: 'Una cuenta bancaria está bloqueada',
+ completeHrSetup: 'Completa la configuración de RR. HH.',
},
},
emptySearchView: {
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index d764be15a28..931fecb6f06 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -9587,6 +9587,7 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
theresAProblemWithYourWallet: 'Il y a un problème avec votre portefeuille',
theresAProblemWithYourWalletTerms: 'Il y a un problème avec les conditions de votre portefeuille',
aBankAccountIsLocked: 'Un compte bancaire est verrouillé',
+ completeHrSetup: 'Terminer la configuration RH',
},
},
emptySearchView: {
diff --git a/src/languages/it.ts b/src/languages/it.ts
index f1240aee0d7..f91f5d6e65f 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -9543,6 +9543,7 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`,
theresAProblemWithYourWallet: 'Si è verificato un problema con il tuo portafoglio',
theresAProblemWithYourWalletTerms: 'C’è un problema con i termini del tuo portafoglio',
aBankAccountIsLocked: 'Un conto bancario è bloccato',
+ completeHrSetup: 'Completa la configurazione HR',
},
},
emptySearchView: {
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index f5e6745882f..37240f1f219 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -9422,6 +9422,7 @@ ${reportName}`,
theresAProblemWithYourWallet: 'ウォレットに問題があります',
theresAProblemWithYourWalletTerms: 'ウォレットの利用規約に問題があります',
aBankAccountIsLocked: '銀行口座がロックされています',
+ completeHrSetup: '人事設定を完了する',
},
},
emptySearchView: {
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 01764358e9b..e495cac0f2a 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -9509,6 +9509,7 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`,
theresAProblemWithYourWallet: 'Er is een probleem met je wallet',
theresAProblemWithYourWalletTerms: 'Er is een probleem met de voorwaarden van je wallet',
aBankAccountIsLocked: 'Een bankrekening is geblokkeerd',
+ completeHrSetup: 'HR-configuratie voltooien',
},
},
emptySearchView: {
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index aeefb8e21a0..ebddb2a6ac6 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -9495,6 +9495,7 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`,
theresAProblemWithYourWallet: 'Wystąpił problem z Twoim portfelem',
theresAProblemWithYourWalletTerms: 'Wystąpił problem z warunkami Twojego portfela',
aBankAccountIsLocked: 'Konto bankowe jest zablokowane',
+ completeHrSetup: 'Dokończ konfigurację HR',
},
},
emptySearchView: {
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 85a71c0b595..e9bca36936f 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -9499,6 +9499,7 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`,
theresAProblemWithYourWallet: 'Há um problema com sua carteira',
theresAProblemWithYourWalletTerms: 'Há um problema com os termos da sua carteira',
aBankAccountIsLocked: 'Uma conta bancária está bloqueada',
+ completeHrSetup: 'Concluir configuração de RH',
},
},
emptySearchView: {
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 967b91f9938..5c1eb07f667 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -9246,6 +9246,7 @@ ${reportName}`,
theresAProblemWithYourWallet: '您的钱包出现问题',
theresAProblemWithYourWalletTerms: '您的钱包条款存在问题',
aBankAccountIsLocked: '银行账户已锁定',
+ completeHrSetup: '完成人力资源设置',
},
},
emptySearchView: {
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
@codex review |
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.
|
|
@ikevin127 can you please update the checklist to the new template https://raw.githubusercontent.com/Expensify/App/main/contributingGuides/REVIEWER_CHECKLIST.md? |
|
@luacmartins Done, updated the checklist with new screenshots that show the latest updates + dev mode activated. 🟢 LGTM |
|
@ikevin127 it seems like the checklist is still outdated (it has 59 items, instead of the 50 items on the new checklist). Can you update it to this template? https://raw.githubusercontent.com/Expensify/App/main/contributingGuides/REVIEWER_CHECKLIST.md |
|
Merging since @grgia had already approved |
|
🚧 @luacmartins 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.4.16-1 🚀
Bundle Size Analysis (Sentry): |
|
🤖 No help site changes are required. I reviewed the changes in this PR against the help site articles under This is a Why no docs update is needed:
Because no changes are required, I did not open a draft help site PR. @mhawryluk, if you believe a customer-facing help article should exist for the Merge HR connection flow (separate from this indicator change), let me know and I can draft one. |
| "../../tests/unit/usePolicyIndicatorChecksTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 11 | ||
| "../../tests/unit/usePolicyIndicatorChecksTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 14 |
There was a problem hiding this comment.
Please don't increase instances of this. Use partial object types instead.
| "../../tests/unit/useWorkspacesTabIndicatorStatusTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 5 | ||
| "../../tests/unit/useWorkspacesTabIndicatorStatusTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 6 |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.4.16-5 🚀
|
9 participants
Explanation of Change
Shows a green dot in the LHN/tab bar Workspaces button when there is a workspace with Merge HR "complete setup" state and there are no workspaces with errors.
Fixed Issues
$ #93445
PROPOSAL: N/A
Tests
mergeHRConnectionsbeta enabled.Offline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
iOS: mWeb Safari
MacOS: Chrome / Safari