Skip to content
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.

Sign up for GitHub

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

Prevent additional requests on settled request reports #19033

Merged
merged 11 commits into from
May 17, 2023
Merged

Prevent additional requests on settled request reports #19033

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d1faf96
prevent additional requests on settled request reports
Julesssss May 16, 2023
aa43c09
block send money option in all non-chat reports
Julesssss May 16, 2023
82a6ddb
resolve conflicts
luacmartins May 17, 2023
7b4ffdf
Merge branch 'main' into jules-preventRequestsOnSettledReports
mountiny May 17, 2023
5d8e419
Show the correct options for money requests
mountiny May 17, 2023
2783f0a
Merge branch 'main' into jules-preventRequestsOnSettledReports
mountiny May 17, 2023
e6f4e8b
Fix tests
mountiny May 17, 2023
872ca23
Make sure the Money request header works for the sent money requests too
mountiny May 17, 2023
0f218a7
Cast to Boolean to get rid of console warning
mountiny May 17, 2023
8f8e58f
Revert podfile lock change
mountiny May 17, 2023
f2631e0
Use has to find if a IOUDetails field exists
mountiny May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/components/MoneyRequestHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@ const defaultProps = {
};

const MoneyRequestHeader = (props) => {
// These are only used for the single transaction view and not "money requests"
const transactionAmount = lodashGet(props.parentReportAction, ['originalMessage', 'amount']);
const transactionCurrency = lodashGet(props.parentReportAction, ['originalMessage', 'currency']);
const transactionDescription = lodashGet(props.parentReportAction, ['originalMessage', 'comment']);
// These are only used for the single transaction view and not for expense and iou reports
const {amount: transactionAmount, currency: transactionCurrency, comment: transactionDescription} = ReportUtils.getMoneyRequestAction(props.parentReportAction);
const formattedTransactionAmount = transactionAmount && transactionCurrency && CurrencyUtils.convertToDisplayString(transactionAmount, transactionCurrency);
const transactionDate = lodashGet(props.parentReportAction, ['created']);
const formattedTransactionDate = DateUtils.getDateStringFromISOTimestamp(transactionDate);
Expand Down
5 changes: 1 addition & 4 deletions src/components/ReportWelcomeText.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,7 @@ const ReportWelcomeText = (props) => {
</Text>
)}
{(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.SEND) || moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.REQUEST)) && (
<Text>
{/* Need to confirm copy for the below with marketing, and then add to translations. */}
{props.translate('reportActionsView.usePlusButton')}
</Text>
<Text>{props.translate('reportActionsView.usePlusButton')}</Text>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just confirmed in slack the comment is not needed anymore

)}
</Text>
</>
Expand Down
6 changes: 3 additions & 3 deletions src/libs/ReportActionsUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ function getFormattedAmount(reportAction) {
*/
function isTransactionThread(parentReportAction) {
const originalMessage = lodashGet(parentReportAction, 'originalMessage', {});
return (
return Boolean(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: @mountiny why is this change needed?

The logical operators should return a boolean anyway, right?

Copy link
Member

@rushatgabhane rushatgabhane May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nvm, just saw your comment after a page refresh https://github.com/Expensify/App/pull/19033/files#r1197091432

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

parentReportAction &&
parentReportAction.actionName === CONST.REPORT.ACTIONS.TYPE.IOU &&
(originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE || (originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.PAY && originalMessage.IOUDetails))
parentReportAction.actionName === CONST.REPORT.ACTIONS.TYPE.IOU &&
(originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE || (originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.PAY && originalMessage.IOUDetails)),
);
mountiny marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
14 changes: 13 additions & 1 deletion src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2080,6 +2080,11 @@ function canRequestMoney(report) {
* @returns {Array}
*/
function getMoneyRequestOptions(report, reportParticipants, betas) {
// In the transaction thread, we do not allow any new money requests
if (ReportActionsUtils.isTransactionThread(ReportActionsUtils.getParentReportAction(report))) {
return [];
}

const participants = _.filter(reportParticipants, (email) => currentUserPersonalDetails.login !== email);
const hasExcludedIOUEmails = lodashIntersection(reportParticipants, CONST.EXPENSIFY_EMAILS).length > 0;
const hasMultipleParticipants = participants.length > 1;
Expand All @@ -2088,6 +2093,11 @@ function getMoneyRequestOptions(report, reportParticipants, betas) {
return [];
}

// Additional requests should be blocked for money request reports
if (isMoneyRequestReport(report)) {
return [];
}

// User created policy rooms and default rooms like #admins or #announce will always have the Split Bill option
// unless there are no participants at all (e.g. #admins room for a policy with only 1 admin)
// DM chats will have the Split Bill option only when there are at least 3 people in the chat.
Expand All @@ -2100,7 +2110,9 @@ function getMoneyRequestOptions(report, reportParticipants, betas) {
// Workspace chats should only see the Request money option, as "easy overages" is not available.
return [
...(canRequestMoney(report) ? [CONST.IOU.MONEY_REQUEST_TYPE.REQUEST] : []),
...(Permissions.canUseIOUSend(betas) && !isPolicyExpenseChat(report) ? [CONST.IOU.MONEY_REQUEST_TYPE.SEND] : []),

// Send money option should be visible only in DMs
...(Permissions.canUseIOUSend(betas) && isChatReport(report) && !isPolicyExpenseChat(report) && participants.length === 1 ? [CONST.IOU.MONEY_REQUEST_TYPE.SEND] : []),
];
}

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/ReportUtilsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,8 @@ describe('ReportUtils', () => {
});
});

it('return both iou send and request money', () => {
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions({}, [currentUserEmail, participants], [CONST.BETAS.IOU, CONST.BETAS.IOU_SEND]);
it('return both iou send and request money in DM', () => {
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions({type: 'chat'}, [currentUserEmail, participants[0]], [CONST.BETAS.IOU, CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(2);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.REQUEST)).toBe(true);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.SEND)).toBe(true);
Expand Down