Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: rename message variables to closer represent their purpose in SDK #730

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Sources/Common/Communication/Event.swift
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,16 @@ public struct NewSubscriptionEvent: EventRepresentable {
self.params = params
}
}

/// When in-app SDK has fetched in-app messages from the server.
public struct InAppMessagesFetchedEvent: EventRepresentable {
public let storageId: String
public let params: [String: String]
public let timestamp: Date

public init(storageId: String = UUID().uuidString, timestamp: Date = Date(), params: [String: String] = [:]) {
self.storageId = storageId
self.timestamp = timestamp
self.params = params
}
}
2 changes: 1 addition & 1 deletion Sources/MessagingInApp/Extensions/GistExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ extension Message {
//
// We are logging the Gist campaign id as delivery id because we call it delivery id on our system.
var describeForLogs: String {
"id: \(messageId), queueId: \(queueId ?? "none"), deliveryId: \(gistProperties.campaignId ?? "none")"
"id: \(id ?? "none"), templateId: \(templateId), deliveryId: \(gistProperties.campaignId ?? "none")"
}
}
14 changes: 7 additions & 7 deletions Sources/MessagingInApp/Gist/Gist.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class Gist: GistInstance, GistDelegate {

// To finish initializing of Gist, we want to fetch fonts and other assets for HTML in-app messages.
// To do that, we try to display a message with an empty message id.
_ = InlineMessageManager(siteId: self.siteId, message: Message(messageId: ""))
_ = InlineMessageManager(siteId: self.siteId, message: Message(templateId: ""))
}

