Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions app/src/app_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ static void PlatformOptionsToAppOptions(FIROptions* platform_options, AppOptions
const char* value = platform_options.databaseURL.UTF8String;
if (value) app_options->set_database_url(value);
}
if (!strlen(app_options->ga_tracking_id())) {
const char* value = platform_options.trackingID.UTF8String;
if (value) app_options->set_ga_tracking_id(value);
}
if (!strlen(app_options->storage_bucket())) {
const char* value = platform_options.storageBucket.UTF8String;
if (value) app_options->set_storage_bucket(value);
Expand Down Expand Up @@ -107,9 +103,6 @@ static void PlatformOptionsToAppOptions(FIROptions* platform_options, AppOptions
if (strlen(app_options.database_url())) {
platform_options.databaseURL = @(app_options.database_url());
}
if (strlen(app_options.ga_tracking_id())) {
platform_options.trackingID = @(app_options.ga_tracking_id());
}
if (strlen(app_options.storage_bucket())) {
platform_options.storageBucket = @(app_options.storage_bucket());
}
Expand Down
4 changes: 4 additions & 0 deletions app/tests/app_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,13 @@ TEST_F(AppTest, TestSetDatabaseUrl) {
EXPECT_STREQ("http://abc-xyz-123.firebaseio.com", options.database_url());
}

#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__
Comment on lines +210 to +216

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());
}


TEST_F(AppTest, TestSetStorageBucket) {
AppOptions options;
Expand Down Expand Up @@ -241,10 +243,12 @@ TEST_F(AppTest, LoadDefault) {
EXPECT_STREQ("fake messaging sender id from resource",
options.messaging_sender_id());
EXPECT_STREQ("fake database url from resource", options.database_url());
#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.

EXPECT_STREQ("fake storage bucket from resource", options.storage_bucket());
EXPECT_STREQ("fake project id from resource", options.project_id());
#if !FIREBASE_PLATFORM_IOS
Expand Down
Loading