Migrating qr_code_scanner to mobile_scanner in talawa mobile #2719
Conversation
…ation/talawa into develop-postgres
- successfully migrated to mobile_scanner - app building on android 14(api 34) - updated old tests
WalkthroughThe pull request introduces a comprehensive migration from the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart (1)
201-228: Avoid plain print statements & confirm consistent disposal.
Usingcontroller.stop()andcontroller.dispose()in rapid succession does not cause race conditions.A potential approach:
- print(code); + debugPrint('Scanned code: $code'); // or use a logging tool - print('invalid app qr'); + debugPrint('Invalid app QR.');lib/locator.dart (1)
104-104: Validate lazy vs singleton registrations.
Registering all services in one place is handy, but re-check if some singletons should be factories (or vice versa) to align with their life cycles.Would you like to modularize this by grouping related services into separate
setupmethods (e.g.,setupViewModels(),setupServices())?lib/splash_screen.dart (2)
15-19: Clarify theisTestingparameter usage.
Adding theisTestingflag is useful for skipping certain logic in tests. Ensure you document how testers should use this parameter in different scenarios.
215-216: Remove or convert debug print statement.
print(widget.isTesting);may be unnecessary in production. Consider removing or replacing it with a debug log.- print(widget.isTesting); + debugPrint('widget.isTesting: ${widget.isTesting}');test/views/after_auth_screens/join_org_after_auth_test/join_organisation_after_auth_test.dart (1)
63-76: Verify the barcode format in test data.The test data structure has been correctly updated to use
BarcodeCapturewithBarcodeFormat.qrCode. However, there's a potential improvement in the test data construction.Consider removing the unnecessary string concatenation:
- displayValue: ' ' + '?orgid=6737904485008f171cf29924', + displayValue: '?orgid=6737904485008f171cf29924',test/widget_tests/pre_auth_screens/splash_screen_test.dart (2)
84-85: Clean up commented code.Remove unnecessary commented-out code to improve readability.
Apply this diff:
- // late StreamController<Uri> uriStreamController; - - // uriStreamController = StreamController<Uri>(); -Also applies to: 90-91
311-323: Simplify test structure by removing commented runAsync blocks.The commented-out
runAsyncblocks should be either removed or properly implemented if async testing is needed.Consider either:
- Removing the commented blocks entirely for cleaner code
- Implementing proper async testing if required
If async testing is needed, consider using
tester.binding.delayedinstead ofrunAsyncfor better control over async operations.Also applies to: 327-339, 343-355, 358-370, 374-385
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
devtools_options.yaml(1 hunks)lib/locator.dart(1 hunks)lib/splash_screen.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(4 hunks)lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart(4 hunks)pubspec.yaml(1 hunks)test/helpers/test_helpers.dart(2 hunks)test/view_model_tests/pre_auth_view_models/set_url_view_model_test.dart(8 hunks)test/views/after_auth_screens/join_org_after_auth_test/join_organisation_after_auth_test.dart(4 hunks)test/widget_tests/pre_auth_screens/set_url_page_test.dart(2 hunks)test/widget_tests/pre_auth_screens/splash_screen_test.dart(5 hunks)
✅ Files skipped from review due to trivial changes (2)
- lib/view_model/pre_auth_view_models/select_organization_view_model.dart
- devtools_options.yaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Checking codebase
🔇 Additional comments (11)
lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart (3)
17-27: Ensure widget state is truly required.
Converting fromStatelessWidgettoStatefulWidgetis appropriate here, given that QR scanning logic depends on state. However, confirm that any potential side effects (e.g., re-initialization of controllers) won’t negatively impact performance when the widget rebuilds.
29-32: Controller lifecycle check.
TheMobileScannerControlleris stored as a field in_JoinOrganisationAfterAuthState, which is fine. Ensure you properly dispose of it indispose()if the user leaves this page prematurely.Would you like to factor out
controllerdisposal to adispose()override so new scans can be re-initialized when returning to this screen?
3-4: Replace or remove unused import.
The blank line before and the import formobile_scannerindicate a transition fromqr_code_scanner. If the replaced library is no longer used elsewhere, consider removing the old import references entirely.To confirm it's fully migrated, run:
✅ Verification successful
Migration to mobile_scanner is complete
The qr_code_scanner package has been fully replaced with mobile_scanner in the codebase. Remaining references are only in build configuration and documentation files, which can be cleaned up separately.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash rg -A 2 "qr_code_scanner"Length of output: 7264
test/views/after_auth_screens/join_org_after_auth_test/join_organisation_after_auth_test.dart (2)
7-8: Update import statements for mobile_scanner migration.Good job replacing the deprecated
qr_code_scannerimport withmobile_scanner. This aligns with the PR objective of migrating to the new scanner library.
98-100: Verify MobileScanner controller initialization.The test is correctly using the new
MobileScannerwidget and its controller. However, there's a potential null safety issue when accessing the controller.Consider adding a null check before accessing the controller or use a more robust way to get the controller instance:
- (tester.widget(find.byType(MobileScanner)) as MobileScanner) - .controller! - .start(); + final scanner = tester.widget(find.byType(MobileScanner)) as MobileScanner; + if (scanner.controller != null) { + scanner.controller!.start(); + }Also applies to: 138-140, 170-172
lib/view_model/pre_auth_view_models/set_url_view_model.dart (1)
244-277: Comprehensive error handling for scanner issues.Excellent implementation of error handling with specific messages for different scanner error scenarios. The error builder provides clear user feedback for various failure cases.
test/view_model_tests/pre_auth_view_models/set_url_view_model_test.dart (1)
294-298: Comprehensive error testing implementation.Good implementation of error testing using
MobileScannerException. The test covers the specific error code scenario.test/widget_tests/pre_auth_screens/splash_screen_test.dart (1)
51-51: Good addition of isTesting flag.The addition of the
isTestingflag helps prevent timer-related issues during testing.test/widget_tests/pre_auth_screens/set_url_page_test.dart (1)
4-4: LGTM! Test updated for new scanner.The test has been correctly updated to verify the
MobileScannerwidget instead of the oldQRViewwidget, aligning with the migration fromqr_code_scannertomobile_scannerpackage.Also applies to: 525-525
test/helpers/test_helpers.dart (1)
8-8: LGTM! Mock configuration updated for new scanner.The mock specifications have been correctly updated to use
MobileScannerControllerinstead ofQRViewController, maintaining test coverage for the new scanner implementation.Also applies to: 90-92
pubspec.yaml (1)
60-60: Verify the mobile_scanner package version.The added
mobile_scannerpackage is at version 6.0.2. Let's verify this version for compatibility and security.
| child: MobileScanner( | ||
| fit: BoxFit.contain, | ||
| controller: controller, | ||
| errorBuilder: (ctx, error, _) { | ||
| String errorMessage = ''; | ||
| switch (error.errorCode) { | ||
| case MobileScannerErrorCode.controllerUninitialized: | ||
| errorMessage = 'camera is not ready'; | ||
| break; | ||
| case MobileScannerErrorCode.permissionDenied: | ||
| errorMessage = | ||
| 'Please provide camera permission to scan QR code'; | ||
| break; | ||
| case MobileScannerErrorCode.unsupported: | ||
| errorMessage = | ||
| 'This device does not support scanning.'; | ||
| break; | ||
| default: | ||
| errorMessage = 'An unkonwn error occurred'; | ||
| } | ||
|
|
||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| navigationService.showTalawaErrorSnackBar( | ||
| errorMessage, | ||
| MessageType.error, | ||
| ); | ||
| }); | ||
|
|
||
| return Center( | ||
| child: Text( | ||
| errorMessage, | ||
| ), | ||
| ); | ||
| }, | ||
| onDetect: (scanData) => _onQRViewCreated(scanData, model), | ||
| overlayBuilder: (context, constraints) { | ||
| return Container( | ||
| width: 250, | ||
| height: 250, | ||
| decoration: BoxDecoration( | ||
| borderRadius: BorderRadius.circular(10), | ||
| border: Border.all( | ||
| color: Theme.of(context).colorScheme.secondary, | ||
| width: 10, | ||
| ), | ||
| ), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor error handling & fix spelling.
In the errorBuilder, the error message “unkonwn” should be corrected, and consider using a more structured approach to handle user-facing error strings.
Here’s a small fix:
- errorMessage = 'An unkonwn error occurred';
+ errorMessage = 'An unknown error occurred';Furthermore, for better i18n or theming, you could store error messages in a dedicated localized resource.
📝 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.
| child: MobileScanner( | |
| fit: BoxFit.contain, | |
| controller: controller, | |
| errorBuilder: (ctx, error, _) { | |
| String errorMessage = ''; | |
| switch (error.errorCode) { | |
| case MobileScannerErrorCode.controllerUninitialized: | |
| errorMessage = 'camera is not ready'; | |
| break; | |
| case MobileScannerErrorCode.permissionDenied: | |
| errorMessage = | |
| 'Please provide camera permission to scan QR code'; | |
| break; | |
| case MobileScannerErrorCode.unsupported: | |
| errorMessage = | |
| 'This device does not support scanning.'; | |
| break; | |
| default: | |
| errorMessage = 'An unkonwn error occurred'; | |
| } | |
| WidgetsBinding.instance.addPostFrameCallback((_) { | |
| navigationService.showTalawaErrorSnackBar( | |
| errorMessage, | |
| MessageType.error, | |
| ); | |
| }); | |
| return Center( | |
| child: Text( | |
| errorMessage, | |
| ), | |
| ); | |
| }, | |
| onDetect: (scanData) => _onQRViewCreated(scanData, model), | |
| overlayBuilder: (context, constraints) { | |
| return Container( | |
| width: 250, | |
| height: 250, | |
| decoration: BoxDecoration( | |
| borderRadius: BorderRadius.circular(10), | |
| border: Border.all( | |
| color: Theme.of(context).colorScheme.secondary, | |
| width: 10, | |
| ), | |
| ), | |
| ); | |
| }, | |
| child: MobileScanner( | |
| fit: BoxFit.contain, | |
| controller: controller, | |
| errorBuilder: (ctx, error, _) { | |
| String errorMessage = ''; | |
| switch (error.errorCode) { | |
| case MobileScannerErrorCode.controllerUninitialized: | |
| errorMessage = 'camera is not ready'; | |
| break; | |
| case MobileScannerErrorCode.permissionDenied: | |
| errorMessage = | |
| 'Please provide camera permission to scan QR code'; | |
| break; | |
| case MobileScannerErrorCode.unsupported: | |
| errorMessage = | |
| 'This device does not support scanning.'; | |
| break; | |
| default: | |
| errorMessage = 'An unknown error occurred'; | |
| } | |
| WidgetsBinding.instance.addPostFrameCallback((_) { | |
| navigationService.showTalawaErrorSnackBar( | |
| errorMessage, | |
| MessageType.error, | |
| ); | |
| }); | |
| return Center( | |
| child: Text( | |
| errorMessage, | |
| ), | |
| ); | |
| }, | |
| onDetect: (scanData) => _onQRViewCreated(scanData, model), | |
| overlayBuilder: (context, constraints) { | |
| return Container( | |
| width: 250, | |
| height: 250, | |
| decoration: BoxDecoration( | |
| borderRadius: BorderRadius.circular(10), | |
| border: Border.all( | |
| color: Theme.of(context).colorScheme.secondary, | |
| width: 10, | |
| ), | |
| ), | |
| ); | |
| }, |
| Future<void> dispose() async { | ||
| super.dispose(); | ||
| await _sub.cancel(); |
There was a problem hiding this comment.
Ensure proper disposal sequence.
Always cancel streams before calling super.dispose() to avoid possible late calls after the State is unmounted.
- Future<void> dispose() async {
- super.dispose();
- await _sub.cancel();
+ @override
+ Future<void> dispose() async {
+ await _sub.cancel();
+ super.dispose();
}📝 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.
| Future<void> dispose() async { | |
| super.dispose(); | |
| await _sub.cancel(); | |
| @override | |
| Future<void> dispose() async { | |
| await _sub.cancel(); | |
| super.dispose(); | |
| } |
| void _onQRViewCreated(BarcodeCapture scanData) { | ||
| if (scanData.raw != null && scanData.barcodes.isNotEmpty) { | ||
| final String code = scanData.barcodes.first.displayValue!; | ||
| try { | ||
| final List<String> data = code.split('?'); | ||
| url.text = data[0]; | ||
| final List<String> queries = data[1].split('&'); | ||
| orgId = queries[0].split('=')[1]; | ||
| Vibration.vibrate(duration: 100); | ||
| controller.stop(); | ||
| controller.dispose(); | ||
| final box = Hive.box('url'); | ||
| box.put(urlKey, url.text); | ||
| box.put(imageUrlKey, "${url.text}/talawa/"); | ||
| graphqlConfig.getOrgUrl(); | ||
| Navigator.pop(navigationService.navigatorKey.currentContext!); | ||
| navigationService.pushScreen('/selectOrg', arguments: orgId); | ||
| } on Exception catch (e) { | ||
| debugPrint(e.toString()); | ||
| navigationService.showTalawaErrorSnackBar( | ||
| "The Camera is not working", | ||
| MessageType.error, | ||
| ); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review QR code data processing logic.
The QR code processing logic has been updated for the new scanner, but there are some improvements possible:
- Add validation for the scanned data format
- Handle empty queries more gracefully
- Consider adding logging for debugging
void _onQRViewCreated(BarcodeCapture scanData) {
if (scanData.raw != null && scanData.barcodes.isNotEmpty) {
final String code = scanData.barcodes.first.displayValue!;
try {
+ if (!code.contains('?')) {
+ throw FormatException('Invalid QR code format');
+ }
final List<String> data = code.split('?');
url.text = data[0];
final List<String> queries = data[1].split('&');
+ if (queries.isEmpty || !queries[0].contains('=')) {
+ throw FormatException('Invalid query format');
+ }
orgId = queries[0].split('=')[1];
+ debugPrint('Successfully processed QR code: ${url.text}, orgId: $orgId');
Vibration.vibrate(duration: 100);📝 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.
| void _onQRViewCreated(BarcodeCapture scanData) { | |
| if (scanData.raw != null && scanData.barcodes.isNotEmpty) { | |
| final String code = scanData.barcodes.first.displayValue!; | |
| try { | |
| final List<String> data = code.split('?'); | |
| url.text = data[0]; | |
| final List<String> queries = data[1].split('&'); | |
| orgId = queries[0].split('=')[1]; | |
| Vibration.vibrate(duration: 100); | |
| controller.stop(); | |
| controller.dispose(); | |
| final box = Hive.box('url'); | |
| box.put(urlKey, url.text); | |
| box.put(imageUrlKey, "${url.text}/talawa/"); | |
| graphqlConfig.getOrgUrl(); | |
| Navigator.pop(navigationService.navigatorKey.currentContext!); | |
| navigationService.pushScreen('/selectOrg', arguments: orgId); | |
| } on Exception catch (e) { | |
| debugPrint(e.toString()); | |
| navigationService.showTalawaErrorSnackBar( | |
| "The Camera is not working", | |
| MessageType.error, | |
| ); | |
| } | |
| }); | |
| } | |
| void _onQRViewCreated(BarcodeCapture scanData) { | |
| if (scanData.raw != null && scanData.barcodes.isNotEmpty) { | |
| final String code = scanData.barcodes.first.displayValue!; | |
| try { | |
| if (!code.contains('?')) { | |
| throw FormatException('Invalid QR code format'); | |
| } | |
| final List<String> data = code.split('?'); | |
| url.text = data[0]; | |
| final List<String> queries = data[1].split('&'); | |
| if (queries.isEmpty || !queries[0].contains('=')) { | |
| throw FormatException('Invalid query format'); | |
| } | |
| orgId = queries[0].split('=')[1]; | |
| debugPrint('Successfully processed QR code: ${url.text}, orgId: $orgId'); | |
| Vibration.vibrate(duration: 100); | |
| controller.stop(); | |
| controller.dispose(); | |
| final box = Hive.box('url'); | |
| box.put(urlKey, url.text); | |
| box.put(imageUrlKey, "${url.text}/talawa/"); | |
| graphqlConfig.getOrgUrl(); | |
| Navigator.pop(navigationService.navigatorKey.currentContext!); | |
| navigationService.pushScreen('/selectOrg', arguments: orgId); | |
| } on Exception catch (e) { | |
| debugPrint(e.toString()); | |
| navigationService.showTalawaErrorSnackBar( | |
| "The Camera is not working", | |
| MessageType.error, | |
| ); | |
| } | |
| } |
| // await locator.unregister<Validator>(); | ||
| final service = MockValidator(); | ||
| //locator.registerSingleton<Validator>(service); | ||
| // locator.registerSingleton<Validator>(service); |
There was a problem hiding this comment.
Uncomment necessary test setup code.
There are commented-out lines that appear to be important for test setup.
Uncomment these lines to ensure proper test initialization:
- // await locator.unregister<Validator>();
+ await locator.unregister<Validator>();
final service = MockValidator();
- // locator.registerSingleton<Validator>(service);
+ locator.registerSingleton<Validator>(service);📝 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.
| // await locator.unregister<Validator>(); | |
| final service = MockValidator(); | |
| //locator.registerSingleton<Validator>(service); | |
| // locator.registerSingleton<Validator>(service); | |
| await locator.unregister<Validator>(); | |
| final service = MockValidator(); | |
| locator.registerSingleton<Validator>(service); |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2719 +/- ##
====================================================
- Coverage 96.80% 96.14% -0.67%
====================================================
Files 189 189
Lines 9997 10026 +29
====================================================
- Hits 9678 9639 -39
- Misses 319 387 +68 ☔ View full report in Codecov by Sentry. |
|
@palisadoes please review and let me know if any changes are required |
|
Please fix the tests |
palisadoes
added
the
ignore-sensitive-files-pr
|
i am in the process of writing improved tests |
|
@palisadoes I am trying to write tests for the scanner functionality particularly testing this behaviour In the tests I have managed to stub the behavior of the Here we can create different tests by changing the error Property to Not sure how to proceed further, could you suggest something? |
|
@noman2002 PTAL |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
pubspec.yaml (1)
22-22: Fix trailing spaces in dependency declaration.Remove trailing spaces after
app_links:to maintain consistent formatting.- app_links: + app_links:🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 22-22: trailing spaces
(trailing-spaces)
lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart (2)
191-191: Replace print with proper logging.Use proper logging mechanism instead of print for better debugging and production monitoring.
- print(code); + debugPrint('Scanned QR code: $code');
210-211: Improve error logging.Replace print statements with proper error logging and provide more descriptive error messages.
- print(e); - print('invalid app qr'); + debugPrint('Error processing QR code: $e'); + debugPrint('Invalid app QR code format');lib/view_model/pre_auth_view_models/set_url_view_model.dart (1)
301-320: Add validation for QR code format.The QR code processing logic should validate the format before processing to prevent potential errors.
if (scanData.barcodes.isNotEmpty) { debugPrint("Barcode: ${scanData.barcodes.first.displayValue}"); try { final String code = scanData.barcodes.first.displayValue!; + if (!code.contains('?')) { + throw FormatException('Invalid QR code format'); + } final List<String> data = code.split('?'); + if (data.length != 2) { + throw FormatException('Invalid QR code structure'); + } url.text = data[0]; final List<String> queries = data[1].split('&'); + if (queries.isEmpty || !queries[0].contains('=')) { + throw FormatException('Invalid query format'); + } orgId = queries[0].split('=')[1];test/view_model_tests/pre_auth_view_models/set_url_view_model_test.dart (1)
669-877: Consolidate duplicate test cases.Multiple test cases have identical descriptions. Consider consolidating them or providing unique descriptions that reflect their specific test scenarios.
Example of better test descriptions:
- 'Check if _onQRViewCreated() is working fine when throws MobileScannerException', + 'MobileScannerException: Handle controller already initialized error',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
android/app/proguard-rules.pro(1 hunks)android/gradle/wrapper/gradle-wrapper.properties(1 hunks)android/settings.gradle(1 hunks)lib/view_model/pre_auth_view_models/set_url_view_model.dart(5 hunks)lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart(4 hunks)pubspec.yaml(2 hunks)test/view_model_tests/pre_auth_view_models/set_url_view_model_test.dart(5 hunks)
✅ Files skipped from review due to trivial changes (2)
- android/app/proguard-rules.pro
- android/gradle/wrapper/gradle-wrapper.properties
🧰 Additional context used
🪛 YAMLlint (1.35.1)
pubspec.yaml
[error] 22-22: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Checking codebase
🔇 Additional comments (6)
android/settings.gradle (1)
22-22: Approve AGP 8.1.1 update for Android 14 support.The update to Android Gradle Plugin 8.1.1 is necessary for Android 14 (API 34) support and aligns with the PR objectives.
Since AGP 8.x introduces breaking changes, verify the following:
Key breaking changes to verify:
- Gradle 8.x compatibility
- R8 full mode is now default
- Jetifier is deprecated
- New SDK version syntax in build.gradle
✅ Verification successful
AGP 8.1.1 update is properly configured for Android 14 support
All necessary configurations for AGP 8.1.1 are in place:
- Gradle 8.1.1 wrapper ✓
- Java 17 compatibility ✓
- Android 14 (SDK 34) targeting ✓
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Gradle configuration and potential breaking changes # Check if gradle-wrapper.properties is updated to compatible version echo "Checking Gradle wrapper version..." cat gradle/wrapper/gradle-wrapper.properties # Check for common breaking changes in build files echo "Checking for common breaking changes..." rg -l 'android\.enableR8' # R8 full mode is now default rg -l 'android\.enableJetifier' # Jetifier is deprecated rg -l 'compileSdkVersion|targetSdkVersion' # New SDK version syntaxLength of output: 445
Script:
#!/bin/bash # Search for gradle-wrapper.properties echo "Searching for gradle-wrapper.properties..." fd "gradle-wrapper.properties" echo -e "\nChecking android/gradle.properties content..." cat android/gradle.properties echo -e "\nChecking android/app/build.gradle content..." cat android/app/build.gradleLength of output: 2637
Script:
#!/bin/bash echo "Checking Gradle wrapper version..." cat android/gradle/wrapper/gradle-wrapper.propertiesLength of output: 366
pubspec.yaml (1)
60-60: LGTM! Good choice of mobile_scanner version.The mobile_scanner version 6.0.2 is appropriate for Android 14 compatibility.
lib/views/after_auth_screens/join_org_after_auth/join_organisation_after_auth.dart (1)
125-157: LGTM! Well-implemented MobileScanner integration.The MobileScanner implementation includes proper error handling and UI feedback. The overlay builder provides good visual guidance for users.
lib/view_model/pre_auth_view_models/set_url_view_model.dart (1)
350-378: LGTM! Comprehensive error handling.The error handling for MobileScanner is well-implemented with specific messages for each error case.
test/view_model_tests/pre_auth_view_models/set_url_view_model_test.dart (2)
156-226: LGTM! Well-structured test cases.The test cases for basic MobileScanner functionality are well-structured and cover the essential scenarios.
603-605:⚠️ Potential issueUncomment test setup code.
Important test setup code is commented out. This should be uncommented to ensure proper test initialization.
- // await locator.unregister<Validator>(); + await locator.unregister<Validator>(); final service = MockValidator(); - // locator.registerSingleton<Validator>(service); + locator.registerSingleton<Validator>(service);Likely invalid or redundant comment.
|
Please fix the failing tests |
What kind of change does this PR introduce?
This PR aims to remove the deprecated qr_code_scanner and replace it with mobile_scanner which is the latest one. Also it fixes the build exception on android 14. Now the project supports the latest android 14 version which is also the highest version that can be supported by the project 3.22.3
Issue Number:
Fixes #2714
Did you add tests for your changes?
Snapshots/Videos:
Summary
Does this PR introduce a breaking change?
yes
Checklist for Repository Standards
coderaabbitaireview suggestions?Other information
The PR also solves a flaky tests for
test\widget_tests\pre_auth_screens\splash_screen_test.dartwhich was failing due a pending timer exception. The error was at the splash screen widget which was waiting for 750 milliseconds on the start screen and then navigating away. The behaviour is hard to test so I have introduced aisTestingflag which prevents the behaviour only while testing.Have you read the contributing guide?
yes
Summary by CodeRabbit
Summary by CodeRabbit
New Features
mobile_scannerpackage.Bug Fixes
Refactor
qr_code_scannerwithmobile_scanneracross multiple components.Chores