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: display inline messages fetched after View constructed #720

Merged
28 changes: 13 additions & 15 deletions Apps/APN-UIKit/APN UIKit/View/DashboardViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,19 @@ class DashboardViewController: BaseViewController {
override func viewDidLoad() {
super.viewDidLoad()

// In a future PR, we will remove the asyncAfter(). this is only for testing in sample apps because when app opens, the local queue is empty. so wait to check messages until first fetch is done.
DispatchQueue.main.asyncAfter(deadline: .now() + 5) { [self] in
inlineInAppViewCreatedInStoryboard.elementId = "dashboard-announcement"

// We want to test that Inline Views can be used by customers who prefer to use code to make the UI.
// Construct a new instance of the View, add it to the ViewController, then set constraints to make it visible.
let newInlineViewUsingUIAsCode = InAppMessageView(elementId: "dashboard-announcement-code")
// Because the Dashboard screen contains a lot of Views and it's designed using Storyboard, we are
// adding this inline View into the UI by adding to a StackView. This allows us to dyanamically add to the Dashboard screen without complexity or breaking any of the constraints set in Storyboard.
buttonStackView.addArrangedSubview(newInlineViewUsingUIAsCode)

// Customers are responsible for setting the width of the View.
newInlineViewUsingUIAsCode.translatesAutoresizingMaskIntoConstraints = false
newInlineViewUsingUIAsCode.widthAnchor.constraint(equalTo: buttonStackView.widthAnchor).isActive = true
}
// For inline Views added with Storyboard, set the elementId to finish setup of the View and begin showing messages.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just removed the DispatchQueue.main.asyncAfter(deadline: .now() + 5) { [self] and formatted the code.

This delay is no longer needed after this commit since View will display messages that are fetched after the UI is in the foreground.

inlineInAppViewCreatedInStoryboard.elementId = "dashboard-announcement"

// We want to test that Inline Views can be used by customers who prefer to use code to make the UI.
// Construct a new instance of the View, add it to the ViewController, then set constraints to make it visible.
let newInlineViewUsingUIAsCode = InAppMessageView(elementId: "dashboard-announcement-code")
// Because the Dashboard screen contains a lot of Views and it's designed using Storyboard, we are
// adding this inline View into the UI by adding to a StackView. This allows us to dyanamically add to the Dashboard screen without complexity or breaking any of the constraints set in Storyboard.
buttonStackView.addArrangedSubview(newInlineViewUsingUIAsCode)

// Customers are responsible for setting the width of the View.
Copy link
Contributor

Choose a reason for hiding this comment

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

i might have missed it but whats the default behaviour if customers dont?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default behavior is that the view does not add any width constraint. We assume that the developer set one for us. As far as what will happen if you don't set a width, that would depend on your app's UI code. AutoLayout can be unpredictable as every layout is different in how it behaves.

I think this requirement is reasonable when working with AutoLayout in UIKit. Especially when working with Storyboard, you're used to setting all of the constraints of all of the Views in your app.

Think of it like in Android, you can set a default width to let's say match parent. But really, it's reasonable to assume that you're going to set a width on your View when you add it to your app, especially when using XML layouts and getting lint warnings for not setting a width on a View. Compose might be different as we may have default parameters set for width. SwiftUI could probably behavior similarly to Compose in that way.

Happy to talk about this in more depth if you have more questions.

newInlineViewUsingUIAsCode.translatesAutoresizingMaskIntoConstraints = false
newInlineViewUsingUIAsCode.widthAnchor.constraint(equalTo: buttonStackView.widthAnchor).isActive = true

configureDashboardRouter()
addNotifierObserver()
Expand Down
19 changes: 19 additions & 0 deletions Sources/MessagingInApp/Communication/InAppEventBusEvents.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import CioInternalCommon
import Foundation

/*
EventBus events that are specific to the in-app SDK.
*/

/// 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
}
}
4 changes: 0 additions & 4 deletions Sources/MessagingInApp/Gist/Gist.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ public class Gist: GistInstance, GistDelegate {
delegate?.action(message: message, currentRoute: currentRoute, action: action, name: name)
}

public func embedMessage(message: Message, elementId: String) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

embedMessage function call removed in this commit. Because it's no longer needed, I removed it here.

This is a public function of Gist, but it's not part of our advertised public API. So removing it should not be considered a breaking change to our SDK.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still need to provide customers with a way to add messages locally, we might need to check with in-app squad about this. We could probably give a method on top of InAppMessageView. Doesn't need to happen in this PR but maybe a ticker or todo so we don't forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this feature isn't on the project roadmap, I'll hold off on creating a ticket or todo. If you think it's important, please pitch it to the team, perhaps in the project Slack channel.

delegate?.embedMessage(message: message, elementId: elementId)
}

func logMessageView(message: Message) {
// This function body reports metrics and makes sure that messages are not shown 2+ times.
// For inline messages, we have not yet implemented either of these features.
Expand Down
1 change: 0 additions & 1 deletion Sources/MessagingInApp/Gist/GistDelegate.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import UIKit

public protocol GistDelegate: AnyObject {
func embedMessage(message: Message, elementId: String)
func messageShown(message: Message)
func messageDismissed(message: Message)
func messageError(message: Message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ class MessageQueueManagerImpl: MessageQueueManager {
DIGraphShared.shared.gist
}

private var eventBus: EventBusHandler {
DIGraphShared.shared.eventBusHandler
}

func getInterval() -> Double {
interval
}
Expand Down Expand Up @@ -118,6 +122,10 @@ class MessageQueueManagerImpl: MessageQueueManager {
for message in fetchedMessages {
handleMessage(message: message)
}

// Notify observers that a fetch has completed and the local queue has been modified.
// This is useful for inline Views that may need to display or dismiss messages.
eventBus.postEvent(InAppMessagesFetchedEvent())
}

private func handleMessage(message: Message) {
Expand Down
2 changes: 0 additions & 2 deletions Sources/MessagingInApp/MessagingInAppImplementation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ class MessagingInAppImplementation: MessagingInAppInstance {
}

extension MessagingInAppImplementation: GistDelegate {
public func embedMessage(message: Message, elementId: String) {}

// Aka: message opened
public func messageShown(message: Message) {
logger.debug("in-app message opened. \(message.describeForLogs)")
Expand Down
17 changes: 17 additions & 0 deletions Sources/MessagingInApp/Views/InAppMessageView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ public class InAppMessageView: UIView {
DIGraphShared.shared.gist
}

private var eventBus: EventBusHandler {
DIGraphShared.shared.eventBusHandler
}

// Can set in the constructor or can set later (like if you use Storyboards)
public var elementId: String? {
didSet {
Expand Down Expand Up @@ -82,6 +86,14 @@ public class InAppMessageView: UIView {
heightConstraint.priority = .required
heightConstraint.isActive = true
layoutIfNeeded()

// Begin listening to the queue for new messages.
eventBus.addObserver(InAppMessagesFetchedEvent.self) { [weak self] _ in
// We are unsure what thread this code will run on. We want to ensure that the View is updated on the main thread.
Task { @MainActor in
self?.checkIfMessageAvailableToDisplay()
}
}
}

private func checkIfMessageAvailableToDisplay() {
Expand All @@ -94,6 +106,11 @@ public class InAppMessageView: UIView {
return // no messages to display, exit early. In the future we will dismiss the View.
}

// 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 {
return // already showing this message, exit early.
}

displayInAppMessage(messageToDisplay)
}

Expand Down
11 changes: 11 additions & 0 deletions Tests/MessagingInApp/Gist/Managers/MessageQueueManagerTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ class MessageQueueManagerTest: UnitTest {
private var manager: MessageQueueManagerImpl!

private let gistMock = GistInstanceMock()
private let eventBusMock = EventBusHandlerMock()

override func setUp() {
super.setUp()

DIGraphShared.shared.override(value: gistMock, forType: GistInstance.self)
DIGraphShared.shared.override(value: eventBusMock, forType: EventBusHandler.self)

manager = MessageQueueManagerImpl()
}
Expand Down Expand Up @@ -89,6 +91,15 @@ class MessageQueueManagerTest: UnitTest {
XCTAssertEqual(modalMessageProcessed.count, 1)
XCTAssertTrue(inlineMessagesProcessed.isEmpty)
}

func test_processFetchedMessages_expectSendEventBusEventAfterProcessing() {
XCTAssertEqual(eventBusMock.postEventCallsCount, 0)

manager.processFetchedMessages([Message.randomInline])

XCTAssertEqual(eventBusMock.postEventCallsCount, 1)
XCTAssertTrue(eventBusMock.postEventArguments is InAppMessagesFetchedEvent)
}
}

extension MessageQueueManagerTest {
Expand Down
87 changes: 87 additions & 0 deletions Tests/MessagingInApp/Views/InAppMessageViewTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,81 @@ class InAppMessageViewTest: UnitTest {

XCTAssertNotNil(getInAppMessageWebView(fromInlineView: inlineView))
}

// MARK: Async fetching of in-app messages

// The in-app SDK fetches for new messages in the background in an async manner.
// We need to test that the View is updated when new messages are fetched.

func test_givenInAppMessageFetchedAfterViewConstructed_expectShowInAppMessageFetched() {
// start with no messages available.
queueMock.getInlineMessagesReturnValue = []

let view = InAppMessageView(elementId: .random)
XCTAssertNil(getInAppMessageWebView(fromInlineView: view))

// Modify queue to return a message after the UI has been constructed and not showing a WebView.
simulateSdkFetchedMessages([Message.random])

XCTAssertNotNil(getInAppMessageWebView(fromInlineView: view))
}

// Test that the eventbus listening does not impact memory management of the View instance.
func test_deinit_givenObservingEventBusEvent_expectNoMemoryLeaks() {
// Before we try to deinit the View, make sure the eventbus observer has executed at least once.
// This is important if the observer holds a strong reference to something preventing the View deinit.
let expectToCheckIfInAppMessagesAvailableToDisplay = expectation(description: "expect to check for in-app messages")
expectToCheckIfInAppMessagesAvailableToDisplay.expectedFulfillmentCount = 2 // once on View init() and once on observer action.
queueMock.getInlineMessagesClosure = { _ in
expectToCheckIfInAppMessagesAvailableToDisplay.fulfill()
return []
}

var view: InAppMessageView? = InAppMessageView(elementId: .random)

DIGraphShared.shared.eventBusHandler.postEvent(InAppMessagesFetchedEvent())

// Wait for the observer to be called.
waitForExpectations()

// Deinit the View and asert deinit actually cleared the instance.
view = nil
XCTAssertNil(view)
}

func test_givenAlreadyShowingInAppMessage_whenNewMessageFetched_expectDoNotReplaceContents() {
let givenOldInlineMessage = Message.randomInline
queueMock.getInlineMessagesReturnValue = [givenOldInlineMessage]

let inlineView = InAppMessageView(elementId: givenOldInlineMessage.elementId!)
let webViewBeforeFetch = getInAppMessageWebView(fromInlineView: inlineView)

// Make sure message is unique, but has same elementId.
let givenNewInlineMessage = Message(messageId: .random, campaignId: .random, elementId: givenOldInlineMessage.elementId)

simulateSdkFetchedMessages([givenNewInlineMessage])

let webViewAfterFetch = getInAppMessageWebView(fromInlineView: inlineView)

// If the WebViews are different, it means the message was reloaded.
XCTAssertTrue(webViewBeforeFetch === webViewAfterFetch)
}

func test_givenAlreadyShowingMessage_whenSameMessageFetched_expectDoNotReloadTheMessageAgain() {
let givenInlineMessage = Message.randomInline
queueMock.getInlineMessagesReturnValue = [givenInlineMessage]

let inlineView = InAppMessageView(elementId: givenInlineMessage.elementId!)

let webViewBeforeFetch = getInAppMessageWebView(fromInlineView: inlineView)

simulateSdkFetchedMessages([givenInlineMessage])

let webViewAfterFetch = getInAppMessageWebView(fromInlineView: inlineView)

// If the WebViews are the same instance, it means the message was not reloaded.
XCTAssertTrue(webViewBeforeFetch === webViewAfterFetch)
}
}

extension InAppMessageViewTest {
Expand All @@ -76,4 +151,16 @@ extension InAppMessageViewTest {

return gistViews.first
}

func simulateSdkFetchedMessages(_ messages: [Message]) {
// Because eventbus operations are async, use an expectation that waits until eventbus event is posted and observer is called.
let expectToCheckIfInAppMessagesAvailableToDisplay = expectation(description: "expect to check for in-app messages")
queueMock.getInlineMessagesClosure = { _ in
expectToCheckIfInAppMessagesAvailableToDisplay.fulfill()
return messages
}
// Imagine the in-app SDK has fetched new messages. It sends an event to the eventbus.
DIGraphShared.shared.eventBusHandler.postEvent(InAppMessagesFetchedEvent())
waitForExpectations()
}
}
Loading