-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
Updated Flutter SDK and Android build dependencies to latest versions #2722
base: develop-postgres
Are you sure you want to change the base?
Updated Flutter SDK and Android build dependencies to latest versions #2722
Conversation
WalkthroughThis pull request focuses on upgrading project dependencies and configurations for Android build compatibility. The changes span multiple files, including GitHub workflows, Android build configurations, and Dart/Flutter dependencies. Key updates include upgrading Flutter to version 3.27.3, updating Gradle to version 8.9, modifying the Android Gradle plugin to version 8.7.1, and replacing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
Other🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise. |
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: 3
🔭 Outside diff range comments (1)
lib/view_model/main_screen_view_model.dart (1)
Line range hint
324-337
: Add error handling for plugin fetching.The Timer.periodic callback makes external calls without proper error handling. This could lead to unhandled exceptions if the plugin fetch fails or if the Hive box is unavailable.
Timer.periodic(Duration(seconds: (testMode ? 1 : 300)), (timer) { + try { FetchPluginList(); final newPluginList = (Hive.box('pluginBox').get('plugins') ?? []) as List<dynamic>; if (listEquals(pluginList, newPluginList)) { // notifyListeners(); } if (testMode) timer.cancel(); + } catch (e) { + print('Failed to fetch plugins: $e'); + // Consider implementing proper error logging + if (testMode) timer.cancel(); + } });
🧹 Nitpick comments (4)
lib/custom_painters/whatsapp_logo.dart (1)
106-106
: Simplify the alpha value calculation.The calculation
(1.0 * 255).toInt()
can be simplified to just255
since it's a constant value representing full opacity.Apply this diff to simplify the alpha value:
- paint0Fill.color = const Color(0xff40c351).withAlpha((1.0 * 255).toInt()); + paint0Fill.color = const Color(0xff40c351).withAlpha(255); - paint1fill.color = const Color(0xfffcfcfc).withAlpha((1.0 * 255).toInt()); + paint1fill.color = const Color(0xfffcfcfc).withAlpha(255);Also applies to: 425-425
lib/custom_painters/telegram_logo.dart (1)
20-21
: Consider keepingwithOpacity(1)
for better readability.The change from
withOpacity(1)
towithAlpha((1.0 * 255).toInt())
adds unnecessary complexity without any significant benefits:
- Both achieve the same result for full opacity
withOpacity(1)
is more readable and commonly used in Flutter- The explicit conversion
(1.0 * 255).toInt()
adds complexity- const Color(0xff37aee2).withAlpha((1.0 * 255).toInt()), - const Color(0xff1e96c8).withAlpha((1.0 * 255).toInt()), + const Color(0xff37aee2).withOpacity(1), + const Color(0xff1e96c8).withOpacity(1),lib/view_model/main_screen_view_model.dart (1)
Line range hint
10-10
: Track commented-out features.There are several commented-out features (Chat, AddPost, Invite) with TODO comments. These should be tracked properly in the issue tracker.
Would you like me to create issues to track these features:
- Chat functionality (TODO on line 71)
- AddPost feature
- Invite feature in the app tour
This will help in better tracking of these pending implementations.
Also applies to: 71-77, 283-285, 461-469, 611-619
lib/widgets/custom_avatar.dart (1)
63-63
: Fix the line formatting.The line exceeds the maximum line length. Consider breaking it into multiple lines.
- Theme.of(context).iconTheme.color!.withAlpha((0.2 * 255).toInt()), + Theme.of(context) + .iconTheme + .color! + .withAlpha((0.2 * 255).toInt()),🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] File required formatting changes
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
lib/custom_painters/language_icon.dart
(1 hunks)lib/custom_painters/talawa_logo.dart
(4 hunks)lib/custom_painters/telegram_logo.dart
(4 hunks)lib/custom_painters/whatsapp_logo.dart
(2 hunks)lib/services/navigation_service.dart
(1 hunks)lib/view_model/access_request_view_model.dart
(1 hunks)lib/view_model/main_screen_view_model.dart
(2 hunks)lib/views/after_auth_screens/chat/widgets/chat_input_field.dart
(2 hunks)lib/views/after_auth_screens/events/edit_agenda_item_page.dart
(1 hunks)lib/views/after_auth_screens/events/manage_agenda_items_screen.dart
(1 hunks)lib/views/after_auth_screens/feed/individual_post.dart
(1 hunks)lib/views/after_auth_screens/org_info_screen.dart
(2 hunks)lib/views/after_auth_screens/profile/edit_profile_page.dart
(1 hunks)lib/views/pre_auth_screens/select_language.dart
(1 hunks)lib/views/pre_auth_screens/set_url.dart
(1 hunks)lib/widgets/agenda_item_tile.dart
(1 hunks)lib/widgets/create_recurring_event_helper_widgets.dart
(1 hunks)lib/widgets/custom_avatar.dart
(2 hunks)lib/widgets/custom_list_tile.dart
(1 hunks)lib/widgets/custom_weekday_selector.dart
(1 hunks)lib/widgets/event_card.dart
(2 hunks)lib/widgets/pinned_carousel_widget.dart
(1 hunks)lib/widgets/venue_card.dart
(2 hunks)talawa_lint/lib/talawa_api_doc/talawa_api_doc_fixer.dart
(3 hunks)test/views/after_auth_screens/events/manage_agenda_items_screen_test.dart
(1 hunks)test/widget_tests/pre_auth_screens/select_language_page_test.dart
(2 hunks)test/widget_tests/widgets/agenda_item_tile_test.dart
(1 hunks)test/widget_tests/widgets/custom_avatar_test.dart
(2 hunks)
✅ Files skipped from review due to trivial changes (13)
- lib/widgets/agenda_item_tile.dart
- lib/views/after_auth_screens/events/manage_agenda_items_screen.dart
- lib/views/pre_auth_screens/set_url.dart
- lib/views/pre_auth_screens/select_language.dart
- test/views/after_auth_screens/events/manage_agenda_items_screen_test.dart
- lib/custom_painters/language_icon.dart
- lib/view_model/access_request_view_model.dart
- lib/widgets/custom_list_tile.dart
- lib/views/after_auth_screens/chat/widgets/chat_input_field.dart
- lib/views/after_auth_screens/feed/individual_post.dart
- lib/custom_painters/talawa_logo.dart
- lib/widgets/event_card.dart
- lib/services/navigation_service.dart
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
lib/widgets/custom_avatar.dart
[warning] File required formatting changes
test/widget_tests/pre_auth_screens/select_language_page_test.dart
[warning] File required formatting changes
lib/widgets/custom_weekday_selector.dart
[warning] File required formatting changes
lib/widgets/pinned_carousel_widget.dart
[warning] File required formatting changes
🔇 Additional comments (21)
lib/custom_painters/whatsapp_logo.dart (1)
106-106
: LGTM! Performance optimization for color opacity.The change from
withOpacity
towithAlpha
is a good optimization as it reduces one multiplication operation. This is consistent with similar changes across the codebase.Also applies to: 425-425
lib/custom_painters/telegram_logo.dart (1)
46-46
: Similar changes in other color definitions.The same feedback applies to other color definitions in the file.
Also applies to: 63-63, 110-111
lib/view_model/main_screen_view_model.dart (2)
181-181
: LGTM! Good optimization.Storing the dialog widget in a local variable before use is a good optimization. This prevents rebuilding the widget unnecessarily and improves code readability.
Also applies to: 199-199
Line range hint
1-724
: Verify Flutter 3.27.3 compatibility.Since this PR includes an upgrade to Flutter 3.27.3, let's verify that all Flutter features used in this file are compatible with the new version.
✅ Verification successful
All Flutter 3.27.3 APIs used in this file are compatible
The code uses standard Flutter patterns and APIs that are stable in Flutter 3.27.3:
- SDK constraints in pubspec.yaml support Flutter 3.27.3
- All dependencies including tutorial_coach_mark are compatible
- No deprecated APIs or breaking changes detected in the used patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any breaking changes in Flutter 3.27.3 that might affect the features used in this file # Check pubspec.yaml for Flutter SDK version constraint echo "Checking Flutter SDK version constraint..." rg "sdk: flutter" -A 2 "pubspec.yaml" # Search for any deprecated APIs used in the file echo "Checking for deprecated APIs..." rg "deprecated.*3\.27" ".dart-api-docs" || echo "No deprecated APIs found" # Check if TutorialCoachMark package is compatible with Flutter 3.27.3 echo "Checking TutorialCoachMark package compatibility..." curl -s "https://pub.dev/packages/tutorial_coach_mark.json" | jq -r '.versions[] | select(.version == "1.2.11") | .pubspec.environment.flutter'Length of output: 737
Script:
#!/bin/bash # Check Flutter SDK version constraint and dependencies echo "Checking Flutter SDK version constraint and dependencies..." cat pubspec.yaml # Check for any deprecated API usage patterns echo -e "\nChecking for potential deprecated API usage..." rg "GlobalKey<ScaffoldState>" -A 2 rg "Timer.periodic" -A 2 rg "BottomNavigationBarItem" -A 2 # Check tutorial_coach_mark version echo -e "\nChecking tutorial_coach_mark version..." rg "tutorial_coach_mark:" -A 1 pubspec.yamlLength of output: 78436
Script:
#!/bin/bash # Check tutorial_coach_mark version and compatibility echo "Checking tutorial_coach_mark version..." rg "tutorial_coach_mark:" -A 1 pubspec.yaml # Check for any breaking changes in Flutter 3.27.3 that might affect used widgets echo -e "\nChecking for potential breaking changes..." rg "class MainScreenViewModel" -B 2 -A 10 lib/view_model/main_screen_view_model.dart # Check GlobalKey usage pattern echo -e "\nChecking GlobalKey usage pattern..." rg "GlobalKey<ScaffoldState>" -A 2 lib/view_model/main_screen_view_model.dart # Check Timer usage echo -e "\nChecking Timer usage..." rg "Timer.periodic" -A 5 lib/view_model/main_screen_view_model.dartLength of output: 1447
Script:
#!/bin/bash # Check navigation service usage pattern echo "Checking navigation service usage..." rg "navigationService" -A 2 lib/view_model/main_screen_view_model.dart # Check for any Material 3 related changes echo -e "\nChecking Material usage..." rg "import.*material" -A 5 lib/view_model/main_screen_view_model.dart # Verify BaseModel implementation echo -e "\nChecking BaseModel implementation..." rg "class BaseModel" -A 10Length of output: 1889
lib/widgets/custom_weekday_selector.dart (2)
41-41
: LGTM! The opacity conversion is mathematically correct.The change from
withOpacity(0.5)
towithAlpha((0.5 * 255).toInt())
will produce visually equivalent results.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] File required formatting changes
41-41
: Fix the formatting issue.The pipeline indicates that this file requires formatting changes.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] File required formatting changes
lib/widgets/venue_card.dart (1)
39-39
: LGTM! The opacity conversions are consistent and mathematically correct.Both changes from
withOpacity(0.3)
towithAlpha((0.3 * 255).toInt())
will produce visually equivalent results:
- For the card's background color when selected
- For the image container's background color
Also applies to: 50-50
lib/widgets/pinned_carousel_widget.dart (1)
33-33
: LGTM! The opacity conversion is mathematically correct.The change from
withOpacity(0.5)
towithAlpha((0.5 * 255).toInt())
will produce visually equivalent results.🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] File required formatting changes
lib/widgets/create_recurring_event_helper_widgets.dart (2)
238-238
: LGTM! The opacity conversion is mathematically correct.The change from
withOpacity(0.4)
towithAlpha((0.4 * 255).toInt())
will produce visually equivalent results for the disabled text state.
Line range hint
41-41
: Consider documenting the rationale for switching from withOpacity to withAlpha.While all the opacity to alpha conversions are mathematically correct and will produce visually equivalent results, it would be helpful to document the reasoning behind this systematic change across the codebase (e.g., performance optimization, Flutter SDK compatibility, or standardization effort).
Also applies to: 39-39, 50-50, 33-33, 238-238
test/widget_tests/widgets/custom_avatar_test.dart (1)
137-137
: LGTM! Improved color opacity handling.The change from
withOpacity(0.2)
towithAlpha((0.2 * 255).toInt())
is a good optimization. UsingwithAlpha
with an integer value (0-255) is more efficient thanwithOpacity
with a double value (0.0-1.0) as it avoids floating-point calculations.test/widget_tests/widgets/agenda_item_tile_test.dart (1)
350-350
: LGTM! Consistent color opacity handling.The change to use
withAlpha((0.5 * 255).toInt())
aligns with the codebase-wide standardization of color opacity handling.test/widget_tests/pre_auth_screens/select_language_page_test.dart (1)
154-154
: LGTM! Consistent color opacity handling in both themes.The changes to use
withAlpha((0.15 * 255).toInt())
in both light and dark mode tests align with the standardized approach to color opacity.Also applies to: 258-258
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] File required formatting changes
lib/views/after_auth_screens/profile/edit_profile_page.dart (1)
60-60
: LGTM! Consistent color opacity handling.The change to use
withAlpha((0.2 * 255).toInt())
for the CircleAvatar's background color aligns with the standardized approach across the codebase.lib/views/after_auth_screens/org_info_screen.dart (2)
102-102
: LGTM! Performance improvement in color handling.The change from
withOpacity
towithAlpha
is a good optimization. ThewithAlpha
method is more efficient as it directly sets the alpha channel value instead of performing floating-point calculations at runtime.
344-344
: LGTM! Consistent color handling for shadows.The shadow color opacity change maintains consistency with the new color handling approach across the codebase.
lib/views/after_auth_screens/events/edit_agenda_item_page.dart (1)
82-82
: LGTM! Consistent toast notification styling.The color opacity change for the toast notification maintains consistency with the new color handling approach.
lib/widgets/custom_avatar.dart (1)
45-45
: LGTM! Consistent avatar background styling.The color opacity changes for both avatar states (with and without image) maintain consistency with the new color handling approach.
Also applies to: 63-63
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] File required formatting changes
talawa_lint/lib/talawa_api_doc/talawa_api_doc_fixer.dart (3)
85-87
: LGTM! Improved code readability.Adding braces for the return statement improves code clarity and maintainability.
100-102
: LGTM! Consistent control flow style.Adding braces maintains consistency in control flow statements.
111-113
: LGTM! Complete control flow standardization.Adding braces completes the standardization of control flow statements throughout the file.
test/widget_tests/pre_auth_screens/select_language_page_test.dart
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2722 +/- ##
=================================================
Coverage 96.80% 96.81%
=================================================
Files 189 189
Lines 9997 10010 +13
=================================================
+ Hits 9678 9691 +13
Misses 319 319 ☔ View full report in Codecov by Sentry. |
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
🔭 Outside diff range comments (2)
lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart (1)
Line range hint
146-190
: Resource leak: QR controller not properly disposedThe removal of
controller.dispose()
could lead to memory leaks. The camera resources need to be properly released when the QR view is no longer needed.Add proper disposal of the QR controller when the bottom sheet is closed:
void _onQRViewCreated( QRViewController controller, SelectOrganizationViewModel model, ) { + // Ensure controller is disposed when the bottom sheet is closed + WidgetsBinding.instance.addPostFrameCallback((_) { + if (navigationService.navigatorKey.currentContext != null) { + final navigator = Navigator.of(navigationService.navigatorKey.currentContext!); + navigator.focusScopeNode.addListener(() { + if (!navigator.focusScopeNode.hasFocus) { + controller.dispose(); + } + }); + } + }); controller.scannedDataStream.listen((scanData) { // ... existing code ... }); }lib/view_model/pre_auth_view_models/set_url_view_model.dart (1)
Line range hint
391-456
: Resource leak: Apply consistent QR controller disposalFor consistency with the changes in
join_organisation_after_auth.dart
, we need to implement proper controller disposal here as well.Apply the same controller disposal pattern:
void _onQRViewCreated(QRViewController controller) { + // Ensure controller is disposed when the bottom sheet is closed + WidgetsBinding.instance.addPostFrameCallback((_) { + if (navigationService.navigatorKey.currentContext != null) { + final navigator = Navigator.of(navigationService.navigatorKey.currentContext!); + navigator.focusScopeNode.addListener(() { + if (!navigator.focusScopeNode.hasFocus) { + controller.dispose(); + } + }); + } + }); controller.scannedDataStream.listen((scanData) { // ... existing code ... }); }
🧹 Nitpick comments (1)
lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart (1)
Line range hint
146-190
: Enhance error handling in QR scanningThe error handling could be improved to catch specific QR-related exceptions.
Add comprehensive error handling:
void _onQRViewCreated( QRViewController controller, SelectOrganizationViewModel model, ) { controller.scannedDataStream.listen((scanData) { if (scanData.code!.isNotEmpty) { print(scanData.code); try { final List<String> data = scanData.code!.split('?'); final String url = data[0]; Vibration.vibrate(duration: 100); if (url == GraphqlConfig.orgURI) { final List<String> queries = data[1].split('&'); model.orgId = queries[0].split('=')[1]; controller.stopCamera(); Navigator.pop(navigationService.navigatorKey.currentContext!); model.initialise(model.orgId); } else { navigationService.showTalawaErrorSnackBar( "Organisation on different server, logout and scan qr again", MessageType.error, ); } - } on Exception catch (e) { + } on CameraException catch (e) { + print(e); + navigationService.showTalawaErrorSnackBar( + "Camera error: ${e.description}", + MessageType.error, + ); + } on FormatException catch (e) { + print(e); + navigationService.showTalawaErrorSnackBar( + "Invalid QR code format", + MessageType.error, + ); + } on Exception catch (e) { print(e); print('invalid app qr'); + navigationService.showTalawaErrorSnackBar( + "Invalid QR code", + MessageType.error, + ); } } }); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lock
is excluded by!**/*.lock
📒 Files selected for processing (14)
android/app/proguard-rules.pro
(1 hunks)android/build.gradle
(1 hunks)lib/custom_painters/language_icon.dart
(2 hunks)lib/custom_painters/talawa_logo.dart
(5 hunks)lib/custom_painters/telegram_logo.dart
(5 hunks)lib/custom_painters/whatsapp_logo.dart
(3 hunks)lib/view_model/pre_auth_view_models/select_organization_view_model.dart
(1 hunks)lib/view_model/pre_auth_view_models/set_url_view_model.dart
(1 hunks)lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart
(1 hunks)pubspec.yaml
(3 hunks)test/helpers/test_helpers.dart
(1 hunks)test/view_model_tests/pre_auth_view_models/set_url_view_model_test.dart
(1 hunks)test/views/after_auth_screens/join_org_after_auth_test/join_organisation_after_auth_test.dart
(1 hunks)test/widget_tests/pre_auth_screens/set_url_page_test.dart
(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- android/app/proguard-rules.pro
- test/widget_tests/pre_auth_screens/set_url_page_test.dart
- test/views/after_auth_screens/join_org_after_auth_test/join_organisation_after_auth_test.dart
- lib/view_model/pre_auth_view_models/select_organization_view_model.dart
- test/helpers/test_helpers.dart
- test/view_model_tests/pre_auth_view_models/set_url_view_model_test.dart
🚧 Files skipped from review as they are similar to previous changes (6)
- lib/custom_painters/language_icon.dart
- lib/custom_painters/talawa_logo.dart
- android/build.gradle
- lib/custom_painters/whatsapp_logo.dart
- pubspec.yaml
- lib/custom_painters/telegram_logo.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Checking codebase
🔇 Additional comments (3)
lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart (1)
3-3
: Verify qr_code_scanner_plus compatibility with Flutter 3.27.3The migration to qr_code_scanner_plus is appropriate for Flutter 3.x compatibility.
lib/view_model/pre_auth_view_models/set_url_view_model.dart (2)
3-3
: LGTM! Package migration is consistentThe migration to qr_code_scanner_plus is consistently applied across files.
Line range hint
391-456
: LGTM! Comprehensive error handlingThe error handling in this file is more comprehensive than in
join_organisation_after_auth.dart
, properly catching and handling specific QR-related exceptions.
@palisadoes Please Add |
Hey, @palisadoes please tell how to proceed? |
Hey, I had to replace I've also updated Flutter to version 3.27.3 (the latest). The Android build is now passing, and all checks have cleared except for one related to docs deployment, which should pass soon. Other than that, I don’t see any issues. |
What kind of change does this PR introduce?
Issue Number:
Did you add tests for your changes?
Not required
Tests are written for all changes made in this PR.
Test coverage meets or exceeds the current coverage (~90/95%).
Summary
Key changes include:
Flutter Version
: Upgraded to 3.27.3 in GitHub workflow files.Dart SDK Version
: Updated to 3.6.1 in pubspec.yaml and lock files.Android Gradle Plugin
: Upgraded to 8.7.1.Gradle Version
: Updated to 8.9.Dependencies
: Replaced the outdated timelines package (last updated 3 years ago) with timelines_plus, which is a drop-in replacement updated for the latest Flutter version.Does this PR introduce a breaking change?
Checklist for Repository Standards
coderaabbitai
review suggestions?Have you read the contributing guide?
Summary by CodeRabbit
Release Notes
Dependencies
timelines
package withtimelines_plus
Build Configuration
Infrastructure
UI Enhancements