[No QA][HR Import] Generalize Workflows page for any HR connection#91324
[No QA][HR Import] Generalize Workflows page for any HR connection#91324jmusial wants to merge 9 commits into
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 |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@codex review |
![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: 98dc455df8
ℹ️ 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 isLastApprover = index === approvalWorkflow.approvers.length - 1; | ||
| if (isLastApprover) { | ||
| return translate('workflowsPage.finalApprover'); | ||
| } |
There was a problem hiding this comment.
In HR manager mode this unconditionally labels the last row as Final approver, but convertPolicyEmployeesToApprovalWorkflows() only appends a real final approver when mergeConfig.finalApprover is set. During setup/migration states where manager mode is enabled but finalApprover is still null, the chain can end with a manager and this UI will mislabel that manager as the final approver, which is misleading for admins reviewing routing.
Useful? React with 👍 / 👎.
|
@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] |
🦜 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 1c54bf62..6295ac0e 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -2618,7 +2618,7 @@ ${amount} für ${merchant} – ${date}`,
`Genehmigungen werden über deine ${provider}-Integration verwaltet. Um deinen Genehmigungsworkflow zu aktualisieren, gehe zu deinen ${provider}-Verbindungseinstellungen.`,
goToHRSettings: ({provider}: {provider: string}) => `Zu den ${provider}-Einstellungen gehen`,
approverFromProvider: ({provider}: {provider: string}) => `von ${provider}`,
- finalApprover: 'Endgenehmiger',
+ finalApprover: 'Letzte*r Genehmigende*r',
manager: 'Manager',
},
workflowsDelayedSubmissionPage: {
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 4325d6d8..75ef5fcf 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -2442,7 +2442,7 @@ ${amount} para ${merchant} - ${date}`,
goToHRSettings: ({provider}: {provider: string}) => `Ir a la configuración de ${provider}`,
approverFromProvider: ({provider}: {provider: string}) => `de ${provider}`,
finalApprover: 'Aprobador final',
- manager: 'Gerente',
+ manager: 'Responsable',
makeOrTrackPaymentsTitle: 'Realizar o seguir pagos',
makeOrTrackPaymentsDescription: 'Añade un pagador autorizado para los pagos realizados en Expensify o realiza un seguimiento de los pagos realizados en otro lugar.',
customApprovalWorkflowEnabled:
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 85a07208..a49db1c9 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -2622,7 +2622,7 @@ ${amount} pour ${merchant} - ${date}`,
hrApprovalWorkflowLockedPrompt: ({provider}: {provider: string}) =>
`Les validations sont gérées par votre intégration ${provider}. Pour mettre à jour votre workflow de validation, accédez aux paramètres de connexion ${provider}.`,
goToHRSettings: ({provider}: {provider: string}) => `Aller aux paramètres ${provider}`,
- approverFromProvider: ({provider}: {provider: string}) => `de ${provider}`,
+ approverFromProvider: ({provider}: {provider: string}) => `de la part de ${provider}`,
finalApprover: 'Approbateur final',
manager: 'Manager',
},
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 1a323c49..2d1a6233 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -2613,8 +2613,8 @@ ${amount} per ${merchant} - ${date}`,
`Le approvazioni sono gestite dalla tua integrazione con ${provider}. Per aggiornare il flusso di approvazione, vai alle impostazioni di connessione di ${provider}.`,
goToHRSettings: ({provider}: {provider: string}) => `Vai alle impostazioni di ${provider}`,
approverFromProvider: ({provider}: {provider: string}) => `da ${provider}`,
- finalApprover: 'Approvatore finale',
- manager: 'Manager',
+ finalApprover: 'Approvazione finale',
+ manager: 'Responsabile',
},
workflowsDelayedSubmissionPage: {
autoReportingFrequencyErrorMessage: 'Impossibile modificare la frequenza di invio. Riprova oppure contatta l’assistenza.',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 8a8d570c..c7070e68 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -2610,7 +2610,7 @@ ${amount} voor ${merchant} - ${date}`,
`Goedkeuringen worden beheerd door je ${provider}-integratie. Ga naar je ${provider}-verbindingsinstellingen om je goedkeuringsworkflow bij te werken.`,
goToHRSettings: ({provider}: {provider: string}) => `Ga naar ${provider}-instellingen`,
approverFromProvider: ({provider}: {provider: string}) => `van ${provider}`,
- finalApprover: 'Eindgoedkeurder',
+ finalApprover: 'Laatste fiatteur',
manager: 'Manager',
},
workflowsDelayedSubmissionPage: {
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 989b21cb..9e3d09c4 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -2605,9 +2605,9 @@ ${amount} dla ${merchant} - ${date}`,
hrApprovalWorkflowLockedPrompt: ({provider}: {provider: string}) =>
`Zatwierdzanie jest zarządzane przez Twoją integrację z ${provider}. Aby zaktualizować swój proces zatwierdzania, przejdź do ustawień połączenia z ${provider}.`,
goToHRSettings: ({provider}: {provider: string}) => `Przejdź do ustawień ${provider}`,
- approverFromProvider: ({provider}: {provider: string}) => `z ${provider}`,
- finalApprover: 'Końcowy zatwierdzający',
- manager: 'Kierownik',
+ approverFromProvider: ({provider}: {provider: string}) => `od ${provider}`,
+ finalApprover: 'Ostateczny akceptujący',
+ manager: 'Menedżer',
},
workflowsDelayedSubmissionPage: {
autoReportingFrequencyErrorMessage: 'Nie udało się zmienić częstotliwości wysyłania. Spróbuj ponownie lub skontaktuj się z pomocą techniczną.',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01990a9031
ℹ️ 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".
| function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, firstApprover, localeCompare, currentUserLogin}: PolicyConversionParams): PolicyConversionResult { | ||
| const employees = policy?.employeeList ?? {}; | ||
| const defaultApprover = getDefaultApprover(policy); | ||
| const defaultApprover = getMergeHRFinalApprover(policy) ?? getDefaultApprover(policy); |
There was a problem hiding this comment.
Using getMergeHRFinalApprover(policy) as defaultApprover can set the default workflow anchor to an email that is not present in employeeList; when that happens, no workflow is created for that approver and the later default-workflow insertion path ends up with calculateApprovers(...) returning an empty approver chain. This produces an empty "Everyone" workflow in read-only HR basic/manager setups instead of showing the required approver, so admins lose the routing visibility this change is trying to provide.
Useful? React with 👍 / 👎.
|
Looks like this requires a synced merge provider, but the sync keeps failing on my side. When I sync it manually, I get. When I let it sync automatically, I get a timed-out error. Is there something missing to do? |
|
|
||
| const approverTitle = (index: number) => | ||
| approvalWorkflow.approvers.length > 1 ? `${toLocaleOrdinal(index + 1, true)} ${translate('workflowsPage.approver').toLowerCase()}` : `${translate('workflowsPage.approver')}`; | ||
| const fromProviderSuffix = hrProviderName ? ` (${translate('workflowsPage.approverFromProvider', {provider: hrProviderName})})` : ''; |
There was a problem hiding this comment.
I think we should define this right before it's being used here
| return translate('workflowsPage.approver'); | ||
| } | ||
| const isLastApprover = index === approvalWorkflow.approvers.length - 1; | ||
| if (isLastApprover && approvalWorkflow.approvers.length > 1) { |
There was a problem hiding this comment.
approvalWorkflow.approvers.length > 1 is always true
| } | ||
|
|
||
| /** Returns the Merge HR finalApprover when the integration is in "basic" approval mode, or null otherwise. */ | ||
| function getMergeHRBasicModeFinalApprover(policy: OnyxEntry<Policy>): string | null { |
There was a problem hiding this comment.
NAB: this file has become so large that syntax highlighting has stopped working. Put Merge.dev / HR sync stuff in a new dedicated file.
| const effectiveSubmitsTo = shouldFilterOutExpensifyTeam ? (findFirstNonExpensifyApprover(employees, submitsTo) ?? submitsTo) : submitsTo; | ||
|
|
||
| if (!employees[effectiveSubmitsTo]) { | ||
| if (!employees[submitsTo] && !isMergeHRManagerMode) { |
There was a problem hiding this comment.
NAB: we have two back-to-back if statements that just do continue. Maybe it would be cleaner to combine them.
| hrProviderName?: string; | ||
|
|
||
| /** When true, uses HR manager mode labels: "Manager (from {provider})" then "Final approver" */ | ||
| isHRManagerMode?: boolean; |
There was a problem hiding this comment.
The term "manager mode" is kind of confusing because there are three canonical approval modes in our backend:
- Basic
- Advanced (I think this is what we're calling "manager mode")
- Manual
imo it would be better to use the same terms in the front-end and the back-end in the code, even though the product shows the modes as Basic, Manager, and Custom.
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Core sprint issue
5 participants
Explanation of Change
This PR safeguards the workflow modification pages when
basicormanagerapproval modes are set and makes sure that the workflow chains display themanagerwith label when manager workflow is set.Fixed Issues
$ #90604
PROPOSAL:
Tests
You muse have a workspace with synced merge dev provider.
BasicManagerCustomBasicOffline tests
Same as tests.
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
Screen.Recording.2026-05-22.at.17.54.55.mov
MacOS: Chrome / Safari
Screen.Recording.2026-05-22.at.17.41.03.mov