Show deleted comments in conversation history #6819

aswin-s · 2021-12-17T19:55:41Z

Details

This PR fixes an issue where deleted comments get removed completely from conversations. Instead it replaces deleted messages with a placeholder text [Comment deleted]

Fixed Issues

$ #6759

Tests

Send a message in any conversation window.
Delete the comment
Verify that message gets replaced with placeholder text [Comment deleted]
Repeat steps 2 & 3 for any previous messages.
Try to delete attachments from web and verify that it gets replaced with placeholder text.

NB: Native apps doesn't seem to offer delete option at the moment. Reported here

QA Steps

Send a message in any conversation window.
Delete the comment
Verify that message gets replaced with placeholder text [Comment deleted]
Repeat steps 2 & 3 for any previous messages.
Verify that status gets updated at receiving end as well

Tested On

Screenshots

Web

1.mp4

Mobile Web

Desktop

iOS

Android

2.mp4

parasharrajat

Does not look like the supporting color is applied at all? I think you should use styles instead of passing the color.

parasharrajat · 2021-12-17T20:01:34Z

src/pages/home/report/ReportActionItem.js

+    isDeleted() {
+        // A deleted comment has either an empty array or an object with html field with empty string as value
+        return lodashGet(this.props.action, 'message[0].html', '') === '';
+    }


Move this to ReportUtils with isDeletedAction

parasharrajat · 2021-12-17T20:02:48Z

src/pages/home/report/ReportActionItem.js

+     */
+    isDeleted() {
+        // A deleted comment has either an empty array or an object with html field with empty string as value
+        return lodashGet(this.props.action, 'message[0].html', '') === '';


Don't think we need to explicitly compare it with ===' '

parasharrajat · 2021-12-17T20:03:53Z

src/pages/home/report/ReportActionItemMessage.js

-                />
-            ))}
+            {isDeleted
+                ? <ExpensifyText color={themeColors.textSupporting}>{`[${Localize.translateLocal('common.deletedCommentMessage')}]`}</ExpensifyText>


use WithLocalize instead.

src/pages/home/report/ReportActionsView.js

Co-authored-by: Rajat Parashar <[email protected]>

aswin-s · 2021-12-17T20:39:40Z

Does not look like the supporting color is applied at all? I think you should use styles instead of passing the color.

I took the video right before changing the text style. It was getting confusing with normal comments. Updated video below.

final.mp4

parasharrajat · 2021-12-17T21:16:21Z

Thanks for the video. I am not a designer guy ✏️ so I will request @shawnborton to review the video and decide the color and font styles.

parasharrajat · 2021-12-17T21:20:58Z

Which one look better?

aswin-s · 2021-12-17T21:24:25Z

Shouldn't format be the same as that used in LHN? @parasharrajat

parasharrajat · 2021-12-17T21:30:27Z

yup, that's a valid point. Just thought of sharing a different perspective. But I definitely think that we should reduce the text size.

shawnborton · 2021-12-17T21:31:25Z

I actually like what's shown in this comment here: #6819 (comment)

parasharrajat

a few changes...

parasharrajat · 2021-12-17T21:53:14Z

src/libs/reportUtils.js

@@ -200,8 +200,19 @@ function canShowReportRecipientLocalTime(personalDetails, myPersonalDetails, rep
        && moment().tz(currentUserTimezone.selected).utcOffset() !== moment().tz(reportRecipientTimezone.selected).utcOffset();
 }

+/**
+ * Check if the comment is deleted
+ * @param {Array<Object>} action


Wrong type...

parasharrajat · 2021-12-17T21:53:45Z

src/pages/home/report/ReportActionItemMessage.js

@@ -42,4 +54,4 @@ ReportActionItemMessage.propTypes = propTypes;
 ReportActionItemMessage.defaultProps = defaultProps;
 ReportActionItemMessage.displayName = 'ReportActionItemMessage';

-export default withNetwork()(ReportActionItemMessage);
+export default compose(withNetwork(), withLocalize)(ReportActionItemMessage);


NAB: May be wrap this to the next lines like the other components?

parasharrajat · 2021-12-17T21:54:31Z

src/libs/reportUtils.js

+/**
+ * Check if the comment is deleted
+ * @param {Array<Object>} action
+ * @returns {boolean}


Suggested change

* @returns {boolean}

* @returns {Boolean}

parasharrajat

LGTM.

cc: @Luke9389

Luke9389

Lookin good! Thanks

OSBotify · 2022-01-04T01:53:11Z

🚀 Deployed to staging by @Luke9389 in version: 1.1.24-19 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	failure ❌
🕸 web 🕸	failure ❌

francoisl · 2022-01-04T02:25:15Z

This failed because of the import ExpensifyText from '../../../components/ExpensifyText'; statement - looks like that component has been renamed or removed since the last commit. Can someone follow up with a fix PR please? Thanks!

OSBotify · 2022-01-04T02:40:09Z

🚀 Deployed to production by @francoisl in version: 1.1.24-19 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	failure ❌
🕸 web 🕸	failure ❌

marcaaron · 2022-01-04T02:43:36Z

@francoisl Fix here #7006

marcaaron · 2022-01-04T02:47:10Z

Guessing this did not make it to staging and so does not need to be CP'd to staging but not sure just saw it was breaking my GH actions in open PRs I'm working on.

Jag96 · 2022-01-04T03:00:11Z

The changes weren't deployed to staging, but the staging branch does have the broken code on it so I think we should CP so we can get this fixed on the production branch faster.

aswin-s added 4 commits December 18, 2021 00:47

Remove condition which filtered out deleted comments

cb5737b

Hide context menu and hover icons for deleted comments

421f317

Render placeholder text for deleted comment

5d84848

Use lighter color to differentiate placeholder text

1ac5337

aswin-s requested a review from a team as a code owner December 17, 2021 19:55

MelvinBot requested review from Luke9389 and parasharrajat and removed request for a team December 17, 2021 19:55

parasharrajat suggested changes Dec 17, 2021

View reviewed changes

aswin-s and others added 3 commits December 18, 2021 01:50

Move isDeleted method into ReportUtils

bb13eba

use withLocalize HOC

4c0dcdd

Remove extra parentheses

4a6ee78

Co-authored-by: Rajat Parashar <[email protected]>

aswin-s requested a review from parasharrajat December 17, 2021 20:39

parasharrajat suggested changes Dec 17, 2021

View reviewed changes

aswin-s added 2 commits December 20, 2021 10:01

Fix param types & return types for isDeletedAction

a6e24f4

Wrap arguments of compose HOC

17bd856

parasharrajat approved these changes Dec 20, 2021

View reviewed changes

Luke9389 approved these changes Jan 3, 2022

View reviewed changes

Luke9389 merged commit 19085f3 into Expensify:main Jan 3, 2022

botify mentioned this pull request Jan 4, 2022

[HOLD for payment 2022-01-11] [250$] Deleted comment status is only visible on LHN and not in the conversation, should be visible on both #6759

Closed

OSBotify mentioned this pull request Jan 4, 2022

Deploy Checklist: New Expensify 2022-01-04 #7005

Closed

45 tasks

marcaaron mentioned this pull request Jan 4, 2022

[No QA] Fix broken ExpensifyText #7006

Merged

5 tasks

Jag96 mentioned this pull request Jan 4, 2022

Ping deployers if PR is merged with failing lint rules #7026

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show deleted comments in conversation history #6819

Show deleted comments in conversation history #6819

aswin-s commented Dec 17, 2021

parasharrajat left a comment •

edited

Loading

parasharrajat Dec 17, 2021

aswin-s Dec 17, 2021

parasharrajat Dec 17, 2021

aswin-s Dec 17, 2021

parasharrajat Dec 17, 2021

aswin-s Dec 17, 2021

aswin-s commented Dec 17, 2021 •

edited

Loading

parasharrajat commented Dec 17, 2021

parasharrajat commented Dec 17, 2021

aswin-s commented Dec 17, 2021

parasharrajat commented Dec 17, 2021

shawnborton commented Dec 17, 2021

parasharrajat left a comment

parasharrajat Dec 17, 2021

aswin-s Dec 20, 2021

parasharrajat Dec 17, 2021

aswin-s Dec 20, 2021

parasharrajat Dec 17, 2021

aswin-s Dec 20, 2021

parasharrajat left a comment

Luke9389 left a comment

OSBotify commented Jan 4, 2022

francoisl commented Jan 4, 2022

OSBotify commented Jan 4, 2022

marcaaron commented Jan 4, 2022

marcaaron commented Jan 4, 2022

Jag96 commented Jan 4, 2022

Show deleted comments in conversation history #6819

Show deleted comments in conversation history #6819

Conversation

aswin-s commented Dec 17, 2021

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

parasharrajat left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aswin-s commented Dec 17, 2021 • edited Loading

parasharrajat commented Dec 17, 2021

parasharrajat commented Dec 17, 2021

aswin-s commented Dec 17, 2021

parasharrajat commented Dec 17, 2021

shawnborton commented Dec 17, 2021

parasharrajat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat left a comment

Choose a reason for hiding this comment

Luke9389 left a comment

Choose a reason for hiding this comment

OSBotify commented Jan 4, 2022

francoisl commented Jan 4, 2022

OSBotify commented Jan 4, 2022

marcaaron commented Jan 4, 2022

marcaaron commented Jan 4, 2022

Jag96 commented Jan 4, 2022

parasharrajat left a comment •

edited

Loading

aswin-s commented Dec 17, 2021 •

edited

Loading