Skip to content

feat: Remove trackingID from iOS #1775

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

Merged
merged 2 commits into from
Jul 16, 2025
Merged

feat: Remove trackingID from iOS #1775

merged 2 commits into from
Jul 16, 2025

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented Jul 16, 2025

Description

Provide details of the change, and generalize the change in the PR title above.

Firebase 12 Apple SDK has been released and has removed the deprecated, unused trackingID property.


Testing

Describe how you've tested these changes. Link any manually triggered Integration tests or CPP binary SDK Packaging Github Action workflows, if applicable.

Did not test locally, relying on CI for full coverage.


Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

Notes

  • Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
  • Read the contribution guidelines CONTRIBUTING.md.
  • Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @ncooke3, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request updates the Firebase C++ SDK to reflect changes in the latest Firebase Apple SDK (version 12). The primary goal is to remove all references and handling of the trackingID property, which has been deprecated and removed from the upstream iOS SDK. This ensures the C++ SDK remains compatible and free of unused code paths related to this property on Apple platforms.

Highlights

  • iOS Option Mapping Update: I've removed the mapping logic for the trackingID property in app/src/app_ios.mm. This includes removing the code that previously set ga_tracking_id in AppOptions from FIROptions.trackingID and vice-versa, as the underlying Firebase Apple SDK no longer uses this property.
  • Platform-Specific Test Adjustments: In app/tests/app_test.cc, I've updated the test suite to conditionally compile out ga_tracking_id related tests on Apple platforms. This ensures that tests for a deprecated and removed property on iOS are no longer run, aligning the test coverage with the updated SDK behavior.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes the usage of the deprecated trackingID property from the iOS implementation. The changes look good, but there are a couple of suggestions for the test file to improve correctness and clarity. One test seems to be disabled unnecessarily, and another change could be made more readable.

Comment on lines +210 to +216
#ifndef __APPLE__
TEST_F(AppTest, TestSetGaTrackingId) {
AppOptions options;
options.set_ga_tracking_id("UA-12345678-1");
EXPECT_STREQ("UA-12345678-1", options.ga_tracking_id());
}
#endif // __APPLE__

Choose a reason for hiding this comment

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

medium

This test verifies the functionality of set_ga_tracking_id and ga_tracking_id methods. Since AppOptions is platform-independent and the ga_tracking_id field still exists, this test should still pass on non-Apple platforms. Consider re-enabling this test by removing the #ifndef __APPLE__ and #endif directives.

Suggested change
#ifndef __APPLE__
TEST_F(AppTest, TestSetGaTrackingId) {
AppOptions options;
options.set_ga_tracking_id("UA-12345678-1");
EXPECT_STREQ("UA-12345678-1", options.ga_tracking_id());
}
#endif // __APPLE__
TEST_F(AppTest, TestSetGaTrackingId) {
AppOptions options;
options.set_ga_tracking_id("UA-12345678-1");
EXPECT_STREQ("UA-12345678-1", options.ga_tracking_id());
}

Comment on lines 246 to 251
#ifndef __APPLE__
#if FIREBASE_PLATFORM_IOS || FIREBASE_PLATFORM_TVOS
// GA tracking ID can currently only be configured on iOS.
EXPECT_STREQ("fake ga tracking id from resource", options.ga_tracking_id());
#endif // FIREBASE_PLATFORM_IOS
#endif // __APPLE__

Choose a reason for hiding this comment

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

medium

The combination of preprocessor directives (#ifndef __APPLE__ and #if FIREBASE_PLATFORM_IOS || FIREBASE_PLATFORM_TVOS) makes the enclosed code unreachable on any platform. If the intention is to permanently remove this check for ga_tracking_id, it would be clearer to delete this block of code entirely.

@ncooke3 ncooke3 requested review from a-maurice and removed request for a-maurice July 16, 2025 16:14
@ncooke3
Copy link
Member Author

ncooke3 commented Jul 16, 2025

@a-maurice, I see there are likely some other changes needed in https://github.com/firebase/firebase-cpp-sdk/blob/main/app/src/include/firebase/app.h

This seems potentially trickier than intended if the other platforms also expose this property. Does this PR seem feasible to do if it still exists on other platforms? I don't want to introduce too much complexity, so I'd be fine adding the property back upstream.

@a-maurice
Copy link
Contributor

@a-maurice, I see there are likely some other changes needed in https://github.com/firebase/firebase-cpp-sdk/blob/main/app/src/include/firebase/app.h

This seems potentially trickier than intended if the other platforms also expose this property. Does this PR seem feasible to do if it still exists on other platforms? I don't want to introduce too much complexity, so I'd be fine adding the property back upstream.

Will see what needs to be done for the field on other platforms, but for now this should be good for iOS. Thanks!

@a-maurice a-maurice added the skip-release-notes Skip release notes check label Jul 16, 2025
@a-maurice a-maurice merged commit 2d52424 into main Jul 16, 2025
34 of 46 checks passed
@a-maurice a-maurice deleted the remove-trackingid branch July 16, 2025 20:17
@github-actions github-actions bot added the tests: in-progress This PR's integration tests are in progress. label Jul 16, 2025
Copy link

github-actions bot commented Jul 16, 2025

❌  Integration test FAILED

Requested by @a-maurice on commit 2d52424
Last updated: Wed Jul 16 15:20 PDT 2025
View integration test log & download artifacts

Failures Configs
auth [TEST] [FAILURE] [iOS] [macos] [1/2 ios_device: simulator_target]
(1 failed tests)  FirebaseAuthTest.TestSignInWithBadEmailFails
firestore [TEST] [FLAKINESS] [Android] [1/3 os: windows] [1/2 android_device: android_target]
(1 failed tests)  CRASH/TIMEOUT

Add flaky tests to go/fpl-cpp-flake-tracker

@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Jul 16, 2025
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-release-notes Skip release notes check tests: failed This PR's integration tests failed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants