-
Notifications
You must be signed in to change notification settings - Fork 4k
[Simplified Actions] Implement DeleteAppReport #58020
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
Changes from all commits
82f636e
04737c3
a67f18f
1615ce2
f161477
6333b12
8a59591
29e383b
6cca61c
9cbf409
5b721c2
5d12b7f
19d72ad
ac728eb
f1ce165
35e06c5
bc904c7
8106089
527ba70
1ec0bfc
ae72bf6
9f416a2
8b07977
9fbc154
78e96c3
bf65471
be88e98
a53e566
6121e5c
c38491a
c36e1c0
9f261e2
557caa0
324b779
602d991
f388b13
ba2b8e5
e9ef5c5
a90bd0d
df01e66
7f22c66
7420bb6
19b6020
bd8d7ce
b9831de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -186,7 +186,6 @@ import type { | |
| TransferParams, | ||
| TrialStartedTitleParams, | ||
| UnapproveWithIntegrationWarningParams, | ||
| UnreportedTransactionParams, | ||
| UnshareParams, | ||
| UntilTimeParams, | ||
| UpdatedCustomFieldParams, | ||
|
|
@@ -952,7 +951,7 @@ const translations = { | |
| deleteReceipt: 'Delete receipt', | ||
| deletedTransaction: ({amount, merchant}: DeleteTransactionParams) => `deleted an expense on this report, ${merchant} - ${amount}`, | ||
| movedTransaction: ({reportUrl, reportName}: MovedTransactionParams) => `moved this expense to <a href="${reportUrl}">${reportName}</a>`, | ||
| unreportedTransaction: ({reportUrl, reportName}: UnreportedTransactionParams) => `removed this expense from <a href="${reportUrl}">${reportName}</a>`, | ||
| unreportedTransaction: 'moved this expense to your personal space', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was missing a hyperlink so we updated it in #68586
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hey i think we didn't want a hyperlink at the time of the PR, so i don't this this PR would be the offending one, please update the C+ checklist accordingly 😄 c.c. @luacmartins |
||
| pendingMatchWithCreditCard: 'Receipt pending match with card transaction', | ||
| pendingMatch: 'Pending match', | ||
| pendingMatchWithCreditCardDescription: 'Receipt pending match with card transaction. Mark as cash to cancel.', | ||
|
|
@@ -1016,6 +1015,8 @@ const translations = { | |
| one: 'Are you sure that you want to delete this expense?', | ||
| other: 'Are you sure that you want to delete these expenses?', | ||
| }), | ||
| deleteReport: 'Delete report', | ||
| deleteReportConfirmation: 'Are you sure that you want to delete this report?', | ||
| settledExpensify: 'Paid', | ||
| done: 'Done', | ||
| settledElsewhere: 'Paid elsewhere', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| type DeleteAppReportParams = { | ||
| reportID: string; | ||
| transactionIDToReportActionAndThreadData?: string; | ||
| selfDMReportID?: string; | ||
| selfDMCreatedReportActionID?: string; | ||
| }; | ||
|
|
||
| export default DeleteAppReportParams; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -338,15 +338,11 @@ function isMoveTransactionAction(reportTransactions: Transaction[], reportAction | |
| return canMoveExpense; | ||
| } | ||
|
|
||
| function isDeleteAction(report: Report, reportTransactions: Transaction[], reportActions?: ReportAction[]): boolean { | ||
| const transactionThreadReportID = getOneTransactionThreadReportID(report.reportID, reportActions ?? []); | ||
| function isDeleteAction(report: Report): boolean { | ||
|
allgandalf marked this conversation as resolved.
|
||
| const isExpenseReport = isExpenseReportUtils(report); | ||
| const isIOUReport = isIOUReportUtils(report); | ||
|
|
||
| // This should be removed when is merged https://github.com/Expensify/App/pull/58020 | ||
| const isSingleTransaction = reportTransactions.length === 1; | ||
|
|
||
| if ((!isExpenseReport && !isIOUReport) || !isSingleTransaction || (!!reportActions && !transactionThreadReportID)) { | ||
| if (!isExpenseReport && !isIOUReport) { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -448,7 +444,7 @@ function getSecondaryReportActions( | |
|
|
||
| options.push(CONST.REPORT.SECONDARY_ACTIONS.VIEW_DETAILS); | ||
|
|
||
| if (isDeleteAction(report, reportTransactions, reportActions)) { | ||
| if (isDeleteAction(report)) { | ||
| options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should update the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @allgandalf we're also missing the action setup here (the current one is for the delete expense case) so we need to add one for the delete report case. And the correct modal/callback here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Working on this Now |
||
| } | ||
|
|
||
|
|
@@ -464,10 +460,10 @@ function getSecondaryTransactionThreadActions(parentReport: Report, reportTransa | |
|
|
||
| options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.VIEW_DETAILS); | ||
|
|
||
| if (isDeleteAction(parentReport, [reportTransaction])) { | ||
| if (isDeleteAction(parentReport)) { | ||
| options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.DELETE); | ||
| } | ||
|
|
||
| return options; | ||
| } | ||
| export {getSecondaryReportActions, getSecondaryTransactionThreadActions}; | ||
| export {getSecondaryReportActions, getSecondaryTransactionThreadActions, isDeleteAction}; | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line caused a bug - #63094 (comment)
we should have waiting for deleteAppReport to complete before navigating