add LHN label for deleted messages #3979
Conversation
| @@ -166,7 +166,7 @@ function getSimplifiedReportObject(report) { | |||
| // We convert the line-breaks in html to space ' ' before striping the tags | |||
| const lastMessageText = lodashGet(lastReportAction, ['message', 'html'], '') | |||
| .replace(/((<br[^>]*>)+)/gi, ' ') | |||
| .replace(/(<([^>]+)>)/gi, ''); | |||
| .replace(/(<([^>]+)>)/gi, '') || `[${translateLocal('common.deletedCommentMessage')}]`; | |||
There was a problem hiding this comment.
hmmmm, can you make sure to escape any character that has a meaning in regex? Right now there's no risk, but it would be very easy for a translation to include a dot or some other character that regexes would treat differently than just a text (same below).
There was a problem hiding this comment.
I'm sorry I'm not following. The translated text is not checked against any regular expression and isn't treated as a regular expression. I'm not sure what's the concern here. Can you please rephrase it somehow maybe?
There was a problem hiding this comment.
Oh sorry, you are right, I misread this.
|
Tagging @NikkiWines since this PR also fixes #3293 . |
| @@ -166,7 +166,7 @@ function getSimplifiedReportObject(report) { | |||
| // We convert the line-breaks in html to space ' ' before striping the tags | |||
| const lastMessageText = lodashGet(lastReportAction, ['message', 'html'], '') | |||
| .replace(/((<br[^>]*>)+)/gi, ' ') | |||
| .replace(/(<([^>]+)>)/gi, ''); | |||
| .replace(/(<([^>]+)>)/gi, '') || `[${translateLocal('common.deletedCommentMessage')}]`; | |||
There was a problem hiding this comment.
Oh sorry, you are right, I misread this.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging in version: 1.0.77-6🚀
|
|
The PR caused a deploy blocker so I had to manually revert it here. |
|
@chiragsalian Yes, this code probably should not have been deployed to staging, however the PR itself is good. The issue you are referring to is caused by #3737. We discussed it here with @iwiznia : #3743 (comment) Should I create a new PR that reverts your revert, so we don't forget about it until #3737 is fixed? |
|
Yeah so i think my revert should be cherry picked to staging so that it unblocks deploy. You can go ahead and create a revert of my revert now or later so that its not forgotten, whichever your preference is. Then once #3737 is fixed you'll just merge the new PR so that both go to staging together. Does that plan sound good with you? |
|
Yes, that sounds good to me. As far as I understand the fix for #3737 is BE only, so we wouldn't need to care about those fixes going to staging together as much. But other than that your plan sounds good. I'll create a PR later. Thanks. |
|
🚀 Deployed to production in version: 1.0.79-4🚀
|
Details
This PR fixes two LHN issues related to comment deletion by showing a translatable "[Comment deleted]" message, if last message was deleted.
Fixed Issues
$ #3743
$ #3293
Tests / QA Steps
Tested On
Screenshots
Web
3747-3293-web.mp4
Mobile Web
Simulator.Screen.Recording.-.iPhone.8.-.2021-07-12.at.15.19.55.mp4
Desktop
Screen.Recording.2021-07-12.at.15.22.22.mov
iOS
Simulator.Screen.Recording.-.iPhone.12.-.2021-07-12.at.15.28.56.mp4
Android
3747-3293-android.mp4