feat(reply_private): Enhance reply private quote

[existentialcoder]

☑️ Resolves

• Fix Reply privately does not quote message #6301

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

Scenario	🏚️ Before	🏡 After
On Group convo	—
On DM	—
After message post on receiver chat	—

🏁 Checklist

• 🌏 Tested with different browsers / clients:
  • Chromium (Chrome / Edge / Opera / Brave)
  • Firefox
  • Safari
  • Talk Desktop
  • Integrations with Files sidebar and other apps
  • Not risky to browser differences / client
• 🖌️ Design was reviewed, approved or inspired by the design team
• ⛑️ Tests are included or not possible
• 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

---

🛠️ API Checklist

🚧 Tasks

• Removed the roomId check on CommentManager::getParentComment
• Store meta information on the childComment about the original parent message copy, channel name and poster.

Suggestions needed

Still the following frontend changes are to be made (backend changes are implemented). Suggestions needed to implement to show the message, channel and poster copy for the below scenarios
- Parent message deleted
- User left parent conversation
- Parent conversation deleted

🏁 Checklist

• ⛑️ Tests (unit and/or integration) are included or not possible
• 📘 API documentation in docs/ has been updated or is not required
• 🔖 Capability is added or not needed

[nickvergessen]

This is a security violation.
It basically allows you to reply to any message, extracting it into your private chat, even if you are not part of the original conversation

[existentialcoder]

I thought so. The initial idea was to pass on the parentConvoToken from the frontend and validate it like this

targetRoomId = isPrivateReply ? parentRoomId : $this->roomId
if ($comment->getObjectType() !== 'chat' || $comment->getObjectId() !== targetRoomId) {
		if ($comment->getObjectType() !== 'chat') {

Along with this we need to check if the logged in user

• is part of the parent room
• is part of the parent message

These three checks would ensure that it is indeed a valid private reply. Do you think this would work? @nickvergessen

[nickvergessen]

• What if the "replied to" user is not part of the parent room anymore?
• What if one of them gets removed afterwards, should they still see the message content?
• What if the message was edited, should it update?
  • Yes: In case a password was posted or something similar
  • No: Because access could have changed for either parts
  • Performance: I would not know how we can best find out which messages need updating?

That's basically exactly the problems why we didn't solve this issue yet 🙈

[nickvergessen]

You have any thoughts on that (before continuing to create more code)

[existentialcoder]

Sure. Let me finalize this and make code changes. This is how I thought it could work based on how it works on Whatsapp

When a A replies privately to B from convo C, during the operation, validate and proceed if and only if

• A and B are part of C
• current room is a dm of A and B
• logged in user is either A or B

Once these are checked, take a snapshop of room C's name, the original parent message and store it in the new message (the changes are already made) and always show that. If the parent message / room exists and the users have access to it, the redirects would work. We do not care about the edits to the message later as well

To answer your questions

What if the "replied to" user is not part of the parent room anymore?
They should still see the snapshot in the DM.
Redirecting to the original room will not work in this case.

What if one of them gets removed afterwards, should they still see the message content?
Yes. Again the snapshot of the original message.

What if the message was edited, should it update?
No.

[existentialcoder]

I'll discuss it with the team next week

Let me know if anything was discussed. Thanks

[nickvergessen]

So our idea would be the following:

• On replying-privately, the message ID of the "message to reply to" and the conversation token of the "conversation where the message to reply to is in", need to be handed in
• When both users can access the replyTo-conversation, we create a new message with a specific verb
• Those messages are not rendered actually in the chat, but only as parents
• They should have the meta-data information of "actual message id" and "actual conversation token", so frontend/clients can link to them

I hope this makes sense to you?

[existentialcoder]

Yes. Have few follow up questions in terms of the UX

• What parent message do we show in the replyTo- conversation. Will it be something like a generic "Go to parent message"? Asking because we do not have the copy of the original message on this reference parent message we create. Should the original message be picked / fetched on the fly and shown? Will it impact the performance.

And how to handle the same

• in case the parent message is deleted
• user loses access to parent message

Also wanted to understand anything that is specifically concerning in the snapshot approach mentioned above

[Antreesy]

Also wanted to understand anything that is specifically concerning in the snapshot approach mentioned above

We agreed on having it as a snapshot, basically. As a follow-up, we can discuss whether to expire this snapshot somehow, if the original was edited/deleted/access changed.

From UI perspective, I expect that this should result in following for now:

{
 ...originalSnapshot,
 id: <new message id as it's a new entry in DB>
 token: <token of reply-to 1-1 conversation>
 messageType: <i guess we're introducing a new type, e.g. 'comment_hidden'>
 ...{ some fields might need to be overwritten, e.g. reactions, threadId, parent}
 metaData: {
  originalId: <from original message>
  originalToken: <from original message>
  }
}

in case the parent message is deleted

Copy stays in 1-1 chat, it now links to deleted message (we still render those in original place)

user loses access to parent message

Copy stays in 1-1 chat, click redirects to 403 or 404 - should already be handled by frontend at least

[existentialcoder]

Sure makes sense. I should be pushing the changes before next Sunday. Thanks

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)