#5898 Quick Reply

[MarlowBrown]

First time contributor checklist

• I have read the README and CONTRIBUTING documents
• I have signed the Contributor Licence Agreement

Contributor checklist

• My commits are rebased on the latest main branch
• My commits are in nice logical chunks
• My contribution is fully baked and is ready to be merged as is
• I have tested my contribution on these devices:

• iPhone 16, iOS 18.2

---

Description

This pull request fixes #5898 by allowing users to see which message they quick replied to. Previously before this PR users were not able to see the message that they replied to by using quick reply in the Notification Content Extension. I tested this PR by performing this user requested feature to make sure that it functioned as laid out in the user issue ticket.

QuickReply.mov

[sashaweiss-signal]

FWIW, there are a lot of changes in this diff that are unrelated to the meaningful change, which is to build and pass along a quoted reply model. Those changes (e.g., renaming methods, restructuring methods without functional change) make it harder to review the change and verify that it's correct/safe. Generally, it's easier for us to accept changes that only touch the required lines.

[MarlowBrown]

I made those extra changes because it was pretty difficult to keep track of what indentation level that I was in, so I flattened out the nested calls. It makes it easier to read and modify in the future, but at a cost of more time to QA it.

[sashaweiss-signal]

I understand, and I don't disagree that working with this code is tricky. At the same time, it's now difficult for me to verify that something subtle hasn't changed in the functionality. For example, there are now a lot of unnecessary calls to firstly that will result in a lot of unnecessary hopping between dispatch queues.

I'd appreciate if, now that we understand what the desired change is here, we could reset the refactoring and keep only the changes related to fetching and using a DraftQuotedReplyModel. That'll make it a lot easier for me to verify that the new code uses Promises appropriately.

[MarlowBrown]

Yeah, I can do that! I'll undo the renamings and the restructurings and focus just on the bits that need to change to still implement the feature. Since I now know how the final code turned out it should be easy to do. I'll go get to work on it!

[MarlowBrown]

@sashaweiss-signal Just made the changes. Let me know what you think!

[sashaweiss-signal]

There's no need for these to be their own Promises. We can construct the reply inline here:

...
let thread = notificationMessage.thread
let interaction = notificationMessage.interaction

let draftQuotedReplyModelForSending = try SKEnvironment.shared.databaseStorageRef.read { tx -> DraftQuotedReplyModel.ForSending? in
  if
    let incomingMessage = interaction as? TSIncomingMessage,
    let draftQuotedReplyModel = DependenciesBridge.shared.quotedReplyManager.buildDraftQuotedReply(originalMessage: incomingMessage, tx: txasV2Read)
  {
    return try DependenciesBridge.shared.quotedReplyManager.prepareDraftForSending(draftQuotedReplyModel)
  }

  return nil
}

[MarlowBrown]

IIRC If you put DependenciesBridge.shared.quotedReplyManager.prepareDraftForSending inside of a SSKEnvironment.shared.databaseStorageRef.read or SSKEnvironment.shared.databaseStorageRef.write block it will cause a database reentrant error because prepareDraftForSending is also reading from the database.

[sashaweiss-signal]

Ah, annoying. Then something like:

...
let thread = notificationMessage.thread
let interaction = notificationMessage.interaction

let draftQuotedReplyModel = try SKEnvironment.shared.databaseStorageRef.read { tx -> DraftQuotedReplyModel? in
  if let incomingMessage = interaction as? TSIncomingMessage {
    return DependenciesBridge.shared.quotedReplyManager.buildDraftQuotedReply(originalMessage: incomingMessage, tx: txasV2Read)
  }

  return nil
}

let draftQuotedReplyModelForSending = try draftQuotedReplyModel.map { try DependenciesBridge.shared.quotedReplyManager.prepareDraftForSending($0) }

[sashaweiss-signal]

I see you already did something similar, thanks.

[sashaweiss-signal]

Thanks for making this PR much easier to review. I'll work on getting it merged internally!

[sashaweiss-signal]

This has been merged internally as 6268f7f80a, which should become public in a week or two with an upcoming release. Thanks again!