Added a progress tracker to the bug report view (#513)

* created a progress tracker class and passed it in the user notification to be observed by the progress view * improved the publishing by dispatching it on RunLoop.main * bug report struct created and progress tracker class moved into the Other folder * some swiftlint adjustments * fixed tests * fixed another test BugReportServiceTests * changelog 495 - change * added a mock preview * fixing some linting suggestions * no need to use KVO, achieve the same result using a publisher * some refactors to address PR comments * some code improvements * fixed the issue that prevented the avatar of the room to be displayed in the mocks, and updated the tests * Revert "fixed the issue that prevented the avatar of the room to be displayed in the mocks, and updated the tests" This reverts commit 113d609.
element-hq · Feb 8, 2023 · e5522e7 · e5522e7
1 parent 4d87701
commit e5522e7
Show file tree

Hide file tree

Showing 12 changed files with 168 additions and 71 deletions.
diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj
@@ -345,6 +345,7 @@
 		A663FE6704CB500EBE782AE1 /* AnalyticsPromptCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = B4DE1CF8F5EFD353B1A5E36F /* AnalyticsPromptCoordinator.swift */; };
 		A69A54FF11A3F9EA0660E6BF /* NSE.appex in Embed App Extensions */ = {isa = PBXBuildFile; fileRef = 0D8F620C8B314840D8602E3F /* NSE.appex */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; };
 		A6DEC1ADEC8FEEC206A0FA37 /* AttributedStringBuilderProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 72F37B5DA798C9AE436F2C2C /* AttributedStringBuilderProtocol.swift */; };
+		A7CC2102298ACB1700DBE1C7 /* ProgressTracker.swift in Sources */ = {isa = PBXBuildFile; fileRef = A7CC2101298ACB1700DBE1C7 /* ProgressTracker.swift */; };
 		A7D48E44D485B143AADDB77D /* Strings+Untranslated.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1A18F6CE4D694D21E4EA9B25 /* Strings+Untranslated.swift */; };
 		A7FD7B992E6EE6E5A8429197 /* RoomSummaryDetails.swift in Sources */ = {isa = PBXBuildFile; fileRef = 142808B69851451AC32A2CEA /* RoomSummaryDetails.swift */; };
 		A851635B3255C6DC07034A12 /* RoomScreenCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8108C8F0ACF6A7EB72D0117 /* RoomScreenCoordinator.swift */; };
@@ -902,6 +903,7 @@
 		A65F140F9FE5E8D4DAEFF354 /* RoomProxy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomProxy.swift; sourceTree = "<group>"; };
 		A6B891A6DA826E2461DBB40F /* PHGPostHogConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PHGPostHogConfiguration.swift; sourceTree = "<group>"; };
 		A72232816DCE2B76D48E1367 /* nb-NO */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "nb-NO"; path = "nb-NO.lproj/Localizable.strings"; sourceTree = "<group>"; };
+		A7CC2101298ACB1700DBE1C7 /* ProgressTracker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressTracker.swift; sourceTree = "<group>"; };
 		A8903A9F615BBD0E6D7CD133 /* ApplicationProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ApplicationProtocol.swift; sourceTree = "<group>"; };
 		A8F48EB9B52E70285A4BCB07 /* ur */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ur; path = ur.lproj/Localizable.strings; sourceTree = "<group>"; };
 		A9873374E72AA53260AE90A2 /* fa */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fa; path = fa.lproj/Localizable.strings; sourceTree = "<group>"; };
@@ -2258,6 +2260,7 @@
 				6AD1A853D605C2146B0DC028 /* MatrixEntityRegex.swift */,
 				C789E7BFC066CF39B8AE0974 /* NetworkMonitor.swift */,
 				F754E66A8970963B15B2A41E /* PermalinkBuilder.swift */,
+				A7CC2101298ACB1700DBE1C7 /* ProgressTracker.swift */,
 				53482ECA4B6633961EC224F5 /* ScrollViewAdapter.swift */,
 				BB3073CCD77D906B330BC1D6 /* Tests.swift */,
 				1F2529D434C750ED78ADF1ED /* UserAgentBuilder.swift */,
@@ -3096,6 +3099,7 @@
 				B5903E48CF43259836BF2DBF /* EncryptedRoomTimelineView.swift in Sources */,
 				02D8DF8EB7537EB4E9019DDB /* EventBasedTimelineItemProtocol.swift in Sources */,
 				33D630461FC4562CC767EE9F /* FileCache.swift in Sources */,
