Android build playstore #2333
Android build playstore #2333
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several modifications to the Android deployment workflow and project configuration. Key updates include changes to the deployment workflow file for the Play Store, enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
mobile-v3/android/app/build.gradle (2)
Line range hint
3-3: Consider specifying the Gradle plugin versionsWhile not mandatory, specifying plugin versions can help maintain build consistency and avoid unexpected issues due to plugin updates.
For example:
- id "com.android.application" + id "com.android.application" version "8.1.0"Ensure to use the appropriate versions compatible with your project.
97-110: Review the flavor dimension configurationUsing the same name
"airqo"for both the flavor dimension and one of the flavors might cause confusion. Consider renaming the flavor dimension to something more general, like"environment", to improve clarity.Apply this diff to rename the flavor dimension:
- flavorDimensions "airqo" + flavorDimensions "environment" - airqo { - dimension "airqo" + airqo { + dimension "environment"mobile-v3/android/Gemfile (1)
3-3: Specify the version for the 'fastlane' gemTo ensure consistent builds and avoid potential issues with gem updates, it's advisable to specify a version for the
fastlanegem.Apply this diff to specify the version:
- gem "fastlane" + gem "fastlane", "~> 2.214.0"Replace
2.214.0with the desired version compatible with your project.mobile-v3/android/fastlane/Fastfile (1)
18-26: Add validation for successful build before uploadThe lane should verify that the build was successful before attempting to upload to Play Store.
sh "flutter build appbundle --build-number #{_new_build_number} --flavor airqo" +aab_path = "../build/app/outputs/bundle/airqoRelease/app-airqo-release.aab" +UI.user_error!("AAB file not found at #{aab_path}") unless File.exist?(aab_path) upload_to_play_store( release_status: "completed", json_key_data: ENV['MOBILE_ANDROID_PLAYSTORE_SA'], package_name:"com.airqo.app", - aab: "../build/app/outputs/bundle/airqoRelease/app-airqo-release.aab", + aab: aab_path, skip_upload_metadata: true, skip_upload_images: true, skip_upload_screenshots: true )🧰 Tools
🪛 rubocop (1.69.1)
[convention] 25-25: Avoid comma after the last parameter of a method call.
(Style/TrailingCommaInArguments)
.github/workflows/deploy-android-to-play-store.yml (1)
81-87: Improve changelog management logicThe current changelog management could be more robust:
- The hardcoded version in the default release notes
- No validation of the release notes content
mkdir -p fastlane/metadata/android/en-US/changelogs touch fastlane/metadata/android/en-US/changelogs/default.txt if [ ! -f fastlane/release_notes.txt ]; then - echo "App release version 3.0.2" > fastlane/release_notes.txt + VERSION=$(grep 'version:' ../../pubspec.yaml | awk '{print $2}') + echo "App release version ${VERSION}" > fastlane/release_notes.txt fi +# Validate release notes content +if [ ! -s fastlane/release_notes.txt ]; then + echo "Error: Release notes file is empty" + exit 1 +fimobile-v3/pubspec.yaml (1)
Line range hint
19-19: Review version and build number strategyThe version number (3.0.2) follows semantic versioning, but the build number (+0) seems unusually low for a production Play Store release. Consider:
- Incrementing the build number based on your previous Play Store releases
- Documenting the versioning strategy in the project
mobile-v3/android/fastlane/README.md (1)
24-32: Enhance deployment documentationThe lane descriptions could be more detailed. Consider adding:
- Required environment variables
- Necessary credentials/keys
- Example usage with parameters
- Troubleshooting tips
-Deploy a new version to play store +Deploy a new version to Play Store + +Required setup: +- play-store-service-account.json in the fastlane directory +- Properly configured version in build.gradle + +Example: +```sh +fastlane android play_store track:internal +``` -Deploy to new version to Firebase App Distribution +Deploy a new version to Firebase App Distribution + +Required setup: +- Firebase CLI configuration +- FIREBASE_APP_ID environment variable + +Example: +```sh +fastlane android app_distribution groups:"testers" +```🧰 Tools
🪛 Markdownlint (0.37.0)
26-26: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
mobile-v3/android/Gemfile.lockis excluded by!**/*.lockmobile-v3/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/workflows/deploy-android-to-play-store.yml(3 hunks)mobile-v3/.gitignore(1 hunks)mobile-v3/android/Gemfile(1 hunks)mobile-v3/android/app/build.gradle(4 hunks)mobile-v3/android/fastlane/Appfile(1 hunks)mobile-v3/android/fastlane/Fastfile(1 hunks)mobile-v3/android/fastlane/Pluginfile(1 hunks)mobile-v3/android/fastlane/README.md(1 hunks)mobile-v3/android/fastlane/report.xml(1 hunks)mobile-v3/pubspec.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- mobile-v3/android/fastlane/report.xml
🧰 Additional context used
🪛 rubocop (1.69.1)
mobile-v3/android/fastlane/Fastfile
[convention] 5-6: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
[convention] 25-25: Avoid comma after the last parameter of a method call.
(Style/TrailingCommaInArguments)
[convention] 40-40: Avoid comma after the last parameter of a method call.
(Style/TrailingCommaInArguments)
🪛 Markdownlint (0.37.0)
mobile-v3/android/fastlane/README.md
4-4: Expected: setext; Actual: atx
Heading style
(MD003, heading-style)
14-14: Expected: setext; Actual: atx
Heading style
(MD003, heading-style)
16-16: Expected: setext; Actual: atx
Heading style
(MD003, heading-style)
18-18: Expected: setext; Actual: atx
Heading style
(MD003, heading-style)
26-26: Expected: setext; Actual: atx
Heading style
(MD003, heading-style)
🔇 Additional comments (7)
mobile-v3/android/app/build.gradle (1)
112-119: Verify that all signing properties are available
In the signingConfigs.release block, the code assumes that all required properties are present in releaseKeystoreProperties. If prod-key.properties is missing or incomplete, this could lead to null values and runtime exceptions. Make sure to handle missing properties appropriately.
mobile-v3/android/fastlane/Pluginfile (1)
1-1: Addition of Firebase App Distribution plugin looks good
Including the fastlane-plugin-firebase_app_distribution gem enhances your app distribution workflow. This will streamline the process of distributing your app to testers.
mobile-v3/android/fastlane/Appfile (1)
1-2: Configuration settings are correctly implemented
The json_key_file and package_name are properly set, aligning with the deployment requirements for the Play Store.
.github/workflows/deploy-android-to-play-store.yml (2)
18-18: Validate working directory changes
The working directory has been updated from mobile to mobile-v3. Let's verify the directory structure.
Also applies to: 67-68
67-68: Verify Ruby version compatibility
The Ruby version has been updated to 3.1.0. Let's ensure all gems are compatible with this version.
✅ Verification successful
Ruby 3.1.0 is compatible with the current dependencies
The Gemfile.lock shows all dependencies are compatible with the Ruby platform being used. The key components:
- fastlane (2.226.0) and its plugins are fully compatible with Ruby 3.1.0
- All gem dependencies are using versions that support Ruby 3.1.0
- The bundle is successfully locked with bundler 2.5.22, indicating successful dependency resolution
The setup appears stable and well-maintained with no version conflicts or compatibility issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any version constraints in Gemfile
cd mobile-v3/android
if [ -f Gemfile ]; then
grep -A 5 "ruby '[0-9]" Gemfile
grep "gem 'fastlane'" Gemfile
fi
Length of output: 118
Script:
#!/bin/bash
# Let's check if Gemfile exists in other directories
fd Gemfile
# Check for any fastlane related files
fd fastlane
# Look for ruby version specifications in any fastlane files
fd -t f -e rb | xargs grep -l "ruby.*version" 2>/dev/null || true
Length of output: 242
Script:
#!/bin/bash
# Let's check the content of the Gemfile in mobile-v3/android
cat mobile-v3/android/Gemfile
# Also check fastlane configuration
cat mobile-v3/android/fastlane/Fastfile 2>/dev/null || true
Length of output: 1764
Script:
#!/bin/bash
# Let's check the Gemfile.lock to see the actual dependencies and their versions
cat mobile-v3/android/Gemfile.lock
# Also check if there's a Pluginfile with additional dependencies
cat mobile-v3/android/fastlane/Pluginfile 2>/dev/null || true
Length of output: 6633
mobile-v3/pubspec.yaml (2)
92-93: Ensure proper environment file security
Good separation of development and production environments. However, please ensure:
- Both .env files are added to .gitignore
- A template file (.env.example) is provided for documentation
- CI/CD pipeline is configured to use the correct environment file
✅ Verification successful
Environment files are properly secured in .gitignore
The verification confirms that environment files are properly secured:
.env,.env.*, and*.envpatterns are present in .gitignore- However, an
.env.exampletemplate file is missing, which would be helpful for documentation
Consider adding an .env.example file with placeholder values to help other developers set up their environment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if env files are properly gitignored
cat mobile-v3/.gitignore | grep -E "\.env\.|\.env$"
# Check for presence of example template
fd -t f ".env.example" mobile-v3/
Length of output: 105
50-50: Verify flutter_sticky_header upgrade compatibility
The upgrade from ^0.6.5 to ^0.7.0 is a major version change that might introduce breaking changes. Please ensure:
- All existing sticky header implementations have been tested
- Any breaking changes have been addressed
✅ Verification successful
Limited impact upgrade - only basic SliverStickyHeader usage found
The codebase only uses the basic SliverStickyHeader widget with simple header configuration in dashboard_page.dart. The upgrade from ^0.6.5 to ^0.7.0 should be safe as:
- Only one file uses the package
- Implementation uses basic sticky header functionality
- No complex customizations or deprecated features detected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all uses of flutter_sticky_header to identify potential impact
rg -l "flutter_sticky_header" --type dart
Length of output: 99
Script:
#!/bin/bash
# Check the actual usage of flutter_sticky_header in the dashboard page
rg "flutter_sticky_header" mobile-v3/lib/src/app/dashboard/pages/dashboard_page.dart -B 2 -A 2
Length of output: 322
Script:
#!/bin/bash
# Let's check how StickyHeader is actually used in the implementation
ast-grep --pattern 'StickyHeader' mobile-v3/lib/src/app/dashboard/pages/dashboard_page.dart
Length of output: 91
Script:
#!/bin/bash
# Let's try a different pattern to find StickyHeaderBuilder usage
ast-grep --pattern 'StickyHeaderBuilder' mobile-v3/lib/src/app/dashboard/pages/dashboard_page.dart
# Also search for any sticky header related content
rg "Sticky" mobile-v3/lib/src/app/dashboard/pages/dashboard_page.dart -B 2 -A 5
Length of output: 430
| def googleMapApiKeyDev = appProperties.getProperty('google.maps.key.dev') | ||
| if (googleMapApiKeyDev == null) { | ||
| throw new GradleException("Google Maps Key not found. Define google.maps.key_dev in the key.properties file.") | ||
| } |
There was a problem hiding this comment.
Correct the property key name in the exception message
There's an inconsistency between the property key you're accessing and the one mentioned in the exception message. The property key is 'google.maps.key.dev', but the exception message refers to 'google.maps.key_dev'. Ensure both use the same key name for clarity and correctness.
Apply this diff to correct the exception message:
- throw new GradleException("Google Maps Key not found. Define google.maps.key_dev in the key.properties file.")
+ throw new GradleException("Google Maps Key not found. Define google.maps.key.dev in the key.properties file.")📝 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.
| def googleMapApiKeyDev = appProperties.getProperty('google.maps.key.dev') | |
| if (googleMapApiKeyDev == null) { | |
| throw new GradleException("Google Maps Key not found. Define google.maps.key_dev in the key.properties file.") | |
| } | |
| def googleMapApiKeyDev = appProperties.getProperty('google.maps.key.dev') | |
| if (googleMapApiKeyDev == null) { | |
| throw new GradleException("Google Maps Key not found. Define google.maps.key.dev in the key.properties file.") | |
| } |
| releaseKeystoreProperties.load(new FileInputStream(releaseKeystorePropertiesFile)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure proper resource management when loading properties
The FileInputStream used to load releaseKeystoreProperties isn't closed, which can lead to resource leaks. Consider using withInputStream to automatically manage the resource.
Apply this diff to use withInputStream:
- releaseKeystoreProperties.load(new FileInputStream(releaseKeystorePropertiesFile))
+ releaseKeystorePropertiesFile.withInputStream { stream ->
+ releaseKeystoreProperties.load(stream)
+ }📝 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.
| releaseKeystoreProperties.load(new FileInputStream(releaseKeystorePropertiesFile)) | |
| } | |
| releaseKeystorePropertiesFile.withInputStream { stream -> | |
| releaseKeystoreProperties.load(stream) | |
| } | |
| } |
| /android/google-services.json | ||
| /android/play-store-service-account.json | ||
| /android/fastlane/metadata |
There was a problem hiding this comment.
💡 Codebase verification
Additional sensitive files need to be ignored
Found an exposed Google Maps API key in iOS configuration and several configuration files that should be considered for gitignore:
- Add
/ios/Runner/AppDelegate.swiftto gitignore as it contains an exposed API key - Consider adding the following configuration files:
/ios/Flutter/*.xcconfig/android/app/src/main/res/xml/network_security_config.xml
While the current additions are good, these additional sensitive files should also be protected to prevent accidental commits of credentials or configuration.
🔗 Analysis chain
Good security practice! Properly ignoring sensitive configuration files.
The additions to .gitignore for google-services.json, play-store-service-account.json, and Fastlane metadata are essential for security. However, let's verify if we're catching all sensitive files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any potentially sensitive files that might have been committed
rg -l "AIza[0-9A-Za-z-_]{35}" # Search for Google API keys
rg -l "BEGIN PRIVATE KEY" # Search for private key files
Length of output: 95
Script:
#!/bin/bash
# Let's check the content of the file that might contain sensitive information
cat mobile/ios/Runner/AppDelegate.swift
# Also check for any other potential sensitive files in the mobile directories
fd -t f "(google-services|credentials|key|secret|auth|config)" mobile/
Length of output: 1479
| _google_play_track_version_codes = google_play_track_version_codes( | ||
| package_name: 'com.airqo.app', | ||
| track: 'production', | ||
| json_key_data: ENV['MOBILE_ANDROID_PLAYSTORE_SA'] | ||
| ) | ||
| _previous_build_number = _google_play_track_version_codes[0] | ||
| _new_build_number = _previous_build_number + 1 | ||
| sh "flutter build appbundle --build-number #{_new_build_number} --flavor airqo" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for version code retrieval
The version code management logic could benefit from error handling in case the Play Store API call fails or returns unexpected results.
_google_play_track_version_codes = google_play_track_version_codes(
package_name: 'com.airqo.app',
track: 'production',
json_key_data: ENV['MOBILE_ANDROID_PLAYSTORE_SA']
)
+if _google_play_track_version_codes.nil? || _google_play_track_version_codes.empty?
+ UI.user_error!("Failed to retrieve version codes from Play Store")
+end
_previous_build_number = _google_play_track_version_codes[0]📝 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.
| _google_play_track_version_codes = google_play_track_version_codes( | |
| package_name: 'com.airqo.app', | |
| track: 'production', | |
| json_key_data: ENV['MOBILE_ANDROID_PLAYSTORE_SA'] | |
| ) | |
| _previous_build_number = _google_play_track_version_codes[0] | |
| _new_build_number = _previous_build_number + 1 | |
| sh "flutter build appbundle --build-number #{_new_build_number} --flavor airqo" | |
| _google_play_track_version_codes = google_play_track_version_codes( | |
| package_name: 'com.airqo.app', | |
| track: 'production', | |
| json_key_data: ENV['MOBILE_ANDROID_PLAYSTORE_SA'] | |
| ) | |
| if _google_play_track_version_codes.nil? || _google_play_track_version_codes.empty? | |
| UI.user_error!("Failed to retrieve version codes from Play Store") | |
| end | |
| _previous_build_number = _google_play_track_version_codes[0] | |
| _new_build_number = _previous_build_number + 1 | |
| sh "flutter build appbundle --build-number #{_new_build_number} --flavor airqo" |
| sh "flutter build appbundle --flavor airqo" | ||
| firebase_app_distribution( | ||
| app: ENV['MOBILE_ANDROID_ID_FIREBASE'], | ||
| android_artifact_path:"../build/app/outputs/bundle/airqoRelease/app-airqo-release.aab", | ||
| release_notes_file: "fastlane/release_notes.txt", | ||
| service_credentials_file: "fastlane/firebase-app-distribution.json", | ||
| android_artifact_type: "AAB", | ||
| groups: "airqo-team", | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for Firebase App Distribution
The app_distribution lane should include error handling for the build and upload process.
sh "flutter build appbundle --flavor airqo"
+unless $?.success?
+ UI.user_error!("Flutter build failed")
+end
+
+release_notes_path = "fastlane/release_notes.txt"
+service_credentials_path = "fastlane/firebase-app-distribution.json"
+
+UI.user_error!("Release notes not found") unless File.exist?(release_notes_path)
+UI.user_error!("Firebase credentials not found") unless File.exist?(service_credentials_path)
+
firebase_app_distribution(
app: ENV['MOBILE_ANDROID_ID_FIREBASE'],
android_artifact_path:"../build/app/outputs/bundle/airqoRelease/app-airqo-release.aab",
- release_notes_file: "fastlane/release_notes.txt",
- service_credentials_file: "fastlane/firebase-app-distribution.json",
+ release_notes_file: release_notes_path,
+ service_credentials_file: service_credentials_path,
android_artifact_type: "AAB",
groups: "airqo-team"
)📝 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.
| sh "flutter build appbundle --flavor airqo" | |
| firebase_app_distribution( | |
| app: ENV['MOBILE_ANDROID_ID_FIREBASE'], | |
| android_artifact_path:"../build/app/outputs/bundle/airqoRelease/app-airqo-release.aab", | |
| release_notes_file: "fastlane/release_notes.txt", | |
| service_credentials_file: "fastlane/firebase-app-distribution.json", | |
| android_artifact_type: "AAB", | |
| groups: "airqo-team", | |
| ) | |
| sh "flutter build appbundle --flavor airqo" | |
| unless $?.success? | |
| UI.user_error!("Flutter build failed") | |
| end | |
| release_notes_path = "fastlane/release_notes.txt" | |
| service_credentials_path = "fastlane/firebase-app-distribution.json" | |
| UI.user_error!("Release notes not found") unless File.exist?(release_notes_path) | |
| UI.user_error!("Firebase credentials not found") unless File.exist?(service_credentials_path) | |
| firebase_app_distribution( | |
| app: ENV['MOBILE_ANDROID_ID_FIREBASE'], | |
| android_artifact_path:"../build/app/outputs/bundle/airqoRelease/app-airqo-release.aab", | |
| release_notes_file: release_notes_path, | |
| service_credentials_file: service_credentials_path, | |
| android_artifact_type: "AAB", | |
| groups: "airqo-team" | |
| ) |
🧰 Tools
🪛 rubocop (1.69.1)
[convention] 40-40: Avoid comma after the last parameter of a method call.
(Style/TrailingCommaInArguments)
| Make sure you have the latest version of the Xcode command line tools installed: | ||
|
|
||
| ```sh | ||
| xcode-select --install | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary Xcode requirement for Android deployment
The README includes Xcode command line tools installation, which isn't necessary for Android deployment. Consider:
- Removing the Xcode-specific instructions
- Adding Android-specific prerequisites (Java, Android SDK)
-Make sure you have the latest version of the Xcode command line tools installed:
-
-```sh
-xcode-select --install
-```
+Make sure you have the following prerequisites installed:
+- Java Development Kit (JDK)
+- Android SDK
+- Ruby (for Fastlane)
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
GemfileandPluginfilefor managing dependencies and plugins.Bug Fixes
Documentation
Chores
.gitignoreto manage sensitive files and prevent tracking of unnecessary metadata.pubspec.yaml.