Skip to content

#5898 Quick Reply #5957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MarlowBrown wants to merge 9 commits into from
Closed

#5898 Quick Reply #5957

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8dd9b4d
Added in QuotedMessage to be send attached to an Outgoing message for…
Jan 31, 2025
054aa2b
Refactored a subfunction out of a bigger function.
Feb 1, 2025
37043b3
Renamed a function to be more descriptive in its functionality.
Feb 1, 2025
b336799
Finally implemented the feature! :)
Feb 2, 2025
71d1f3e
One last message.
Feb 2, 2025
8066820
Removed comments and made minor renaming and formatting changes.
Feb 5, 2025
08c0794
Reverted renaming and restructing changes.
Feb 6, 2025
e47a99a
Reimplemented the quick reply draft feature by only changing code tha…
Feb 7, 2025
514369b
Compacted the function even more.
Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Signal/ConversationView/ConversationViewController+GestureRecognizers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,6 @@ public struct CVLongPressHandler {

switch gestureLocation {
case .`default`:
// TODO: Rename from "Text view item" to "default"?
delegate.didLongPressTextViewItem(cell,
itemViewModel: itemViewModel,
shouldAllowReply: shouldAllowReply)
Expand Down
83 changes: 77 additions & 6 deletions Signal/Notifications/NotificationActionHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,13 @@ public class NotificationActionHandler {
private class func reply(userInfo: [AnyHashable: Any], replyText: String) throws -> Promise<Void> {
return firstly { () -> Promise<NotificationMessage> in
self.notificationMessage(forUserInfo: userInfo)
}.then(on: DispatchQueue.global()) { (notificationMessage: NotificationMessage) -> Promise<Void> in
let thread = notificationMessage.thread
let interaction = notificationMessage.interaction
}.then(on: DispatchQueue.global()) { (notificationMessage: NotificationMessage) -> Promise<(DraftQuotedReplyModel?, NotificationMessage)> in
return getDraftQuotedReplyModelTupleFromIncomingMessage(notificationMessage: notificationMessage)
}.then(on: DispatchQueue.global()) { (informationTuple: (draft: DraftQuotedReplyModel?, notificationMessage: NotificationMessage)) -> Promise<(DraftQuotedReplyModel.ForSending?, NotificationMessage)> in
return getDraftQuotedReplyModelForSendingTupleFromDraftAndIncomingMessage(optionalDraftModel: informationTuple.draft, notificationMessage: informationTuple.notificationMessage)
}.then(on: DispatchQueue.global()) { (informationTuple: (draft: DraftQuotedReplyModel.ForSending?, notificationMessage: NotificationMessage)) -> Promise<Void> in
Copy link
Contributor

@sashaweiss-signal sashaweiss-signal Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
}

Copy link
Author

@MarlowBrown MarlowBrown Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@sashaweiss-signal sashaweiss-signal Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) }

Copy link
Contributor

@sashaweiss-signal sashaweiss-signal Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you already did something similar, thanks.

let thread = informationTuple.notificationMessage.thread
let interaction = informationTuple.notificationMessage.interaction
guard (interaction is TSOutgoingMessage) || (interaction is TSIncomingMessage) else {
throw OWSAssertionError("Unexpected interaction type.")
}
Expand All @@ -135,7 +139,7 @@ public class NotificationActionHandler {
// If we're replying to a group story reply, keep the reply within that context.
if
let incomingMessage = interaction as? TSIncomingMessage,
notificationMessage.isGroupStoryReply,
informationTuple.notificationMessage.isGroupStoryReply,
let storyTimestamp = incomingMessage.storyTimestamp,
let storyAuthorAci = incomingMessage.storyAuthorAci
{
Expand All @@ -156,7 +160,7 @@ public class NotificationActionHandler {
explicitRecipients: [],
skippedRecipients: [],
transaction: transaction
))
), quotedReplyDraft: informationTuple.draft)
do {
let preparedMessage = try unpreparedMessage.prepare(tx: transaction)
return ThreadUtil.enqueueMessagePromise(message: preparedMessage, transaction: transaction)
Expand All @@ -169,7 +173,74 @@ public class NotificationActionHandler {
SSKEnvironment.shared.notificationPresenterRef.notifyUserOfFailedSend(inThread: thread)
throw error
}.then(on: DispatchQueue.global()) { () -> Promise<Void> in
self.markMessageAsRead(notificationMessage: notificationMessage)
self.markMessageAsRead(notificationMessage: informationTuple.notificationMessage)
}
}
}

