-
Notifications
You must be signed in to change notification settings - Fork 7
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
Remove ad id usage #72
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request involves updates to several configuration files in an Android and Flutter project. The changes primarily focus on updating dependency versions, removing specific configurations related to Google Play services and advertising, and refreshing plugin and package references. The modifications appear to be part of a routine maintenance and version update process, ensuring the project uses the latest library versions and removing deprecated or unnecessary configurations. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/android/app/build.gradle (1)
104-104
: Remove extra empty lines at end of fileClean up the extra empty lines at the end of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/android_deploy.yml
(1 hunks)app/android/app/build.gradle
(1 hunks)app/android/app/src/main/AndroidManifest.xml
(1 hunks)
🔇 Additional comments (3)
app/android/app/src/main/AndroidManifest.xml (2)
14-17
: LGTM: Appropriate removal of AD_ID permission
The permission removal is correctly implemented using tools:node="remove"
to ensure the AD_ID permission is explicitly removed.
Line range hint 41-43
: LGTM: Firebase Analytics ad ID collection properly disabled
The meta-data entry correctly disables Firebase Analytics ad ID collection, complementing the permission removal above.
app/android/app/build.gradle (1)
97-101
: LGTM: Google Ads group properly excluded
The configuration correctly excludes the Google Ads group, which aligns with the removal of ad ID usage.
.github/workflows/android_deploy.yml
Outdated
branches: | ||
- main | ||
workflow_dispatch: | ||
on: push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert workflow trigger to be more specific
The change to trigger on any push seems unrelated to ad ID removal and could lead to unintended deployments. Consider reverting to the previous configuration that only triggered on:
- pushes to main branch
- manual workflow dispatch
-on: push
+on:
+ push:
+ branches:
+ - main
+ workflow_dispatch:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
on: push | |
on: | |
push: | |
branches: | |
- main | |
workflow_dispatch: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.idea/libraries/Flutter_Plugins.xml (1)
1-53
: Ensure comprehensive removal of ad ID usageWhile the plugin updates support removing ad ID usage, please verify:
- All Firebase Analytics instances have ad ID collection disabled
- No remaining ad ID permissions in Android manifest
- No ad ID related code in the application logic
Consider documenting these privacy-enhancing changes in the README or documentation to maintain consistency in future updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.idea/libraries/Dart_Packages.xml
(0 hunks).idea/libraries/Flutter_Plugins.xml
(1 hunks)app/android/app/build.gradle
(1 hunks)app/android/app/src/main/AndroidManifest.xml
(3 hunks)data/.flutter-plugins-dependencies
(1 hunks)
💤 Files with no reviewable changes (1)
- .idea/libraries/Dart_Packages.xml
✅ Files skipped from review due to trivial changes (1)
- data/.flutter-plugins-dependencies
🚧 Files skipped from review as they are similar to previous changes (2)
- app/android/app/src/main/AndroidManifest.xml
- app/android/app/build.gradle
🔇 Additional comments (4)
.idea/libraries/Flutter_Plugins.xml (4)
10-10
: Review permission handler changes for ad ID implications
The permission handler plugin has been updated (html-0.1.3+5, 11.3.1). We should verify that no ad ID related permissions are requested.
Also applies to: 26-26
4-53
: Verify no remaining ad-related dependencies
The plugin updates appear comprehensive, but we should verify no ad-related dependencies remain.
✅ Verification successful
No ad-related dependencies found in the codebase
The verification shows that there are no ad-related dependencies in the pubspec.yaml or ad-related package imports in the Dart files. The only matches found are for legitimate animation-related imports (fade_in_switcher.dart
and cross_fade_animation.dart
), which are not related to advertising.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining ad-related dependencies in pubspec.yaml
fd -g "pubspec.yaml" -x cat {} \; | grep -i "ad"
# Check for any remaining ad-related imports in Dart files
rg "import.*package:.*ad"
Length of output: 814
4-4
: Verify Firebase Analytics configuration aligns with ad ID removal
The Firebase plugin updates (core-web, analytics-web, core, crashlytics) are present, but we should verify that these versions support disabling ad ID collection.
Also applies to: 8-8, 22-24
✅ Verification successful
Let me gather more information about the Firebase Analytics configuration to verify if ad ID collection is properly disabled.
Firebase Analytics ad ID collection is properly disabled
The verification shows that ad ID collection is explicitly disabled in the application:
- AndroidManifest.xml has disabled advertising ID collection through:
- Removal of AD_ID permission
- Setting
google_analytics_adid_collection_enabled
configuration
- The Podfile.lock shows the transition to
WithoutAdIdSupport
Firebase components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Firebase Analytics configuration in the app matches the intent to remove ad ID collection
rg -l "firebase_analytics" | xargs rg "(?i)(ad.*id|advertising.*id)"
# Verify Firebase configuration files
fd -g "*google-services.json" -x cat {} \; | jq -r '.client[].client_info | select(.android_client_info != null) | .android_client_info'
Length of output: 635
Script:
#!/bin/bash
# Check Firebase Analytics initialization and configuration in the app
ast-grep --pattern 'FirebaseAnalytics.$$$'
# Look for any Firebase options or configuration setup
rg -A 3 "FirebaseOptions|firebase.*options"
# Check for any ad ID related configuration in build files
fd -e gradle -e xml | xargs rg "(?i)(ad.*id|advertising.*id)"
Length of output: 1099
31-32
: Confirm removal of photo_manager version with potential ad ID usage
The change from photo_manager-3.6.2 to 3.6.3 might be significant. Let's verify if this version update relates to ad ID removal.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores