Add unreportedExpense to report page#60352
Conversation
| const animatedHighlightStyle = useAnimatedHighlightStyle({ | ||
| borderRadius: variables.componentBorderRadius, | ||
| shouldHighlight: item?.shouldAnimateInHighlight ?? false, | ||
| highlightColor: theme.messageHighlightBG, | ||
| backgroundColor: theme.highlightBG, | ||
| }); |
There was a problem hiding this comment.
This hook is used for highlighting items that pop up on Search page, while the user is focused on it. Since we don't suppport highlighting new items on AddUnreportedExpense page(i hope :D), I think we can just remove this hook from here.
| const animatedHighlightStyle = useAnimatedHighlightStyle({ | |
| borderRadius: variables.componentBorderRadius, | |
| shouldHighlight: item?.shouldAnimateInHighlight ?? false, | |
| highlightColor: theme.messageHighlightBG, | |
| backgroundColor: theme.highlightBG, | |
| }); |
There was a problem hiding this comment.
This is used as the style of the currently selected components by an external checkbox.
…age_final_branch # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportView.tsx # src/styles/index.ts
sumo-slonik
force-pushed
the
feature/kuba-nowakowski/unreported_expense_page_final_branch
branch
from
99d23ff to
f77f2a8
Compare
|
I'm still wrestling 🤼 with TypeScript here, but the code is probably ready for review. |
|
@ 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] |
Can you post about this in a new thread in migrate please? I am not sure I exactly follow what you mean |
I'll record a video then, to provide better context. |
…age_final_branch # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportView.tsx # src/pages/home/report/ReportActionItemContentCreated.tsx
|
All check errors seem to be fixed, I think we can start the review. |
|
|
Nice, looks good to me 👍 |
|
Why is there more padding on the bottom of these than on the top?
|
…age_final_branch # Conflicts: # src/components/SelectionList/types.ts # src/styles/utils/spacing.ts
Good shout! I think @Kicu is fixing that in a different PR, right? |
|
It seems to me that it's done here: |
|
There are some issues after adding the main, I'm already working on the fixes. |
|
If there’s nothing else here, I think this can be merged. |
@allgandalf Would you have a chance to take a look at it today? |
|
Working on it right now |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@sumo-slonik This PR is failing because of issue #62057 bug.mp4 |
| const sections: Array<SectionListDataType<Transaction & ListItem>> = [ | ||
| { | ||
| shouldShow: true, | ||
| data: transactions.filter((t): t is Transaction & ListItem => t !== undefined), |
There was a problem hiding this comment.
We had an issue recently originating from this implementation:
we addressed it by mapping transactions and passing isDisabled: isTransactionPendingDelete(transaction) to prevent offline deleted unreported expenses from being added to a report.
| onSelectRow={(item) => { | ||
| if (selectedIds.has(item.transactionID)) { | ||
| selectedIds.delete(item.transactionID); | ||
| } else { | ||
| selectedIds.add(item.transactionID); | ||
| } | ||
| }} |
There was a problem hiding this comment.
The onSelectRow block was missing logic to clear existing errorMessage once an option was actually selected, leading to this issue:
which we addressed by clearing the error and moving the Set implementation to state (see comment for details).
|
This PR introduced a bug where the LHN preview total only included one track expense after adding multiple unreported expenses to a workspace report. This happened because the aggregation logic didn’t account for the possibility of multiple track expenses in this context |
| const {translate} = useLocalize(); | ||
| const [errorMessage, setErrorMessage] = useState<string>(''); | ||
|
|
||
| function getUnreportedTransactions(transactions: OnyxCollection<Transaction>) { |
There was a problem hiding this comment.
Per diem transactions can be moved to any workspace, they can only be readded to same workspace and we do not filter per diem transactions if workspace per diem settings is disabled. It caused #65402
| if (!transactions) { | ||
| return []; | ||
| } | ||
| return Object.values(transactions || {}).filter((item) => item?.reportID === '0'); |
There was a problem hiding this comment.
We needed an additional check if unreported transactions are owned by the current user. We implemented this in #70639
14 participants
Explanation of Change
Added the ability to move unreported expenses to existing reports according to the design doc. For now, the view is only accessible via a direct link, as header buttons are not yet available on main. Recordings are from internal web environments only.
Fixed Issues
$ #57655
PROPOSAL:
Tests
Create an expense for yourself
Manually create a new expense entry where you are both the payer and the receiver.
Create a report
Generate a new report to ensure existing expenses are reported and separated from unreported ones.
Use a direct link to access the unreported expenses tab
Append
/add-unreported-expenseto the report URL to directly navigate to the "Unreported Expenses" tab, even if there’s no header button yet.Visit the link again and validate the behavior
Open the same link again.
Check if an empty state is shown for unreported expenses.
Verify that the "Add Expense" button works correctly from this view.
Verify that no errors appear in the JS console
Offline tests
Unnesesary
QA Steps
Same as test
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
Screen.Recording.2025-04-23.at.11.37.36.2.mov
iOS: Native
iOS: mWeb Safari
Screen.Recording.2025-04-23.at.11.42.29.mov
MacOS: Chrome / Safari
desktop.mov
MacOS: Desktop