Allow full-access copilots to add new company card feeds - #98600
Conversation
Co-authored-by: Shubham Agrawal <shubham1206agra@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
🤖 I looked into the failing checks — there's no code fix to push. Both failures are process/checklist checks, not code issues:
Every actual code check is green: To clear the two failures:
Since these aren't code defects, I didn't push any commit. Let me know if you'd like me to dig into anything else. |
|
🤖 Re-checked — one real check is now failing ( Failing test: Why it's unrelated: This PR changes only two company-card files ( Suggested fix (in that unrelated file, for its owner): add The other two failures are still the process checks — Full check status
|
|
@MelvinBot Merge main please |
…Feed Co-authored-by: Shubham Agrawal <shubham1206agra@users.noreply.github.com>
|
🤖 Merged The merge was clean — no conflicts. The PR's net diff against
Merge commit: |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| shouldEnablePickerAvoiding={false} | ||
| > | ||
| <DelegateNoAccessWrapper accessDeniedVariants={[CONST.DELEGATE.DENIED_ACCESS_VARIANTS.DELEGATE]} /> | ||
| <DelegateNoAccessWrapper accessDeniedVariants={[CONST.DELEGATE.DENIED_ACCESS_VARIANTS.SUBMITTER]} /> |
There was a problem hiding this comment.
@MelvinBot What is the reason for changing this line?
There was a problem hiding this comment.
Line 95 swaps the DelegateNoAccessWrapper variant from DENIED_ACCESS_VARIANTS.DELEGATE to DENIED_ACCESS_VARIANTS.SUBMITTER so the inner guard matches the new outer gate and only blocks limited/submitter copilots — not full-access ones.
The two variants apply different predicates in DelegateNoAccessWrapper.tsx:22-26:
DELEGATE→isDelegate(account)— true for every delegate (full-access included)SUBMITTER→isSubmitter(account)— true only for delegate-only submitters (limited access)
Since the outer branch condition was changed to isDelegateAccessRestricted (acting as delegate and submitter-only), the wrapper's own predicate has to change too, otherwise the two layers would disagree about who is blocked. This makes the whole page consistently allow full-access copilots into the add-feed flow while still blocking limited/submitter copilots — mirroring the Expensify Card issue-new page (DynamicIssueNewCardPage.tsx:149) from #81817, which already uses the SUBMITTER variant.
|
Analyzed the failing checks — no code fix is needed. Every check that validates the code passes (Bun tests, The two remaining ❌ are process gates, not code failures, so there's nothing to push:
Both clear on their own once this PR is marked "Ready for review" and a reviewer approves + completes their checklist. As a reminder, the human co-author still needs to fill in the |
|
No fix to push — the code is green. All checks that validate code pass (Bun tests, The only two ❌ are non-code process gates that clear through review, not by pushing commits:
To move this forward: fill in the |
|
@MelvinBot Merge main please |
…Feed Co-authored-by: Shubham Agrawal <shubham1206agra@users.noreply.github.com>
|
🤖 Merged |
|
There's no failing check that a code change can fix here — nothing to push. The only failing check is Every actual code/build/test check is green:
The one remaining Next step: get an independent reviewer to approve the PR to satisfy the |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-08-19.at.5.58.57.PM.mov |
|
@MelvinBot Ready this PR for review |
|
@shubham1206agra 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] |
|
🤖 Marked this PR as ready for review. |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #98328 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
@shubham1206agra, the PR author checklists and test steps are not filled, is it expected to be left empty? |
|
@mollfpr Yes |
|
All yours @joekaufmanexpensify cc @trjExpensify |
trjExpensify
left a comment
There was a problem hiding this comment.
@shubham1206agra in your video evidence, please go through the flow and actually add a new card feed as a copilot.
|
@trjExpensify Here you go Screen.Recording.2026-08-20.at.5.46.00.PM.mov |
Did you do a connected card feed as well like "Mock bank"? |
@trjExpensify I think this is an Internal QA task as I don't I can add any feed apart from CSV. |
Yeah, you can. Click |
Screen.Recording.2026-08-24.at.8.19.39.PM.mov@trjExpensify Here you go |
|
@mollfpr feel free to merge. |
|
@mollfpr Bump here |
|
🚧 mollfpr has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mollfpr in version: 9.4.66-0 🚀
|
|
Yes, help site changes are required. Draft PR: #100095 Why: this PR lets a Full Access copilot open the add-new-feed flow, but the feed-setup articles only said "Any Workspace Admin can…", so they read as admin-only and never mentioned copilots at all. What changed in the docs PR
Two things to check when you review:
Articles I checked and left alone
@shubham1206agra, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to staging by https://github.com/mollfpr in version: 9.4.68-0 🚀
|
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.4.68-1 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
PR #52103 restricted Copilots (delegates) from adding new company card feeds. The add-new-feed flow gated on
isActingAsDelegatewith theDENIED_ACCESS_VARIANTS.DELEGATEwrapper variant, whose predicate is justisDelegate— so it blocks every copilot, full-access included.That's too broad: a full-access copilot can already add a card feed in Expensify Classic (a Domain Admin action), and NewDot already allows full Copilot access for Expensify Card issuing (#81817). This PR mirrors #81817 exactly — it swaps the gate from "any delegate" to "limited/submitter delegates only":
DynamicAddNewCardPage.tsx— sourceisDelegateAccessRestrictedfromuseDelegateNoAccessState()(asDynamicIssueNewCardPagedoes), gate on it instead ofisActingAsDelegate, and change theDelegateNoAccessWrappervariant fromDELEGATE→SUBMITTER.WorkspaceCompanyCardPageEmptyState.tsx— gate the empty-state "Add cards" CTA onisDelegateAccessRestrictedinstead ofisActingAsDelegate.isDelegateAccessRestrictedisisActingAsDelegate && isDelegateOnlySubmitter(account), so after this change a full-access copilot passes into the add-new-feed flow (BYOC and CSV), while limited/submitter copilots still hit the no-access view. Assigning already-issued cards and issuing Expensify Cards were already allowed and are unchanged.Fixed Issues
$ #98328
PROPOSAL: #98328 (comment)
Tests
// TODO: The human co-author must fill out the tests before marking this PR "ready for review". Suggested coverage: (1) as a full-access copilot of an account that admins a workspace with Company cards enabled, open the workspace Company cards page, click "Add cards", and verify the add-new-feed flow opens (no "no access" modal); (2) as a limited/submitter copilot, verify "Add cards" still shows the no-access modal / view; (3) as the account owner (not a copilot), verify no regression to the add-new-feed flow.
Offline tests
QA Steps
// TODO: The human co-author must fill out the QA steps before marking this PR "ready for review".
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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
iOS: mWeb Safari
MacOS: Chrome / Safari