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

Jump to bottom

Fix deleting parent thread message temporary disappearing #26548

Merged
merged 9 commits into from
Sep 11, 2023
Merged

Fix deleting parent thread message temporary disappearing #26548

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9cc6747
temporary message disappears
Nodebrute Sep 2, 2023
bffb906
Merge branch 'Expensify:main' into threadHeader
Nodebrute Sep 4, 2023
39a1074
set optimistic childType
Nodebrute Sep 4, 2023
30b84db
Merge branch 'Expensify:main' into threadHeader
Nodebrute Sep 6, 2023
acb86ef
improve code readability
Nodebrute Sep 6, 2023
1185100
Merge branch 'Expensify:main' into threadHeader
Nodebrute Sep 6, 2023
0de1ea8
apply suggested changes
Nodebrute Sep 6, 2023
fa514e4
Merge branch 'Expensify:main' into threadHeader
Nodebrute Sep 10, 2023
326010b
comments fixed
Nodebrute Sep 10, 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
13 changes: 10 additions & 3 deletions src/libs/ReportActionsUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,18 @@ function isWhisperAction(action) {
}

/**
*
Beamanator marked this conversation as resolved.
Show resolved Hide resolved
* Returns whether the comment is a thread parent message/the first message in a thread
*
* @param {Object} reportAction
* @param {Number} reportID
Copy link
Contributor

@fedirjh fedirjh Sep 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
* @param {Number} reportID
* @param {String} reportID

reportID has type string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's address this comment 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.

I'm not sure how I overlooked this, but I've just corrected it.

* @returns {Boolean}
*/
function hasCommentThread(reportAction) {
return lodashGet(reportAction, 'childType', '') === CONST.REPORT.TYPE.CHAT && lodashGet(reportAction, 'childVisibleActionCount', 0) > 0;
function isThreadParentMessage(reportAction, reportID) {
return (
lodashGet(reportAction, 'childType', '') === CONST.REPORT.TYPE.CHAT &&
(lodashGet(reportAction, 'childVisibleActionCount', 0) > 0 || (!_.isUndefined(reportAction.childReportID) && reportAction.childReportID.toString() === reportID))
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function isThreadParentMessage(reportAction, reportID) {
return (
lodashGet(reportAction, 'childType', '') === CONST.REPORT.TYPE.CHAT &&
(lodashGet(reportAction, 'childVisibleActionCount', 0) > 0 || (!_.isUndefined(reportAction.childReportID) && reportAction.childReportID.toString() === reportID))
);
function isThreadParentMessage(reportAction = {}, reportID) {
const {childType, childVisibleActionCount = 0, childReportID} = reportAction;
return childType === CONST.REPORT.TYPE.CHAT && (childVisibleActionCount > 0 || String(childReportID) === reportID);

NAB: Let's improve the code readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestions; I've just applied the recommended changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @Nodebrute Thanks for the update , let's make sure to resolve other comments as well.

}

/**
Expand Down Expand Up @@ -624,7 +631,7 @@ export {
getLastClosedReportAction,
getLatestReportActionFromOnyxData,
isMoneyRequestAction,
hasCommentThread,
isThreadParentMessage,
getLinkedTransactionID,
getMostRecentReportActionLastModified,
getReportPreviewAction,
Expand Down
6 changes: 3 additions & 3 deletions src/libs/actions/Report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,12 +528,12 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p
onyxData.optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newReportObject.parentReportID}`,
value: {[parentReportActionID]: {childReportID: reportID}},
value: {[parentReportActionID]: {childReportID: reportID, childType: CONST.REPORT.TYPE.CHAT}},
});
onyxData.failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newReportObject.parentReportID}`,
value: {[parentReportActionID]: {childReportID: '0'}},
value: {[parentReportActionID]: {childReportID: '0', childType: ''}},
});
}
}
Expand Down Expand Up @@ -926,7 +926,7 @@ function deleteReportComment(reportID, reportAction) {
html: '',
text: '',
isEdited: true,
isDeletedParentAction: ReportActionsUtils.hasCommentThread(reportAction),
isDeletedParentAction: ReportActionsUtils.isThreadParentMessage(reportAction, reportID),
},
];
const optimisticReportActions = {
Expand Down
3 changes: 2 additions & 1 deletion src/pages/home/report/ReportActionItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ function ReportActionItem(props) {
{!props.draftMessage ? (
<View style={props.displayAsGroup && hasBeenFlagged ? styles.blockquote : {}}>
<ReportActionItemMessage
reportID={props.report.reportID}
action={props.action}
displayAsGroup={props.displayAsGroup}
isHidden={isHidden}
Expand Down Expand Up @@ -576,7 +577,7 @@ function ReportActionItem(props) {
<OfflineWithFeedback
onClose={() => ReportActions.clearReportActionErrors(props.report.reportID, props.action)}
pendingAction={props.draftMessage ? null : props.action.pendingAction}
shouldHideOnDelete={!ReportActionsUtils.hasCommentThread(props.action)}
shouldHideOnDelete={!ReportActionsUtils.isThreadParentMessage(props.action, props.report.reportID)}
errors={props.action.errors}
errorRowStyles={[styles.ml10, styles.mr2]}
needsOffscreenAlphaCompositing={ReportActionsUtils.isMoneyRequestAction(props.action)}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItemFragment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function ReportActionItemFragment(props) {
// While offline we display the previous message with a strikethrough style. Once online we want to
// immediately display "[Deleted message]" while the delete action is pending.

if ((!props.network.isOffline && props.hasCommentThread && props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) || props.fragment.isDeletedParentAction) {
if ((!props.network.isOffline && props.isThreadParentMessage && props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) || props.fragment.isDeletedParentAction) {
Beamanator marked this conversation as resolved.
Show resolved Hide resolved
return <RenderHTML html={`<comment>${props.translate('parentReportAction.deletedMessage')}</comment>`} />;
}

Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItemMessage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function ReportActionItemMessage(props) {
fragment={fragment}
isAttachment={props.action.isAttachment}
iouMessage={iouMessage}
hasCommentThread={ReportActionsUtils.hasCommentThread(props.action)}
isThreadParentMessage={ReportActionsUtils.isThreadParentMessage(props.action, props.reportID)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add this prop reportID to the propTypes definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @Nodebrute , props.reportID is not defined in the propTypes , it should be added to this block

App/src/pages/home/report/ReportActionItemMessage.js

Line 13 in 1223e1e

const propTypes = {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just fixed this one as well. I apologize for the numerous mistakes.

attachmentInfo={props.action.attachmentInfo}
pendingAction={props.action.pendingAction}
source={lodashGet(props.action, 'originalMessage.source')}
Expand Down
Loading