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.

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

[CP Staging] Fix empty workspace detail in money request page #27742

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 1 deletion src/components/OptionsList/BaseOptionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,11 @@ function BaseOptionsList({
const renderItem = ({item, index, section}) => {
const isItemDisabled = isDisabled || section.isDisabled || !!item.isDisabled;
const isSelected = _.some(selectedOptions, (option) => {
if (option.accountID === item.accountID) {
if (option.accountID && option.accountID === item.accountID) {
return true;
}

if (option.reportID && option.reportID === item.reportID) {
return true;
}

Expand Down
9 changes: 8 additions & 1 deletion src/libs/MoneyRequestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,11 @@ function isDistanceRequest(iouType: ValueOf<typeof CONST.IOU.MONEY_REQUEST_TYPE>
return iouType === CONST.IOU.MONEY_REQUEST_TYPE.REQUEST && selectedTab === CONST.TAB.DISTANCE;
}

export {stripCommaFromAmount, stripSpacesFromAmount, addLeadingZero, validateAmount, replaceAllDigits, isDistanceRequest};
/**
* Check if scan request or not
*/
function isScanRequest(selectedTab: ValueOf<typeof CONST.TAB>): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is inconsistent with isDistanceRequest which also accepts iouType as param but I think that's fine as we'll be supporting split bill on scan request as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I didn't include that because of that reason

return selectedTab === CONST.TAB.SCAN;
}

export {stripCommaFromAmount, stripSpacesFromAmount, addLeadingZero, validateAmount, replaceAllDigits, isDistanceRequest, isScanRequest};
113 changes: 56 additions & 57 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,31 +92,29 @@ Onyx.connect({
});

/**
* Get the options for a policy expense report.
* Get the option for a policy expense report.
* @param {Object} report
* @returns {Array}
* @returns {Object}
*/
function getPolicyExpenseReportOptions(report) {
function getPolicyExpenseReportOption(report) {
const expenseReport = policyExpenseReports[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`];
const policyExpenseChatAvatarSource = ReportUtils.getWorkspaceAvatar(expenseReport);
const reportName = ReportUtils.getReportName(expenseReport);
return [
{
...expenseReport,
keyForList: expenseReport.policyID,
text: reportName,
alternateText: Localize.translateLocal('workspace.common.workspace'),
icons: [
{
source: policyExpenseChatAvatarSource,
name: reportName,
type: CONST.ICON_TYPE_WORKSPACE,
},
],
selected: report.selected,
isPolicyExpenseChat: true,
},
];
return {
...expenseReport,
keyForList: expenseReport.policyID,
text: reportName,
alternateText: Localize.translateLocal('workspace.common.workspace'),
icons: [
{
source: policyExpenseChatAvatarSource,
name: reportName,
type: CONST.ICON_TYPE_WORKSPACE,
},
],
selected: report.selected,
isPolicyExpenseChat: true,
};
}

/**
Expand Down Expand Up @@ -201,37 +199,34 @@ function isPersonalDetailsReady(personalDetails) {
}

/**
* Get the participant options for a report.
* @param {Array<Object>} participants
* Get the participant option for a report.
* @param {Object} participant
* @param {Array<Object>} personalDetails
* @returns {Array}
* @returns {Object}
*/
function getParticipantsOptions(participants, personalDetails) {
const details = getPersonalDetailsForAccountIDs(_.pluck(participants, 'accountID'), personalDetails);
return _.map(participants, (participant) => {
const detail = details[participant.accountID];
const login = detail.login || participant.login;
const displayName = detail.displayName || LocalePhoneNumber.formatPhoneNumber(login);
return {
keyForList: String(detail.accountID),
login,
accountID: detail.accountID,
text: displayName,
firstName: lodashGet(detail, 'firstName', ''),
lastName: lodashGet(detail, 'lastName', ''),
alternateText: LocalePhoneNumber.formatPhoneNumber(login) || displayName,
icons: [
{
source: UserUtils.getAvatar(detail.avatar, detail.accountID),
name: login,
type: CONST.ICON_TYPE_AVATAR,
id: detail.accountID,
},
],
phoneNumber: lodashGet(detail, 'phoneNumber', ''),
selected: participant.selected,
};
});
function getParticipantsOption(participant, personalDetails) {
const detail = getPersonalDetailsForAccountIDs([participant.accountID], personalDetails)[participant.accountID];
const login = detail.login || participant.login;
const displayName = detail.displayName || LocalePhoneNumber.formatPhoneNumber(login);
return {
keyForList: String(detail.accountID),
login,
accountID: detail.accountID,
text: displayName,
firstName: lodashGet(detail, 'firstName', ''),
lastName: lodashGet(detail, 'lastName', ''),
alternateText: LocalePhoneNumber.formatPhoneNumber(login) || displayName,
icons: [
{
source: UserUtils.getAvatar(detail.avatar, detail.accountID),
name: login,
type: CONST.ICON_TYPE_AVATAR,
id: detail.accountID,
},
],
phoneNumber: lodashGet(detail, 'phoneNumber', ''),
selected: participant.selected,
};
}

/**
Expand Down Expand Up @@ -895,10 +890,10 @@ function getOptions(
}

// Always exclude already selected options and the currently logged in user
const loginOptionsToExclude = [...selectedOptions, {login: currentUserLogin}];
const optionsToExclude = [...selectedOptions, {login: currentUserLogin}];

_.each(excludeLogins, (login) => {
loginOptionsToExclude.push({login});
optionsToExclude.push({login});
});

if (includeRecentReports) {
Expand All @@ -919,7 +914,11 @@ function getOptions(
}

// If we're excluding threads, check the report to see if it has a single participant and if the participant is already selected
if (!includeThreads && reportOption.login && _.some(loginOptionsToExclude, (option) => option.login === reportOption.login)) {
if (
!includeThreads &&
(reportOption.login || reportOption.reportID) &&
_.some(optionsToExclude, (option) => (option.login && option.login === reportOption.login) || (option.reportID && option.reportID === reportOption.reportID))
) {
continue;
}

Expand All @@ -943,15 +942,15 @@ function getOptions(

// Add this login to the exclude list so it won't appear when we process the personal details
if (reportOption.login) {
loginOptionsToExclude.push({login: reportOption.login});
optionsToExclude.push({login: reportOption.login});
}
}
}

if (includePersonalDetails) {
// Next loop over all personal details removing any that are selectedUsers or recentChats
_.each(allPersonalDetailsOptions, (personalDetailOption) => {
if (_.some(loginOptionsToExclude, (loginOptionToExclude) => loginOptionToExclude.login === personalDetailOption.login)) {
if (_.some(optionsToExclude, (optionToExclude) => optionToExclude.login === personalDetailOption.login)) {
return;
}
const {searchText, participantsList, isChatRoom} = personalDetailOption;
Expand Down Expand Up @@ -979,7 +978,7 @@ function getOptions(
_.every(selectedOptions, (option) => option.login !== searchValue) &&
((Str.isValidEmail(searchValue) && !Str.isDomainEmail(searchValue) && !Str.endsWith(searchValue, CONST.SMS.DOMAIN)) ||
(parsedPhoneNumber.possible && Str.isValidPhone(LoginUtils.getPhoneNumberWithoutSpecialChars(parsedPhoneNumber.number.input)))) &&
!_.find(loginOptionsToExclude, (loginOptionToExclude) => loginOptionToExclude.login === addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) &&
!_.find(optionsToExclude, (optionToExclude) => optionToExclude.login === addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) &&
(searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas)) &&
!excludeUnknownUsers
) {
Expand Down Expand Up @@ -1322,8 +1321,8 @@ export {
getIOUConfirmationOptionsFromParticipants,
getSearchText,
getAllReportErrors,
getPolicyExpenseReportOptions,
getParticipantsOptions,
getPolicyExpenseReportOption,
getParticipantsOption,
isSearchStringMatch,
shouldOptionShowTooltip,
getLastMessageTextForReport,
Expand Down
8 changes: 5 additions & 3 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -676,9 +676,10 @@ function requestMoney(report, amount, currency, created, merchant, payeeEmail, p
function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, existingSplitChatReportID = '') {
const currentUserEmailForIOUSplit = OptionsListUtils.addSMSDomainIfPhoneNumber(currentUserLogin);
const participantAccountIDs = _.map(participants, (participant) => Number(participant.accountID));
const existingSplitChatReport = existingSplitChatReportID
? allReports[`${ONYXKEYS.COLLECTION.REPORT}${existingSplitChatReportID}`]
: ReportUtils.getChatByParticipants(participantAccountIDs);
const existingSplitChatReport =
existingSplitChatReportID || participants[0].reportID
? allReports[`${ONYXKEYS.COLLECTION.REPORT}${existingSplitChatReportID || participants[0].reportID}`]
: ReportUtils.getChatByParticipants(participantAccountIDs);
const splitChatReport = existingSplitChatReport || ReportUtils.buildOptimisticChatReport(participantAccountIDs);
const isOwnPolicyExpenseChat = splitChatReport.isOwnPolicyExpenseChat;

Expand Down Expand Up @@ -1023,6 +1024,7 @@ function splitBillAndOpenReport(participants, currentUserLogin, currentUserAccou
transactionID: splitData.transactionID,
reportActionID: splitData.reportActionID,
createdReportActionID: splitData.createdReportActionID,
policyID: splitData.policyID,
},
onyxData,
);
Expand Down
9 changes: 3 additions & 6 deletions src/pages/iou/SplitBillDetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,11 @@ function SplitBillDetailsPage(props) {
let participants;
if (ReportUtils.isPolicyExpenseChat(props.report)) {
participants = [
...OptionsListUtils.getParticipantsOptions([{accountID: participantAccountIDs[0], selected: true}], props.personalDetails),
...OptionsListUtils.getPolicyExpenseReportOptions({...props.report, selected: true}),
OptionsListUtils.getParticipantsOption({accountID: participantAccountIDs[0], selected: true}, props.personalDetails),
OptionsListUtils.getPolicyExpenseReportOption({...props.report, selected: true}),
];
} else {
participants = OptionsListUtils.getParticipantsOptions(
_.map(participantAccountIDs, (accountID) => ({accountID, selected: true})),
props.personalDetails,
);
participants = _.map(participantAccountIDs, (accountID) => OptionsListUtils.getParticipantsOption({accountID, selected: true}, props.personalDetails));
}
const payeePersonalDetails = props.personalDetails[reportAction.actorAccountID];
const participantsExcludingPayee = _.filter(participants, (participant) => participant.accountID !== reportAction.actorAccountID);
Expand Down
7 changes: 4 additions & 3 deletions src/pages/iou/steps/MoneyRequestConfirmPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ function MoneyRequestConfirmPage(props) {
const reportID = useRef(lodashGet(props.route, 'params.reportID', ''));
const participants = useMemo(
() =>
lodashGet(props.iou.participants, [0, 'isPolicyExpenseChat'], false)
? OptionsListUtils.getPolicyExpenseReportOptions(props.iou.participants[0])
: OptionsListUtils.getParticipantsOptions(props.iou.participants, props.personalDetails),
_.map(props.iou.participants, (participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? OptionsListUtils.getPolicyExpenseReportOption(participant) : OptionsListUtils.getParticipantsOption(participant, props.personalDetails);
}),
[props.iou.participants, props.personalDetails],
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function MoneyRequestParticipantsPage({iou, selectedTab, route}) {
const iouType = useRef(lodashGet(route, 'params.iouType', ''));
const reportID = useRef(lodashGet(route, 'params.reportID', ''));
const isDistanceRequest = MoneyRequestUtils.isDistanceRequest(iouType.current, selectedTab);
const isScanRequest = MoneyRequestUtils.isScanRequest(selectedTab);
const isSplitRequest = iou.id === CONST.IOU.MONEY_REQUEST_TYPE.SPLIT;
const [headerTitle, setHeaderTitle] = useState();

Expand Down Expand Up @@ -117,6 +118,7 @@ function MoneyRequestParticipantsPage({iou, selectedTab, route}) {
safeAreaPaddingBottomStyle={safeAreaPaddingBottomStyle}
iouType={iouType.current}
isDistanceRequest={isDistanceRequest}
isScanRequest={isScanRequest}
/>
</View>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {View} from 'react-native';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import lodashGet from 'lodash/get';
import ONYXKEYS from '../../../../ONYXKEYS';
import styles from '../../../../styles/styles';
import OptionsSelector from '../../../../components/OptionsSelector';
Expand Down Expand Up @@ -58,6 +59,9 @@ const propTypes = {
/** Whether the money request is a distance request or not */
isDistanceRequest: PropTypes.bool,

/** Whether the money request is a scan request or not */
isScanRequest: PropTypes.bool,

...withLocalizePropTypes,
};

Expand All @@ -69,6 +73,7 @@ const defaultProps = {
reports: {},
betas: [],
isDistanceRequest: false,
isScanRequest: false,
};

function MoneyRequestParticipantsSelector({
Expand All @@ -84,6 +89,7 @@ function MoneyRequestParticipantsSelector({
safeAreaPaddingBottomStyle,
iouType,
isDistanceRequest,
isScanRequest,
}) {
const [searchTerm, setSearchTerm] = useState('');
const [newChatOptions, setNewChatOptions] = useState({
Expand All @@ -105,7 +111,10 @@ function MoneyRequestParticipantsSelector({

newSections.push({
title: undefined,
data: OptionsListUtils.getParticipantsOptions(participants, personalDetails),
data: _.map(participants, (participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? OptionsListUtils.getPolicyExpenseReportOption(participant) : OptionsListUtils.getParticipantsOption(participant, personalDetails);
}),
shouldShow: true,
indexOffset,
});
Expand Down Expand Up @@ -159,14 +168,27 @@ function MoneyRequestParticipantsSelector({
*/
const addParticipantToSelection = useCallback(
(option) => {
const isOptionInList = _.some(participants, (selectedOption) => selectedOption.accountID === option.accountID);

const isOptionSelected = (selectedOption) => {
if (selectedOption.accountID && selectedOption.accountID === option.accountID) {
return true;
}

if (selectedOption.reportID && selectedOption.reportID === option.reportID) {
return true;
}

return false;
};
const isOptionInList = _.some(participants, isOptionSelected);
let newSelectedOptions;

if (isOptionInList) {
newSelectedOptions = _.reject(participants, (selectedOption) => selectedOption.accountID === option.accountID);
newSelectedOptions = _.reject(participants, isOptionSelected);
} else {
newSelectedOptions = [...participants, {accountID: option.accountID, login: option.login, selected: true}];
newSelectedOptions = [
...participants,
{accountID: option.accountID, login: option.login, isPolicyExpenseChat: option.isPolicyExpenseChat, reportID: option.reportID, selected: true},
];
}

onAddParticipants(newSelectedOptions);
Expand Down Expand Up @@ -227,10 +249,17 @@ function MoneyRequestParticipantsSelector({
});
}, [betas, reports, participants, personalDetails, translate, searchTerm, setNewChatOptions, iouType, isDistanceRequest]);

// Right now you can't split a request with a workspace and other additional participants
// This is getting properly fixed in https://github.com/Expensify/App/issues/27508, but as a stop-gap to prevent
// the app from crashing on native when you try to do this, we'll going to hide the button if you have a workspace and other participants
const hasPolicyExpenseChatParticipant = _.some(participants, (participant) => participant.isPolicyExpenseChat);
const shouldShowConfirmButton = !(participants.length > 1 && hasPolicyExpenseChatParticipant);
const isAllowedToSplit = !isDistanceRequest && !isScanRequest;

return (
<View style={[styles.flex1, styles.w100, participants.length > 0 ? safeAreaPaddingBottomStyle : {}]}>
<OptionsSelector
canSelectMultipleOptions
canSelectMultipleOptions={isAllowedToSplit}
shouldShowMultipleOptionSelectorAsButton
multipleOptionSelectorButtonText={translate('iou.split')}
onAddToSelection={addParticipantToSelection}
Expand All @@ -242,7 +271,7 @@ function MoneyRequestParticipantsSelector({
ref={forwardedRef}
headerMessage={headerMessage}
boldStyle
shouldShowConfirmButton
shouldShowConfirmButton={shouldShowConfirmButton && isAllowedToSplit}
confirmButtonText={translate('iou.addToSplit')}
onConfirmSelection={navigateToSplit}
textInputLabel={translate('optionsSelector.nameEmailOrPhoneNumber')}
Expand Down
Loading