[HOLD for Payment 2023-05-16][$1000] Inconsistency in delete comment button for video file on archived workspace in web and android #16092

kavimuru · 2023-03-18T01:40:25Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

First open chrome web and create a new workspace
Go to announce of the created workspace and upload a video file
Next, delete the workspace and come back to the announce section again
Make sure to refresh the page
Right-click on the video file attachment and see that the delete button doesn’t appear for archived workspace
Next , open android app with the same user account and visit the announce section again
Now long press on the video file (you need to long press on the video file attachment itself) and see that the delete comment button is present. Thus, no consistency is there between web and android for archived workspace. In web ,delete comment button is not present whereas on android delete comment button is present.

Expected Result:

Android should not display the delete button for video file comments on archived workspaces

Actual Result:

The delete comment button is seen on android (incorrect), but is not seen on web chrome (correct) for video files in archived workspace

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Version Number: 1.2.87-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

android.mp4

archived-2023-03-17_21.59.13.mp4

Recording.1726.mp4

Expensify/Expensify Issue URL:
Issue reported by: @avi-shek-jha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679070606602909

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0109ece2dd91782609
Upwork Job ID: 1640725629625151488
Last Price Increase: 2023-04-11

MelvinBot · 2023-03-18T01:40:30Z

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

MelvinBot · 2023-03-18T01:40:34Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

MelvinBot · 2023-03-21T18:22:41Z

@greg-schroeder Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

greg-schroeder · 2023-03-28T14:29:43Z

Sorry for the delay, I was OOO just after this was assigned.

greg-schroeder · 2023-03-28T14:30:37Z

I was able to reproduce this ... with that said I'm not 100% sure what the expected behavior is, but I expect it's to have the ability to delete the video on web. Asking the team in Slack

greg-schroeder · 2023-03-28T14:32:19Z

Discussion here: https://expensify.slack.com/archives/C01SKUP7QR0/p1680013927834489

greg-schroeder · 2023-03-28T14:40:20Z

Confirmed with the team:

I would say the web behavior is correct - when the workspace is deleted you shouldn’t be able to edit or delete the comments in that workspace chat

Updated OP accordingly

MelvinBot · 2023-03-28T14:40:51Z

Job added to Upwork: https://www.upwork.com/jobs/~0109ece2dd91782609

MelvinBot · 2023-03-28T14:40:57Z

Triggered auto assignment to @CortneyOfstad (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

MelvinBot · 2023-03-28T14:41:17Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

MelvinBot · 2023-03-28T14:41:26Z

Triggered auto assignment to @tylerkaraszewski (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

ShogunFire · 2023-03-28T16:36:41Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

On Android, when a workplace is closed we can still delete the comment

What is the root cause of that problem?

We don't always pass the parameter isArchivedRoom to showContextMenu

When we don't pass the parameter isArchivedRoom it takes the value false.

We then use this value to determine if Delete comment is possible or not here:

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 240 to 241 in d57b331

    
           shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport) => type === CONTEXT_MENU_TYPES.REPORT_ACTION 
        
               && ReportUtils.canDeleteReportAction(reportAction) && !isArchivedRoom && !isChronosReport,

What changes do you think we should make in order to solve the problem?

We should pass the right value of isArchivedRoom in parameters of showContextMenu() here and here

We can use the method ReportUtils.isArchivedRoom() to get the right value

What alternative solutions did you explore? (Optional)

hoangzinh · 2023-03-28T17:29:54Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Android/IOS should not display the delete button for video file comments on archived workspaces

What is the root cause of that problem?

Take a look at the condition to show "Delete Comment" in the Context Menu

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 240 to 241 in 752ac61

    
           shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport) => type === CONTEXT_MENU_TYPES.REPORT_ACTION 
        
               && ReportUtils.canDeleteReportAction(reportAction) && !isArchivedRoom && !isChronosReport,

