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

[WEB-861] Consent Management feature flag #1764

Merged
merged 7 commits into from
Jan 17, 2023
Merged

Conversation

scottkicks
Copy link
Contributor

@scottkicks scottkicks commented Jan 3, 2023

📲 What

This PR makes the necessary code changes to add a new 'Consent Management' feature flag.

🤔 Why

With iOS 14.5+, iPadOS 14.5+, we need to receive member's permission through the Apple's AppTrackingTransparency framework in order to track them or access their device’s advertising identifier.

This flag will gate the work that will be done to implement this.

More details around this initiative can be found here

🛠 How

Boilerplate code that adds a new case to the OptimizelyFeature enum and then a helper function that checks user defaults before returning the value from the Optimizely SDK. Finally a case for the new consentManagementEnabled feature case in OptimizelyFeatureFlagTools

👀 See

Optimizely Feature Flag menu in our Beta Tools

| After 🦋 |

| |

Important Notes

  • Need to update snapshot tests
  • Due to ongoing CircleCI issues I've changed the base branch to a new circleci/pipeline-queue branch off of main
  • We'll need to update OptimizelyFeatureFlagToolsViewControllerTests once the snapshot fixes pr goes out

@scottkicks scottkicks added this to the release-5.7.0 milestone Jan 3, 2023
@scottkicks scottkicks self-assigned this Jan 3, 2023
@scottkicks scottkicks changed the title [WEB-861] Consent Management Dialog feature flag [WEB-861] Consent Management feature flag Jan 4, 2023
@scottkicks scottkicks marked this pull request as ready for review January 4, 2023 15:40
@scottkicks
Copy link
Contributor Author

@msadoon I'll just need some help with the snapshot tests

Copy link
Contributor

@msadoon msadoon left a comment

Choose a reason for hiding this comment

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

Hey Scott,
Before we start down this path, I just want to understand the scope of the feature and its' necessity. I read your consent management document and left some comments. Can we talk about those first before proceeding?

@scottkicks
Copy link
Contributor Author

@msadoon definitely. i'll slack you to setup a time to discuss.

Copy link
Contributor

@msadoon msadoon left a comment

Choose a reason for hiding this comment

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

A few changes, nothing major.

One thing, instead of merging to main...because our CI pipelines are broken, let's merge to a new branch called circleci/pipeline-queue.

This new branch can be created off main and pushed up and going forward let's merge everything into it until we get our cirlce ci pipelines back.

Small thing: Any Snapshot tests that get need to be re-recorded can be skipped on your M1. Just add notes to this PR please, so we can keep track of which screens need re-records. At least that way we merge PR's that don't have any failing tests.

@scottkicks scottkicks changed the base branch from main to circleci/pipeline-queue January 10, 2023 18:56
@scottkicks scottkicks requested a review from msadoon January 10, 2023 19:16
@scottkicks scottkicks removed the request for review from msadoon January 11, 2023 16:40
@kickstarter kickstarter deleted a comment from nativeksr Jan 12, 2023
Didn't realize that if we removed the feature from the mock client, the recording would would still have it in the snapshot and our old snapshot wouldn't have been updated yet.
@msadoon msadoon self-requested a review January 16, 2023 16:38
Copy link
Contributor

@msadoon msadoon left a comment

Choose a reason for hiding this comment

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

good to go!

@codecov
Copy link

codecov bot commented Jan 16, 2023

Codecov Report

Merging #1764 (c528ef6) into circleci/pipeline-queue (cd6fe2b) will decrease coverage by 0.04%.
The diff coverage is 84.00%.

@@                     Coverage Diff                     @@
##           circleci/pipeline-queue    #1764      +/-   ##
===========================================================
- Coverage                    85.29%   85.25%   -0.05%     
===========================================================
  Files                         1276     1276              
  Lines                       116541   116550       +9     
  Branches                     30722    30734      +12     
===========================================================
- Hits                         99408    99365      -43     
- Misses                       16066    16117      +51     
- Partials                      1067     1068       +1     
Impacted Files Coverage Δ
...ptimizelyFeatureFlagToolsViewControllerTests.swift 0.00% <ø> (-100.00%) ⬇️
Library/OptimizelyFeature.swift 0.00% <0.00%> (ø)
...ewModels/OptimizelyFeatureFlagToolsViewModel.swift 84.52% <66.66%> (-1.38%) ⬇️
Library/OptimizelyFeature+Helpers.swift 80.00% <80.00%> (ø)
Library/OptimizelyClientTypeTests.swift 95.74% <100.00%> (ø)
Library/OptimizelyFeature+HelpersTests.swift 100.00% <100.00%> (ø)
...els/OptimizelyFeatureFlagToolsViewModelTests.swift 100.00% <100.00%> (ø)
...ler/OptimizelyFeatureFlagToolsViewController.swift 0.00% <0.00%> (-86.37%) ⬇️
Library/Navigation.swift 85.20% <0.00%> (-0.65%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@scottkicks scottkicks merged commit dd9a5b2 into circleci/pipeline-queue Jan 17, 2023
@scottkicks scottkicks deleted the web-861 branch January 17, 2023 14:47
scottkicks added a commit that referenced this pull request Jan 24, 2023
* [WEB-861] Consent Management feature flag (#1764)

* Add new Consent Management Dialog feature flag

* Add new Consent Management Dialog feature flag

* remove failing view controller test assertion

* formatting

* Adding a small commit so tests pass on CI.

Didn't realize that if we removed the feature from the mock client, the recording would would still have it in the snapshot and our old snapshot wouldn't have been updated yet.

* formatting

Co-authored-by: Mubarak Sadoon <msadoon@gmail.com>

* [WEB-696] FacebookResetPassword Improvements (#1765)

* remove ResetYourFacebookPasswordViewController snapshots

* rename viewmodel and viewcontroller

* viewmodel form logic suggestions

* update viewmodel tests

* ensure the scrollview is not hidden by the keyboard

* set returnKeyType to go and make api call on tap

* update accessibility on set your password view controller

* updated accessibility for facebookresetpasswordviewcontroller.

* formatting

Co-authored-by: Mubarak Sadoon <msadoon@gmail.com>

* [WEB-669] Facebook Conversions API Feature Flag (#1766)

* add facebook conversions api feature flag

* add facebook conversions api feature flag

* fix test

* [WEB-862] AppTrackingTransparency Authorization (#1772)

* set NSUserTrackingUsageDescription in plist

* request ATTrackingAuthorization on app applicationdidFinishLaunching

* formatting

* gate behind consent management dialog feature flag

* pr feedback
* ATTrackingAuthorizationStatus to its own file
* Use `.ksr_debounce` on Signal instead of `asyncAfter`
* Handle `restricted` and `@unknown` requestTrackingAuthorization status cases
* Improve unit test

* use ksr_delay insted of ksr_debounce

Co-authored-by: Mubarak Sadoon <msadoon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants