From 93c2f0f503d56908900944deb01b211e1ccf428c Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Tue, 13 Sep 2022 10:51:26 +1000 Subject: [PATCH] Fixed some bugs with disappearing messages Fixed an issue where the DisappearingMessages job could incorrectly overwrite it's nextRunTimestamp Fixed an issue where sent/self-send messages wouldn't correctly trigger the disappearing messages job Fixed an issue where sending the mnemonic along with an attachment wasn't showing the warning prompt Fixed an issue where the home screen wasn't updating on launch if the displayed messages were removed disappearing messages Fixed a small UI glitch where the input field wouldn't immediately get cleared when sending a message (unfortunately there is a slight delay before the message appears still) --- .../ConversationVC+Interaction.swift | 33 ++++++++++----- Session/Conversations/ConversationVC.swift | 2 +- Session/Meta/AppDelegate.swift | 18 ++++++--- .../Database/Models/Interaction.swift | 9 ++++- .../Jobs/Types/DisappearingMessagesJob.swift | 13 +++--- .../Typing Indicators/TypingIndicators.swift | 4 +- SessionUtilitiesKit/JobRunner/JobRunner.swift | 40 +++++++++++-------- 7 files changed, 72 insertions(+), 47 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index 1a7e3e38a..34e695aaf 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -361,12 +361,11 @@ extension ConversationVC: let modal = SendSeedModal() modal.modalPresentationStyle = .overFullScreen modal.modalTransitionStyle = .crossDissolve - modal.proceed = { self.sendMessage(hasPermissionToSendSeed: true) } + modal.proceed = { [weak self] in self?.sendMessage(hasPermissionToSendSeed: true) } return present(modal, animated: true, completion: nil) } - // Clearing this out immediately (even though it already happens in 'messageSent') to prevent - // "double sending" if the user rapidly taps the send button + // Clearing this out immediately to make this appear more snappy DispatchQueue.main.async { [weak self] in self?.snInputView.text = "" self?.snInputView.quoteDraftInfo = nil @@ -462,7 +461,7 @@ extension ConversationVC: ) } - func sendAttachments(_ attachments: [SignalAttachment], with text: String, onComplete: (() -> ())? = nil) { + func sendAttachments(_ attachments: [SignalAttachment], with text: String, hasPermissionToSendSeed: Bool = false, onComplete: (() -> ())? = nil) { guard !showBlockedModalIfNeeded() else { return } for attachment in attachments { @@ -472,6 +471,25 @@ extension ConversationVC: } let text = replaceMentions(in: snInputView.text.trimmingCharacters(in: .whitespacesAndNewlines)) + + if text.contains(mnemonic) && !viewModel.threadData.threadIsNoteToSelf && !hasPermissionToSendSeed { + // Warn the user if they're about to send their seed to someone + let modal = SendSeedModal() + modal.modalPresentationStyle = .overFullScreen + modal.modalTransitionStyle = .crossDissolve + modal.proceed = { [weak self] in + self?.sendAttachments(attachments, with: text, hasPermissionToSendSeed: true, onComplete: onComplete) + } + return present(modal, animated: true, completion: nil) + } + + // Clearing this out immediately to make this appear more snappy + DispatchQueue.main.async { [weak self] in + self?.snInputView.text = "" + self?.snInputView.quoteDraftInfo = nil + + self?.resetMentions() + } // Note: 'shouldBeVisible' is set to true the first time a thread is saved so we can // use it to determine if the user is creating a new thread and update the 'isApproved' @@ -538,13 +556,6 @@ extension ConversationVC: } func handleMessageSent() { - DispatchQueue.main.async { [weak self] in - self?.snInputView.text = "" - self?.snInputView.quoteDraftInfo = nil - - self?.resetMentions() - } - if Storage.shared[.playNotificationSoundInForeground] { let soundID = Preferences.Sound.systemSoundId(for: .messageSent, quiet: true) AudioServicesPlaySystemSound(soundID) diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index a7c406903..e165c03b0 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -859,7 +859,7 @@ final class ConversationVC: BaseVC, ConversationSearchControllerDelegate, UITabl deleteSectionsAnimation: .none, insertSectionsAnimation: .none, reloadSectionsAnimation: .none, - deleteRowsAnimation: .bottom, + deleteRowsAnimation: .fade, insertRowsAnimation: .none, reloadRowsAnimation: .none, interrupt: { itemChangeInfo?.isInsertAtTop == true || $0.changeCount > ConversationViewModel.pageSize } diff --git a/Session/Meta/AppDelegate.swift b/Session/Meta/AppDelegate.swift index 057c16478..ef84f5c0d 100644 --- a/Session/Meta/AppDelegate.swift +++ b/Session/Meta/AppDelegate.swift @@ -242,16 +242,22 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD Configuration.performMainSetup() JobRunner.add(executor: SyncPushTokensJob.self, for: .syncPushTokens) - // Trigger any launch-specific jobs and start the JobRunner - JobRunner.appDidFinishLaunching() - /// Setup the UI /// - /// **Note:** This **MUST** be run before calling `AppReadiness.setAppIsReady()` otherwise if - /// we are launching the app from a push notification the HomeVC won't be setup yet and it won't open the - /// related thread + /// **Note:** This **MUST** be run before calling: + /// - `AppReadiness.setAppIsReady()`: + /// If we are launching the app from a push notification the HomeVC won't be setup yet + /// and it won't open the related thread + /// + /// - `JobRunner.appDidFinishLaunching()`: + /// The jobs which run on launch (eg. DisappearingMessages job) can impact the interactions + /// which get fetched to display on the home screen, if the PagedDatabaseObserver hasn't + /// been setup yet then the home screen can show stale (ie. deleted) interactions incorrectly self.ensureRootViewController(isPreAppReadyCall: true) + // Trigger any launch-specific jobs and start the JobRunner + JobRunner.appDidFinishLaunching() + // Note that this does much more than set a flag; // it will also run all deferred blocks (including the JobRunner // 'appDidBecomeActive' method) diff --git a/SessionMessagingKit/Database/Models/Interaction.swift b/SessionMessagingKit/Database/Models/Interaction.swift index 933a86b9f..fe0830dcb 100644 --- a/SessionMessagingKit/Database/Models/Interaction.swift +++ b/SessionMessagingKit/Database/Models/Interaction.swift @@ -519,8 +519,13 @@ public extension Interaction { .asRequest(of: Int64.self) .fetchAll(db) - // Don't bother continuing if there are not interactions to mark as read - guard !interactionIdsToMarkAsRead.isEmpty else { return } + // If there are no other interactions to mark as read then just schedule the jobs + // for this interaction (need to ensure the disapeparing messages run for sync'ed + // outgoing messages which will always have 'wasRead' as false) + guard !interactionIdsToMarkAsRead.isEmpty else { + scheduleJobs(interactionIds: [interactionId]) + return + } // Update the `wasRead` flag to true try interactionQuery.updateAll(db, Columns.wasRead.set(to: true)) diff --git a/SessionMessagingKit/Jobs/Types/DisappearingMessagesJob.swift b/SessionMessagingKit/Jobs/Types/DisappearingMessagesJob.swift index 9ed31c7e7..d6897e1ff 100644 --- a/SessionMessagingKit/Jobs/Types/DisappearingMessagesJob.swift +++ b/SessionMessagingKit/Jobs/Types/DisappearingMessagesJob.swift @@ -17,7 +17,7 @@ public enum DisappearingMessagesJob: JobExecutor { deferred: @escaping (Job) -> () ) { // The 'backgroundTask' gets captured and cleared within the 'completion' block - let timestampNowMs: TimeInterval = (Date().timeIntervalSince1970 * 1000) + let timestampNowMs: TimeInterval = ceil(Date().timeIntervalSince1970 * 1000) var backgroundTask: OWSBackgroundTask? = OWSBackgroundTask(label: #function) let updatedJob: Job? = Storage.shared.write { db in @@ -29,12 +29,9 @@ public enum DisappearingMessagesJob: JobExecutor { // Update the next run timestamp for the DisappearingMessagesJob (if the call // to 'updateNextRunIfNeeded' returns 'nil' then it doesn't need to re-run so // should have it's 'nextRunTimestamp' cleared) - return updateNextRunIfNeeded(db) - .defaulting( - to: try job - .with(nextRunTimestamp: 0) - .saved(db) - ) + return try updateNextRunIfNeeded(db) + .defaulting(to: job.with(nextRunTimestamp: 0)) + .saved(db) } success(updatedJob ?? job, false) @@ -65,7 +62,7 @@ public extension DisappearingMessagesJob { return try? Job .filter(Job.Columns.variant == Job.Variant.disappearingMessages) .fetchOne(db)? - .with(nextRunTimestamp: ((nextExpirationTimestampMs / 1000) + 1)) + .with(nextRunTimestamp: ceil(nextExpirationTimestampMs / 1000)) .saved(db) } diff --git a/SessionMessagingKit/Sending & Receiving/Typing Indicators/TypingIndicators.swift b/SessionMessagingKit/Sending & Receiving/Typing Indicators/TypingIndicators.swift index cdda8b025..9e50e80b8 100644 --- a/SessionMessagingKit/Sending & Receiving/Typing Indicators/TypingIndicators.swift +++ b/SessionMessagingKit/Sending & Receiving/Typing Indicators/TypingIndicators.swift @@ -98,7 +98,7 @@ public class TypingIndicators { withTimeInterval: (direction == .outgoing ? 3 : 5), repeats: false ) { _ in - Storage.shared.write { db in + Storage.shared.writeAsync { db in TypingIndicators.didStopTyping(db, threadId: threadId, direction: direction) } } @@ -123,7 +123,7 @@ public class TypingIndicators { withTimeInterval: 10, repeats: false ) { [weak self] _ in - Storage.shared.write { db in + Storage.shared.writeAsync { db in self?.scheduleRefreshCallback(db) } } diff --git a/SessionUtilitiesKit/JobRunner/JobRunner.swift b/SessionUtilitiesKit/JobRunner/JobRunner.swift index d336f624b..2d08e817a 100644 --- a/SessionUtilitiesKit/JobRunner/JobRunner.swift +++ b/SessionUtilitiesKit/JobRunner/JobRunner.swift @@ -888,30 +888,36 @@ private final class JobQueue { // but we want at least 1 second to pass before doing so - the job itself should // really update it's own 'nextRunTimestamp' (this is just a safety net) case .recurring where job.nextRunTimestamp <= Date().timeIntervalSince1970: + guard let jobId: Int64 = job.id else { break } + Storage.shared.write { db in - _ = try job - .with(nextRunTimestamp: (Date().timeIntervalSince1970 + 1)) - .saved(db) + _ = try Job + .filter(id: jobId) + .updateAll( + db, + Job.Columns.failureCount.set(to: 0), + Job.Columns.nextRunTimestamp.set(to: (Date().timeIntervalSince1970 + 1)) + ) } - // For `recurringOnLaunch/Active` jobs which have already run, we want to clear their - // `failureCount` and `nextRunTimestamp` to prevent them from endlessly running over - // and over and reset their retry backoff in case they fail next time + // For `recurringOnLaunch/Active` jobs which have already run but failed once, we need to + // clear their `failureCount` and `nextRunTimestamp` to prevent them from endlessly running + // over and over again case .recurringOnLaunch, .recurringOnActive: - if + guard let jobId: Int64 = job.id, job.failureCount != 0 && job.nextRunTimestamp > TimeInterval.leastNonzeroMagnitude - { - Storage.shared.write { db in - _ = try Job - .filter(id: jobId) - .updateAll( - db, - Job.Columns.failureCount.set(to: 0), - Job.Columns.nextRunTimestamp.set(to: 0) - ) - } + else { break } + + Storage.shared.write { db in + _ = try Job + .filter(id: jobId) + .updateAll( + db, + Job.Columns.failureCount.set(to: 0), + Job.Columns.nextRunTimestamp.set(to: 0) + ) } default: break