[HOLD for payment 2024-02-01] [$500] HIGH: Enable threading on all system messages

[quinthar]

Problem: Our chat-centric design is to allow you to chat on basically anything. However, right now you can't thread up a response to a system message, for no clear reason. One specific place this is a limitation is when there is a system message in the #admins room -- such as when someone adds an outsider to a room. By not allowing threading, we disrupt the most natural place to discuss this with the employee.

Solution: Enable threading on all system messages from a user, exactly as if they had manually posted that same message as a comment. This means:

• Name the thread based on the parent comment that is being threaded
• Invite the author of the parent comment (in this case, the system message) to the thread even if they don't have access to the parent room. (This can happen because users can cause system messages to posted "from" them to rooms they aren't in).

So far as I know this should be doable entirely on the client side without any back-end work. However, if you determine that's not true (and if you need some back end help) please just let us know.

Test: Verify:

1. Alice creates a workspace Alice's Apples
2. Alice invites Bob to the workspace
3. Bob creates a room #bagel-lovers that Alice is not in
4. Bob invites Cathy (who is not a member of the workspace) to #bagel-lovers
   • This cause Bob to post a system message to the #admins room of Alice's Apples saying: Bob invited Cathy to #bagel-lovers
5. Alice opens a thread on that system message
6. Alice posts "What up Bob, why did you invite Cathy?" in that thread
7. Confirm Bob is invited to that thread and sees Alice's message
   Upwork Automation - Do Not Edit

   • Upwork Job URL: https://www.upwork.com/jobs/~0143d329144a3feb35
   • Upwork Job ID: 1740548696855482368
   • Last Price Increase: 2024-01-05
   • Automatic offers:
   • aimane-chnaif | Reviewer | 28088772

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0143d329144a3feb35

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[dukenv0307]

This will be fixed here https://github.com/Expensify/App/pull/33490.

[melvin-bot[bot]]

@sonialiap, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot[bot]]

@sonialiap, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[quinthar]

Done!

[quinthar]

Actually reopening -- something seems weird about threads on system messages in the #admins room. Specifically, do all the original steps, and then check the new step at the end:

1. Alice creates a workspace Alice's Apples
2. Alice invites Bob to the workspace
3. Bob creates a room #bagel-lovers that Alice is not in
4. Bob invites Cathy (who is not a member of the workspace) to #bagel-lovers
   • This cause Bob to post a system message to the #admins room of Alice's Apples saying: Bob invited Cathy to #bagel-lovers
5. Alice opens a thread on that system message
6. Alice posts "What up Bob, why did you invite Cathy?" in that thread
7. Confirm Bob is invited to that thread and sees Alice's message
8. **Mention @cathy@croissants.com
   • What should happen: Cathy should be added to the thread
   • What actually happenes: Cathy is not invited
9. **Click into the room header and go to Members
   • What should happen: It should be possible to add/remove members
   • What actually happens: It shows the member list but without any option to add/remove

It seems like the thread from the #admins room is somehow inheriting the invite restrictions of #admins. Please remove this restriction and allow members of the thread to add/remove people at will, both via @mentions and the Add/Remove button in the Members list.

[quinthar]

Actually it looks like this restriction is on all threads of system messages: for some reason you can't add people to the thread after created, either by mention or the Members page.

[quinthar]

Ok, so ya things are really screwed up with threading on system messages. There are multiple issues, I'll try to list them here.

1) Consider this #expensify-test-room, created by @nathanmetcalf:

2) Note how at the bottom it says invited @nathan.d.metcalf@gmail.com:

3) When Nathan clicks into it, he sees this:

3) There are multiple problems. First, note how the header (and LHN) just says invited -- it should show show invited @nathan.d.metcalf@gmail.com, just like the original comment:

4) But when I click into it, I see this:

5) This has all the problems Nathan saw, but more. Check how the first message in the chat, which shows the parent that is being threaded from, shows some kind of phantom 6-reply thread (it should not be showing any replies there):

6) Needless to say, when I try to click into the phantom replies, it breaks:

7) Finally, it's not letting me invite/remove members from this thread:

Something seriously screwed up is happening, that is causing:

• The LHN and header says invited for both of us, rather than invited @nathan.d.metcalf@gmail.com
• His parent chat correctly shows invited @nathan.d.metcalf@gmail.com, but mine shows invited @hidden. @puneetlath This is likely due to some kind of isKnown issue, can you investigate this?
• My parent chat also shows a bunch of phantom replies equal to the number of comments I made to this thread.
• I can't add/remove people to this thread from the Members page
• I can't @mention invite anyone to this thread either