It will return false (which mean not show) when the argument isArchivedRoom is true.

Deep dive into the ReportActionItemFragment component of video attachment comment. We will find out it use BaseAnchorForAttachmentsOnly component to render. In the onLongPress handler of this component, we didn't pass isArchivedRoom flag to the showContextMenuForReport method. So isArchivedRoom is always false => possible to show "Delete Comment" even the Report is archived

App/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.js

Lines 66 to 72 in 752ac61

    
           onLongPress={event => showContextMenuForReport( 
        
               event, 
        
               anchor, 
        
               reportID, 
        
               action, 
        
               checkIfContextMenuActive, 
        
           )}

It also explains why we thought it only happen in native devices => because it only happen if we do a long press => the bug can be reproduce in Web/Desktop Mac if you do a long press. Let's see screenshot below

What changes do you think we should make in order to solve the problem?

General idea is we should update the onLongPress handler that I mentioned above so that It can pass the flag isArchivedRoom into the Context menu. More details:

We should store the isArchivedRoom into the ShowContextMenuContext here
Then update the showContextMenuForReport method to receive an additional argument which is isArchivedRoom

Finally, we get the isArchivedRoom from the context, and pass it as new argument that we added in step 2

App/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.js

Lines 66 to 72 in 752ac61

    
           onLongPress={event => showContextMenuForReport( 
        
               event, 
        
               anchor, 
        
               reportID, 
        
               action, 
        
               checkIfContextMenuActive, 
        
           )}

podrascanindragan47 · 2023-03-28T19:03:53Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

The delete comment button should not be displayed in archived workspaces on web and android platforms.
On web, the button should disappear without refreshing the page.
On android, the button should not appear at all.

What is the root cause of that problem?

In web

This bug occurs because ReportActionsView does not re-render when the workspace is deleted. The shouldComponentUpdate function prevents it from updating, so the props for report.stateNum and report.statusNum are not passed down to the children components.

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

Lines 123 to 170 in 3a9ce6f

    
           shouldComponentUpdate(nextProps, nextState) { 
        
               if (!_.isEqual(nextProps.reportActions, this.props.reportActions)) { 
        
                   this.mostRecentIOUReportActionID = ReportActionsUtils.getMostRecentIOURequestActionID(nextProps.reportActions); 
        
                   return true; 
        
               } 
        
               if (lodashGet(nextProps.network, 'isOffline') !== lodashGet(this.props.network, 'isOffline')) { 
        
                   return true; 
        
               } 
        
               if (nextProps.report.isLoadingMoreReportActions !== this.props.report.isLoadingMoreReportActions) { 
        
                   return true; 
        
               } 
        
               if (nextProps.report.isLoadingReportActions !== this.props.report.isLoadingReportActions) { 
        
                   return true; 
        
               } 
        
               if (nextProps.report.lastReadTime !== this.props.report.lastReadTime) { 
        
                   return true; 
        
               } 
        
               if (nextState.isFloatingMessageCounterVisible !== this.state.isFloatingMessageCounterVisible) { 
        
                   return true; 
        
               } 
        
               if (nextState.newMarkerReportActionID !== this.state.newMarkerReportActionID) { 
        
                   return true; 
        
               } 
        
               if (this.props.isSmallScreenWidth !== nextProps.isSmallScreenWidth) { 
        
                   return true; 
        
               } 
        
               if (this.props.isDrawerOpen !== nextProps.isDrawerOpen) { 
        
                   return true; 
        
               } 
        
               if (lodashGet(this.props.report, 'hasOutstandingIOU') !== lodashGet(nextProps.report, 'hasOutstandingIOU')) { 
        
                   return true; 
        
               } 
        
               if (this.props.isComposerFullSize !== nextProps.isComposerFullSize) { 
        
                   return true; 
        
               } 
        
               return !_.isEqual(lodashGet(this.props.report, 'icons', []), lodashGet(nextProps.report, 'icons', [])); 
        
           }

