Simplified Actions on Report Header and Preview V2#59999
Conversation
melvin-bot
bot
requested review from
MarioExpensify
and removed request for
a team
|
@MarioExpensify 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] |
SzymczakJ
force-pushed
the
jnowakow/simplified-actions-on-header-v2
branch
from
b493343 to
59cca89
Compare
luacmartins
requested review from
luacmartins
and removed request for
MarioExpensify
…ied-actions-on-header-v2
…-actions-on-header-v2
|
This is a large PR and has many unrelated ESLint errors. I'm gonna merge it as is and we can tackle those in a follow up. |
|
@luacmartins looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
See comment above |
Related to eslint error by rule |
|
✋ 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/luacmartins in version: 9.1.33-0 🚀
|
| const shouldShowSettlementButton = !shouldShowSubmitButton && (shouldShowPayButton || shouldShowApproveButton) && !showRTERViolationMessage && !shouldShowBrokenConnectionViolation; | ||
|
|
||
| const shouldPromptUserToAddBankAccount = (hasMissingPaymentMethod(userWallet, iouReportID) || hasMissingInvoiceBankAccount(iouReportID)) && !iouSettled; | ||
| const shouldPromptUserToAddBankAccount = (hasMissingPaymentMethod(userWallet, iouReportID) || hasMissingInvoiceBankAccount(iouReportID)) && !isSettled(iouReportID); |
There was a problem hiding this comment.
I think the changes in this PR may be causing this deploy blocker: #61005
Not 100% sure because it can't be reverted easily
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.35-1 🚀
|
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.36-3 🚀
|
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.37-1 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.37-3 🚀
|
| const goBackRoute = getNavigationUrlOnMoneyRequestDelete(transaction.transactionID, parentReportAction, true); | ||
| navigateBackOnDeleteTransaction(goBackRoute); |
There was a problem hiding this comment.
We should have just called the onBackButtonPress function here. This redirection logic caused #62075 later on.
| return parentReport?.invoiceReceiver?.accountID === getCurrentUserAccountID(); | ||
| } | ||
|
|
||
| return policy?.role === CONST.POLICY.ROLE.ADMIN; |
There was a problem hiding this comment.
Coming from #61315, we should have checked the invoice receiver policy.
| ), | ||
| [CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE]: ( | ||
| <Button | ||
| text={translate('iou.approve')} |
| onSelected: () => { | ||
| if (!moneyRequestReport) { | ||
| return; | ||
| } | ||
| approveMoneyRequest(moneyRequestReport); | ||
| }, |
|
|
||
| setIsDeleteRequestModalVisible(false); | ||
| }, [canDeleteRequest]); | ||
| const applicableSecondaryActions = secondaryActions.map((action) => secondaryActionsImplemenation[action]); |
There was a problem hiding this comment.
Came from this issue when primary action is again shown in the secondary action for the submitter
More context here
| const submitToAccountID = getSubmitToAccountID(policy, report); | ||
|
|
||
| if (submitToAccountID === report.ownerAccountID && policy?.preventSelfApproval) { | ||
| return false; | ||
| } | ||
|
|
||
| return isExpenseReport && isReportSubmitter && isOpenReport && isManualSubmitEnabled; | ||
| return isExpenseReport && isReportSubmitter && isOpenReport && isManualSubmitEnabled && reportTransactions.length !== 0 && transactionAreComplete; |
There was a problem hiding this comment.
There's missing case of submit button being shown for second admin when Prevent self-approval is enabled.
17 participants
This reverts commit 3daab6e.
cc @luacmartins @borys3kk
Explanation of Change
Fixed Issues
$ #59963
$ #59960
PROPOSAL:
Tests
Test cases:
Prerequisites: Workspace without accounting with approvals enabled, workspace with connected accounting system
Test case 1:
Test case 2:
Test case 3:
Test case 4:
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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