Let's get some proposals for cleaning all this up.

[puneetlath]

His parent chat correctly shows invited @nathan.d.metcalf@gmail.com, but mine shows invited @hidden. @puneetlath This is likely due to some kind of isKnown issue, can you investigate this?

Agreed, it seems like this is because you don't "know" the user he invited. Though, hmm this is a worksapce room. So you should "know" all other room members. So it's more likely that it's that we aren't returning the personalDetails of the mentioned user when returning the reportAction.

[sonialiap]

I've posted this issue in expensify-bugs and in callstack

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[narefyev91]

Hi, I'm Nicolay from Callstack - expert contributor group - and I will try to help here.

[melvin-bot[bot]]

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[sonialiap]

Thanks Nicolay! I'm going to assign you to the issue so that we can keep track who's working on this and to keep melvin-bot happy

[narefyev91]

@quinthar @puneetlath @aimane-chnaif Hey guys! Just some updates from my side: Seems we need to have some work with BE team.

1. For this one "The LHN and header says invited for both of us, rather than invited @nathan.d.metcalf@gmail.com" - for this current place we just pick information which coming from API side - you can see on screenshot - that we have a lot of free space between invited and to. And just the same happened when we will invite 2 persons as well:

Will write some FE logic to build correct naming for channels and threads as well

2. For the case of @hidden instead of real user name - seems that it's expected - because we show hidden if mentioned user is not logged in inside the system (you can see on the first image - that one user is @hidden and one already has previously logged in the app and admin already had some interaction with that user). @puneetlath were right that admin does not know "who you invited" - because we store information inside "personalDetails" - for the current user (in example just to @nathanmetcalf). That's why in example "@nathanmetcalf" sees correct name instead of @hidden, but admin does not know that user.

3. For the rest of the issues - seems most of the points were fixed before - i could not reproduce problem with ghost replies - neither in code no in the app. And also i can invite any new member either with @ or inside right panel in settings

   

Let me know if any of those points makes sense!

[quinthar]

Hi there, thanks for that! I'm not 100% if you are waiting on answers. If so, can you please ask the specific questions in a numbered list, and tag who you are asking the question to? Thanks!

[quinthar]

Also, can you give an ETA for completion? Can it be done by Monday? Thanks!

[narefyev91]

Hey @quinthar ! Yup i think we will be good to go here before Monday for sure.

[quinthar]

Can you give a new ETA? Can we get this merged today?

[narefyev91]

Can you give a new ETA? Can we get this merged today?

Hey! It's mostly depends when @aimane-chnaif will take a review - PR opened since last friday

[quinthar]

@aimane-chnaif What's your ETA? Can we finish the review and merge today?

[aimane-chnaif]

started review. will complete today

[quinthar]

Did you complete yesterday? What's the update?

[melvin-bot[bot]]

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

[aimane-chnaif]

review completed with some NAB comments

[quinthar]

Ok, so what's next? It's sat for 12 hours with no blockers, what is stopping us from merging it?

[blimpich]

@quinthar after Aimane reviews the PR it passes on to the Contributor Manager Engineer (who is me in this case) for final review. Aimane finished their review in the wee hours of the morning PST time, so it sat there for 12 hours because I didn't get to it till this morning. Here is my review. I left some comments, so we are now waiting for those to be addressed, and then we can merge.

[blimpich]

Merged ✅

[quinthar]

Woohoooo!!!!!

[melvin-bot[bot]]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/34362

If no regressions arise, payment will be issued on 2024-02-01. :confetti_ball:

For reference, here are some details about the assignees on this issue:

• @narefyev91 does not require payment (Contractor)
• @aimane-chnaif requires payment automatic offer (Reviewer)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [ ] [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [ ] [@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [ ] [@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [ ] [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [ ] [@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [ ] [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[sonialiap]

@aimane-chnaif $500 - paid ✔️ please complete the checklist

[aimane-chnaif]

This is not bug but feature.

Regression Test Proposal

1. Alice creates a workspace Alice's Apples
2. Alice invites Bob to the workspace
3. Bob creates a room #bagel-lovers that Alice is not in
4. Bob invites Cathy (who is not a member of the workspace) to #bagel-lovers
5. This cause Bob to post a system message to the #admins room of Alice's Apples saying: Bob invited Cathy to #bagel-lovers
6. Alice opens a thread on that system message
7. Alice posts "What up Bob, why did you invite Cathy?" in that thread
8. Confirm Bob is invited to that thread and sees Alice's message
9. Mention by @ should be available
10. Click into the room header and go to Members
11. Confirm that it shows the member list with options to add/remove