[MM-57409] Notify error sharing on iOS #8408

Willyfrog · 2024-12-11T15:01:54Z

Ticket: MM-57409

Approach to solve it:

a local notification for failure is scheduled.
the device tries to share
1. If the share action succeeds, the notification is removed from the queue
2. if the share action is known to fail, we remove the notification and launch a new one immediately
3. if the share action takes too long, notification kicks in and next time we remove attempts to keep trying.

Why such a complex approach:

iOS might kill the share action process, by having this queued notification we'll ensure the user doesn't expect it to be shared
iOS waits for up to 9 days for the user to come online to try and do the sharing action, while this might be fine for other apps, it goes a bit against the timing expected by our users. I set up a 5 min deadline, but we can change it to something else (bigger/smaller)
we don't have control on when the share action is executing

Context:

This is only for iOS. Since the code path is very different we can process this and I'll tackle Android as soon as possible, depending on other priorities.
Timeout needs to be defined.
The message should be reviewed. I kept a vague message for the scheduled notification since there could be a number of issues at play, but it can definitely be improved.

Pending:

There are no tests for this part and I'm not sure how to do them, so any pointers would be great.
Localization, since this is not part of the regular app, I haven't yet done any localization.
Android.
Unit testing.
allow users to specify their own timeout within some sensible range. This could be discussed if it makes sense.

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
Have tested against the 5 core themes to ensure consistency between them.
Have run E2E tests by adding label E2E iOS tests for PR.

Device Information

iOS simulator

Screenshots

Release Note

Improved error notification when sharing on iOS

enahum

Additionally, what happens when:

The use taps on the notification that was presented.
The user has the application opened when the notification triggers
Tap on the notification inside the app as shown in 2.

enahum · 2024-12-12T00:12:32Z

ios/Gekidou/Package.resolved

Why the changes on this file?

I'll remove them, they were added by mistake

enahum · 2024-12-12T07:37:47Z

ios/Gekidou/Sources/Gekidou/Networking/ShareExtension+Delegate.swift

+        if let timeout = data.failTimeout, timeout < Date() {
+            backgroundSession?.invalidateAndCancel()
+            os_log(
+                "Mattermost BackgroundSession: postMessageIfUploadFinished session %{public}@ timed out",
+                session.configuration.identifier ?? "no identifier"
+            )
+            return
+        }


shouldn't this be done after checking if the last file was actually uploaded? because if that is the case, this is cancelling the whole thing instead of just posting the message

Yup! The reason is that we already told the user that the sharing failed due to a timeout. Since we can't control when this happens this could be a day later making things confusing. So we cancel the first opportunity we have

enahum · 2024-12-12T07:39:40Z

ios/Gekidou/Sources/Gekidou/Networking/ShareExtension+Post.swift

@@ -56,6 +61,7 @@ extension ShareExtension {
                                filename,
                                id
                            )
+                            return "There was an error when trying to upload. Please try again"


before the other two returns you added a notifyFailureNow call, but not here, why?

enahum · 2024-12-12T07:41:43Z

ios/Gekidou/Sources/Gekidou/Networking/ShareExtension.swift

        )
    }
 }

+// TODO: setup some configuration for allowing users to figure out the right ammount of time to wait.


I don't think we should allow the user to set this, we should come up with a number that makes sense.. maybe an hour or so

That's cool with me too. O just didn't feel entitled to make a decision on either option

enahum · 2024-12-12T07:43:41Z

ios/Gekidou/Sources/Gekidou/Networking/ShareExtension.swift

+let NETWORK_ERROR_MESSAGE: String = "Mattermost App couldn't acknowledge the message was posted due to network conditions, please review and try again"
+let FILE_ERROR_MESSAGE: String = "The shared file couldn't be processed for sharing"


Ideally this would be localized. You can check the code in the Share Extension around the Views on how is done with the use of NSLocalizedString or String.localizedString.

enahum · 2024-12-12T07:44:14Z

ios/Gekidou/Sources/Gekidou/Networking/ShareExtension.swift

+
+    func scheduleFailNotification(timeout: Double = SHARE_TIMEOUT, description: String = NETWORK_ERROR_MESSAGE) -> String {
+        let failNotification = UNMutableNotificationContent()
+        failNotification.title = "Share content failed"


this should also be localized.

Willyfrog · 2024-12-12T10:46:32Z

Additionally, what happens when:

The use taps on the notification that was presented.

The user has the application opened when the notification triggers

Tap on the notification inside the app as shown in 2.

for 1 and 3, it opens the app.
2 is not showing, seems I need to also handle the local notification within the app, right? I'll work on that next as well as the translations and update the PR.

Willyfrog added 2 commits December 5, 2024 16:27

ios share extension handle error

7586206

move info to preferences and cancel if we are over the timeout

ec3f621

Willyfrog added 2: Dev Review Requires review by a core commiter 2: UX Review Requires review by a UX Designer labels Dec 11, 2024

Willyfrog requested review from rahimrahman and enahum December 11, 2024 15:01

Willyfrog self-assigned this Dec 11, 2024

mm-cloud-bot added release-note labels Dec 11, 2024

Willyfrog added the 2: PM Review Requires review by a product manager label Dec 11, 2024

Willyfrog requested a review from esethna December 11, 2024 15:02

enahum requested changes Dec 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-57409] Notify error sharing on iOS #8408

[MM-57409] Notify error sharing on iOS #8408

Willyfrog commented Dec 11, 2024 •

edited by jira bot

Loading

enahum left a comment

enahum Dec 12, 2024

Willyfrog Dec 12, 2024

enahum Dec 12, 2024

Willyfrog Dec 12, 2024

enahum Dec 12, 2024

enahum Dec 12, 2024

Willyfrog Dec 12, 2024

enahum Dec 12, 2024

enahum Dec 12, 2024

Willyfrog commented Dec 12, 2024

		let NETWORK_ERROR_MESSAGE: String = "Mattermost App couldn't acknowledge the message was posted due to network conditions, please review and try again"
		let FILE_ERROR_MESSAGE: String = "The shared file couldn't be processed for sharing"

[MM-57409] Notify error sharing on iOS #8408

Are you sure you want to change the base?

[MM-57409] Notify error sharing on iOS #8408

Conversation

Willyfrog commented Dec 11, 2024 • edited by jira bot Loading

Checklist

Device Information

Screenshots

Release Note

enahum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Willyfrog commented Dec 12, 2024

Willyfrog commented Dec 11, 2024 •

edited by jira bot

Loading