Filter unreported expenses by owner to prevent showing member expenses to admin#73230
Conversation
|
@iwiznia I think the card for 70639-web-chrome-issue.mp4 |
|
Damnit! I see here we are not returning all cards to newDot (cash, personal cards not returned, not sure if some others). |
|
Not me :). cc @grgia https://github.com/Expensify/Auth/pull/9223 for personal cards |
|
Ah thanks for that. I don't see a real reason not to add them, just that we did not want them to display them in the cards table. I think we should've done that in the frontend instead. So if @grgia agrees, I think we would:
Thoughts? |
|
Yeah @iwiznia, now that we are using personal cards in ND, makes sense |
|
We are? Where? How if we are not returning them from auth? |
|
@grgia Can you please help confirm if the BE changes are in production already? Thanks |
|
@grgia please respond to the question above so we can get this moving forward |
|
No update on the BE implementation, I only worked on ECards @iwiznia. Is there an issue for the BE? |
|
ok talked to @grgia, I am sending the backend change right now. We need to do this, but maybe we need to do it in 3 steps so we don't break anything, so:
Sounds good @rojiphil? If so, can you start working on the PR for the first item? |
|
PR for the backend is here https://github.com/Expensify/Auth/pull/18448 will merge once the PR for item 1 in my comment above is deployed |
@iwiznia That sounds good as first step to me as well so that we don’t break anything. Meanwhile, I would need your help with the following as I am unable to figure out a way to filter. Thanks.
An access to the relevant design document is also good enough if that helps. Btw, I am partially available today and tomorrow due to travel but hope to work on the PR over the weekend. |
|
Looks like a solid plan if you will need a C+ for the other PR I can work on it as well. |
Are you asking how to identify which cards are personal vs non personal and how to differentiate the cash personal card from other cards? If so:
|
|
Can we keep this moving please @rojiphil ? |
|
so according to this plan seems like we are now pending on the second point #73230 (comment) |
|
The second point is https://github.com/Expensify/Auth/pull/18448 I believe @iwiznia |
|
I am lost. Did we do step 1 here already? |
|
Sure ,, just resolved the conflicts |
|
Conflicts again @abzokhattab. Please resolve. Working on checklist now |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp70639-android-hybrid-001.mp4Android: mWeb Chrome70639-mweb-chrome-001.mp4iOS: HybridApp70639-ios-hybrid-001.mp4iOS: mWeb Safari70639-mweb-safari-001.mp4MacOS: Chrome / Safari79772-web-chrome-002.mp4 |
| const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${getNonEmptyStringOnyxID(report?.policyID)}`, {canBeMissing: true}); | ||
| const [hasMoreUnreportedTransactionsResults] = useOnyx(ONYXKEYS.HAS_MORE_UNREPORTED_TRANSACTIONS_RESULTS, {canBeMissing: true}); | ||
| const [isLoadingUnreportedTransactions] = useOnyx(ONYXKEYS.IS_LOADING_UNREPORTED_TRANSACTIONS, {canBeMissing: true}); | ||
| const [cardList] = useOnyx(ONYXKEYS.CARD_LIST, {selector: filterPersonalCards, canBeMissing: true}); |
There was a problem hiding this comment.
@abzokhattab We should not include the selector here as we should look into the entire cardlist to determine if the transaction belongs to this user
There was a problem hiding this comment.
| const [cardList] = useOnyx(ONYXKEYS.CARD_LIST, {selector: filterPersonalCards, canBeMissing: true}); | |
| const [cardList] = useOnyx(ONYXKEYS.CARD_LIST, {canBeMissing: true}); |
|
Completed checklist. Tests well after applying the suggestion here. |
…es-ownership-check
|
LGTM 79772-web-chrome-002.mp4 |
rojiphil
left a comment
There was a problem hiding this comment.
Thanks @abzokhattab for the updates.
@iwiznia Changes LGTM.
Over to you. Thanks.
…es-ownership-check
|
FYI: To incorporate this comment, I negated the
i am afriad that the current changes bring regressions since we are now checking for cc @rojiphil |
@abzokhattab I think this will get addressed once we resolve the conflicts here. |
|
resolved the conflicts |
|
Changes LGTM 73230-web-chrome-003.mp4 |
|
🚧 @iwiznia 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.3.13-1 🚀
|
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.3.15-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.15-10 🚀
|

Explanation of Change
Fixed Issues
$ #70639
PROPOSAL: #70639 (comment)
Tests
Precondition:
Steps:
Offline tests
Same as tests
QA Steps
same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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-01-17.at.16.13.02.mov
Android: mWeb Chrome
Screen.Recording.2026-01-17.at.16.15.59.mov
iOS: Native
Screen.Recording.2026-01-17.at.16.08.38.mov
iOS: mWeb Safari
Screen.Recording.2026-01-17.at.16.09.49.mov
MacOS: Chrome / Safari
Screen.Recording.2026-01-17.at.16.03.15.mov