MEDIUM: Show full thread ancestry (back end, 2/2)

[quinthar]

Note: Please see #32618 for the complete problem/solution statement. This issue only relates to implementing the back-end subset of the issue.

• Back-end: (To be done by an internal engineer.) Update OpenReport to return the the entire ancestry (i.e., the parentReportID, parentReportActionID and the reportAction to display), in the same exact format as if you had manually navigated down the tree, as far as it is shared with this user
  • Most of the time you will find a conversation by descending "down the tree", meaning you will already have the full ancestry locally cached by the time you get to the leaf thread conversation.
  • However, if you are mentioned in a branch, or switch platforms while in the middle of a chat, it's possible that you will be on a client that doesn't already know its full ancestry, so this change will ensure when you open the chat, you always populate its full ancestry.
  • The ancestry itself is stored using standard reportActions (exactly like you had navigated there from the top); nothing fancy:
    • reportActions_1: { 10: {original comment} }
    • reportActions_2: { 20: {child comment} }
    • reportActions_3: { 30: {grandchild comment} }
    • report_2: {parentReportID: 1, parentReportActionID: 10}
    • report_3: {parentReportID: 2, parentReportActionID: 20}
  • OpenReport should do a recursive query as far as the caller has access -- recognizing that not all users will be able to access the full ancestry (eg, if you are pulled into a thread in a private chat, and then that thread has another thread, you'll be able to see a partial ancestry -- including the original message in the private chat that was threaded in the first place -- but you won't be able to navigate back to that private chat, nor see any other chats that it might be a thread inside of). There are likely a few ways to do this:
  • Perhaps use a recursive common table expression to pull all the necessary ancestor data in a single query
  • Alternatively, create some kind of recursive function like Report::getAncestry which accepts parentReportID to look up the parent report, and then its parent, so on recursively, adding results to the response.

[marcaaron]

I've got a Wave issue assigned now and haven't started on this. Going to remove myself to not be a blocker here. Doing a recursive SQLite query sounds pretty unique though so if nobody grabs it I'm happy to take it on and/or follow along in the reviews.

[quinthar]

Nobody's picked it up yet, I'd love your help!

[marcaaron]

Alright I'm back in 😄

[marcaaron]

The recursive query part wasn't too bad! Turns out we already had an existing example in the code to follow. Did have a question on this one though...

if you are pulled into a thread in a private chat, and then that thread has another thread, you'll be able to see a partial ancestry

Thinking about workspace/non-workspace members for a sec... this is how I might imagine it could work and want to confirm first...

Case 1:

• action: Invite anyone to a thread at any level inside a public room.
• expectation: They can see the full ancestry since the top level room is public.

Case 2:

• action: Invite anyone to a thread at any level inside a private room
• expectation: They can only see the thread they were invited to.

Case 3:

• action: Invite an off-workspace person to a thread on a workspace room
• expectation: This works the same as a private room for the off workspace person. They are not workspace members and would have to be explicitly invited to the parent room in order to access the full ancestry.

Case 4:

• action: Invite workspace member to a thread inside a workspace room
• expectation: They can see the full ancestry as they are a policy member.

Mostly not sure about Case 3. I could see the usefulness of inviting an off-workspace person to a thread and also providing some way to optionally grant access to the entire contents of the room. But also feels like something we'd ask your permission to do first.

[marcaaron]

Update: I ran through all of these cases to see how things work now. And I'm gonna assume that the cases I laid out above is the direction we want to go in. We can always change how this works later. 👍

ETA: This change is going to take some extra time because the logic for building report action data is complex and we are still building them the PHP layer. The "easy" path to getting that to work is:

1. Make workspace members participants of the ancestor reports they have not joined yet, but can access.
2. Give them notificationPreference = 'hidden' so they do not show up in the LHN
3. PHP can then get the report data so it can build the reportActions we need to return to the client

The hard path would be to migrate all report actions to Auth. But this would be a very large refactor that I'm not convinced we need to do right now.

[marcaaron]

This is in review!

[marcaaron]

Auth PR Merged.

Web PR holding on the Auth deploy.

[melvin-bot]

This issue has not been updated in over 15 days. @marcaaron eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[melvin-bot]

@marcaaron, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.