-
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
[Manual Requests] Create thread for Manual Request when opening IOUPreview #18760
Changes from 20 commits
202122a
fa1b3dc
e30b603
a5e043e
94c9125
94c19d6
b2c926f
6e3bf2c
b4a991b
c0e2430
4cfefbb
9d518ba
f83e240
03f8c6f
dfc4e79
33afd33
6e6eb06
35c3c79
22f9a4a
bcb6855
3ec51b5
9acde61
e9d4c4a
df1462a
42db583
e800564
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 |
---|---|---|
|
@@ -16,6 +16,11 @@ import Navigation from '../../libs/Navigation/Navigation'; | |
import ROUTES from '../../ROUTES'; | ||
import styles from '../../styles/styles'; | ||
import * as IOUUtils from '../../libs/IOUUtils'; | ||
import * as OptionsListUtils from '../../libs/OptionsListUtils'; | ||
import * as ReportUtils from '../../libs/ReportUtils'; | ||
import * as Report from '../../libs/actions/Report'; | ||
import withLocalize, {withLocalizePropTypes} from '../withLocalize'; | ||
import * as CurrencyUtils from '../../libs/CurrencyUtils'; | ||
|
||
const propTypes = { | ||
/** All the data of the action */ | ||
|
@@ -56,6 +61,14 @@ const propTypes = { | |
isHovered: PropTypes.bool, | ||
|
||
network: networkPropTypes.isRequired, | ||
|
||
/** Session info for the currently logged in user. */ | ||
session: PropTypes.shape({ | ||
/** Currently logged in user email */ | ||
email: PropTypes.string, | ||
}), | ||
|
||
...withLocalizePropTypes, | ||
}; | ||
|
||
const defaultProps = { | ||
|
@@ -67,15 +80,46 @@ const defaultProps = { | |
iouReport: {}, | ||
reportActions: {}, | ||
isHovered: false, | ||
session: { | ||
email: null, | ||
}, | ||
}; | ||
|
||
const MoneyRequestAction = (props) => { | ||
const hasMultipleParticipants = lodashGet(props.chatReport, 'participants', []).length > 1; | ||
const onIOUPreviewPressed = () => { | ||
if (hasMultipleParticipants) { | ||
if (lodashGet(props.action, 'originalMessage.type', '') === CONST.IOU.REPORT_ACTION_TYPE.SPLIT && hasMultipleParticipants) { | ||
Navigation.navigate(ROUTES.getReportParticipantsRoute(props.chatReportID)); | ||
return; | ||
} | ||
|
||
// If the childReportID is not present, we need to create a new thread | ||
const childReportID = lodashGet(props.action, 'childReportID', '0'); | ||
if (childReportID === '0') { | ||
const participants = _.uniq([props.session.email, props.action.actorEmail]); | ||
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. 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. it did not work for me that well when I had that before 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. 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. Can we just filter those out? 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. @mountiny To make sure we are on the same page , the PR checklist was created with the changes that I suggested : const participants = lodashGet(props.action, 'originalMessage.participants'); With this actual code, the report is created for user 1 and shared to the DM but the user can’t access the report , That's because participants doesn’t include user2 mail const participants = _.uniq([props.session.email, props.action.actorEmail]); |
||
const formattedUserLogins = _.map(participants, (login) => OptionsListUtils.addSMSDomainIfPhoneNumber(login).toLowerCase()); | ||
const thread = ReportUtils.buildOptimisticChatReport( | ||
formattedUserLogins, | ||
props.translate('iou.threadReportName', { | ||
formattedAmount: CurrencyUtils.convertToDisplayString(lodashGet(props.action, 'originalMessage.amount', 0), lodashGet(props.action, 'originalMessage.currency', '')), | ||
comment: props.action.originalMessage.comment, | ||
}), | ||
'', | ||
CONST.POLICY.OWNER_EMAIL_FAKE, | ||
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. Coming from IOU - Workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view:
|
||
CONST.POLICY.OWNER_EMAIL_FAKE, | ||
false, | ||
'', | ||
undefined, | ||
CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, | ||
props.action.reportActionID, | ||
props.chatReportID, // Needs to be changed to iouReportID | ||
); | ||
Comment on lines
+99
to
+116
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. Eventually we should move this to reportUtils but now for urgency just leaving this pragmatically here, I hope that works 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. Yea, we should create an utils method. Seems ok to leave it for now given that this is blocking other issues |
||
|
||
Report.openReport(thread.reportID, thread.participants, thread, props.action.reportActionID); | ||
Navigation.navigate(ROUTES.getReportRoute(thread.reportID)); | ||
} else { | ||
Navigation.navigate(ROUTES.getIouDetailsRoute(props.chatReportID, props.action.originalMessage.IOUReportID)); | ||
Report.openReport(childReportID); | ||
Navigation.navigate(ROUTES.getReportRoute(childReportID)); | ||
} | ||
}; | ||
|
||
|
@@ -125,6 +169,7 @@ MoneyRequestAction.defaultProps = defaultProps; | |
MoneyRequestAction.displayName = 'MoneyRequestAction'; | ||
|
||
export default compose( | ||
withLocalize, | ||
withOnyx({ | ||
chatReport: { | ||
key: ({chatReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${chatReportID}`, | ||
|
@@ -136,6 +181,9 @@ export default compose( | |
key: ({chatReportID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReportID}`, | ||
canEvict: false, | ||
}, | ||
session: { | ||
key: ONYXKEYS.SESSION, | ||
}, | ||
}), | ||
withNetwork(), | ||
)(MoneyRequestAction); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1001,6 +1001,19 @@ function getMoneyRequestReportName(report) { | |
return Localize.translateLocal('iou.payerOwesAmount', {payer: payerName, amount: formattedAmount}); | ||
} | ||
|
||
/** | ||
* Given a parent IOU report action get report name for the LHN. | ||
* | ||
* @param {Object} reportAction | ||
* @returns {String} | ||
*/ | ||
function getTransactionReportName(reportAction) { | ||
return Localize.translateLocal('iou.threadReportName', { | ||
formattedAmount: CurrencyUtils.convertToDisplayString(lodashGet(reportAction, 'originalMessage.amount', 0), lodashGet(reportAction, 'originalMessage.currency', '')), | ||
comment: lodashGet(reportAction, 'originalMessage.comment'), | ||
}); | ||
} | ||
|
||
/** | ||
* Get the title for a report. | ||
* | ||
|
@@ -1011,6 +1024,9 @@ function getReportName(report) { | |
let formattedName; | ||
if (isThread(report)) { | ||
const parentReportAction = ReportActionsUtils.getParentReportAction(report); | ||
if (ReportActionsUtils.isTransactionThread(parentReportAction)) { | ||
return getTransactionReportName(parentReportAction); | ||
Comment on lines
+1027
to
+1028
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. The reportName is specific for the transaction threads so hijackign this flow |
||
} | ||
const parentReportActionMessage = lodashGet(parentReportAction, ['message', 0, 'text'], '').replace(/(\r\n|\n|\r)/gm, ' '); | ||
return parentReportActionMessage || Localize.translateLocal('parentReportAction.deletedMessage'); | ||
} | ||
|
@@ -1388,8 +1404,8 @@ function buildOptimisticChatReport( | |
oldPolicyName = '', | ||
visibility = undefined, | ||
notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, | ||
parentReportActionID, | ||
parentReportID, | ||
parentReportActionID = '', | ||
bondydaa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
parentReportID = '', | ||
) { | ||
const currentTime = DateUtils.getDBTime(); | ||
return { | ||
|
@@ -2035,6 +2051,22 @@ function getWhisperDisplayNames(participants) { | |
return _.map(participants, (login) => getDisplayNameForParticipant(login, !isWhisperOnlyVisibleToCurrentUSer)).join(', '); | ||
} | ||
|
||
/** | ||
* Used to determine if a thread exists already for a given reportActionID | ||
* | ||
* @param {String} reportActionID | ||
* @returns {Boolean} | ||
*/ | ||
function getThreadForReportActionID(reportActionID) { | ||
return _.find(allReports, (report) => { | ||
if (!report.parentReportActionID) { | ||
return false; | ||
} | ||
|
||
return report.parentReportActionID === reportActionID; | ||
}); | ||
} | ||
|
||
/** | ||
* Show subscript on IOU or expense report | ||
* @param {Object} report | ||
|
@@ -2141,6 +2173,7 @@ export { | |
canRequestMoney, | ||
getWhisperDisplayNames, | ||
getWorkspaceAvatar, | ||
getThreadForReportActionID, | ||
isThread, | ||
isThreadParent, | ||
isThreadFirstChat, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -404,6 +404,20 @@ function openReport(reportID, participantList = [], newReportObject = {}, parent | |
// Add the createdReportActionID parameter to the API call | ||
params.createdReportActionID = optimisticCreatedAction.reportActionID; | ||
} | ||
|
||
// If we are creating a thread, ensure the report action has childReportID property added | ||
if (newReportObject.parentReportID && parentReportActionID) { | ||
onyxData.optimisticData.push({ | ||
onyxMethod: Onyx.METHOD.MERGE, | ||
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newReportObject.parentReportID}`, | ||
value: {[parentReportActionID]: {childReportID: reportID}}, | ||
}); | ||
onyxData.failureData.push({ | ||
onyxMethod: Onyx.METHOD.MERGE, | ||
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newReportObject.parentReportID}`, | ||
value: {[parentReportActionID]: {childReportID: '0'}}, | ||
}); | ||
} | ||
Comment on lines
+409
to
+420
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 makes sure that when we create a report, the parent report action gets the childReportID key added. It should be updated by pusher, but if you are offline we would keep creating new threads by clicking on the preview or we could also crete new ones by quickly clicking on the preview before the pusher update comes through 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 ma not sure but I think no successData is needed here because we are only adding a property in ptimistic data and no pending value |
||
} | ||
|
||
API.write('OpenReport', params, onyxData); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import compose from '../../../libs/compose'; | |
import withLocalize from '../../../components/withLocalize'; | ||
import ReportActionItem from './ReportActionItem'; | ||
import reportActionPropTypes from './reportActionPropTypes'; | ||
import * as ReportActionsUtils from '../../../libs/ReportActionsUtils'; | ||
|
||
const propTypes = { | ||
/** The id of the report */ | ||
|
@@ -41,6 +42,11 @@ const defaultProps = { | |
|
||
const ReportActionItemParentAction = (props) => { | ||
const parentReportAction = props.parentReportActions[`${props.report.parentReportActionID}`]; | ||
|
||
// In case of transaction threads, we do not want to render the parent report action. | ||
if (ReportActionsUtils.isTransactionThread(parentReportAction)) { | ||
return null; | ||
} | ||
Comment on lines
+46
to
+49
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 is to ensure we do not show any report Action at the top of the transactions thread 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'm working on a change that might affect this line, can you clarify what the goal here was? @mountiny |
||
return ( | ||
<OfflineWithFeedback | ||
pendingAction={lodashGet(props.report, 'pendingFields.addWorkspaceRoom') || lodashGet(props.report, 'pendingFields.createChat')} | ||
|
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.
We only going to navigate to the details page in case of IOU split preview