-
Notifications
You must be signed in to change notification settings - Fork 16
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
git actions for build apk #630
Conversation
…hint text and info description for search referral (#435)
# Conflicts: # apps/health_campaign_field_worker_app/pubspec.lock # apps/health_campaign_field_worker_app/pubspec.yaml # packages/attendance_management/pubspec.yaml # packages/inventory_management/pubspec.lock # packages/inventory_management/pubspec.yaml # packages/referral_reconciliation/pubspec.yaml # packages/registration_delivery/pubspec.lock # packages/registration_delivery/pubspec.yaml
added new service for foreground separated sync service as a package updated package dependencies
added subscription listener in boundary and reports from utils
added permissions for data sync
updated hcm and packages version to latest dart_mappable_builder and generated mappers for the new version dart analyze issue fixes
…ckground service start
…n of reactive forms
…441) * updated the integer picker and added a new text block component * added a field to change the width of button * modified text block component to conditionally render children --------- Co-authored-by: rachana-egov <[email protected]>
Co-authored-by: rachana-egov <[email protected]>
…ted reactive forms
Co-authored-by: rachana-egov <[email protected]>
* updated checklist for a new type boolean * added To Do to fix hard code options --------- Co-authored-by: rachana-egov <[email protected]>
* HLM-6283:: IRS- Household Details Fields added * HLM-6283:: House Structure Details Page
…ield to selection card (#450) Co-authored-by: rachana-egov <[email protected]>
* HLM-6283:: IRS- Household Details Fields added * HLM-6283:: House Structure Details Page * HLM-6371:: Beneficiary Registration Summary Page * Updated registration_delivery script
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: 4
🧹 Nitpick comments (1)
apps/health_campaign_field_worker_app/android/app/src/main/kotlin/com/digit/hcm/MainActivity.kt (1)
43-70
: Consider optimizing location update interval and error handlingThe service management implementation could be improved:
- The default interval of 1 minute might drain battery unnecessarily
- The timestamp conversion lacks error handling
Consider these improvements:
- val interval = (call.argument<Number>("interval")?.toLong()) ?: 60000L - val stopAfterTimestamp = (call.argument<Number>("stopAfterTimestamp")?.toLong()) - ?: (System.currentTimeMillis() + 60000L) + val interval = try { + call.argument<Number>("interval")?.toLong() ?: 300000L // Default to 5 minutes + } catch (e: NumberFormatException) { + 300000L + } + val stopAfterTimestamp = try { + call.argument<Number>("stopAfterTimestamp")?.toLong() + ?: (System.currentTimeMillis() + 3600000L) // Default to 1 hour + } catch (e: NumberFormatException) { + System.currentTimeMillis() + 3600000L + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/health_campaign_field_worker_app/android/app/src/main/AndroidManifest.xml
is excluded by!**/*.xml
📒 Files selected for processing (2)
apps/health_campaign_field_worker_app/android/app/build.gradle
(4 hunks)apps/health_campaign_field_worker_app/android/app/src/main/kotlin/com/digit/hcm/MainActivity.kt
(2 hunks)
🔇 Additional comments (3)
apps/health_campaign_field_worker_app/android/app/build.gradle (2)
28-28
: SDK version configuration is properly aligned with Flutter
Using Flutter's SDK version properties ensures consistency with Flutter's requirements and recommendations.
Also applies to: 49-50
32-33
: Java 17 configuration looks good
The upgrade to Java 17 has been confirmed as compatible with the current Flutter version. However, let's verify that all related configuration files are aligned.
Also applies to: 37-37
apps/health_campaign_field_worker_app/android/app/src/main/kotlin/com/digit/hcm/MainActivity.kt (1)
73-79
: Review security implications of exported receiver
Using Context.RECEIVER_EXPORTED
on Android 13+ makes the broadcast receiver accessible to other apps. Consider if this level of access is necessary and document any security implications.
Consider adding runtime permissions check and implementing security measures:
+ // Check for required permissions before registering
+ if (checkSelfPermission(android.Manifest.permission.ACCESS_FINE_LOCATION)
+ != PackageManager.PERMISSION_GRANTED) {
+ // Handle missing permissions
+ return
+ }
+
val filter = IntentFilter("LocationUpdate")
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
+ // Add security flags if needed
+ filter.priority = IntentFilter.SYSTEM_HIGH_PRIORITY
registerReceiver(locationReceiver, filter, Context.RECEIVER_EXPORTED)
} else {
registerReceiver(locationReceiver, filter)
}
apps/health_campaign_field_worker_app/android/app/src/main/kotlin/com/digit/hcm/MainActivity.kt
Outdated
Show resolved
Hide resolved
apps/health_campaign_field_worker_app/android/app/src/main/kotlin/com/digit/hcm/MainActivity.kt
Outdated
Show resolved
Hide resolved
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/health_campaign_field_worker_app/lib/data/sync_service_mapper.dart (1)
34-80
:⚠️ Potential issueAdd missing
break
statements in switch cases.Each case in the switch statement should end with a
break
statement to prevent unintended fall-through behavior. This is particularly important in Dart where fall-through is disallowed.
🧹 Nitpick comments (5)
apps/health_campaign_field_worker_app/lib/pages/authenticated.dart (2)
112-112
: Review or remove the metadata comment.“// INFO : Need to add bloc of package Here” might no longer be necessary. Remove the comment if no further action is required or replace it with more precise documentation.
120-120
: Ensure consistent dependency injection.Instantiating SyncService directly here might be acceptable, but consider injecting it from a higher-level DI container for maintainability and testability, especially if SyncService grows more complex.
apps/health_campaign_field_worker_app/lib/data/sync_service_mapper.dart (3)
83-85
: Enhance error handling with context.The current error handling simply rethrows exceptions without providing context about which entity type failed or what operation was being performed.
Consider wrapping the error:
} catch (e) { - rethrow; + throw Exception('Failed to write entities to DB: $e'); }
715-782
: Document the commented complaints sync code.The commented code block for complaints sync needs proper documentation explaining why it's commented out and when it should be enabled.
Add a more descriptive comment:
- // Note: Uncomment the following code block to enable complaints sync down + /** + * Complaints sync down is currently disabled. + * TODO: Enable this when the complaints feature is ready for production. + * This implementation handles PGR service requests differently from other entities + * as it requires individual request lookups instead of bulk search. + */
795-798
: Move constants to class level.These constants are duplicated from the syncDownEntityResponse method. They should be defined once at the class level.
Move the constants to class level:
class SyncServiceMapper extends SyncEntityMapperListener { + static const taskResourceIdKey = 'taskResourceId'; + static const individualIdentifierIdKey = 'individualIdentifierId'; + static const householdAddressIdKey = 'householdAddressId'; + static const individualAddressIdKey = 'individualAddressId'; // ... rest of the class
🛑 Comments failed to post (2)
apps/health_campaign_field_worker_app/lib/data/sync_service_mapper.dart (2)
134-788: 🛠️ Refactor suggestion
Extract common sync down logic to reduce duplication.
The method contains significant duplication in server ID updates and error handling across different entity types. This makes maintenance difficult and increases the chance of bugs.
Consider extracting the common pattern into a helper method:
Future<void> handleEntitySync<T extends EntityModel>({ required T entity, required T? responseEntity, required LocalRepository<EntityModel, EntitySearchModel> local, required DataOperation operation, List<AdditionalId>? additionalIds, }) async { final serverGeneratedId = responseEntity?.id; final rowVersion = responseEntity?.rowVersion; if (serverGeneratedId != null) { await local.opLogManager.updateServerGeneratedIds( model: UpdateServerGeneratedIdModel( clientReferenceId: entity.clientReferenceId, serverGeneratedId: serverGeneratedId, additionalIds: additionalIds, dataOperation: operation, rowVersion: rowVersion, ), ); } else { final bool markAsNonRecoverable = await local.opLogManager.updateSyncDownRetry(entity.clientReferenceId); if (markAsNonRecoverable) { await local.update( entity.copyWith(nonRecoverableError: true), createOpLog: false, ); } } }
89-132: 🛠️ Refactor suggestion
Reduce code duplication in entity type checking.
The method contains duplicate switch cases for checking entity types. This can be simplified using a set of allowed types.
Consider refactoring to:
int getSyncCount(List<OpLog> opLogs) { + final allowedTypes = { + DataModelType.household, + DataModelType.individual, + DataModelType.householdMember, + DataModelType.projectBeneficiary, + DataModelType.task, + DataModelType.stock, + DataModelType.stockReconciliation, + DataModelType.sideEffect, + DataModelType.referral, + DataModelType.hFReferral, + DataModelType.attendance, + }; + return opLogs.where((element) { - if (element.syncedDown == false && element.syncedUp == true) { - switch (element.entityType) { - // ... existing cases - } - } else { - switch (element.entityType) { - // ... existing cases - } - } + if (element.syncedDown == false && element.syncedUp == true) { + return allowedTypes.contains(element.entityType); + } + return allowedTypes.contains(element.entityType); }).length; }Committable suggestion skipped: line range outside the PR's diff.
added log import in main activity
No description provided.