In android.

The onLongPress handler in the BaseAnchorForAttachment component invokes the showContextMenuForReport function, which does not have the isArchivedRoom and isChronosReport parameters.

What changes do you think we should make in order to solve the problem?

In web

To fix this bug, we need to add the deleting conditions in the shouldComponentUpdate function as shown below.

        if (nextProps.report.stateNum !== this.props.report.stateNum) {
            return true;
        }

        if (nextProps.report.statusNum !== this.props.report.statusNum) {
            return true;
        }

This is result video.

video_02.mp4

In android

Instead of passing reportID, I suggest passing report in the ShowContextMenuContext.

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

Lines 174 to 181 in 2398744

    
           <ShowContextMenuContext.Provider 
        
               value={{ 
        
                   anchor: this.popoverAnchor, 
        
                   reportID: this.props.report.reportID, 
        
                   action: this.props.action, 
        
                   checkIfContextMenuActive: this.checkIfContextMenuActive, 
        
               }} 
        
           >

report: this.props.report,

This way, we can access the additional parameters (isArchivedRoom, isChronosReport) in the showContextMenuForReport function.

App/src/components/ShowContextMenuContext.js

Lines 23 to 35 in 2398744

    
           function showContextMenuForReport(event, anchor, reportID, action, checkIfContextMenuActive) { 
        
               ReportActionContextMenu.showContextMenu( 
        
                   ContextMenuActions.CONTEXT_MENU_TYPES.REPORT_ACTION, 
        
                   event, 
        
                   '', 
        
                   anchor, 
        
                   reportID, 
        
                   action, 
        
                   '', 
        
                   checkIfContextMenuActive, 
        
                   checkIfContextMenuActive, 
        
               ); 
        
           }

function showContextMenuForReport(event, anchor, reportID, action, checkIfContextMenuActive, isArchivedRoom = false, isChronosReport = false) {
    ReportActionContextMenu.showContextMenu(
        ContextMenuActions.CONTEXT_MENU_TYPES.REPORT_ACTION,
        event,
        '',
        anchor,
        reportID,
        action,
        '',
        checkIfContextMenuActive,
        checkIfContextMenuActive,
        isArchivedRoom,
        isChronosReport
    );
}

We also need to update the onLongPress handler in the BaseAnchorForAttachmentsOnly.js file accordingly.

App/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.js

Lines 66 to 72 in 2398744

    
           onLongPress={event => showContextMenuForReport( 
        
               event, 
        
               anchor, 
        
               reportID, 
        
               action, 
        
               checkIfContextMenuActive, 
        
           )}

    onLongPress={event => showContextMenuForReport(
        event,
        anchor,
        report.reportID,
        action,
        checkIfContextMenuActive,
        ReportUtils.isArchivedRoom(report),
        ReportUtils.chatIncludesChronos(report)
    )}

This is result video.

video_02.mp4

eVoloshchak · 2023-05-03T20:11:28Z

Not overdue, this was deployed to staging yesterday

melvin-bot · 2023-05-11T20:08:06Z

@tylerkaraszewski, @eVoloshchak, @ShogunFire, @greg-schroeder Whoops! This issue is 2 days overdue. Let's get this updated quick!

greg-schroeder · 2023-05-12T06:16:57Z

Hmm - was this deployed @eVoloshchak? I think it was but I'm not sure why the automated comment didn't happen 🤔

eVoloshchak · 2023-05-12T07:01:17Z

@greg-schroeder, yep, this was deployed to production 3 days ago.
The PR description was edited by author, I'm guessing that's why automation failed

greg-schroeder · 2023-05-12T23:26:47Z

Ahhh, got it got it - I will copy over an existing checklist manually posterity

greg-schroeder · 2023-05-12T23:30:47Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.12-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix: Can delete attachments in archived workspace #17329

