New Feature: Support NewDot Unreported Expenses#60077
New Feature: Support NewDot Unreported Expenses#60077marcaaron merged 4 commits intoExpensify:mainfrom
Conversation
marcaaron
left a comment
There was a problem hiding this comment.
Thanks @ikevin127. This is looking really great!
…7-supportUnreportedExpenses
| }, | ||
| }); | ||
|
|
||
| parameters.parentReportActionID = generatedReportActionID; |
There was a problem hiding this comment.
| parameters.parentReportActionID = generatedReportActionID; |
Actually, the backend logic is going to check that we don't have this. It roughly does something like
if (!doesReportExist && !parentReportActionID && unreportedTransactionID) {
// ... create the transaction thread
}
|
Looking good. Almost there - just one small thing to address. Thanks! |
|
@marcaaron Great, applied the changes - shout if other changes are needed 👍 Please let me know when the backend PR is merged and whether this requires futher testing from my side (including steps) or you'll only test internally and we add [NoQA] here. |
|
Gonna test this now. I was going to tag @puneetlath in to help test this. But realized I have a few super old reported expenses in my expensifail.com account. |
|
@rayane-d you can review the code if you want. testing will be hard. |
marcaaron
left a comment
There was a problem hiding this comment.
This is working super well.
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
Actually this will just be a bit too hard to test @rayane-d I'm gonna drop you from the requested reviewers. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Should the "join" button be showing at the top of the unreported expense? I would've though you are automatically a participant in it. |
|
Nice spot. I think I must have missed something in the backend. I'll take a look. I don't think it's super bad to have that button there for now. |
|
Hmm actually seems like they should always be participants according to backend logic. Lemme check the test report again. |
|
Oh but it should have a default "HIDDEN" notification preference: So they are participants, but still must explicitly "Join"? Here's the App logic: https://github.com/Expensify/App/blob/e37d15d210447cdd50bd6e65a3400a476dfd2f04/src/libs/ReportUtils.ts#L9225C1-L9249C2 It must be failing all these conditions. |
|
I'm not too sure what's happening with that because we're using the exact same logic we use when calling |
|
🚀 Deployed to staging by https://github.com/marcaaron in version: 9.1.29-0 🚀
|
|
Interesting. I definitely think it's fine for now and can be handled in a follow-up. Maybe we're treating it the way we do a chat thread with a "hidden" notification preference, which is an indication that you haven't "joined" it. Whereas with an expense report/expense thread, if you are a member of the report, then you have "joined" it, even if you have a "hidden" notification preference. |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.1.29-10 🚀
|



Explanation of Change
Enhanced the
openReportfunction inReport.tsto handle creating transaction threads for unreported expenses by:Updated the Search component to properly handle unreported expenses by:
reportID === CONST.REPORT.UNREPORTED_REPORTIDreportIDfor transaction threadstransactionID✅ The ReportScreen component already had the necessary logic to handle creating transaction threads when receiving a transactionID parameter (from previous draft PR).
🔄 These changes ensure that when a user clicks on an unreported expense in search results, we:
This will allow users to view and interact with all historical unreported expenses, whether or not they were already added to the self DM chat.
Fixed Issues
$ #59863
PROPOSAL:
Tests
Prerequisite: Must have a very old unreported expense that was created in OldDot.
Offline tests
We should still be able to navigate to the transaction thread while offline.
QA Steps
Same as Tests. But can skip step 6.
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))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
MacOS: Desktop