+				A7CC2102298ACB1700DBE1C7 /* ProgressTracker.swift in Sources */,
 				5F06AD3C66884CE793AE6119 /* FileManager.swift in Sources */,
 				6C67774E8387D44426718BD9 /* FilePreviewCoordinator.swift in Sources */,
 				6C9F6C7F2B35288C4230EF3F /* FilePreviewModels.swift in Sources */,

diff --git a/ElementX/Sources/Other/ProgressTracker.swift b/ElementX/Sources/Other/ProgressTracker.swift
@@ -0,0 +1,39 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import Combine
+import Foundation
+
+protocol ProgressListener {
+    var progressSubject: CurrentValueSubject<Double, Never> { get }
+}
+
+protocol ProgressPublisher {
+    var publisher: AnyPublisher<Double, Never> { get }
+}
+
+final class ProgressTracker: ProgressListener, ProgressPublisher {
+    let progressSubject: CurrentValueSubject<Double, Never>
+
+    var publisher: AnyPublisher<Double, Never> {
+        progressSubject
+            .eraseToAnyPublisher()
+    }
+
+    init(initialValue: Double = 0.0) {
+        progressSubject = .init(initialValue)
+    }
+}
diff --git a/ElementX/Sources/Other/UserNotifications/UserNotification.swift b/ElementX/Sources/Other/UserNotifications/UserNotification.swift
@@ -22,9 +22,18 @@ enum UserNotificationType {
 }
 
 struct UserNotification: Equatable, Identifiable {
+    static func == (lhs: UserNotification, rhs: UserNotification) -> Bool {
+        lhs.id == rhs.id &&
+            lhs.type == rhs.type &&
+            lhs.title == rhs.title &&
+            lhs.iconName == rhs.iconName &&
+            lhs.persistent == rhs.persistent
+    }
+
     var id: String = UUID().uuidString
     var type = UserNotificationType.toast
     var title: String
     var iconName: String?
     var persistent = false
+    var progressPublisher: ProgressPublisher?
 }
diff --git a/ElementX/Sources/Other/UserNotifications/UserNotificationModalView.swift b/ElementX/Sources/Other/UserNotifications/UserNotificationModalView.swift
@@ -14,16 +14,22 @@
 // limitations under the License.
 //
 
+import Combine
 import SwiftUI
 
 struct UserNotificationModalView: View {
     let notification: UserNotification
-
+    @State private var progressFraction: Double?
+
     var body: some View {
         ZStack {
             VStack(spacing: 12.0) {
-                ProgressView()
-
+                if let progressFraction = progressFraction {
+                    ProgressView(value: progressFraction)
+                } else {
+                    ProgressView()
+                }
+
                 HStack {
                     if let iconName = notification.iconName {
                         Image(systemName: iconName)
@@ -34,32 +40,36 @@ struct UserNotificationModalView: View {
                 }
             }
             .padding()
-            .frame(minWidth: 150.0)
+            .frame(minWidth: 150.0, maxWidth: 250.0)
             .background(Color.element.quinaryContent)
             .clipShape(RoundedCornerShape(radius: 12.0, corners: .allCorners))
             .shadow(color: .black.opacity(0.1), radius: 10.0, y: 4.0)
+            .onReceive(notification.progressPublisher?.publisher ?? Empty().eraseToAnyPublisher()) { progress in
+                progressFraction = progress
+            }
             .transition(.opacity)
         }
         .id(notification.id)
         .frame(maxWidth: .infinity, maxHeight: .infinity)
         .background(.black.opacity(0.1))
         .ignoresSafeArea()
     }
-
-    private var toastTransition: AnyTransition {
-        AnyTransition
-            .asymmetric(insertion: .move(edge: .top),
-                        removal: .move(edge: .bottom))
-            .combined(with: .opacity)
-    }
 }
 
 struct UserNotificationModalView_Previews: PreviewProvider {
     static var previews: some View {
-        VStack {
+        Group {
+            UserNotificationModalView(notification: UserNotification(type: .modal,
+                                                                     title: "Successfully logged in",
+                                                                     iconName: "checkmark")
+            )
+            .previewDisplayName("Spinner")
             UserNotificationModalView(notification: UserNotification(type: .modal,
                                                                      title: "Successfully logged in",
-                                                                     iconName: "checkmark"))
+                                                                     iconName: "checkmark",
+                                                                     progressPublisher: ProgressTracker(initialValue: 0.5))
+            )
+            .previewDisplayName("Progress Bar")
         }
     }
 }
diff --git a/ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift b/ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift
@@ -51,8 +51,8 @@ final class BugReportCoordinator: CoordinatorProtocol {
             switch result {
             case .cancel:
                 self.completion?(.cancel)
-            case .submitStarted:
-                self.startLoading()
+            case let .submitStarted(progressTracker):
+                self.startLoading(progressPublisher: progressTracker)
             case .submitFinished:
                 self.stopLoading()
                 self.completion?(.finish)
@@ -75,11 +75,14 @@ final class BugReportCoordinator: CoordinatorProtocol {
 
     static let loadingIndicatorIdentifier = "BugReportLoading"
 
-    private func startLoading(label: String = ElementL10n.loading) {
-        parameters.userNotificationController?.submitNotification(UserNotification(id: Self.loadingIndicatorIdentifier,
-                                                                                   type: .modal,
-                                                                                   title: label,
-                                                                                   persistent: true))
+    private func startLoading(label: String = ElementL10n.loading, progressPublisher: ProgressPublisher) {
+        parameters.userNotificationController?.submitNotification(
+            UserNotification(id: Self.loadingIndicatorIdentifier,
+                             type: .modal,
+                             title: label,
+                             persistent: true,
+                             progressPublisher: progressPublisher)
+        )
     }
 
     private func stopLoading() {

diff --git a/ElementX/Sources/Screens/BugReport/BugReportModels.swift b/ElementX/Sources/Screens/BugReport/BugReportModels.swift
@@ -19,7 +19,7 @@ import UIKit
 
 enum BugReportViewModelAction {
     case cancel
-    case submitStarted
+    case submitStarted(progressTracker: ProgressTracker)
     case submitFinished
     case submitFailed(error: Error)
 }

diff --git a/ElementX/Sources/Screens/BugReport/BugReportViewModel.swift b/ElementX/Sources/Screens/BugReport/BugReportViewModel.swift
@@ -49,13 +49,16 @@ class BugReportViewModel: BugReportViewModelType, BugReportViewModelProtocol {
     // MARK: Private
 
     private func submitBugReport() async {
-        callback?(.submitStarted)
+        let progressTracker = ProgressTracker()
+        callback?(.submitStarted(progressTracker: progressTracker))
         do {
-            let result = try await bugReportService.submitBugReport(text: context.reportText,
-                                                                    includeLogs: context.sendingLogsEnabled,
-                                                                    includeCrashLog: true,
-                                                                    githubLabels: [],
-                                                                    files: [])
+            let bugReport = BugReport(text: context.reportText,
+                                      includeLogs: context.sendingLogsEnabled,
+                                      includeCrashLog: true,
+                                      githubLabels: [],
+                                      files: [])
+            let result = try await bugReportService.submitBugReport(bugReport,
+                                                                    progressListener: progressTracker)
             MXLog.info("SubmitBugReport succeeded, result: \(result.reportUrl)")
             callback?(.submitFinished)
         } catch {

diff --git a/ElementX/Sources/Services/BugReport/BugReportService.swift b/ElementX/Sources/Services/BugReport/BugReportService.swift
@@ -14,17 +14,20 @@
 // limitations under the License.
 //
 
+import Combine
 import Foundation
 import GZIP
 import Sentry
 import UIKit
 
-class BugReportService: BugReportServiceProtocol {
+class BugReportService: NSObject, BugReportServiceProtocol {
     private let baseURL: URL
     private let sentryURL: URL
     private let applicationId: String
     private let session: URLSession
     private var lastCrashEventId: String?
+    private let progressSubject = PassthroughSubject<Double, Never>()
+    private var cancellables = Set<AnyCancellable>()
 
     init(withBaseURL baseURL: URL,
          sentryURL: URL,
@@ -34,6 +37,7 @@ class BugReportService: BugReportServiceProtocol {
         self.sentryURL = sentryURL
         self.applicationId = applicationId
         self.session = session
+        super.init()
 
         //  enable SentrySDK
         SentrySDK.start { options in
@@ -74,18 +78,15 @@ class BugReportService: BugReportServiceProtocol {
         SentrySDK.crash()
     }
 
-    func submitBugReport(text: String,
-                         includeLogs: Bool,
-                         includeCrashLog: Bool,
-                         githubLabels: [String],
-                         files: [URL]) async throws -> SubmitBugReportResponse {
-        var params = [MultipartFormData(key: "text", type: .text(value: text))]
+    func submitBugReport(_ bugReport: BugReport,
+                         progressListener: ProgressListener?) async throws -> SubmitBugReportResponse {
+        var params = [MultipartFormData(key: "text", type: .text(value: bugReport.text))]
         params.append(contentsOf: defaultParams)
-        for label in githubLabels {
+        for label in bugReport.githubLabels {
             params.append(MultipartFormData(key: "label", type: .text(value: label)))
         }
-        let zippedFiles = try await zipFiles(includeLogs: includeLogs,
-                                             includeCrashLog: includeCrashLog)
+        let zippedFiles = try await zipFiles(includeLogs: bugReport.includeLogs,
+                                             includeCrashLog: bugReport.includeCrashLog)
         //  log or compressed-log
         if !zippedFiles.isEmpty {
             for url in zippedFiles {
@@ -97,24 +98,14 @@ class BugReportService: BugReportServiceProtocol {
             params.append(MultipartFormData(key: "crash_report", type: .text(value: "<https://sentry.tools.element.io/organizations/element/issues/?project=44&query=\(crashEventId)>")))
         }
 
-        for url in files {
+        for url in bugReport.files {
             params.append(MultipartFormData(key: "file", type: .file(url: url)))
         }
 
         let boundary = "Boundary-\(UUID().uuidString)"
         var body = Data()
         for param in params {
-            body.appendString(string: "--\(boundary)\r\n")
-            body.appendString(string: "Content-Disposition:form-data; name=\"\(param.key)\"")
-            switch param.type {
-            case .text(let value):
-                body.appendString(string: "\r\n\r\n\(value)\r\n")
-            case .file(let url):
-                body.appendString(string: "; filename=\"\(url.lastPathComponent)\"\r\n")
-                body.appendString(string: "Content-Type: \"content-type header\"\r\n\r\n")
-                body.append(try Data(contentsOf: url))
-                body.appendString(string: "\r\n")
-            }
+            try body.appendParam(param, boundary: boundary)
         }
         body.appendString(string: "--\(boundary)--\r\n")
 
@@ -124,7 +115,16 @@ class BugReportService: BugReportServiceProtocol {
         request.httpMethod = "POST"
         request.httpBody = body as Data
 
-        let (data, _) = try await session.data(for: request)
+        let data: Data
+        if let progressListener {
+            progressSubject
+                .receive(on: DispatchQueue.main)
+                .assign(to: \.value, on: progressListener.progressSubject)
+                .store(in: &cancellables)
+            (data, _) = try await session.data(for: request, delegate: self)
+        } else {
+            (data, _) = try await session.data(for: request)
+        }
 
         // Parse the JSON data
         let decoder = JSONDecoder()
@@ -223,6 +223,20 @@ private extension Data {
             append(data)
         }
     }
+
+    mutating func appendParam(_ param: MultipartFormData, boundary: String) throws {
+        appendString(string: "--\(boundary)\r\n")
+        appendString(string: "Content-Disposition:form-data; name=\"\(param.key)\"")
+        switch param.type {
+        case .text(let value):
+            appendString(string: "\r\n\r\n\(value)\r\n")
+        case .file(let url):
+            appendString(string: "; filename=\"\(url.lastPathComponent)\"\r\n")
+            appendString(string: "Content-Type: \"content-type header\"\r\n\r\n")
+            append(try Data(contentsOf: url))
+            appendString(string: "\r\n")
+        }
+    }
 }
 
 private struct MultipartFormData {
@@ -234,3 +248,13 @@ private enum MultipartFormDataType {
     case text(value: String)
     case file(url: URL)
 }
+
+extension BugReportService: URLSessionTaskDelegate {
+    func urlSession(_ session: URLSession, didCreateTask task: URLSessionTask) {
+        task.progress.publisher(for: \.fractionCompleted)
+            .sink { [weak self] value in
+                self?.progressSubject.send(value)
+            }
+            .store(in: &cancellables)
+    }
+}
diff --git a/ElementX/Sources/Services/BugReport/BugReportServiceProtocol.swift b/ElementX/Sources/Services/BugReport/BugReportServiceProtocol.swift
@@ -17,6 +17,14 @@
 import Foundation
 import UIKit
 
+struct BugReport {
+    let text: String
+    let includeLogs: Bool
+    let includeCrashLog: Bool
+    let githubLabels: [String]
+    let files: [URL]
+}
+
 struct SubmitBugReportResponse: Decodable {
     var reportUrl: String
 }
@@ -26,9 +34,6 @@ protocol BugReportServiceProtocol {
 
     func crash()
 
-    func submitBugReport(text: String,
-                         includeLogs: Bool,
-                         includeCrashLog: Bool,
-                         githubLabels: [String],
-                         files: [URL]) async throws -> SubmitBugReportResponse
+    func submitBugReport(_ bugReport: BugReport,
+                         progressListener: ProgressListener?) async throws -> SubmitBugReportResponse
 }