// MARK: User
Expand Down Expand Up @@ -68,7 +68,7 @@ public class Gist: GistInstance, GistDelegate {

public func showMessage(_ message: Message, position: MessagePosition) -> Bool {
if let messageManager = getModalMessageManager() {
Logger.instance.info(message: "Message cannot be displayed, \(messageManager.currentMessage.messageId) is being displayed.")
Logger.instance.info(message: "Message cannot be displayed, \(messageManager.currentMessage.templateId) is being displayed.")
} else {
let messageManager = createMessageManager(siteId: siteId, message: message)
messageManager.showMessage(position: position)
Expand All @@ -93,7 +93,7 @@ public class Gist: GistInstance, GistDelegate {
// MARK: Events

public func messageShown(message: Message) {
Logger.instance.debug(message: "Message with route: \(message.messageId) shown")
Logger.instance.debug(message: "Message with route: \(message.templateId) shown")
if message.gistProperties.persistent != true {
logMessageView(message: message)
} else {
Expand All @@ -103,7 +103,7 @@ public class Gist: GistInstance, GistDelegate {
}

public func messageDismissed(message: Message) {
Logger.instance.debug(message: "Message with id: \(message.messageId) dismissed")
Logger.instance.debug(message: "Message with id: \(message.templateId) dismissed")
removeMessageManager(instanceId: message.instanceId)
delegate?.messageDismissed(message: message)
}
Expand All @@ -126,14 +126,14 @@ public class Gist: GistInstance, GistDelegate {
}

messageQueueManager.removeMessageFromLocalStore(message: message)
if let queueId = message.queueId {
shownModalMessageQueueIds.insert(queueId)
if let id = message.id {
shownModalMessageQueueIds.insert(id)
}
let userToken = UserManager().getUserToken()
LogManager(siteId: siteId, dataCenter: dataCenter)
.logView(message: message, userToken: userToken) { response in
if case .failure(let error) = response {
Logger.instance.error(message: "Failed to log view for message: \(message.messageId) with error: \(error)")
Logger.instance.error(message: "Failed to log view for message: \(message.templateId) with error: \(error)")
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/MessagingInApp/Gist/Managers/LogManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class LogManager {

func logView(message: Message, userToken: String?, completionHandler: @escaping (Result<Void, Error>) -> Void) {
do {
if let queueId = message.queueId, let userToken = userToken {
if let queueId = message.id, let userToken = userToken {
try GistQueueNetwork(siteId: siteId, dataCenter: dataCenter, userToken: userToken)
.request(LogEndpoint.logUserMessageView(queueId: queueId), completionHandler: { response in
switch response {
Expand All @@ -27,7 +27,7 @@ class LogManager {
})
} else {
try GistQueueNetwork(siteId: siteId, dataCenter: dataCenter)
.request(LogEndpoint.logMessageView(messageId: message.messageId), completionHandler: { response in
.request(LogEndpoint.logMessageView(messageId: message.templateId), completionHandler: { response in
switch response {
case .success(let (_, response)):
if response.statusCode == 200 {
Expand Down
10 changes: 5 additions & 5 deletions Sources/MessagingInApp/Gist/Managers/MessageManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,21 @@ class MessageManager {
init(siteId: String, message: Message) {
self.siteId = siteId
self.currentMessage = message
self.currentRoute = message.messageId
self.currentRoute = message.templateId

let engineWebConfiguration = EngineWebConfiguration(
siteId: Gist.shared.siteId,
dataCenter: Gist.shared.dataCenter,
instanceId: message.instanceId,
endpoint: Settings.Network.engineAPI,
messageId: message.messageId,
messageId: message.templateId,
properties: message.toEngineRoute().properties
)

// When EngineWeb instance is constructed, it will begin the rendering process for the in-app message.
// This means that the message begins the process of loading.
// Start a timer that helps us determine how long a message took to load/render.
elapsedTimer.start(title: "Loading message with id: \(currentMessage.messageId)")
elapsedTimer.start(title: "Loading message with id: \(currentMessage.templateId)")
self.engine = EngineWeb(configuration: engineWebConfiguration)

if let engine = engine {
Expand Down Expand Up @@ -84,7 +84,7 @@ extension MessageManager: EngineWebDelegate {
Logger.instance.debug(message: "Bourbon Engine bootstrapped")

// Cleaning after engine web is bootstrapped and all assets downloaded.
if currentMessage.messageId == "" {
if currentMessage.templateId == "" {
engine?.cleanEngineWeb()
}
}
Expand Down Expand Up @@ -193,7 +193,7 @@ extension MessageManager: EngineWebDelegate {
}

func error() {
Logger.instance.error(message: "Error loading message with id: \(currentMessage.messageId)")
Logger.instance.error(message: "Error loading message with id: \(currentMessage.templateId)")
delegate?.messageError(message: currentMessage)
}

Expand Down
10 changes: 5 additions & 5 deletions Sources/MessagingInApp/Gist/Managers/MessageQueueManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class MessageQueueManagerImpl: MessageQueueManager {
}

func removeMessageFromLocalStore(message: Message) {
guard let queueId = message.queueId else {
guard let queueId = message.id else {
return
}
localMessageStore.removeValue(forKey: queueId)
Expand All @@ -81,10 +81,10 @@ class MessageQueueManagerImpl: MessageQueueManager {
}

func addMessageToLocalStore(message: Message) {
guard let queueId = message.queueId else {
guard let id = message.id else {
return
}
localMessageStore.updateValue(message, forKey: queueId)
localMessageStore.updateValue(message, forKey: id)
}

@objc
Expand Down Expand Up @@ -140,8 +140,8 @@ class MessageQueueManagerImpl: MessageQueueManager {
// Rest of logic of function is for Modal messages

// Skip showing Modal messages if already shown.
if let queueId = message.queueId, Gist.shared.shownModalMessageQueueIds.contains(queueId) {
Logger.instance.info(message: "Message with queueId: \(queueId) already shown, skipping.")
if let id = message.id, Gist.shared.shownModalMessageQueueIds.contains(id) {
Logger.instance.info(message: "Message with id: \(id) already shown, skipping.")
return
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ModalMessageManager: MessageManager {
}

override func onDoneLoadingMessage(routeLoaded: String, onComplete: @escaping () -> Void) {
if routeLoaded == currentMessage.messageId, !messageLoaded {
if routeLoaded == currentMessage.templateId, !messageLoaded {
messageLoaded = true

if UIApplication.shared.applicationState == .active {
Expand Down
17 changes: 9 additions & 8 deletions Sources/MessagingInApp/Gist/Managers/Models/Message.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,26 @@ public class GistProperties {

public class Message {
public private(set) var instanceId = UUID().uuidString.lowercased()
public let queueId: String? // used to uniquely identify an in-app message.
public let id: String? // Uniquely identify an in-app message. This ID is also known as the "queue id" in Gist.
public let priority: Int?
public let messageId: String // the messageId refers to the template used to render. For non-HTML messages, that messageId is something like "welcome-demo"
public let templateId: String // Refers to the template used to render. Also known as "message id" in Gist. For non-HTML messages, this value is something like "welcome-demo".
public private(set) var gistProperties: GistProperties

var properties = [String: Any]()

public init(messageId: String) {
self.queueId = nil
public init(templateId: String) {
self.id = nil
self.priority = nil
self.gistProperties = GistProperties(routeRule: nil, elementId: nil, campaignId: nil, position: .center, persistent: false)
self.messageId = messageId
self.templateId = templateId
}

// Used to construct instance from API response
init(queueId: String? = nil, priority: Int? = nil, messageId: String, properties: [String: Any]?) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
init(queueId: String? = nil, priority: Int? = nil, messageId: String, properties: [String: Any]?) {
init(queueId: String? = nil, priority: Int? = nil, templateId: String, properties: [String: Any]?) {

self.queueId = queueId
self.id = queueId
self.priority = priority
self.gistProperties = GistProperties(routeRule: nil, elementId: nil, campaignId: nil, position: .center, persistent: false)
self.messageId = messageId
self.templateId = messageId

if let properties = properties {
self.properties = properties
Expand Down Expand Up @@ -79,7 +80,7 @@ public class Message {
}

func toEngineRoute() -> EngineRoute {
let engineRoute = EngineRoute(route: messageId)
let engineRoute = EngineRoute(route: templateId)
properties.keys.forEach { key in
if let value = properties[key] {
engineRoute.addProperty(key: key, value: value)
Expand Down
4 changes: 3 additions & 1 deletion Sources/MessagingInApp/Type/InAppMessage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ public struct InAppMessage: Equatable {
// in-app messages

init(gistMessage: GistMessage) {
self.messageId = gistMessage.messageId
// Internally, the SDK refers to the message ID as "templateId".
// To keep backwards compatibility, map Message.templateId to "messageId".
self.messageId = gistMessage.templateId
self.deliveryId = gistMessage.gistProperties.campaignId
}
}
2 changes: 1 addition & 1 deletion Sources/MessagingInApp/Views/InAppMessageView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public class InAppMessageView: UIView {
}

// Do not re-show the existing message if already shown to prevent the UI from flickering as it loads the same message again.
if let currentlyShownMessage = inlineMessageManager?.currentMessage, currentlyShownMessage.queueId == messageToDisplay.queueId {
if let currentlyShownMessage = inlineMessageManager?.currentMessage, currentlyShownMessage.id == messageToDisplay.id {
return // already showing this message, exit early.
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class MessageQueueManagerTest: UnitTest {

let actualMessages = manager.getInlineMessages(forElementId: givenElementId)

XCTAssertEqual(actualMessages.map(\.messageId), [givenMessage2.messageId, givenMessage1.messageId, givenMessage3.messageId])
XCTAssertEqual(actualMessages.map(\.id), [givenMessage2.id, givenMessage1.id, givenMessage3.id])
}

// MARK: - processFetchedMessages
Expand Down
Loading