private class func getDraftQuotedReplyModelTupleFromIncomingMessage(notificationMessage: NotificationMessage) -> Promise<(DraftQuotedReplyModel?, NotificationMessage)> {
let (promise, future) = Promise<(DraftQuotedReplyModel?, NotificationMessage)>.pending()
SSKEnvironment.shared.databaseStorageRef.read { readTransaction in
if
let incomingMessage = notificationMessage.interaction as? TSIncomingMessage,
let draftQuotedReplyModel = DependenciesBridge.shared.quotedReplyManager.buildDraftQuotedReply(originalMessage: incomingMessage, tx: readTransaction.asV2Read) {
future.resolve((draftQuotedReplyModel, notificationMessage))
}
return future.resolve((nil, notificationMessage))
}
return promise
}

private class func getDraftQuotedReplyModelForSendingTupleFromDraftAndIncomingMessage(optionalDraftModel: DraftQuotedReplyModel?, notificationMessage: NotificationMessage) -> Promise<(DraftQuotedReplyModel.ForSending?, NotificationMessage)> {
guard let draftModel = optionalDraftModel else {
return Promise.value((nil, notificationMessage))
}

do {
let draftForSending = try DependenciesBridge.shared.quotedReplyManager.prepareDraftForSending(draftModel)
return Promise.value((draftForSending, notificationMessage))
} catch {
return Promise.value((nil, notificationMessage))
}
}

private class func sendReplyToNoficationMessageWithMessageToReplyTo(replyText: String, messageBeingRespondedTo: DraftQuotedReplyModel.ForSending?, notificationMessage: NotificationMessage) throws -> Promise<Void> {
return SSKEnvironment.shared.databaseStorageRef.write { transaction in
let builder: TSOutgoingMessageBuilder = .withDefaultValues(thread: notificationMessage.thread)
builder.messageBody = replyText

// If we're replying to a group story reply, keep the reply within that context.
if
let incomingMessage = notificationMessage.interaction as? TSIncomingMessage,
notificationMessage.isGroupStoryReply,
let storyTimestamp = incomingMessage.storyTimestamp,
let storyAuthorAci = incomingMessage.storyAuthorAci
{
builder.storyTimestamp = storyTimestamp
builder.storyAuthorAci = storyAuthorAci
} else {
// We only use the thread's DM timer for normal messages & 1:1 story
// replies -- group story replies last for the lifetime of the story.
let dmConfigurationStore = DependenciesBridge.shared.disappearingMessagesConfigurationStore
let dmConfig = dmConfigurationStore.fetchOrBuildDefault(for: .thread(notificationMessage.thread), tx: transaction.asV2Read)
builder.expiresInSeconds = dmConfig.durationSeconds
builder.expireTimerVersion = NSNumber(value: dmConfig.timerVersion)
}

let outgoingMessage = TSOutgoingMessage(
outgoingMessageWith: builder,
additionalRecipients: [],
explicitRecipients: [],
skippedRecipients: [],
transaction: transaction
)

let unpreparedMessage = UnpreparedOutgoingMessage.forMessage(outgoingMessage, quotedReplyDraft: messageBeingRespondedTo)
do {
let preparedMessage = try unpreparedMessage.prepare(tx: transaction)
return ThreadUtil.enqueueMessagePromise(message: preparedMessage, transaction: transaction)
} catch {
return Promise(error: error)
}
}
}
Expand Down