If no regressions arise, payment will be issued on 2023-05-16. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

ShogunFire · 2023-05-16T18:10:44Z

@greg-schroeder Hello, I see that the upwork job has been closed. Do I need to do something for the paiement ? It looks like there was no regression

greg-schroeder · 2023-05-16T18:11:36Z

It was closed due to timing out, but I'll recreate it now.

hmm. I can't get Upwork to re-create the issue, it's just hanging indefinitely

greg-schroeder · 2023-05-16T18:37:35Z

I might just try re-adding the label to create one tbh... stand by, not sure if it works that way

Edit: It did not work. I will just have to create offers manually I think, upwork seems to be broken right now when it comes to re-using issues

greg-schroeder · 2023-05-16T18:47:45Z

@ShogunFire can you tell me your Upwork details?

ShogunFire · 2023-05-16T19:31:36Z

@greg-schroeder https://www.upwork.com/freelancers/~01d59f5bcb11fabc56

greg-schroeder · 2023-05-16T20:57:56Z

Thanks! Offers sent to @ShogunFire and @eVoloshchak. Upwork is not working properly right now so I just sent contracts to you both manually.

@avi-shek-jha can you post your Upwork details so I can get you paid for reporting this?

greg-schroeder · 2023-05-16T23:50:46Z

This is paid out with the exception of @avi-shek-jha - please post your details whenever you are able and I'll get you paid for reporting this.

ShogunFire · 2023-05-17T00:37:50Z

Thanks !

avi-shek-jha · 2023-05-17T05:21:38Z

@greg-schroeder Here's my upwork profile link: https://www.upwork.com/freelancers/~01a90e548bf808418c

greg-schroeder · 2023-05-17T05:30:34Z

Thank you! Sent you an offer

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 18, 2023

melvin-bot bot locked and limited conversation to collaborators Mar 18, 2023

melvin-bot bot assigned greg-schroeder Mar 18, 2023

melvin-bot bot added the Overdue label Mar 20, 2023

Expensify unlocked this conversation Mar 21, 2023

melvin-bot bot added Overdue and removed Overdue labels Mar 23, 2023

melvin-bot bot removed the Overdue label Mar 28, 2023

greg-schroeder added the External Added to denote the issue can be worked on by a contributor label Mar 28, 2023

melvin-bot bot changed the title ~~Inconsistency in delete comment button for video file on archived workspace in web and android~~ [$1000] Inconsistency in delete comment button for video file on archived workspace in web and android Mar 28, 2023

melvin-bot bot assigned CortneyOfstad Mar 28, 2023

melvin-bot bot assigned eVoloshchak Mar 28, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 28, 2023

melvin-bot bot assigned tylerkaraszewski Mar 28, 2023

greg-schroeder unassigned CortneyOfstad Mar 28, 2023

greg-schroeder changed the title ~~[$1000] Inconsistency in delete comment button for video file on archived workspace in web and android~~ [HOLD for Payment 2023-05-16][$1000] Inconsistency in delete comment button for video file on archived workspace in web and android May 12, 2023

greg-schroeder added Weekly KSv2 and removed Daily KSv2 labels May 15, 2023

greg-schroeder added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels May 16, 2023

This comment was marked as resolved.

Sign in to view

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 16, 2023

This comment was marked as resolved.

Sign in to view

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 16, 2023

This comment was marked as resolved.

Sign in to view

greg-schroeder added Weekly KSv2 and removed Daily KSv2 labels May 16, 2023

greg-schroeder closed this as completed May 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for Payment 2023-05-16][$1000] Inconsistency in delete comment button for video file on archived workspace in web and android #16092

[HOLD for Payment 2023-05-16][$1000] Inconsistency in delete comment button for video file on archived workspace in web and android #16092

kavimuru commented Mar 18, 2023 •

edited by melvin-bot bot

Loading

MelvinBot commented Mar 18, 2023

MelvinBot commented Mar 18, 2023 •

edited by greg-schroeder

Loading

MelvinBot commented Mar 21, 2023

greg-schroeder commented Mar 28, 2023

greg-schroeder commented Mar 28, 2023

greg-schroeder commented Mar 28, 2023

greg-schroeder commented Mar 28, 2023 •

edited

Loading

MelvinBot commented Mar 28, 2023

MelvinBot commented Mar 28, 2023

MelvinBot commented Mar 28, 2023

MelvinBot commented Mar 28, 2023

ShogunFire commented Mar 28, 2023

hoangzinh commented Mar 28, 2023 •

edited

Loading

podrascanindragan47 commented Mar 28, 2023 •

edited

Loading

eVoloshchak commented May 3, 2023

melvin-bot bot commented May 11, 2023

greg-schroeder commented May 12, 2023

eVoloshchak commented May 12, 2023

greg-schroeder commented May 12, 2023 •

edited

Loading

greg-schroeder commented May 12, 2023

ShogunFire commented May 16, 2023

greg-schroeder commented May 16, 2023 •

edited

Loading

greg-schroeder commented May 16, 2023 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

greg-schroeder commented May 16, 2023

ShogunFire commented May 16, 2023

greg-schroeder commented May 16, 2023

greg-schroeder commented May 16, 2023

ShogunFire commented May 17, 2023

avi-shek-jha commented May 17, 2023

greg-schroeder commented May 17, 2023

[HOLD for Payment 2023-05-16][$1000] Inconsistency in delete comment button for video file on archived workspace in web and android #16092

[HOLD for Payment 2023-05-16][$1000] Inconsistency in delete comment button for video file on archived workspace in web and android #16092

Comments

kavimuru commented Mar 18, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

MelvinBot commented Mar 18, 2023

MelvinBot commented Mar 18, 2023 • edited by greg-schroeder Loading

Bug0 Triage Checklist (Main S/O)

MelvinBot commented Mar 21, 2023

greg-schroeder commented Mar 28, 2023

greg-schroeder commented Mar 28, 2023

greg-schroeder commented Mar 28, 2023

greg-schroeder commented Mar 28, 2023 • edited Loading

MelvinBot commented Mar 28, 2023

MelvinBot commented Mar 28, 2023

MelvinBot commented Mar 28, 2023

MelvinBot commented Mar 28, 2023

ShogunFire commented Mar 28, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

hoangzinh commented Mar 28, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

podrascanindragan47 commented Mar 28, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

In web

In android.

What changes do you think we should make in order to solve the problem?

In web

In android

eVoloshchak commented May 3, 2023

melvin-bot bot commented May 11, 2023

greg-schroeder commented May 12, 2023

eVoloshchak commented May 12, 2023

greg-schroeder commented May 12, 2023 • edited Loading

greg-schroeder commented May 12, 2023

ShogunFire commented May 16, 2023

greg-schroeder commented May 16, 2023 • edited Loading

greg-schroeder commented May 16, 2023 • edited Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

greg-schroeder commented May 16, 2023

ShogunFire commented May 16, 2023

greg-schroeder commented May 16, 2023

greg-schroeder commented May 16, 2023

ShogunFire commented May 17, 2023

avi-shek-jha commented May 17, 2023

greg-schroeder commented May 17, 2023

kavimuru commented Mar 18, 2023 •

edited by melvin-bot bot

Loading

MelvinBot commented Mar 18, 2023 •

edited by greg-schroeder

Loading

greg-schroeder commented Mar 28, 2023 •

edited

Loading

hoangzinh commented Mar 28, 2023 •

edited

Loading

podrascanindragan47 commented Mar 28, 2023 •

edited

Loading

greg-schroeder commented May 12, 2023 •

edited

Loading

greg-schroeder commented May 16, 2023 •

edited

Loading

greg-schroeder commented May 16, 2023 •

edited

Loading