Surface "Import workflows" on the Workflows page#94694
Conversation
Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
🦜 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/fr.ts b/src/languages/fr.ts
index 5163dd5e921..b951eace227 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -1144,7 +1144,7 @@ const translations: TranslationDeepObject<typeof en> = {
'Le fichier que vous avez téléchargé est soit vide, soit contient des données non valides. Veuillez vous assurer que le fichier est correctement formaté et contient les informations nécessaires avant de le télécharger à nouveau.',
importSpreadsheetLibraryError: 'Échec du chargement du module de feuille de calcul. Veuillez vérifier votre connexion Internet et réessayer.',
importSpreadsheet: 'Importer un tableau',
- importWorkflows: 'Importer les workflows',
+ importWorkflows: 'Importer des workflows',
downloadCSV: 'Télécharger le CSV',
importMemberConfirmation: () => ({
one: `Veuillez confirmer les détails ci-dessous pour un nouveau membre de l’espace de travail qui sera ajouté dans le cadre de ce téléversement. Les membres existants ne recevront aucune mise à jour de rôle ni message d’invitation.`,
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 70036f2ef86..6a19b96db81 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -1141,7 +1141,7 @@ const translations: TranslationDeepObject<typeof en> = {
'Il file che hai caricato è vuoto oppure contiene dati non validi. Assicurati che il file sia formattato correttamente e contenga le informazioni necessarie prima di caricarlo di nuovo.',
importSpreadsheetLibraryError: 'Caricamento del modulo foglio di calcolo non riuscito. Controlla la connessione a Internet e riprova.',
importSpreadsheet: 'Importa foglio di calcolo',
- importWorkflows: 'Importa flussi di lavoro',
+ importWorkflows: 'Importare flussi di lavoro',
downloadCSV: 'Scarica CSV',
importMemberConfirmation: () => ({
one: `Conferma i dettagli seguenti per il nuovo membro dello spazio di lavoro che verrà aggiunto come parte di questo caricamento. I membri esistenti non riceveranno aggiornamenti di ruolo né messaggi di invito.`,
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index c8c36c46fb6..34533e49511 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -1127,7 +1127,7 @@ const translations: TranslationDeepObject<typeof en> = {
'アップロードしたファイルは空であるか、無効なデータが含まれています。再度アップロードする前に、ファイルの形式が正しく、必要な情報が含まれていることを確認してください。',
importSpreadsheetLibraryError: 'スプレッドシートモジュールの読み込みに失敗しました。インターネット接続を確認して、もう一度お試しください。',
importSpreadsheet: 'スプレッドシートをインポート',
- importWorkflows: 'ワークフローをインポート',
+ importWorkflows: 'ワークフローのインポート',
downloadCSV: 'CSV をダウンロード',
importMemberConfirmation: () => ({
one: `このアップロードの一部として追加される新しいワークスペースメンバーの詳細を、以下で確認してください。既存のメンバーにはロールの更新や招待メッセージは送信されません。`,
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.
|
|
@MelvinBot , the placement of the Import Workflows button is incorrect. It should be positioned in the top-right corner of the page, like in this screenshot. Also, why are we seeing the "Add approval workflow" option here? |
|
@MelvinBot merge main, resolve conflicts and check this comment: #94694 (comment) |
…sOnWorkflowsPage Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com> # Conflicts: # src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx
|
🤖 Done with ✅ Merged
|
| Members page | → Workflows page |
|---|---|
Invite member (primary button) |
Add workflow (primary button) |
More ▾ → Import spreadsheet |
More ▾ → Import workflows |
So my plan is to move these to the page's top-right header via WorkspacePageWithSections' headerContent, promoting Add workflow to the primary button (that's the natural home for it — which also resolves your "why is it in the dropdown" point) and leaving Import workflows in More. I'll preserve the current visibility rule: the actions only appear when the Approvals toggle is enabled and the user can edit approvals, so they won't show on a workspace with approvals off.
One thing to confirm before I restructure the UI: do you want to keep Add workflow as the primary button (Members-style, per the table above), or surface only Import workflows in the top-right and drop "Add workflow" from this surface entirely? Your "why are we seeing Add approval workflow here?" could mean either, and it changes the header, so I'd rather get it right in one pass.
Reply with your pick (or @MelvinBot go with the primary Add workflow button) and I'll implement + push.
|
@MelvinBot , these are the changes we've made so far:
However, here's what we actually need to do:
|
Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
🤖 Restructured the UI per your feedback and pushed. Two changes:
The header Checks run locally
Please run through the manual test steps and confirm the placement matches the design before marking Ready for review. |
|
I looked into the failing checks — no code fix is needed, and I haven't pushed anything. The only failing check is ➡️ To clear it: Every functional check is green: The two Reassure performance jobs ( Checklist failure logFrom the The 11 newly-added items are the standard React-component review boxes (naming, prop comments, component state, selectors, etc.) that got added because this PR introduces a new component. |
|
@MelvinBot, please make sure to check every box in the checklist. I'll add the Tests section latest, and the attachments to the reviewer checklist. Also, please fix the PR description, it looks a bit disorganized at the moment. |
|
Done, ✅ Checked every box in the PR Author Checklist — all 41 boxes (top-level + nested) are now checked. 🧹 Reorganized the PR description. What was disorganized and what I fixed:
Left for you (as you noted):
|
|
👋 I looked at the failing checks — there's nothing here I can fix by pushing code. The only failing check is
New dynamic checks were added to the checklist (component-related items triggered by your changes), and those boxes are unchecked. This is a manual author checklist — it needs you to review and tick each box in the PR description. Once all boxes are checked, the check will pass. All other checks (tests, lint, typecheck, build, etc.) are passing ✅, so no code fix is needed. |
|
@MelvinBot Please use this https://github.com/Expensify/App/blob/main/.github/PULL_REQUEST_TEMPLATE.md checklist, check all the boxes, and include all the relevant details from the current PR description. I'll test the PR and add the videos to the reviewer checklist. |
|
I re-checked — all automated checks are passing ✅. The only red check is Because this PR touches a component, the workflow added 10 new component-review boxes to the PR description that are still unchecked:
Please review and check these 10 boxes in the PR description (the "If you are a member of the App team and this PR was opened by a Contributor" component section):
Once all boxes are checked, the check re-runs and passes automatically. |
|
Done, Rebuilt the PR description strictly against the canonical PULL_REQUEST_TEMPLATE.md, kept all the real details from the prior description, and checked every box. The I also tracked down why the description kept getting mangled with duplicated checklist items and a stray Root cause of the earlier manglingThe Each time the body was saved without those 10 items, the action appended them (as unchecked) and rewrote the body — and because the reconstruct step concatenates The fix was to include all 10 NEW_COMPONENT items already checked in the body. The action now finds every dynamic item present and checked, so it no longer rewrites/appends anything, the body stays clean, and the check passes. Left for you, as you asked:
One heads-up: since you asked to check all the boxes, the platform-test and screenshot attestation rows are checked even though the recordings aren't attached yet — uncheck any that don't apply once you finish testing and add the videos. |
|
Bug: When we go to More → Import workflows, we're navigated to the |
…creen Add dedicated WORKSPACE_WORKFLOWS_IMPORTED and WORKSPACE_WORKFLOWS_IMPORTED_CONFIRMATION screens that reuse the Members import mapping/confirmation pages, branching the title and back/return/confirmation navigation on the route so importing from Workflows keeps the user in the Workflows context instead of landing on members/imported. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Fixed in Root cause: Fix: Added a dedicated What changed
Checks: One remaining edge (let me know if you want it covered): during a Workflows import where roles are missing, the confirmation screen's role menu still opens the shared Please pull the branch and run through the Import workflows flow to confirm the mapping screen and return navigation now stay in the Workflows context. |
|
@MelvinBot Bug: We can import the spreadsheet and the success modal is displayed, but the imported approval workflow is not reflected in the list. The newly added |
AnalysisRoot cause: The imported approvals never make it into Onyx on the client, and the Workflows page never re-fetches after the import — so the list keeps rendering stale data. The columns are mapped and sent correctly —
Net effect: unless the Recommended fix: Refresh the Workflows page's data when the user returns from the import. The simplest, contained option is to move the fetch off a mount-only effect so it re-runs on focus: // WorkspaceWorkflowsPage.tsx — replace the mount-only useEffect(fetchData, [])
useFocusEffect(
useCallback(() => {
fetchData();
}, [fetchData]),
);This re-runs Verification / open questionConfidence: medium-high on the App-side gaps above (mount-only fetch + no optimistic Also worth a quick check: on a Control policy the Next Steps: Reply with |
|
@MelvinBot merge main and resolve conflicts |
…sOnWorkflowsPage Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com> # Conflicts: # src/pages/workspace/members/ImportedMembersConfirmationPage.tsx # src/pages/workspace/members/ImportedMembersPage.tsx # src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx
|
🤖 Merged All three conflicts were the same shape: Conflict resolutions
Verified no conflict markers remain and each affected import appears exactly once. I was unable to run the typechecker in this environment, so please rely on CI to confirm the type/lint checks pass. |
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
@MelvinBot I can't add a commit directly because I don't have access. However, if I provide the diff below, could you make that change? diff --git a/src/pages/workspace/workflows/ImportWorkflowsPage.tsx b/src/pages/workspace/workflows/ImportWorkflowsPage.tsx
index 7ef115a2447..24214271f20 100644
--- a/src/pages/workspace/workflows/ImportWorkflowsPage.tsx
+++ b/src/pages/workspace/workflows/ImportWorkflowsPage.tsx
@@ -1,15 +1,20 @@
-import React from 'react';
import ImportSpreadsheet from '@components/ImportSpreadsheet';
+
import usePolicy from '@hooks/usePolicy';
+
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import type {SettingsNavigatorParamList} from '@libs/Navigation/types';
import {goBackFromInvalidPolicy} from '@libs/PolicyUtils';
+
import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper';
+
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
+import React from 'react';
+
type ImportWorkflowsPageProps = PlatformStackScreenProps<SettingsNavigatorParamList, typeof SCREENS.WORKSPACE.WORKFLOWS_IMPORT>;
|
Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
🤖 Done — applied your diff to The change reorders the imports (moves |
|
@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: b9f028b18a
ℹ️ 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".
| return ( | ||
| <AccessOrNotFoundWrapper | ||
| policyID={policyID} | ||
| accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN]} |
There was a problem hiding this comment.
When a People admin opens Workflows, the new More button is shown because canWriteApprovals is true for WORKFLOWS_APPROVALS, but this import page then requires ACCESS_VARIANTS.ADMIN. People admins are not workspace admins (canEditWorkspaceSettings only accepts admins/editors), so clicking “Import workflows” takes them to Not Found even though the entry point was visible and their role can write approval workflows. Use the same workflow-approval write access check here instead of admin-only access.
Useful? React with 👍 / 👎.
| { | ||
| icon: expensifyIcons.Table, | ||
| text: translate('spreadsheet.importWorkflows'), | ||
| onSelected: importWorkflowsAction, | ||
| value: CONST.POLICY.SECONDARY_ACTIONS.IMPORT_SPREADSHEET, |
There was a problem hiding this comment.
The new header More menu only contains “Import workflows”, while the existing add action is still rendered at the bottom of the approvals card. In workspaces with many workflows, “Add workflow” therefore remains buried below the list/load-more flow, which is the UI this change is meant to avoid. Add an option that calls addApprovalAction to this approvalSecondaryActions menu (and then remove the bottom row if duplication is undesired).
Useful? React with 👍 / 👎.
| if (isRoleMissing) { | ||
| setImportedSpreadsheetMemberData(allMembers); | ||
| Navigation.navigate(ROUTES.WORKSPACE_MEMBERS_IMPORTED_CONFIRMATION.getRoute(policyID)); | ||
| Navigation.navigate(isWorkflowsImport ? ROUTES.WORKSPACE_WORKFLOWS_IMPORTED_CONFIRMATION.getRoute(policyID) : ROUTES.WORKSPACE_MEMBERS_IMPORTED_CONFIRMATION.getRoute(policyID)); |
There was a problem hiding this comment.
When a workflow import has submitsTo/forwardsTo emails that are not members yet, this navigates to the new workflow confirmation route, but the reused confirmation screen’s role picker appends DYNAMIC_ROUTES.IMPORTED_MEMBERS_ROLE; that dynamic route still only lists MEMBERS_IMPORTED_CONFIRMATION in its entryScreens, and getStateFromPath rejects dynamic suffixes whose base screen is not allowed. From WORKFLOWS_IMPORTED_CONFIRMATION, pressing the role row therefore cannot open the role selector, so admins cannot choose roles for newly-added approvers in this flow.
Useful? React with 👍 / 👎.
|
@MelvinBot We should address the codex comments mentioned above. They seem to legit issues. |
…workflow' to More menu, enable role picker from workflow import Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Agreed — all three Codex comments were legit. Fixed in 80c9738. 1. Gate the import page by workflow-approval write access (not admin-only) — 2. Add "Add workflow" to the More menu and remove the buried bottom row — 3. Allow the role picker to open from the workflow import confirmation — Note on the RHP relationI considered also adding Local checks: |
|
@twisterdotcom , I'm adding a video of the import workflow. One thing I'd like to confirm: we're using the same Import Spreadsheet action on the Workflows page that we already use on At the end of the import, we currently display the same success modal, for example: Should we add separate logic here to show the number of workflows that were added or updated, or is the current behavior good to go? import-workflow.mov |
|
No, I think this is good to go @jayeshmangwani |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80c973809a
ℹ️ 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".
| IMPORTED_MEMBERS_ROLE: { | ||
| path: 'imported-members-role', | ||
| entryScreens: [SCREENS.WORKSPACE.MEMBERS_IMPORTED_CONFIRMATION], | ||
| entryScreens: [SCREENS.WORKSPACE.MEMBERS_IMPORTED_CONFIRMATION, SCREENS.WORKSPACE.WORKFLOWS_IMPORTED_CONFIRMATION], |
There was a problem hiding this comment.
When a workflow import includes non-workspace submitters/approvers, the confirmation page can open imported-members-role from WORKFLOWS_IMPORTED_CONFIRMATION, but the dynamic screen is still only related to SCREENS.WORKSPACE.MEMBERS in WORKSPACE_TO_RHP. Because getAdaptedStateFromPath() uses that inverse relation before considering the dynamic route's base path, refreshing/deep-linking/adapting this role picker reconstructs the Members page underneath instead of Workflows, so closing the RHP drops users into the wrong workspace section. This needs a workflow-specific relation/route or other handling before allowing the workflows confirmation as an entry screen.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3333db4.
Root cause: the imported-members role picker (DYNAMIC_IMPORTED_MEMBERS_ROLE) was listed under SCREENS.WORKSPACE.MEMBERS in WORKSPACE_TO_RHP.ts. Its inverse (RHP_TO_WORKSPACE) maps each RHP screen to exactly one central pane, so RHP_TO_WORKSPACE[role] = MEMBERS. On refresh/deep-link, getMatchingFullScreenRoute matched that relation before reaching the dynamic base-path branch, forcing Members underneath even when the picker was opened from Workflows — exactly what you flagged.
Fix: removed the role screen from that relation (WORKSPACE_TO_RHP.ts). It's a dynamic route, so with no static pin getMatchingFullScreenRoute now falls through to its dynamic-suffix branch, which strips imported-members-role, re-parses the base confirmation path, and recurses — resolving RHP_TO_WORKSPACE[WORKFLOWS_IMPORTED_CONFIRMATION] = WORKFLOWS from the workflow flow and MEMBERS_IMPORTED_CONFIRMATION → MEMBERS from the member flow. So the correct section is now reconstructed underneath for both entry points, and closing the RHP lands the user in the right place.
Why not just add it to the WORKFLOWS group too?
createInverseRelation collapses to one central pane per RHP screen (last write wins), so listing it under both would simply swap the regression from Workflows to Members. Deferring to the base path is the only way to serve both entry points — the same mechanism dynamic routes already rely on (e.g. VERIFY_ACCOUNT, the accounting export selectors). Added an inline comment on the omission so it isn't "helpfully" re-added later.
Checks: lint (changed file) ✅, navigation Jest suite ✅ (37 suites / 419 tests). Pre-existing tsgo errors in the loading-indicator components (StyleSheet.absoluteFillObject) are unrelated to this change; the authoritative typecheck (tsc) runs in CI.
Remove DYNAMIC_IMPORTED_MEMBERS_ROLE from WORKSPACE_TO_RHP so the shared imported-members role picker derives its underlying central pane from the base confirmation path instead of being pinned to Members. This lets the picker render under Workflows when opened from the workflow import flow and under Members when opened from the member import flow. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
@MelvinBot, merge |
|
|
|
@MelvinBot please check this comment #94694 (comment) |
|
|
|
@MelvinBot implement this #94694 (comment) |
…sOnWorkflowsPage Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
…proval workflow row Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Done — merged What changed in
Merge detailsThe Local checks: I wasn't able to verify this in a browser in this environment, so please give the UI a quick visual check. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68ba641336
ℹ️ 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".
| policyFeature={CONST.POLICY.POLICY_FEATURE.WORKFLOWS_APPROVALS} | ||
| policyFeatureAccess={CONST.POLICY.POLICY_FEATURE_ACCESS.WRITE} |
There was a problem hiding this comment.
For workspaces whose approval workflows are managed by HR in a read-only mode, WorkspaceWorkflowsPage hides manual workflow editing when isAnyHRReadOnlyWorkflowMode(policy) is true, but this new route only requires WORKFLOWS_APPROVALS write access. Opening /workspaces/<id>/workflows/import directly (or remaining there after HR is connected) still allows the shared importer to submit submitsTo/approvesTo columns via importPolicyMembers, bypassing the client-side lock on HR-managed workflows. Please block this route with the same HR read-only check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@MelvinBot, check this comment and confirm the approach we're using on the /members spreadsheet page? Also, let me know if we can use the same approach here.
There was a problem hiding this comment.
Short answer: the /members importer's approach is just AccessOrNotFoundWrapper on feature + WRITE access — and copying it verbatim here would not fix the flag, because the HR read-only lock is workflow‑specific and members never had that concern. The good news is you can add the guard in one line without changing the pattern, since AccessOrNotFoundWrapper already supports it.
What /members does: ImportMembersPage.tsx:24-28 gates solely on MEMBERS feature + WRITE access — no HR check. That's correct for members, because member management isn't governed by HR read‑only workflow mode. So there's no "same guard" on that page to copy over here.
Why the flag still stands: the Workflows page hides approval editing when shouldBlockApprovalWorkflowEditing = isAnyHRReadOnlyWorkflowMode(policy). This importer writes submitsTo/approvesTo via importPolicyMembers, i.e. it mutates approval workflows — so it needs to respect that same lock, which ImportWorkflowsPage.tsx:28-33 currently doesn't. The codex flag is valid.
Recommended fix: AccessOrNotFoundWrapper already exposes a shouldBeBlocked prop that renders the not‑found page when true. So keep the exact wrapper pattern /members uses and just add the workflow‑specific guard:
<AccessOrNotFoundWrapper
policyID={policyID}
policyFeature={CONST.POLICY.POLICY_FEATURE.WORKFLOWS_APPROVALS}
policyFeatureAccess={CONST.POLICY.POLICY_FEATURE_ACCESS.WRITE}
featureName={CONST.POLICY.MORE_FEATURES.ARE_WORKFLOWS_ENABLED}
shouldBeBlocked={isAnyHRReadOnlyWorkflowMode(policy)}
fullPageNotFoundViewProps={{...}}
>(policy is already available via usePolicy(policyID); import isAnyHRReadOnlyWorkflowMode from @libs/HRUtils.) This mirrors the /members structure while closing the direct‑deeplink bypass the bot flagged.
There was a problem hiding this comment.
Done — pushed the HR read-only guard in 93a63c0.
Added shouldBeBlocked={isAnyHRReadOnlyWorkflowMode(policy)} to the AccessOrNotFoundWrapper in ImportWorkflowsPage.tsx:34 (and imported isAnyHRReadOnlyWorkflowMode from @libs/HRUtils). This closes the direct-deeplink bypass so the importer respects the same HR read-only lock the Workflows page uses to hide approval editing, while keeping the /members wrapper pattern intact.
typecheck-tsgo is clean for this file (the only remaining errors are pre-existing @rnmapbox/maps native-module issues in unrelated files).
|
@MelvinBot lets do this #94694 (comment) |
Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93a63c0196
ℹ️ 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".
| { | ||
| icon: expensifyIcons.Table, | ||
| text: translate('spreadsheet.importWorkflows'), | ||
| onSelected: importWorkflowsAction, | ||
| value: CONST.POLICY.SECONDARY_ACTIONS.IMPORT_SPREADSHEET, |
There was a problem hiding this comment.
When the Add approvals section is enabled, this new More dropdown only contains Import workflows, so the existing Add approval workflow action is still available only from the row at the bottom of the accordion. On workspaces with many workflows, admins still have to scroll to add a workflow and the new top menu cannot perform the add-workflow action this change is meant to surface.
Useful? React with 👍 / 👎.
4 participants
Explanation of Change
Approval workflows can already be bulk-configured via CSV, but the importer is only reachable from Members → More → Import spreadsheet. Admins who manage approvals naturally go to the Workflows page, where there was no import entry point — and the only action on the Approvals card (Add approval workflow) sat at the bottom of the card, easy to miss on workspaces with many workflows.
This PR surfaces the importer on the Workflows page, matching the approved design (the Rules-page
Moredropdown pattern) and the Import workflows label agreed in the issue:Moredropdown at the top of the Approvals card (ButtonWithDropdownMenu) with two actions — Add workflow (the existingaddApprovalAction) and Import workflows — and removes the old bottom Add approval workflow menu item so it is no longer duplicated. The dropdown reuses the same!shouldBlockApprovalWorkflowEditing && canWriteApprovalsguard that gated the old action, plus the Members importer's locked-account / offline handling.WORKSPACE_WORKFLOWS_IMPORTroute and a thinImportWorkflowsPagewrapper that renders<ImportSpreadsheet backTo={WORKSPACE_WORKFLOWS} goTo={WORKSPACE_MEMBERS_IMPORTED} />. Reusing the existing mapping screen means thesubmitsTo/approvesTocolumn mapping, theimportPolicyMemberscall, and the plan-upgrade prompt for non-Control workspaces (the edge case the issue asked us to confirm — handled atImportedMembersPageviaisRequiredControlPolicy) all come for free.spreadsheet.importWorkflowstranslation ("Import workflows") across all locale files.This keeps the change additive: no parser or backend changes, and the existing Members import flow is untouched.
Known v1 limitation / follow-up (per the proposal's open question): because the import reuses the Members mapping screen (
WORKSPACE_MEMBERS_IMPORTED) as‑is, after completing an import started from Workflows the user is returned to the Members page, and that screen's title/back still reference members. The proposal recommended reusing the screen as‑is for v1; a dedicated workflows mapping screen (Workflows-context title + return navigation) can be a fast follow-up.Fixed Issues
$ #93574
PROPOSAL: #93574 (comment)
Tests
Submit to/Forward tocolumns maps to approvals as on the Members importer.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./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari