-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Violations - Pending Receipts] Display the rter Violation with the Pending Pattern #40354
[Violations - Pending Receipts] Display the rter Violation with the Pending Pattern #40354
Conversation
@ishpaul777 the PR is ready for another round of feedback |
Something seems off with API, i am not able to sign in |
@smelaa I see two statusbar for workspace expense |
@ishpaul777 it should be fixed now. I don't know how often on real life data (if at all) both conditions to show next step banner and rter violation information are satisfied but I assumed rter violation has higher priority than next step. Therefore now if there is pending rter violation, next step banner is not shown. |
@yuwenmemon Can you please confirm if the assumption is correct |
Yep, that's correct! See https://docs.google.com/document/d/1zJqlTe_RajuBtfQYvbMx8PpXgA9CEnUGVyuqZihQ-ok/edit#heading=h.t853jrc7ig0n |
Looks good, checklist done, just one unresolved comment #40354 (comment) |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
@smelaa Seems this PR causes a crash sometimes after submitting the expense. I think we need to use optional chaining for accessing the transactionID. App/src/components/ReportActionItem/ReportPreview.tsx Lines 144 to 145 in e8ae3c5
|
Hey @smelaa Are you available to create PR to fix above ^ |
@ishpaul777 I'm on it! |
🚀 Deployed to staging by https://github.com/yuwenmemon in version: 1.4.75-0 🚀
|
@smelaa @yuwenmemon @Pujan92 Can someone clarify the steps 4 and 5? |
// Transaction A
Onyx.merge(`transactions_(transaction id copied for A)`, {cardID: 1, merchant: 'Transaction A'});
Onyx.merge(`transactionViolations_(transaction id copied for A)`, [{type: 'test', name: 'rter', data: {pendingPattern: true}}]);
// Transaction B
Onyx.merge(`transactions_(transaction id copied for B)`, {cardID: 1, merchant: 'Transaction B'});
Onyx.merge(`transactionViolations_(transaction id copied for B)`, [{type: 'test', name: 'rter', data: {pendingPattern: true}}]);
// Transaction C
Onyx.merge(`transactions_(transaction id copied for C)`, {cardID: 1, merchant: 'Transaction C'});
Onyx.merge(`transactionViolations_(transaction id copied for C)`, [{type: 'test', name: 'rter', data: {pendingPattern: true}}]);
// Transaction D
Onyx.merge(`transactions_(transaction id copied for D)`, {cardID: 1, merchant: 'Transaction D'});
Onyx.merge(`transactionViolations_(transaction id copied for D)`, [{type: 'test', name: 'rter', data: {pendingPattern: true}}]);
// Transaction E
Onyx.merge(`transactions_(transaction id copied for E)`, {cardID: 1, merchant: 'Transaction E'});
Onyx.merge(`transactionViolations_(transaction id copied for E)`, [{type: 'test', name: 'rter', data: {pendingPattern: true}}]);
text and icon is displayed in Money Request Preview and in headers. For reference check video -> MacOS: Chrome / Safari |
@kbecciv Above are steps to QA this PR. |
🚀 Deployed to production by https://github.com/puneetlath in version: 1.4.75-1 🚀
|
Details
Fixed Issues
$#39533
PROPOSAL:
Tests
Create a single Money Request on a different chat. It should display as preview of single request and not aggregate like in point 1.
Create a Money Request in a new workspace (so called One Expense Chat, for more context check Add One Transaction Report View #36934).
Find transactionIDs of all created requests by using
console.log
to be able to mock the data.Mock the data in
ReportActionList.tsx
similarly to how it was done before this commit: 03fb6eeCheck whether
text and icon is displayed in Money Request Preview and in headers. For reference check video -> MacOS: Chrome / Safari
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
For more context on how to navigate in the app to get to screens from screenshots check MacOS: Chrome section.
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2024-05-07.at.10.31.16.mov
ReportPreview
MoneyReportHeader
MoneyReportHeader & MoneyRequestAction (MoneyReportHeader only if all the transactions have RTER violation)
MoneyRequestHeader
MacOS: Desktop