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

Add targeting to PresentedOfferingContext #3730

Merged
merged 22 commits into from
Feb 28, 2024

Conversation

joshdholtz
Copy link
Member

@joshdholtz joshdholtz commented Feb 28, 2024

Motivation

Get more context with targeting information

Description

  • Parse targeting information into Offerings
  • Add targeting context into Offerings.current
  • Add targeting context into getCurrentOffering(forPlacement: String)
  • Post targeting context in post receipts

@joshdholtz joshdholtz changed the base branch from main to guido/fia-2856-rework-ios-add-getcurrentoffering-to-offerings February 28, 2024 06:03
@joshdholtz joshdholtz added the pr:feat A new feature label Feb 28, 2024
@joshdholtz joshdholtz requested review from a team February 28, 2024 15:00
@joshdholtz joshdholtz marked this pull request as ready for review February 28, 2024 15:01
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Left some comments but LGTM!

@@ -41,6 +41,16 @@ import Foundation
}
}

internal final class Targeting: NSObject {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a struct? (maybe not, not entirely sure if it needs to be an NSObject)

Copy link
Member Author

Choose a reason for hiding this comment

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

It probably should 😅 Not sure why I did the extra work of making this an NSObject...

/// Initialize a ``PresentedOfferingContext``.
@objc
public init(
offeringIdentifier: String,
placementIdentifier: String?
placementIdentifier: String?,
targetingContext: TargetingContext?
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to say this could be a breaking change, but I believe the base change hasn't been merged yet, so it should be ok 👍

@@ -602,6 +602,7 @@ platform :ios do

desc "Run BackendIntegrationTests"
lane :backend_integration_tests do |options|
fetch_snapshots
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm is this needed? The snapshot tests for paywalls should be independent of these test I thought?

Copy link
Member Author

Choose a reason for hiding this comment

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

The CI tests keep failing on UI Snapshots tests are missing in the backend tests so this was my test to see if this fixed it 🤷‍♂️ Will remove it stops the flakey

@joshdholtz joshdholtz force-pushed the guido/fia-2856-rework-ios-add-getcurrentoffering-to-offerings branch from 04abfe4 to 3eb9898 Compare February 28, 2024 20:01
Base automatically changed from guido/fia-2856-rework-ios-add-getcurrentoffering-to-offerings to main February 28, 2024 21:11
joshdholtz and others added 21 commits February 28, 2024 15:17
@joshdholtz joshdholtz force-pushed the add-targeting-to-presented-offering-context branch from 4e60748 to 05d4981 Compare February 28, 2024 21:18
@joshdholtz joshdholtz merged commit a9737aa into main Feb 28, 2024
24 checks passed
@joshdholtz joshdholtz deleted the add-targeting-to-presented-offering-context branch February 28, 2024 22:10
joshdholtz added a commit that referenced this pull request Mar 5, 2024
**This is an automatic release.**

### New Features
* Paywalls: add `updateWithDisplayCloseButton` to
`PaywallViewController` (#3708) via Cesar de la Vega (@vegaro)
* New `syncAttributesAndOfferingsIfNeeded` method (#3709) via Burdock
(@lburdock)
* Add targeting to `PresentedOfferingContext` (#3730) via Josh Holtz
(@joshdholtz)
* Add `currentOffering(forPlacement: String)` to `Offerings` (#3707) via
Guido Torres (@guido732)
* New `Package.presentedOfferingContext` (#3712) via Josh Holtz
(@joshdholtz)
### Bugfixes
* Mark methods with StaticString for appUserID as deprecated (#3739) via
Mark Villacampa (@MarkVillacampa)
### Other Changes
* [EXTERNAL] Spelling typo fix to comment (#3713) via @vdeaugustine
(#3740) via Mark Villacampa (@MarkVillacampa)

---------

Co-authored-by: Josh Holtz <me@joshholtz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:feat A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants