[MOB-21131] Display Proposition api to support multiple propositions. by akhiljain1907 · Pull Request #112 · adobe/aepsdk-optimize-ios

akhiljain1907 · 2024-09-24T19:42:28Z

Description

This PR adds an api "trackDisplayedOffers" which takes list of offers as an argument and sends display interaction event to Experience Edge.

Through this additional api, we now support display event to be called on multiple propositions which can be send in batches and not per offer (read: proposition) and so we've moved generateInteractionXDM and trackWithData methods from Offer class to public class.

This api supports calling display interaction event on multiple offers. Since a proposition could contain more than one offers and our generateInteractionXDM method takes list of propositions, we first extract list of unique propositions from passed offers and create new proposition with relevant offer ids to be tracked. We have also moved generateInteractionXDM and trackWithData methods to OptimizeTrackingUtils from Offer+Tracking extension to reuse them within the module.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

…ired for multiple propositions moved generateInteractionXDM and trackWithData method from Offers class to public Optimize extension added relevant OptimizePuiblicApiTests to validate code functioning for multiple propositions

…timeout

…changed default value of aepError to unexpected

…sition]? instead of [DecisionScope]? in callback and code review changes

…ve AEPOptimizeError as argument and minor changes

…ty and code optimization

…upport tracking based on Offers.

Sources/AEPOptimize/Optimize+Tracking.swift

spoorthipujariadobe · 2025-04-07T22:57:16Z

Sources/AEPOptimize/Optimize+Tracking.swift

+            OptimizeConstants.JsonKeys.EXPERIENCE_EVENT_TYPE: eventType,
+            OptimizeConstants.JsonKeys.EXPERIENCE: [
+                OptimizeConstants.JsonKeys.EXPERIENCE_DECISIONING: [
+                    OptimizeConstants.JsonKeys.DECISIONING_PROPOSITION_EVENT_TYPE: propositionEventType,


@siddique-adobe this seems to be missing in the Android implementation. Could you check if it needs to be added?

Sources/AEPOptimize/Optimize+Tracking.swift

…lass OptimizeTrackingUtils

…rror

akhiljain1907 · 2025-04-08T13:58:13Z

Hi @spoorthipujariadobe, I have addressed your comments and made few changes. Can you please review again?

spoorthipujariadobe · 2025-04-08T17:34:44Z

Tests/AEPOptimizeTests/UnitTests/OptimizePublicAPITests.swift

+        wait(for: [expectation], timeout: 2)
+    }
+
+    func testGenerateInteractionXdm_multiplePropositions() throws {


nit: can we move this test to a separate OptimizeTrackingUtilsTests file?

Would be good to add a few more tests here to cover cases like empty propositions and all offers belonging to the same proposition. You can check the Android PR for more cases
https://github.com/adobe/aepsdk-optimize-android/pull/92/files#diff-710393a6e27a3c8cd7cb485acc24b9df4631442f2c7efc5a5aa7b7ed24686b38

I have moved testGenerateInteractionXdm_multiplePropositions() in OptimizePropositionTests as that's were relevant generate xdm tests were present.
Also i have added a new test file OptimizeTrackingTests which contains test for new displayed api.

spoorthipujariadobe

Logic looks good. Would be nice to have someone more familiar with iOS review it as well, maybe @ishwetansh ?

ishwetansh · 2025-04-09T07:05:27Z

Sources/AEPOptimize/OptimizeProposition.swift

        try container.encode(scope, forKey: .scope)
        try container.encode(AnyCodable.from(dictionary: scopeDetails), forKey: .scopeDetails)
-        try container.encode(offers, forKey: .items)
+        try container.encode(items, forKey: .items)


Please confirm if we need items or offers to be encoded here

Yes, we need to encode offers itself. Removed this.

ishwetansh · 2025-04-09T11:42:50Z

Sources/AEPOptimize/OptimizeTrackingUtils.swift

+import Foundation
+
+/// Internal helper enum for tracking functionality
+enum OptimizeTrackingUtils {


I don't see the use case of using an enum here, it is generally used to define and handle finite cases.
can we please use a struct here.

since we are using only static methods here, it';s recommended to use enum.
FYI when i use struct, i get this warning - Convenience Type Violation: Types used for hosting only static members should be implemented as a caseless enum to avoid instantiation (convenience_type)

ishwetansh · 2025-04-09T11:46:44Z

Sources/AEPOptimize/OptimizeTrackingUtils.swift

+        var propositionDetailsData: [[String: Any]] = []
+
+        for proposition in propositions {
+            let propositionMap: [String: Any] = [
+                OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_ID: proposition.id,
+                OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_SCOPE: proposition.scope,
+                OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_SCOPEDETAILS: proposition.scopeDetails,
+                OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_ITEMS: proposition.offers.map { offer in
+                    [
+                        OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_ITEMS_ID: offer.id
+                    ]
+                }
+            ]
+            propositionDetailsData.append(propositionMap)
+        }


We can use a transformable map function here to build the dictionary instead of traditional loop.

let propositionDetailsData: [[String: Any]] = propositions.map { proposition in return [ OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_ID: proposition.id, OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_SCOPE: proposition.scope, OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_SCOPEDETAILS: proposition.scopeDetails, OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_ITEMS: proposition.offers.map { offer in return [ OptimizeConstants.JsonKeys.DECISIONING_PROPOSITIONS_ITEMS_ID: offer.id ] } ] }

ishwetansh · 2025-04-09T11:59:18Z

Tests/AEPOptimizeTests/UnitTests/Propositions+OptimizeTests.swift

+
+import Foundation
+
+class Propositions_OptimizeTests {


I guess, we don't usually use an _ in class names,
can we use PropositionsOptimizeTests

I previously kept this file in Utils folder of Unit test where this naming convention was followed. But now i have moved these test data in a new file PropositionsTestData which is kept in TestHelpers folder.

ishwetansh · 2025-04-09T12:02:29Z

Tests/AEPOptimizeTests/UnitTests/OptimizePublicAPITests.swift

            XCTAssertEqual(propositions.count, 1)
            XCTAssertEqual(propositions.first?.id, proposition.id)
            XCTAssertEqual(propositions.first?.scope, proposition.scope)
+


nit: please remove the blank line.

akhiljain1907 · 2025-04-14T10:19:23Z

Hi @spoorthipujariadobe, @ishwetansh can you please review and approve this PR

akhiljain1907 added 12 commits September 24, 2024 23:07

Synced with 5.0.2 branch | updated test cases.

97adcf0

minor formatting change

2cf3997

minor code formatting

54f37f8

added test cases for updateProposition api callback and verified for …

4600bf8

…timeout

minor change and formatting

6022eb5

mapped aepError values in OptimizeError class on basis of status and …

6392765

…changed default value of aepError to unexpected

updated updateProposition api to return [DecisionScope: OptimizePropo…

87bd3d1

…sition]? instead of [DecisionScope]? in callback and code review changes

removed createErrorResponseEvent(_ error: AEPError) as now we will ha…

3345a87

…ve AEPOptimizeError as argument and minor changes

added updateProposition api without callback for backward compatibili…

c7dd5d6

…ty and code optimization

minor formatting change for code optimization

13293fc

Minor formatting

ebd0f32

akhiljain1907 changed the base branch from dev-v5.0.2 to main September 24, 2024 19:56

akhiljain1907 changed the base branch from main to dev-v5.0.2 September 24, 2024 19:57

akhiljain1907 requested a review from spoorthipujariadobe September 24, 2024 20:00

akhiljain1907 changed the base branch from dev-v5.0.2 to main September 25, 2024 09:38

akhiljain1907 changed the base branch from main to dev-v5.0.2 September 25, 2024 09:38

akhiljain1907 added 4 commits September 25, 2024 15:47

Merge branch 'dev-v5.0.2' into MOB-21131

559535f

formatting fix

58e84f2

Merge branch 'MOB-20807' into MOB-21131

0dd2361

Merge branch 'dev-v5.0.2' into MOB-21131

15c3eff

akhiljain1907 requested a review from sbenedicadb October 8, 2024 07:06

Merge branch 'main' into MOB-21131

3c5d5a0

akhiljain1907 changed the base branch from dev-v5.1.0 to dev-v5.3.1 April 1, 2025 14:22

akhiljain1907 added 2 commits April 3, 2025 19:56

modified trackDisplayedPropositions api to accept list of Offers to s…

9a8026d

…upport tracking based on Offers.

made UI changes to support batch tracking

6575a7a

akhiljain1907 requested review from ishwetansh and sachauhan-adobe April 4, 2025 13:43

spoorthipujariadobe reviewed Apr 7, 2025

View reviewed changes

Sources/AEPOptimize/Optimize+Tracking.swift Outdated Show resolved Hide resolved

spoorthipujariadobe reviewed Apr 7, 2025

View reviewed changes

Sources/AEPOptimize/Optimize+Tracking.swift Show resolved Hide resolved

spoorthipujariadobe reviewed Apr 7, 2025

View reviewed changes

Sources/AEPOptimize/Optimize+Tracking.swift Outdated Show resolved Hide resolved

spoorthipujariadobe reviewed Apr 7, 2025

View reviewed changes

Sources/AEPOptimize/Optimize+Tracking.swift Outdated Show resolved Hide resolved

akhiljain1907 added 3 commits April 8, 2025 18:29

moved generateInteractionXDM and trackWithDataMethods to a internal c…

8563deb

…lass OptimizeTrackingUtils

removed extra offer data from items array and fixed testapp's build e…

10a7a42

…rror

fix lint - removed redundant internal keyword

a079f5d

spoorthipujariadobe reviewed Apr 8, 2025

View reviewed changes

ishwetansh reviewed Apr 9, 2025

View reviewed changes

akhiljain1907 added 3 commits April 13, 2025 11:20

code review comments and optimizations

34a1f06

obejctive C api compatibility and test app changes

b0bafa1

minor lint fix

81be776

spoorthipujariadobe approved these changes Apr 14, 2025

View reviewed changes

ishwetansh approved these changes Apr 15, 2025

View reviewed changes

akhiljain1907 merged commit 4043d69 into adobe:dev-v5.3.1 Apr 21, 2025
6 checks passed

Conversation

akhiljain1907 commented Sep 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

akhiljain1907 commented Apr 8, 2025

Uh oh!

spoorthipujariadobe Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spoorthipujariadobe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ishwetansh Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akhiljain1907 commented Apr 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

akhiljain1907 commented Sep 24, 2024 •

edited

Loading

spoorthipujariadobe Apr 8, 2025 •

edited

Loading

ishwetansh Apr 9, 2025 •

edited

Loading