From 6c9724c411a6741121a8771b7c7429e89f632be0 Mon Sep 17 00:00:00 2001 From: Anh Do <18567+quanganhdo@users.noreply.github.com> Date: Tue, 26 Nov 2024 09:54:30 -0500 Subject: [PATCH] Send PPro feedback to support inbox (#3567) Task/Issue URL: https://app.asana.com/0/1200019156869587/1207999338171708/f Tech Design URL: CC: **Description**: Integrates with backend to submit issue reports to support inbox. **Optional E2E tests**: - [ ] Run PIR E2E tests Check this to run the Personal Information Removal end to end tests. If updating CCF, or any PIR related code, tick this. **Steps to test this PR**: 1. Go through the unified feedback flow 2. When choosing to submit an issue, there'll be a new optional email field 3. Filling in the form with a valid email would trigger the backend integration **Definition of Done**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- .../UserText+NetworkProtection.swift | 7 +- .../UnifiedMetadataCollector.swift | 11 +++ .../UnifiedFeedbackFormView.swift | 7 ++ .../UnifiedFeedbackFormViewController.swift | 30 +++++-- .../UnifiedFeedbackFormViewModel.swift | 81 +++++++++++++++++- .../UnifiedFeedbackFormViewModelTests.swift | 85 +++++++++++++++++-- 6 files changed, 204 insertions(+), 17 deletions(-) diff --git a/DuckDuckGo/Common/Localizables/UserText+NetworkProtection.swift b/DuckDuckGo/Common/Localizables/UserText+NetworkProtection.swift index 231bedea7c..9aa4698efb 100644 --- a/DuckDuckGo/Common/Localizables/UserText+NetworkProtection.swift +++ b/DuckDuckGo/Common/Localizables/UserText+NetworkProtection.swift @@ -82,7 +82,7 @@ extension UserText { // "general.feedback-form.category.vpn" - Description for the feedback form when the issue is related to VPN static let generalFeedbackFormCategoryVPN = "VPN" // "general.feedback-form.category.pir" - Description for the feedback form when the issue is related to Personal Info Removal (PIR) - static let generalFeedbackFormCategoryPIR = "Personal Info Removal" + static let generalFeedbackFormCategoryPIR = "Personal Information Removal" // "general.feedback-form.category.itr" - Description for the feedback form when the issue is related to Identity Theft Restoration (ITR) static let generalFeedbackFormCategoryITR = "Identity Theft Restoration" // "ppro.feedback-form.category.select-category" - Title for the category selection state of the feedback form @@ -145,6 +145,11 @@ extension UserText { // "ppro.feedback-form.disclaimer" - Text for the disclaimer of the PPro feedback form static let pproFeedbackFormDisclaimer = "Reports are anonymous and sent to DuckDuckGo to help improve our service" + // "ppro.feedback-form.email.label" - Label for the email field of the PPro feedback form + static let pproFeedbackFormEmailLabel = "Provide an email if you’d like us to contact you about this issue (we may not be able to respond to all issues):" + // "ppro.feedback-form.email.placeholder" - Placeholder for the email field of the PPro feedback form + static let pproFeedbackFormEmailPlaceholder = "Email (optional)" + // "ppro.feedback-form.sending-confirmation.title" - Title for the feedback sent view title of the feedback form static let pproFeedbackFormSendingConfirmationTitle = "Thank you!" // "ppro.feedback-form.sending-confirmation.description" - Title for the feedback sent view description of the feedback form diff --git a/DuckDuckGo/UnifiedFeedbackForm/MetadataCollectors/UnifiedMetadataCollector.swift b/DuckDuckGo/UnifiedFeedbackForm/MetadataCollectors/UnifiedMetadataCollector.swift index a720ac8bdc..8ab3c7d277 100644 --- a/DuckDuckGo/UnifiedFeedbackForm/MetadataCollectors/UnifiedMetadataCollector.swift +++ b/DuckDuckGo/UnifiedFeedbackForm/MetadataCollectors/UnifiedMetadataCollector.swift @@ -26,6 +26,7 @@ protocol UnifiedMetadataCollector { protocol UnifiedFeedbackMetadata: Encodable { func toBase64() -> String + func toString() -> String } extension UnifiedFeedbackMetadata { @@ -39,4 +40,14 @@ extension UnifiedFeedbackMetadata { return "Failed to encode metadata to JSON, error message: \(error.localizedDescription)" } } + + func toString() -> String { + let encoder = JSONEncoder() + do { + let encodedMetadata = try encoder.encode(self) + return String(data: encodedMetadata, encoding: .utf8) ?? "" + } catch { + return "Failed to encode metadata to JSON string, error message: \(error.localizedDescription)" + } + } } diff --git a/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormView.swift b/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormView.swift index 011f00efc9..0b9c204696 100644 --- a/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormView.swift +++ b/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormView.swift @@ -128,6 +128,13 @@ private struct FeedbackFormBodyView: View { await viewModel.process(action: .faqClick) } } + } content: { + Text(UserText.pproFeedbackFormEmailLabel) + .multilineTextAlignment(.leading) + .lineLimit(nil) + .fixedSize(horizontal: false, vertical: true) + TextField(UserText.pproFeedbackFormEmailPlaceholder, text: $viewModel.userEmail) + .textFieldStyle(.roundedBorder) } footer: { Text(UserText.pproFeedbackFormText2) VStack(alignment: .leading) { diff --git a/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewController.swift b/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewController.swift index 8fa9d63fdc..175e17a3f4 100644 --- a/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewController.swift +++ b/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewController.swift @@ -21,6 +21,8 @@ import AppKit import SwiftUI import Combine import PixelKit +import Subscription +import Networking final class UnifiedFeedbackFormViewController: NSViewController { // Using a dynamic height in the form was causing layout problems and couldn't be completed in time for the release that needed this form. @@ -28,10 +30,10 @@ final class UnifiedFeedbackFormViewController: NSViewController { // This should be cleaned up later, and eventually use the `sizingOptions` property of NSHostingController. enum Constants { static let landingPageHeight = 260.0 + static let feedbackFormMiniHeight = 350.0 static let feedbackFormCompactHeight = 430.0 - static let feedbackFormHeight = 650.0 + static let feedbackFormHeight = 740.0 static let feedbackSentHeight = 350.0 - static let feedbackErrorHeight = 560.0 } private let defaultSize = CGSize(width: 480, height: Constants.landingPageHeight) @@ -46,6 +48,8 @@ final class UnifiedFeedbackFormViewController: NSViewController { source: UnifiedFeedbackSource = .default) { self.feedbackSender = feedbackSender self.viewModel = UnifiedFeedbackFormViewModel( + accountManager: DefaultSubscriptionManager().accountManager, + apiService: DefaultAPIService(), vpnMetadataCollector: DefaultVPNMetadataCollector(accountManager: Application.appDelegate.subscriptionManager.accountManager), feedbackSender: feedbackSender, source: source @@ -90,13 +94,17 @@ final class UnifiedFeedbackFormViewController: NSViewController { .receive(on: DispatchQueue.main) .sink { [weak self] _ in self?.updateViewHeight() - } - .store(in: &cancellables) + } + .store(in: &cancellables) - viewModel.$selectedReportType - .receive(on: DispatchQueue.main) - .sink { [weak self] _ in - self?.updateViewHeight() + Publishers.MergeMany( + viewModel.$selectedReportType, + viewModel.$selectedCategory, + viewModel.$selectedSubcategory + ) + .receive(on: DispatchQueue.main) + .sink { [weak self] _ in + self?.updateViewHeight() } .store(in: &cancellables) } @@ -106,6 +114,10 @@ final class UnifiedFeedbackFormViewController: NSViewController { case .feedbackPending: if UnifiedFeedbackReportType(rawValue: viewModel.selectedReportType) == .prompt { heightConstraint?.constant = Constants.landingPageHeight + } else if UnifiedFeedbackReportType(rawValue: viewModel.selectedReportType) == .reportIssue, + UnifiedFeedbackCategory(rawValue: viewModel.selectedCategory) == .prompt || + viewModel.selectedSubcategory == PrivacyProFeedbackSubcategory.prompt.rawValue { + heightConstraint?.constant = Constants.feedbackFormMiniHeight } else { heightConstraint?.constant = viewModel.usesCompactForm ? Constants.feedbackFormCompactHeight : Constants.feedbackFormHeight } @@ -114,7 +126,7 @@ final class UnifiedFeedbackFormViewController: NSViewController { case .feedbackSent: heightConstraint?.constant = Constants.feedbackSentHeight case .feedbackSendingFailed: - heightConstraint?.constant = Constants.feedbackErrorHeight + heightConstraint?.constant = (viewModel.usesCompactForm ? Constants.feedbackFormCompactHeight : Constants.feedbackFormHeight) + 20.0 } } diff --git a/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewModel.swift b/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewModel.swift index 5d89a79282..963a5403a6 100644 --- a/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewModel.swift +++ b/DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewModel.swift @@ -20,12 +20,17 @@ import Foundation import Combine import SwiftUI import PixelKit +import Networking +import Subscription protocol UnifiedFeedbackFormViewModelDelegate: AnyObject { func feedbackViewModelDismissedView(_ viewModel: UnifiedFeedbackFormViewModel) } final class UnifiedFeedbackFormViewModel: ObservableObject { + private static let feedbackEndpoint = URL(string: "https://subscriptions.duckduckgo.com/api/feedback")! + private static let platform = "macos" + enum ViewState { case feedbackPending case feedbackSending @@ -42,6 +47,12 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { } } + enum Error: String, Swift.Error { + case missingAccessToken + case invalidRequest + case invalidResponse + } + enum ViewAction { case cancel case submit @@ -51,6 +62,26 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { case reportFAQClick } + struct Payload: Codable { + let userEmail: String + let feedbackSource: String + let platform: String + let problemCategory: String + + let feedbackText: String + let problemSubCategory: String + let customMetadata: String + + func toData() -> Data? { + try? JSONEncoder().encode(self) + } + } + + struct Response: Codable { + let message: String? + let error: String? + } + @Published var viewState: ViewState { didSet { updateSubmitButtonStatus() @@ -90,6 +121,12 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { } } + @Published var userEmail = "" { + didSet { + updateSubmitButtonStatus() + } + } + private var selectedSubcategoryPrompt: String { switch UnifiedFeedbackCategory(rawValue: selectedCategory) { case .selectFeature, nil: return "" @@ -113,18 +150,24 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { weak var delegate: UnifiedFeedbackFormViewModelDelegate? + private let accountManager: any AccountManager + private let apiService: any Networking.APIService private let vpnMetadataCollector: any UnifiedMetadataCollector private let defaultMetadataCollector: any UnifiedMetadataCollector private let feedbackSender: any UnifiedFeedbackSender let source: UnifiedFeedbackSource - init(vpnMetadataCollector: any UnifiedMetadataCollector, + init(accountManager: any AccountManager, + apiService: any Networking.APIService, + vpnMetadataCollector: any UnifiedMetadataCollector, defaultMetadataCollector: any UnifiedMetadataCollector = EmptyMetadataCollector(), feedbackSender: any UnifiedFeedbackSender = DefaultFeedbackSender(), source: UnifiedFeedbackSource = .default) { self.viewState = .feedbackPending + self.accountManager = accountManager + self.apiService = apiService self.vpnMetadataCollector = vpnMetadataCollector self.defaultMetadataCollector = defaultMetadataCollector self.feedbackSender = feedbackSender @@ -204,6 +247,7 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { switch UnifiedFeedbackCategory(rawValue: selectedCategory) { case .vpn: let metadata = await vpnMetadataCollector.collectMetadata() + try await submitIssue(metadata: metadata) try await feedbackSender.sendReportIssuePixel(source: source, category: selectedCategory, subcategory: selectedSubcategory, @@ -211,6 +255,7 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { metadata: metadata as? VPNMetadata) default: let metadata = await defaultMetadataCollector.collectMetadata() + try await submitIssue(metadata: metadata) try await feedbackSender.sendReportIssuePixel(source: source, category: selectedCategory, subcategory: selectedSubcategory, @@ -219,8 +264,33 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { } } + private func submitIssue(metadata: UnifiedFeedbackMetadata?) async throws { + guard !userEmail.isEmpty else { return } + + guard let accessToken = accountManager.accessToken else { + throw Error.missingAccessToken + } + + let payload = Payload(userEmail: userEmail, + feedbackSource: source.rawValue, + platform: Self.platform, + problemCategory: selectedCategory, + feedbackText: feedbackFormText, + problemSubCategory: selectedSubcategory, + customMetadata: metadata?.toString() ?? "") + let headers = APIRequestV2.HeadersV2(additionalHeaders: [HTTPHeaderKey.authorization: "Bearer \(accessToken)"]) + guard let request = APIRequestV2(url: Self.feedbackEndpoint, method: .post, headers: headers, body: payload.toData()) else { + throw Error.invalidRequest + } + + let response: Response = try await apiService.fetch(request: request).decodeBody() + if let error = response.error, !error.isEmpty { + throw Error.invalidResponse + } + } + private func updateSubmitButtonStatus() { - self.submitButtonEnabled = viewState.canSubmit && !feedbackFormText.isEmpty + self.submitButtonEnabled = viewState.canSubmit && !feedbackFormText.isEmpty && (userEmail.isEmpty || userEmail.isValidEmail) } private func updateSubmitShowStatus() { @@ -236,3 +306,10 @@ final class UnifiedFeedbackFormViewModel: ObservableObject { }() } } + +private extension String { + var isValidEmail: Bool { + guard let regex = try? NSRegularExpression(pattern: #"[^\s]+@[^\s]+\.[^\s]+"#) else { return false } + return matches(regex) + } +} diff --git a/UnitTests/UnifiedFeedbackForm/UnifiedFeedbackFormViewModelTests.swift b/UnitTests/UnifiedFeedbackForm/UnifiedFeedbackFormViewModelTests.swift index 2e7dae8803..5c31d96b52 100644 --- a/UnitTests/UnifiedFeedbackForm/UnifiedFeedbackFormViewModelTests.swift +++ b/UnitTests/UnifiedFeedbackForm/UnifiedFeedbackFormViewModelTests.swift @@ -18,22 +18,53 @@ import XCTest @testable import DuckDuckGo_Privacy_Browser +@testable import TestUtils +@testable import Networking final class UnifiedFeedbackFormViewModelTests: XCTestCase { + enum Error: String, Swift.Error { + case generic + } func testWhenCreatingViewModel_ThenInitialStateIsFeedbackPending() throws { let collector = MockVPNMetadataCollector() let sender = MockVPNFeedbackSender() - let viewModel = UnifiedFeedbackFormViewModel(vpnMetadataCollector: collector, feedbackSender: sender) + let viewModel = UnifiedFeedbackFormViewModel(accountManager: MockAccountManager(), + apiService: MockAPIService(apiResponse: .failure(Error.generic)), + vpnMetadataCollector: collector, + feedbackSender: sender) XCTAssertEqual(viewModel.viewState, .feedbackPending) } - func testWhenSendingFeedbackSucceeds_ThenFeedbackIsSent() async throws { + func testGivenNoEmail_WhenSendingFeedbackSucceeds_ThenFeedbackIsSent() async throws { + let collector = MockVPNMetadataCollector() + let sender = MockVPNFeedbackSender() + let viewModel = UnifiedFeedbackFormViewModel(accountManager: MockAccountManager(), + apiService: MockAPIService(apiResponse: .failure(Error.generic)), + vpnMetadataCollector: collector, + feedbackSender: sender) + viewModel.selectedReportType = UnifiedFeedbackReportType.reportIssue.rawValue + let text = "Some feedback report text" + viewModel.feedbackFormText = text + + XCTAssertFalse(sender.sentMetadata) + await viewModel.process(action: .submit) + XCTAssertTrue(sender.sentMetadata) + XCTAssertEqual(sender.receivedData!.4, text) + } + + func testGivenEmail_WhenSendingFeedbackSucceeds_ThenFeedbackIsSent() async throws { let collector = MockVPNMetadataCollector() let sender = MockVPNFeedbackSender() - let viewModel = UnifiedFeedbackFormViewModel(vpnMetadataCollector: collector, feedbackSender: sender) + let payload = UnifiedFeedbackFormViewModel.Response(message: "something", error: nil) + let response = APIResponseV2(data: try! JSONEncoder().encode(payload), httpResponse: HTTPURLResponse()) + let viewModel = UnifiedFeedbackFormViewModel(accountManager: MockAccountManager(), + apiService: MockAPIService(apiResponse: .success(response)), + vpnMetadataCollector: collector, + feedbackSender: sender) viewModel.selectedReportType = UnifiedFeedbackReportType.reportIssue.rawValue + viewModel.userEmail = "hello@example.com" let text = "Some feedback report text" viewModel.feedbackFormText = text @@ -46,8 +77,49 @@ final class UnifiedFeedbackFormViewModelTests: XCTestCase { func testWhenSendingFeedbackFails_ThenFeedbackIsNotSent() async throws { let collector = MockVPNMetadataCollector() let sender = MockVPNFeedbackSender() - let viewModel = UnifiedFeedbackFormViewModel(vpnMetadataCollector: collector, feedbackSender: sender) + let viewModel = UnifiedFeedbackFormViewModel(accountManager: MockAccountManager(), + apiService: MockAPIService(apiResponse: .failure(Error.generic)), + vpnMetadataCollector: collector, + feedbackSender: sender) + viewModel.selectedReportType = UnifiedFeedbackReportType.reportIssue.rawValue + let text = "Some feedback report text" + viewModel.feedbackFormText = text + sender.throwErrorWhenSending = true + + XCTAssertFalse(sender.sentMetadata) + await viewModel.process(action: .submit) + XCTAssertFalse(sender.sentMetadata) + XCTAssertEqual(viewModel.viewState, .feedbackSendingFailed) + } + + func testGivenInvalidEmail_WhenSendingFeedbackFails_ThenFeedbackIsNotSent() async throws { + let collector = MockVPNMetadataCollector() + let sender = MockVPNFeedbackSender() + let viewModel = UnifiedFeedbackFormViewModel(accountManager: MockAccountManager(), + apiService: MockAPIService(apiResponse: .failure(Error.generic)), + vpnMetadataCollector: collector, + feedbackSender: sender) + viewModel.selectedReportType = UnifiedFeedbackReportType.reportIssue.rawValue + viewModel.userEmail = "invalid-email" + let text = "Some feedback report text" + viewModel.feedbackFormText = text + sender.throwErrorWhenSending = true + + XCTAssertFalse(sender.sentMetadata) + await viewModel.process(action: .submit) + XCTAssertFalse(sender.sentMetadata) + XCTAssertEqual(viewModel.viewState, .feedbackSendingFailed) + } + + func testGivenValidEmail_WhenSendingFeedbackFails_ThenFeedbackIsNotSent() async throws { + let collector = MockVPNMetadataCollector() + let sender = MockVPNFeedbackSender() + let viewModel = UnifiedFeedbackFormViewModel(accountManager: MockAccountManager(), + apiService: MockAPIService(apiResponse: .failure(Error.generic)), + vpnMetadataCollector: collector, + feedbackSender: sender) viewModel.selectedReportType = UnifiedFeedbackReportType.reportIssue.rawValue + viewModel.userEmail = "hello@example.com" let text = "Some feedback report text" viewModel.feedbackFormText = text sender.throwErrorWhenSending = true @@ -62,7 +134,10 @@ final class UnifiedFeedbackFormViewModelTests: XCTestCase { let collector = MockVPNMetadataCollector() let sender = MockVPNFeedbackSender() let delegate = MockVPNFeedbackFormViewModelDelegate() - let viewModel = UnifiedFeedbackFormViewModel(vpnMetadataCollector: collector, feedbackSender: sender) + let viewModel = UnifiedFeedbackFormViewModel(accountManager: MockAccountManager(), + apiService: MockAPIService(apiResponse: .failure(Error.generic)), + vpnMetadataCollector: collector, + feedbackSender: sender) viewModel.delegate = delegate XCTAssertFalse(delegate.receivedDismissedViewCallback)