[HOLD for payment July 6] Unable to delete messages with attachments

[isagoico]

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:

1. Log in to e.cash
2. Navigate to a conversation
3. Send an image
4. Try to delete the message with the image (via right click or action hover menu)

Expected Result:

User should be able to delete messages with attachments.

Actual Result:

User is unable to delete messages with attachments

Workaround:

N/A

Platform:

Where is this issue occurring?

Web ✔️
iOS ✔️
Android ✔️
Desktop App ✔️
Mobile Web ✔️

Version Number: 1.0.74-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @aliabbasmalik8 https://expensify.slack.com/archives/C01GTK53T8Q/p1624844541157000

didn't find a delete option on my message that has an attachment.

[MelvinBot]

Triggered auto assignment to @MariaHCD (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MariaHCD]

@sketchydroide @roryabraham Would you be able to confirm if the inability to delete attachments is expected behavior?

[sketchydroide]

I think it is expected behaviour. @roryabraham can correct me on this, but I think that is expected.

[roryabraham]

It may have been expected behavior at the time when we implemented the Delete Message function, but is there any reason we can't fix it now? I wonder if we can use the same approach for attachments as we do for other reportActions, and just set the reportAction.message.html to '' ?

I think this was prettymuch a remnant from Edit Message. Not being able to delete attachments seems like an odd restriction, but not being able to edit attachments makes sense because we would have to come up with some better UI to do that.

[parasharrajat]

Yeah, totally agree. Deleting attachment is just like setting html to null on reportAction. We should have this behaviour.

[MariaHCD]

Thanks for the clarification!

[MelvinBot]

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

[rdjuric]

Proposal

We change our shouldShow condition for the delete option to be a new function canDeleteReportAction. It will be similar to our canEditReportAction, but won't check if the message is an attachment.

This will allow us to separate the checks for editing and deleting a message (they're the same at the moment, but we don't want to show the edit button in our attachments).

We already set the html of a deleted comment to '' (both locally and in the backend), so we don't need to change the behavior of our deleteReportComment action.

[roryabraham]

@rdjuric Your proposal looks good! Nice and simple! Feel free to submit a PR once you've been hired on Upwork

@JmillsExpensify Let's make sure that we get @rdjuric hired on Upwork.

[rdjuric]

Thanks @roryabraham! PR is up at #3804

[JmillsExpensify]

Just seeing this. Doing this now!

[JmillsExpensify]

@rdjuric Upwork job is here! https://www.upwork.com/jobs/~014420b47526ca3ac6

[MelvinBot]

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

[Jag96]

Offer sent @rdjuric! Updating title to keep track of payment

[Jag96]

Tested and paid!