Handle inbox schema type propositions + inbox display tracking#419
Handle inbox schema type propositions + inbox display tracking#419spoorthipujariadobe wants to merge 5 commits intoadobe:feature/inboxfrom
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| override fun onInboxEvent(event: InboxEvent) { | ||
| when (event) { | ||
| is InboxEvent.Display -> { | ||
| val uiState = event.inboxUIState | ||
| if (!uiState.displayed) { | ||
| ContentCardMapper.instance.getInboxPropositionItem(event.inboxUIState.template.id) | ||
| ?.track(MessagingEdgeEventType.DISPLAY) | ||
| uiState.displayed = true | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
While this inbox tracking approach (uiState.displayed = true inside observer) works, it’s more implicit and couples tracking logic to mutable state living inside the event payload.
Would you be willing to consider the pattern we follow for the cards i.e derive new state (uiState.copy(displayed = true) and publish it via a single owner.
Something to note is that AepUI cards work because each card is an object with internal mutable state (BaseAepUI + updateState), so MessagingEventHandler can safely do:
- read event.aepUi.getState()
- compute copy
- call event.aepUi.updateState(...)
For inbox, there is no equivalent InboxUI object with updateState(...) because state comes from MessagingInboxProvider as a Flow<InboxUIState>. So I am thinking the natural single owner is the MessagingInboxProvider.
Do you think this can work?
There was a problem hiding this comment.
Is the suggestion to have MessagingInboxProvider own the changes to displayed state? It is called by the app developer before the actual display. They might choose to call getInboxUI but never pass the Flow<InboxUIState> to AepInbox composable. So the state needs to be flipped in the LaunchedEffect of AepInbox. But right now, the composable in the UI layer is completely decoupled from the data provider in the Messaging layer so passing the displayed signal from AepInbox to MessagingInboxProvider is not possible.
Thinking more about this, we might not even need the displayed flag if the LaunchedEffect in AepInbox uses the template id i.e activity id as the key since it remains the same across network calls. How about I do away with the flag and its mutation completely?
Description
The tracking implementation provides automatic display tracking for inbox propositions while continuing to maintain the separation between UI components (
aepcomposeui) and messaging SDK logic (messaging).Related Issue
Motivation and Context
1.
InboxUIState.ktInboxUIState.Successcontains adisplayedflag backed by Compose'smutableStateOfdisplayed = false2.
InboxEvent.ktInboxEvent.Display3.
AepInboxEventObserverAepUIEventObserverto add inbox-level event handlingInboxEventObserver- New implementation that coordinates both inbox and item-level eventsvarargitem observers (e.g.,ContentCardEventObserver) for extensibility4.
ContentCardMapper.ktMessagingInboxProviderand retrieved during display tracking inInboxEventObserver.5.
AepInbox.ktAepInboxEventObserver(non-optional)LaunchedEffect(Unit)to trigger display tracking once per compositionData Flow
MessagingInboxProvidercreates statePropositionIteminContentCardMapperInboxUIState.Success(withdisplayed = false)AepInboxcomposable displays and checksdisplayedflagobserver.onInboxEvent(InboxEvent.Display)InboxEventObserverretrievesPropositionItemfromContentCardMapperdisplayed = trueHow Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: