Skip to content

Commit

Permalink
Fix non-sending feedback event for the query search result (#347)
Browse files Browse the repository at this point in the history
  • Loading branch information
kried authored Dec 16, 2024
1 parent 2b69d67 commit bae9ae8
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 31 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ Guide: https://keepachangelog.com/en/1.0.0/

## 2.7.0-rc.1

- [Core] Update dependencies
- [Core] Update dependencies.
- [Tech] Support sending feedback events to Telemetry.

**MapboxCommon**: v24.9.0-rc.1
**MapboxCoreSearch**: v2.7.0-rc.1
Expand Down
4 changes: 4 additions & 0 deletions MapboxSearch.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
2C705F062A137CEB00B8B773 /* SearchNavigationProfile.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C705F052A137CEB00B8B773 /* SearchNavigationProfile.swift */; };
2C7FEBFA2CE78E6300B7ED22 /* PointAnnotation+Search.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C7FEBF92CE78E6300B7ED22 /* PointAnnotation+Search.swift */; };
2C7FEBFC2CE7A62C00B7ED22 /* MapView+Search.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C7FEBFB2CE7A62C00B7ED22 /* MapView+Search.swift */; };
2C88C6532D0C62ED00F46FBF /* EventsServiceProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C88C6522D0C62ED00F46FBF /* EventsServiceProtocol.swift */; };
2CA1E22129F09CD200A533CF /* PlaceAutocomplete.Suggestion+Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2CA1E22029F09CD200A533CF /* PlaceAutocomplete.Suggestion+Tests.swift */; };
2CA1E22329F0A47600A533CF /* PlaceAutocompleteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2CA1E22229F0A47600A533CF /* PlaceAutocompleteTests.swift */; };
2CD6C03C29F1982100D865D1 /* EventsManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2CD6C03B29F1982100D865D1 /* EventsManagerTests.swift */; };
Expand Down Expand Up @@ -666,6 +667,7 @@
2C705F052A137CEB00B8B773 /* SearchNavigationProfile.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SearchNavigationProfile.swift; sourceTree = "<group>"; };
2C7FEBF92CE78E6300B7ED22 /* PointAnnotation+Search.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "PointAnnotation+Search.swift"; sourceTree = "<group>"; };
2C7FEBFB2CE7A62C00B7ED22 /* MapView+Search.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MapView+Search.swift"; sourceTree = "<group>"; };
2C88C6522D0C62ED00F46FBF /* EventsServiceProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventsServiceProtocol.swift; sourceTree = "<group>"; };
2CA1E22029F09CD200A533CF /* PlaceAutocomplete.Suggestion+Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "PlaceAutocomplete.Suggestion+Tests.swift"; sourceTree = "<group>"; };
2CA1E22229F0A47600A533CF /* PlaceAutocompleteTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlaceAutocompleteTests.swift; sourceTree = "<group>"; };
2CD6C03B29F1982100D865D1 /* EventsManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventsManagerTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1721,6 +1723,7 @@
isa = PBXGroup;
children = (
F97FA23F25C95ACD0085A311 /* EventAppMetadata.swift */,
2C88C6522D0C62ED00F46FBF /* EventsServiceProtocol.swift */,
);
path = Telemetry;
sourceTree = "<group>";
Expand Down Expand Up @@ -2687,6 +2690,7 @@
FEEDD2F82508DFE400DC0A98 /* RecordsProviderInteractorNativeCore.swift in Sources */,
F97E9A84268C7BBE00F6353D /* DefaultStringInterpolation+Extensions.swift in Sources */,
14FA657F295355BC00056E5B /* AdministrativeUnits.swift in Sources */,
2C88C6532D0C62ED00F46FBF /* EventsServiceProtocol.swift in Sources */,
F9274FF227394AE600708F37 /* TileRegionError.swift in Sources */,
FE1064F525B9A1C9007837BC /* NavigationOptions.swift in Sources */,
148DE668285755500085684D /* AddressAutofill+Suggestion.swift in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion Sources/Demo/MapRootController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class MapRootController: UIViewController {

@discardableResult
private func present(result: SearchResult) -> Bool {
let detailController = ResultDetailViewController(result: result)
let detailController = ResultDetailViewController(result: result, searchEngine: searchController.searchEngine)
present(detailController, animated: true)
return true
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/Demo/PlaceAutocompleteMainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ final class PlaceAutocompleteMainViewController: UIViewController {

extension PlaceAutocompleteMainViewController: UISearchResultsUpdating {
func updateSearchResults(for searchController: UISearchController) {
guard
let text = searchController.searchBar.text
guard let text = searchController.searchBar.text,
!text.isEmpty
else {
cachedSuggestions = []

Expand Down
43 changes: 40 additions & 3 deletions Sources/Demo/ResultDetailViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ import UIKit
class ResultDetailViewController: UIViewController {
private var tableView = UITableView()
private var mapView: MapView
private var feedbackButton: UIButton

var result: SearchResult
var searchEngine: SearchEngine
private var resultComponents: [(name: String, value: String)] = []

init(result: SearchResult) {
init(result: SearchResult, searchEngine: SearchEngine) {
self.result = result
self.searchEngine = searchEngine
self.resultComponents = result.toComponents()

let inset: CGFloat = 8
Expand All @@ -25,8 +28,36 @@ class ResultDetailViewController: UIViewController {
zoom: 15.5
))
)
self.feedbackButton = UIButton()
feedbackButton.setTitle("Send feedback", for: .normal)
feedbackButton.backgroundColor = .lightGray

super.init(nibName: nil, bundle: nil)

feedbackButton.addTarget(self, action: #selector(showFeedbackAlert), for: .touchUpInside)
}

@objc func showFeedbackAlert() {
let alert = UIAlertController(
title: "Submit Feedback?",
message: nil,
preferredStyle: .alert
)
let okAction = UIAlertAction(title: "OK", style: .default) { [weak self] _ in
self?.sendFeedback()
alert.dismiss(animated: true)
}
alert.addAction(okAction)
let cancelAction = UIAlertAction(title: "Cancel", style: .cancel) { _ in
alert.dismiss(animated: true)
}
alert.addAction(cancelAction)
present(alert, animated: true, completion: nil)
}

func sendFeedback() {
let feedbackEvent = FeedbackEvent(record: result, reason: FeedbackEvent.Reason.name.rawValue, text: nil)
try? searchEngine.feedbackManager.sendEvent(feedbackEvent)
}

@available(*, unavailable)
Expand All @@ -39,8 +70,9 @@ class ResultDetailViewController: UIViewController {

tableView.dataSource = self
tableView.allowsSelection = false
view.backgroundColor = .systemBackground

for child in [tableView, mapView] {
for child in [tableView, feedbackButton, mapView] {
child.translatesAutoresizingMaskIntoConstraints = false
view.addSubview(child)
}
Expand All @@ -51,7 +83,12 @@ class ResultDetailViewController: UIViewController {
mapView.trailingAnchor.constraint(equalTo: view.trailingAnchor),
mapView.heightAnchor.constraint(equalTo: view.heightAnchor, multiplier: 0.4),

tableView.topAnchor.constraint(equalTo: mapView.bottomAnchor),
feedbackButton.heightAnchor.constraint(equalToConstant: 44),
feedbackButton.leadingAnchor.constraint(equalTo: view.leadingAnchor),
feedbackButton.trailingAnchor.constraint(equalTo: view.trailingAnchor),
feedbackButton.topAnchor.constraint(equalTo: mapView.bottomAnchor, constant: 20),

tableView.topAnchor.constraint(equalTo: feedbackButton.bottomAnchor, constant: 20),
tableView.leadingAnchor.constraint(equalTo: view.leadingAnchor),
tableView.trailingAnchor.constraint(equalTo: view.trailingAnchor),
tableView.bottomAnchor.constraint(equalTo: view.bottomAnchor),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import Foundation
import MapboxCommon_Private

protocol EventsServiceProtocol {
func sendEvent(for event: Event, callback: EventsServiceResponseCallback?)
}

extension EventsService: EventsServiceProtocol { }
54 changes: 38 additions & 16 deletions Sources/MapboxSearch/PublicAPI/Telemetry/EventsManager.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import MapboxCommon_Private

/// Report search error or any other to the Mapbox telemetry.
///
Expand All @@ -19,13 +20,39 @@ public class EventsManager: NSObject {
}
}

/// - Parameter json: EventTemplate from CoreSearchEngine
func sendEvent(json: String) {
// TODO: Analytics
private let eventsService: EventsServiceProtocol

override convenience init() {
let options = EventsServerOptions(
sdkInformation: SdkInformation.defaultInfo,
deferredDeliveryServiceOptions: nil
)
let eventsService = EventsService.getOrCreate(for: options)
self.init(eventsService: eventsService)
}

init(eventsService: EventsServiceProtocol) {
self.eventsService = eventsService
super.init()
}

func sendEvent(_ event: Events, attributes: [String: Any], autoFlush: Bool) {
// TODO: Analytics
var commonEventAttributes = attributes
commonEventAttributes["event"] = event.rawValue

// This unhandled parameter must be removed to match the event scheme.
commonEventAttributes.removeValue(forKey: "mapboxId")

let commonEvent = Event(priority: autoFlush ? .immediate : .queued,
attributes: commonEventAttributes,
deferredOptions: nil)
eventsService.sendEvent(for: commonEvent) { expected in
if expected.isError() {
_Logger.searchSDK.error("Failed to send the event \(event.rawValue) due to error: \(expected.error.message)")
} else if expected.isValue() {
_Logger.searchSDK.debug("Sent the event \(event.rawValue)")
}
}
}

/// Report an error to Mapbox Search SDK.
Expand All @@ -40,28 +67,23 @@ public class EventsManager: NSObject {
// TODO: Analytics
}

/// json string from the core side populate the whole json suitable for the server
/// Unfortunately, telemetry SDK doesn't support such kind of event
/// Thats why we adopt the raw json to the telemetry-specific API:
/// eventName + set of event attributes
/// We also have a set of default attributes created automatically: `created` and `userAgent`
/// Telemetry SDK is capable to set them on their own
/// json string from the core side populate the whole json suitable for the server.
/// All the events, exept for the feedback event, are sent from the core side.
/// - Parameter eventTemplate: feedbackEventTemplate from CoreSearchEngine
func prepareEventTemplate(_ eventTemplate: String) throws -> (name: String, attributes: [String: Any]) {
func prepareEventTemplate(_ eventTemplate: String) throws -> [String: Any] {
guard let jsonData = eventTemplate.data(using: .utf8),
var jsonObject = try? JSONSerialization.jsonObject(
let jsonObject = try? JSONSerialization.jsonObject(
with: jsonData,
options: [.mutableContainers]
) as? [String: Any],
let eventName = jsonObject.removeValue(forKey: "event") as? String
let eventName = jsonObject["event"] as? String,
!eventName.isEmpty
else {
reportError(SearchError.incorrectEventTemplate)
throw SearchError.incorrectEventTemplate
}
jsonObject.removeValue(forKey: "created")
jsonObject.removeValue(forKey: "userAgent")

return (eventName, jsonObject)
return jsonObject
}

func prepareEventTemplate(for event: String) -> [String: Any] {
Expand Down
14 changes: 7 additions & 7 deletions Sources/MapboxSearch/PublicAPI/Telemetry/FeedbackManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class FeedbackManager {
attributes["queryString"] = attributePlaceholder
attributes["resultId"] = feedbackAttributes.id
attributes["selectedItemName"] = feedbackAttributes.name
attributes["sessionIdentifier"] = attributePlaceholder

if let coordinate = feedbackAttributes.coordinate {
attributes["resultCoordinates"] = [coordinate.longitude, coordinate.latitude]
Expand Down Expand Up @@ -65,7 +66,9 @@ public class FeedbackManager {
attributes["proximity"] = [proximity.value.longitude, proximity.value.latitude]
}

attributes["responseUuid"] = response.responseUUID
if !response.responseUUID.isEmpty {
attributes["responseUuid"] = response.responseUUID
}

// Result parameters
// Mandatory field set -1 if no data available
Expand Down Expand Up @@ -185,12 +188,9 @@ public class FeedbackManager {
try delegate.engine.makeFeedbackEvent(
request: response.request,
result: nil,
callback: { [eventsManager] eventTemplateName in
callback: { [eventsManager] eventJson in
do {
let attributes = try eventsManager.prepareEventTemplate(
eventTemplateName
).attributes

let attributes = try eventsManager.prepareEventTemplate(eventJson)
completion(attributes)
} catch {
_Logger.searchSDK.error(
Expand All @@ -208,7 +208,7 @@ public class FeedbackManager {
do {
let attributes = try eventsManager.prepareEventTemplate(
eventTemplateName
).attributes
)

completion(attributes)
} catch {
Expand Down
30 changes: 29 additions & 1 deletion Tests/MapboxSearchTests/Telemetry/EventsManagerTests.swift
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
@testable import MapboxSearch
import MapboxCommon_Private
import XCTest

final class EventsServiceMock: EventsServiceProtocol {
var events: [Event] = []

func sendEvent(for event: Event, callback: EventsServiceResponseCallback?) {
events.append(event)
callback?(.init(value: NSNull()))
}
}

final class EventsManagerTests: XCTestCase {
var eventsManager: EventsManager!
var eventsService: EventsServiceMock!

override func setUp() {
super.setUp()

eventsManager = EventsManager()
eventsService = EventsServiceMock()
eventsManager = EventsManager(eventsService: eventsService)
}

func testUserAgent() {
Expand All @@ -18,4 +30,20 @@ final class EventsManagerTests: XCTestCase {
let event = eventsManager.prepareEventTemplate(for: "templateEvent")
XCTAssertEqual(event["userAgent"] as? String, "search-sdk-ios/\(mapboxSearchSDKVersion)")
}

func testSendEvent() {
let attributes = ["a": "b", "mapboxId": "value"]
let expectedAttributes = ["a": "b", "event": "search.feedback"]

eventsManager.sendEvent(.feedback, attributes: attributes, autoFlush: true)
let sentEvent1 = eventsService.events[0]
XCTAssertEqual(sentEvent1.priority, .immediate)
XCTAssertEqual(sentEvent1.attributes as? [String: String], expectedAttributes)
XCTAssertNil(sentEvent1.deferredOptions)

eventsManager.sendEvent(.feedback, attributes: attributes, autoFlush: false)
let sentEvent2 = eventsService.events[1]
XCTAssertEqual(sentEvent2.priority, .queued)

}
}

0 comments on commit bae9ae8

Please sign in to comment.