Allow card assignment on shared workspaces via linkedPolicyIDs#89673
Allow card assignment on shared workspaces via linkedPolicyIDs#89673Beamanator merged 3 commits intomainfrom
Conversation
…licyIDs The permission check in useIsAllowedToIssueCompanyCard only allowed card assignment when the feed's domainID matched the current workspace's workspaceAccountID. For feeds shared to other workspaces via linkedPolicyIDs, this check failed, causing the assign button to be greyed out even for workspace admins. This adds a linkedPolicyIDs check so policy admins on any linked workspace can assign cards. Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
…id-values lint rule Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
|
Fixed the failing ESLint check: replaced |
|
@linhvovan29546 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
joekaufmanexpensify and
madmax330
and removed request for
a team
|
@madmax330 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] |
![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: 3896482f25
ℹ️ 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".
|
@MelvinBot can you address the PR review feedback? |
…nkedPolicyIDs check Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
|
@Beamanator 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] |
|
@linhvovan29546 can you please review? |
|
@madmax330 I’m OOO. I’ll check this in about two hours once I’m back home. |
|
Reviewing... |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppN/A — the change only relates to logic, so there’s no difference across platforms. Android: mWeb ChromeN/A — the change only relates to logic, so there’s no difference across platforms. iOS: HybridApp
iOS: mWeb Safari
MacOS: Chrome / SafariBefore 
After 
|
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Fix makes sense
|
🚧 @Beamanator 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/Beamanator in version: 9.3.68-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a bug fix that corrects the permission logic in |
|
@madmax330 @linhvovan29546 No Qa steps, OK to checkoff? |
|
@m-natarajan The QA steps are the same as the test steps. |
|
Can you be more clear with step 2, also SO link is not available for QA team
cc @linhvovan29546 @Beamanator @joekaufmanexpensify @heyjennahay @madmax330 |
|
@IuliiaHerets i'm not sure you can QA this easily so you can just leave it |
|
@madmax330, could you please check it internally? |
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.3.68-3 🚀
|
8 participants
Explanation of Change
When a company card feed is shared to a non-preferred workspace via
linkedPolicyIDs, the "Assign card" button was greyed out becauseuseIsAllowedToIssueCompanyCardonly checked if the feed'sdomainIDmatched the current workspace'sworkspaceAccountID. For shared feeds, thedomainIDbelongs to the owning workspace, so the check failed and fell through to a domain admin check — which also fails for users who are only workspace admins.This adds a
linkedPolicyIDscheck so that policy admins on any workspace that has the feed shared vialinkedPolicyIDsare authorized to assign cards, matching the behavior described in the help article.Fixed Issues
$ #89672
Tests
Verify that no errors appear in the JS console
Create a new account with a domain in OD
Add a corporate card feed with multiple cards and some domain members following this SO: https://stackoverflowteams.com/c/expensify/questions/67
Go to the workspace in ND and link the domain feed
Make sure the assign button is not greyed out and that clicking it assigns the card to the user:
Offline tests
N/A - This change only affects permission logic for card assignment UI state.
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