Console beta release #760
Console beta release #760
Conversation
HCMPRE-2824 : Validation changes
Integrated fetching non mobile users Added new events to attendance_bloc.dart to fetch non mobile users Modified query to fetch same tagged users Modified Non-Mobile users UI to integrate with dynamic data
Removed curllogger
…istration & Delivery Acknowledgement pages (#734)
Fixed code rabbit comments Removed extra BeneficiaryTypes
Published transit_post Updated digit_scanner
Added role check for qr page
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/digit_data_model/lib/data/data_repository.dart (2)
314-324: Correct unreachablesearchPathbranch
The secondDataOperation.singleCreatecheck should beDataOperation.searchso thesearchPathis used as intended.• File:
packages/digit_data_model/lib/data/data_repository.dart
• Lines: 320–324- } else if (operation == DataOperation.singleCreate) { - url = searchPath; + } else if (operation == DataOperation.search) { + url = searchPath; }
148-153: Remove unintendedawaiton non-Futuremap access across multiple filesThe code currently does things like
await responseMap[key], but simpleMaplookups aren’t asynchronous. Usingawaithere will fail static analysis (the operand isn’t aFuture) and should be removed everywhere it occurs.Affected locations:
- packages/digit_data_model/lib/data/data_repository.dart (line 148)
- packages/digit_data_model/lib/data/repositories/remote/boundary.dart (line 63)
- packages/digit_data_model/lib/data/repositories/remote/product_variant.dart (line 76)
- packages/digit_data_model/lib/data/repositories/remote/project_facility.dart (line 86)
- packages/digit_data_model/lib/data/repositories/remote/facility.dart (line 89)
- packages/digit_data_model/lib/data/repositories/remote/project_resource.dart (line 77)
- packages/digit_data_model/lib/blocs/user/user.dart (line 48)
- packages/inventory_management/lib/data/repositories/remote/stock.dart (line 93)
- packages/attendance_management/lib/data/repositories/remote/attendance_register.dart (line 62)
- packages/attendance_management/lib/data/repositories/remote/attendance_logs.dart (line 73)
Apply this pattern for each occurrence:
- final entityResponse = await responseMap[key]; + final entityResponse = responseMap[key];
🧹 Nitpick comments (4)
packages/digit_data_model/CHANGELOG.md (2)
5-8: Add migration details for user_action schema changes.The changelog should note migration/versioning specifics and operational guidance.
Proposed additions:
* Updated user_action entity and table with beneficiaryTag and resourceTag fields + - Migration: add nullable columns `beneficiaryTag` and `resourceTag` to `user_action`. + - Backfill: if existing analytics/filters depend on these tags, plan a one-time backfill. + - Indexing: add indexes if used in WHERE clauses for search/reporting. + - Compatibility: reads remain backward compatible; writes from older app versions will not populate the new columns.
1-3: Update CHANGELOG entry for DioException context enrichmentThe existing entry
* Modified DioException to throw captured errorcan be clearer about what was actually changed and how it impacts consumers. Although our search found no downstream code accessing
exception.errordirectly, it’s still helpful to explicitly document the enrichment and potential breaking change.Recommended revision in packages/digit_data_model/CHANGELOG.md:
## 1.2.0-dev.2-console -* Modified DioException to throw captured error +* Enrich DioException rethrows in DataRepository with HTTP context + - Include HTTP method, endpoint URL, status code, request/response details in the `error` string. + - BREAKING: `exception.error` now returns the full context string (original error plus HTTP details); code that only needs the original error message should use `exception.message` instead.packages/digit_data_model/lib/data/data_repository.dart (2)
410-417: Optional: redact or truncate request/response to avoid PII leakage and log bloat.Embedding full payloads in exceptions risks exposing PII and producing massive logs (e.g., images or large arrays). Redact sensitive keys and limit size.
Add helpers (outside the selected range) and then call them before composing the message:
// Place near the top of the file. const _redactKeys = {'password', 'otp', 'token', 'authorization', 'aadhaar', 'ssn'}; String _safeJson(Object? data, {int maxChars = 5000}) { try { final enc = const JsonEncoder.withIndent(' '); var s = enc.convert(data); if (s.length > maxChars) s = '${s.substring(0, maxChars)}… [truncated]'; return s; } catch (_) { final s = data?.toString() ?? 'null'; return s.length > maxChars ? '${s.substring(0, maxChars)}… [truncated]' : s; } } Object? _redact(Object? value) { if (value is Map) { return value.map((k, v) => MapEntry(k, _redactKeys.contains('$k'.toLowerCase()) ? '***REDACTED***' : _redact(v))); } if (value is List) return value.map(_redact).toList(); return value; }Then replace conversions:
- errorResponse = encoder.convert(error.response?.data); + errorResponse = _safeJson(_redact(error.response?.data)); - requestBody = encoder.convert(error.requestOptions.data); + requestBody = _safeJson(_redact(error.requestOptions.data));
253-265: Confirm delete endpoint for delete operationCurrently,
delete()always posts tocreatePathwith an"apiOperation": "DELETE"flag. If your backend supports routing deletes through the create endpoint with an operation switch, you can leave this as-is. Otherwise, introduce a dedicateddeletePathgetter and use it when configured:+ /// Use a dedicated delete endpoint if provided, otherwise fall back to createPath + String get deletePath => actionMap[ApiOperation.delete] ?? ''; FutureOr<Response> delete(D entity) async { return executeFuture( future: () async { - return await dio.post( - createPath, + return await dio.post( + deletePath.isNotEmpty ? deletePath : createPath, data: { EntityPlurals.getPluralForEntityName(entityName): [entity.toMap()], "apiOperation": "DELETE", }, ); }, ); }This ensures that if you’ve configured a separate delete endpoint in
actionMap, it will be used; otherwise, the existing create-plus-operation approach continues to work.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
.github/workflows/build-apk.ymlis excluded by!**/*.ymlpackages/digit_data_model/pubspec.yamlis excluded by!**/*.yamlpackages/registration_delivery/pubspec.yamlis excluded by!**/*.yamlpackages/sync_service/pubspec.yamlis excluded by!**/*.yaml
📒 Files selected for processing (2)
packages/digit_data_model/CHANGELOG.md(1 hunks)packages/digit_data_model/lib/data/data_repository.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-05T09:37:40.694Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#620
File: packages/digit_data_model/lib/data/data_repository.dart:300-300
Timestamp: 2024-12-05T09:37:40.694Z
Learning: In `packages/digit_data_model/lib/data/data_repository.dart`, the `DigitDataModelSingleton().errorDumpApiPath` is guaranteed to be non-empty, so additional validation is not required.
Applied to files:
packages/digit_data_model/lib/data/data_repository.dart
🔇 Additional comments (1)
packages/digit_data_model/lib/data/data_repository.dart (1)
407-438: Preserve original error and stack trace; use message for enriched textGreat improvement adding rich context. Two adjustments will make it safer and more diagnosable:
- Keep the underlying error object in
error(don’t overwrite it with a String).- Preserve and associate the captured
stackTrace.- Put the human-readable details into
message.Apply this diff:
} on DioException catch (error, stackTrace) { const encoder = JsonEncoder.withIndent(' '); String errorResponse; String requestBody; try { errorResponse = encoder.convert(error.response?.data); } catch (_) { errorResponse = 'Could not parse error response'; } try { requestBody = encoder.convert(error.requestOptions.data); } catch (_) { requestBody = 'Could not parse request body'; } final endpoint = error.requestOptions.uri.toString(); final method = error.requestOptions.method; - throw DioException( - requestOptions: error.requestOptions, - response: error.response, - type: error.type, - error: '${error.error}\n' - 'Endpoint: $method $endpoint\n' - 'Status: ${error.response?.statusCode}\n' - 'Request: $requestBody\n' - 'Response: $errorResponse', - ); + final enrichedMessage = [ + if (error.message != null && error.message!.isNotEmpty) error.message!, + 'Endpoint: $method $endpoint', + 'Status: ${error.response?.statusCode}', + 'Request: $requestBody', + 'Response: $errorResponse', + ].join('\n'); + + // Preserve original cause in `error`, attach enriched text in `message`, + // and rethrow with the captured stackTrace. + Error.throwWithStackTrace( + DioException( + requestOptions: error.requestOptions, + response: error.response, + type: error.type, + error: error.error, + message: enrichedMessage, + stackTrace: stackTrace, + ), + stackTrace, + );Please verify before merging that:
- Your project depends on Dio 5.x or later.
- The
DioExceptionconstructor in your codebase actually accepts bothmessageandstackTraceparameters.If confirmed, apply the diff above; otherwise adjust the change to match your existing exception signature.
| ## 1.2.0-dev.1-console | ||
|
|
||
| * Renamed referenceId to referenceID in project model | ||
| * Made individualClientReferenceId mandatory in identifier | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Call out breaking changes and give upgrade guidance.
Renaming referenceId → referenceID and making individualClientReferenceId mandatory are breaking. Please mark them clearly and provide migration hints.
Proposed edit:
* Renamed referenceId to referenceID in project model
* Made individualClientReferenceId mandatory in identifier
+ - BREAKING: JSON keys and DB columns (if persisted) now use `referenceID`. Update serialization, queries, and migrations accordingly.
+ - Migration: rename column/field and ensure `individualClientReferenceId` is populated (consider generating client refs for legacy rows).
+ - Validation: API contracts and app-side validators must reject missing `individualClientReferenceId`.Quick checks to ensure repo-wide consistency:
🏁 Script executed:
#!/bin/bash
# Confirm all occurrences were updated
rg -nP --glob '!**/build/**' '\breferenceId\b'
rg -nP --glob '!**/build/**' '\bindividualClientReferenceId\b'Length of output: 87885
Looking at the verification results, I can see that the repository contains numerous occurrences of both referenceId and individualClientReferenceId fields throughout the codebase. This indicates that the breaking changes mentioned in the changelog are indeed significant and affect many files across multiple packages.
Let me check specifically for the project model to understand the scope of the referenceId to referenceID change:
🏁 Script executed:
#!/bin/bash
# Look for project model files and any referenceID usage
echo "=== Searching for project model files ==="
fd -t f -e dart | xargs grep -l "project.*model\|ProjectModel" | head -10
echo -e "\n=== Searching for referenceID (with capital D) usage ==="
rg -nP --glob '!**/build/**' '\breferenceID\b' | head -20
echo -e "\n=== Checking if there are any migration scripts or documentation ==="
fd -t f -e sql -e md | xargs grep -l "migration\|referenceId.*referenceID\|breaking" | head -10Length of output: 3680
Clarify Breaking Changes & Add Migration Guidance in CHANGELOG.md
The current entries for 1.2.0-dev.1-console list two changes but don’t flag them as breaking or explain how to upgrade. Please update packages/digit_data_model/CHANGELOG.md to:
- Clearly mark each item under a BREAKING CHANGES header
- Provide actionable migration steps for downstream consumers
For example:
## 1.2.0-dev.1-console
-* Renamed referenceId to referenceID in project model
-* Made individualClientReferenceId mandatory in identifier
+### BREAKING CHANGES
+- Renamed `referenceId` → `referenceID` in ProjectModel
+ - JSON & DB: JSON keys and DB column names now use `referenceID`.
+ • Run an ALTER TABLE or migration script to rename `reference_id` columns.
+ • Update any serialization/deserialization logic to reference `referenceID`.
+- Made `individualClientReferenceId` mandatory in IdentifierModel
+ - Migration: backfill or generate `individualClientReferenceId` for existing records (e.g. copy from `clientReferenceId` or run a one-off script).
+ - Validation: ensure API clients and app-side validators reject payloads missing this field. Additional actions:
- Review and update
packages/digit_data_model/README.mdif it references the old field names. - Consider adding a top-level BREAKING CHANGES section in your release notes for consistency.
These updates will make it explicit to integrators that they must run migrations and adjust their code when upgrading.
📝 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.
| ## 1.2.0-dev.1-console | |
| * Renamed referenceId to referenceID in project model | |
| * Made individualClientReferenceId mandatory in identifier | |
| ## 1.2.0-dev.1-console | |
| ### BREAKING CHANGES | |
| - Renamed `referenceId` → `referenceID` in ProjectModel | |
| - JSON & DB: JSON keys and DB column names now use `referenceID`. | |
| • Run an ALTER TABLE or migration script to rename `reference_id` columns. | |
| • Update any serialization/deserialization logic to reference `referenceID`. | |
| - Made `individualClientReferenceId` mandatory in IdentifierModel | |
| - Migration: backfill or generate `individualClientReferenceId` for existing records (e.g. copy from `clientReferenceId` or run a one-off script). | |
| - Validation: ensure API clients and app-side validators reject payloads missing this field. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
packages/digit_forms_engine/CHANGELOG.md (4)
3-5: Wording: "Bug fix" and pluralize for clarity."BugFix" → "Bug fix" and "empty string" → "empty strings" reads better.
Apply:
-### BugFix -- Removed min and maxLength validation for empty string +### Bug fix +- Removed minLength and maxLength validation for empty strings
9-11: Clarify the "mobile number" entry.Consider making it explicit that this is a format/validation for string type.
-### Added -- Added mobile number for type string +### Added +- Added mobile-number format/validation for string type
43-45: Fix nested list indentation (markdownlint MD007).Indent nested bullets by 2 spaces under “Field types supported:”.
- - `text`, `dropdown`, `latLng`, `locality`, etc. + - `text`, `dropdown`, `latLng`, `locality`, etc.
1-2: Add release date for traceability.Optional but helpful to include dates per version header.
-## 0.0.4-dev +## 0.0.4-dev - 2025-08-26packages/digit_forms_engine/lib/helper/validator_helper.dart (7)
13-26: Flatten composeOR for minLength and keep empty/null bypass explicit.Reduces nesting and improves readability without changing behavior.
- if (schema.type != PropertySchemaType.integer) { - if (parsedValue != null) { - validators.add(Validators.composeOR([ - Validators.minLength(parsedValue) as Validator<T>, - Validators.composeOR( - [Validators.equals(''), Validators.equals(null)]), - ]) as Validator<T>); - } - } + if (schema.type != PropertySchemaType.integer && parsedValue != null) { + validators.add( + Validators.composeOR([ + Validators.minLength(parsedValue) as Validator<T>, + Validators.equals('') as Validator<T>, + Validators.equals(null) as Validator<T>, + ]) as Validator<T>, + ); + }
28-38: Same simplification for maxLength.Mirror the minLength refactor for consistency.
- if (parsedValue != null && - schema.type != PropertySchemaType.integer) { - validators.add(Validators.composeOR([ - Validators.maxLength(parsedValue) as Validator<T>, - Validators.composeOR( - [Validators.equals(''), Validators.equals(null)]), - ]) as Validator<T>); - } + if (schema.type != PropertySchemaType.integer && parsedValue != null) { + validators.add( + Validators.composeOR([ + Validators.maxLength(parsedValue) as Validator<T>, + Validators.equals('') as Validator<T>, + Validators.equals(null) as Validator<T>, + ]) as Validator<T>, + ); + }
40-60: Consider allowing empty string for non-required numeric fields; verify control value semantics.If number inputs can produce '' (empty string) when cleared, min/max will currently fail unless the control coerces to null. If form controls don’t normalize '' to null, add equals('') to the OR for min/max as you did for strings.
- validators.add(Validators.composeOR([ - Validators.min(parsedValue) as Validator<T>, - Validators.equals(null), - ]) as Validator<T>); + validators.add(Validators.composeOR([ + Validators.min(parsedValue) as Validator<T>, + Validators.equals(null), + // Optional: uncomment if empty string is observed for numeric controls + // Validators.equals(''), + ]) as Validator<T>);Same consideration for maxValue.
84-89: Prefer debugPrint and centralized logging.debugPrint truncates long messages gracefully and is idiomatic in Flutter. Consider also surfacing unknown validations via telemetry to catch config drifts.
- if (kDebugMode) { - print('Unknown validation type: ${rule.type}'); - } + if (kDebugMode) { + debugPrint('Unknown validation type: ${rule.type}'); + }
7-11: Dependency coupling via parseIntValue; avoid potential import cycles.parseIntValue coming from form_builder_helper ties helpers together. Extract parsing into a small parsing_helper.dart used by both to prevent any cyclic import risk and improve cohesion.
I can draft the new helper and update imports if you want.
96-146: Helpers are straightforward; consider generic numeric support if schema evolves.If PropertySchema supports non-integer numeric types (e.g., number/decimal), these should parse to num and expose double-aware getters to avoid truncation.
Would you like me to propose num-based variants that remain backward compatible?
7-94: Add unit tests for the empty-string bypass behavior.Tests should assert:
- minLength/maxLength do not error on '' or null when not required.
- min/max do not error on null (and optionally on '' if applicable).
- pattern does not error on ''/null and honors multiple patterns.
I can generate test cases with reactive_forms FormControl setups to cover these.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
packages/digit_forms_engine/pubspec.yamlis excluded by!**/*.yaml
📒 Files selected for processing (2)
packages/digit_forms_engine/CHANGELOG.md(1 hunks)packages/digit_forms_engine/lib/helper/validator_helper.dart(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/digit_forms_engine/CHANGELOG.md
[grammar] ~3-~3: There might be a mistake here.
Context: ## 0.0.4-dev ### BugFix - Removed min and maxLength validation for...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...r empty string ## 0.0.3-dev ### Added - Added mobile number for type string ## 0.0.2...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...Built-in navigation and page order logic - Integration-ready with FormsBloc for l...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...locfor loading and managing form state - Theming support viadigit_ui_components...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...g for scanner, location picker, and more - Form state management using BLoC pattern...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...Form state management using BLoC pattern - Multi-page form support with automatic n...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...e form support with automatic navigation - Conditional validation - Form submission...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...atic navigation - Conditional validation - Form submission with data collection and...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...mmary generation ### Technical Features - JSON-driven form configuration - Automat...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...eatures - JSON-driven form configuration - Automatic form control generation - Fiel...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...tion - Automatic form control generation - Field-level and page-level validation - ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... - Field-level and page-level validation - Custom widget builders for different fie...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...idget builders for different field types - Localization support - Responsive design...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...erent field types - Localization support - Responsive design with digit_ui_componen...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ... design with digit_ui_components theming - Type-safe form data handling - Memory-ef...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...s theming - Type-safe form data handling - Memory-efficient form state management ...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...0.1-dev ### Initial Development Release - Dynamic form rendering engine based on J...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...rm rendering engine based on JSON schema - Page configuration now supports **map-ba...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ..., dropdown, latLng, locality, etc. - Validation support using `reactive_forms...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...Built-in navigation and page order logic - Integration-ready with FormsBloc for l...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...locfor loading and managing form state - Theming support viadigit_ui_components...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...g for scanner, location picker, and more
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
packages/digit_forms_engine/CHANGELOG.md
44-44: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (2)
packages/digit_forms_engine/CHANGELOG.md (1)
1-12: Verify pubspec version bump and changelog alignmentPlease manually confirm the following in the same commit:
- packages/digit_forms_engine/pubspec.yaml
•version: 0.0.4-devis set under thename: digit_forms_enginesection- packages/digit_forms_engine/CHANGELOG.md
• The “## 0.0.4-dev” entry appears as shownThis ensures the new release version matches the changelog entry.
packages/digit_forms_engine/lib/helper/validator_helper.dart (1)
78-81: Required handling looks good.Required is only applied when rule.value == true; aligns with expectations.
| case 'pattern': | ||
| if (rule.value is List && rule.value.isNotEmpty) { | ||
| validators.add(Validators.composeOR([ | ||
| Validators.pattern(RegExp(rule.value.first)) as Validator<T>, | ||
| Validators.composeOR( | ||
| [Validators.equals(''), Validators.equals(null)]), | ||
| ]) as Validator<T>); | ||
| } else if (rule.value is String && rule.value.isNotEmpty) { | ||
| validators.add(Validators.composeOR([ | ||
| Validators.pattern(RegExp(rule.value)) as Validator<T>, | ||
| Validators.composeOR( | ||
| [Validators.equals(''), Validators.equals(null)]), | ||
| ]) as Validator<T>); | ||
| } | ||
| break; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Support multiple patterns and guard against invalid regex.
Currently only the first item in a list is used, and a bad pattern can throw and crash. Build OR across all provided patterns and catch FormatException.
- case 'pattern':
- if (rule.value is List && rule.value.isNotEmpty) {
- validators.add(Validators.composeOR([
- Validators.pattern(RegExp(rule.value.first)) as Validator<T>,
- Validators.composeOR(
- [Validators.equals(''), Validators.equals(null)]),
- ]) as Validator<T>);
- } else if (rule.value is String && rule.value.isNotEmpty) {
- validators.add(Validators.composeOR([
- Validators.pattern(RegExp(rule.value)) as Validator<T>,
- Validators.composeOR(
- [Validators.equals(''), Validators.equals(null)]),
- ]) as Validator<T>);
- }
+ case 'pattern': {
+ List<String> patterns = const [];
+ if (rule.value is List && rule.value.isNotEmpty) {
+ patterns = (rule.value as List).whereType<String>().toList();
+ } else if (rule.value is String && (rule.value as String).isNotEmpty) {
+ patterns = [rule.value as String];
+ }
+ if (patterns.isNotEmpty) {
+ final List<Validator<T>> patternValidators = [];
+ for (final p in patterns) {
+ try {
+ patternValidators.add(
+ Validators.pattern(RegExp(p)) as Validator<T>,
+ );
+ } on FormatException {
+ if (kDebugMode) {
+ // ignore invalid regex in release builds
+ print('Invalid regex pattern skipped: $p');
+ }
+ }
+ }
+ if (patternValidators.isNotEmpty) {
+ validators.add(
+ Validators.composeOR([
+ ...patternValidators,
+ Validators.equals('') as Validator<T>,
+ Validators.equals(null) as Validator<T>,
+ ]) as Validator<T>,
+ );
+ }
+ }
+ break;
+ }📝 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.
| case 'pattern': | |
| if (rule.value is List && rule.value.isNotEmpty) { | |
| validators.add(Validators.composeOR([ | |
| Validators.pattern(RegExp(rule.value.first)) as Validator<T>, | |
| Validators.composeOR( | |
| [Validators.equals(''), Validators.equals(null)]), | |
| ]) as Validator<T>); | |
| } else if (rule.value is String && rule.value.isNotEmpty) { | |
| validators.add(Validators.composeOR([ | |
| Validators.pattern(RegExp(rule.value)) as Validator<T>, | |
| Validators.composeOR( | |
| [Validators.equals(''), Validators.equals(null)]), | |
| ]) as Validator<T>); | |
| } | |
| break; | |
| case 'pattern': { | |
| List<String> patterns = const []; | |
| if (rule.value is List && rule.value.isNotEmpty) { | |
| patterns = (rule.value as List).whereType<String>().toList(); | |
| } else if (rule.value is String && (rule.value as String).isNotEmpty) { | |
| patterns = [rule.value as String]; | |
| } | |
| if (patterns.isNotEmpty) { | |
| final List<Validator<T>> patternValidators = []; | |
| for (final p in patterns) { | |
| try { | |
| patternValidators.add( | |
| Validators.pattern(RegExp(p)) as Validator<T>, | |
| ); | |
| } on FormatException { | |
| if (kDebugMode) { | |
| // ignore invalid regex in release builds | |
| print('Invalid regex pattern skipped: $p'); | |
| } | |
| } | |
| } | |
| if (patternValidators.isNotEmpty) { | |
| validators.add( | |
| Validators.composeOR([ | |
| ...patternValidators, | |
| Validators.equals('') as Validator<T>, | |
| Validators.equals(null) as Validator<T>, | |
| ]) as Validator<T>, | |
| ); | |
| } | |
| } | |
| break; | |
| } |
🤖 Prompt for AI Agents
In packages/digit_forms_engine/lib/helper/validator_helper.dart around lines 62
to 76, the 'pattern' rule currently only uses the first list item and constructs
RegExp without guarding against invalid patterns; update it to iterate over all
provided patterns (if rule.value is List) and build an OR composed validator
that includes Validators.pattern(RegExp(p)) for each pattern p, while wrapping
RegExp construction in a try/catch to handle FormatException (on error fall back
to skip the invalid pattern or add a validator that always succeeds for that
branch), and ensure the final composed validator still allows empty/null values
by OR-ing with Validators.equals('') and Validators.equals(null).
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
apps/health_campaign_field_worker_app/lib/utils/background_service.dart (4)
126-128: Location processing disabled — confirm intent and clean up.You still set tracker tenant/project above but skip processing. Either re-enable or guard the setup behind a feature flag and remove unused init to avoid confusion.
145-149: Return immediately after requesting stop to avoid extra work in the same tick.Apply:
- service.invoke("stopService"); + service.invoke("stopService"); + return; // exit periodic callback early
222-224: Stop-after-sync: also exit this tick (and consider cancelling the timer).Apply:
- service.invoke("stopService"); + service.invoke("stopService"); + return;Optional: keep a handle to the periodic timer and cancel it when stopping to avoid re-entry until the service is torn down.
Example outside this hunk:
final timer = makePeriodicTimer(...); ... service.invoke("stopService"); timer.cancel(); return;
235-237: Null interval path: return after requesting stop.Apply:
- service.invoke("stopService"); + service.invoke("stopService"); + return;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
apps/health_campaign_field_worker_app/pubspec.yamlis excluded by!**/*.yamlpackages/registration_delivery/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/registration_delivery/pubspec.yamlis excluded by!**/*.yamlpackages/sync_service/pubspec.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
apps/health_campaign_field_worker_app/lib/utils/background_service.dart(7 hunks)
🔇 Additional comments (1)
apps/health_campaign_field_worker_app/lib/utils/background_service.dart (1)
70-81: Verify FGS type=dataSync — confirm AndroidManifest, permissions, and plugin supportAutomated check failed (no android/ directory in sandbox). Manually verify:
- Service entry declares android:foregroundServiceType="dataSync" (check android/app/src/main/AndroidManifest.xml and merged manifests).
- Manifest contains and any Android 14 FGS-related declarations/permissions (e.g., notification/runtime permissions).
- pubspec.lock shows flutter_background_service and flutter_local_notifications versions that support AndroidForegroundType.dataSync; upgrade or change FGS type if unsupported.
| // auto start service | ||
| autoStart: false, | ||
| isForegroundMode: true, | ||
| initialNotificationContent: 'BackGround Service Started at ${DateTime.now()}', |
There was a problem hiding this comment.
Fix user-facing text: “BackGround” → “Background”.
Apply:
- initialNotificationContent: 'BackGround Service Started at ${DateTime.now()}',
+ initialNotificationContent: 'Background service started at ${DateTime.now()}',📝 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.
| initialNotificationContent: 'BackGround Service Started at ${DateTime.now()}', | |
| initialNotificationContent: 'Background service started at ${DateTime.now()}', |
🤖 Prompt for AI Agents
In apps/health_campaign_field_worker_app/lib/utils/background_service.dart
around line 76, the user-facing string uses "BackGround" which is
misspelled/capitalized incorrectly; change the literal
initialNotificationContent: 'BackGround Service Started at ${DateTime.now()}' to
use "Background" (initialNotificationContent: 'Background Service Started at
${DateTime.now()}') and verify there are no other occurrences of the same typo
in this file.
| void onStart(ServiceInstance service) async { | ||
| // Only available for flutter 3.0.0 and later | ||
| DartPluginRegistrant.ensureInitialized(); | ||
| // DartPluginRegistrant.ensureInitialized(); |
There was a problem hiding this comment.
Re-enable plugin registration in background isolate (prevents MissingPluginException).
Plugins used below (battery_plus, local_notifications, package_info_plus, isar, dio) require registration in the service isolate.
Apply:
void onStart(ServiceInstance service) async {
- // Only available for flutter 3.0.0 and later
- // DartPluginRegistrant.ensureInitialized();
+ // Ensure plugins are registered in the background isolate.
+ WidgetsFlutterBinding.ensureInitialized();
+ DartPluginRegistrant.ensureInitialized();📝 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.
| // DartPluginRegistrant.ensureInitialized(); | |
| void onStart(ServiceInstance service) async { | |
| // Ensure plugins are registered in the background isolate. | |
| WidgetsFlutterBinding.ensureInitialized(); | |
| DartPluginRegistrant.ensureInitialized(); |
🤖 Prompt for AI Agents
In apps/health_campaign_field_worker_app/lib/utils/background_service.dart
around line 104, the line that calls DartPluginRegistrant.ensureInitialized() is
commented out which causes MissingPluginException for plugins used in the
background isolate; re-enable plugin registration by uncommenting that call and
placing it at the very start of the background isolate entry point (before any
plugin usage), add the appropriate import for the generated dart plugin
registrant (e.g. import the project's dart_plugin_registrant/generated
registrant file or the package that exposes DartPluginRegistrant) so
DartPluginRegistrant.ensureInitialized() resolves, and ensure this import is
included in pubspec/build if necessary; keep the call synchronous and before
initializing battery_plus, local_notifications, package_info_plus, isar, dio or
any other plugins.
| service.invoke('serviceRunning', { | ||
| "enablesManualSync": true, | ||
| }); | ||
| service.stopSelf(); | ||
| service.invoke("stopService"); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Division-by-zero risk after bandwidth error.
On exception you break, leaving speedArray empty, then later divide by speedArray.length. Exit the callback immediately.
Apply:
- service.invoke('serviceRunning', {
- "enablesManualSync": true,
- });
- service.invoke("stopService");
- break;
+ service.invoke('serviceRunning', {"enablesManualSync": true});
+ service.invoke("stopService");
+ return; // prevents avg on empty arrayAdditionally, guard right after the loop:
+ if (speedArray.isEmpty) {
+ return;
+ }
double sum = speedArray.fold(0, (p, c) => p + c);📝 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.
| service.invoke('serviceRunning', { | |
| "enablesManualSync": true, | |
| }); | |
| service.stopSelf(); | |
| service.invoke("stopService"); | |
| break; | |
| } | |
| } | |
| service.invoke('serviceRunning', {"enablesManualSync": true}); | |
| service.invoke("stopService"); | |
| return; // prevents avg on empty array | |
| } | |
| } | |
| if (speedArray.isEmpty) { | |
| return; | |
| } | |
| double sum = speedArray.fold(0, (p, c) => p + c); |
🤖 Prompt for AI Agents
In apps/health_campaign_field_worker_app/lib/utils/background_service.dart
around lines 174-180, the callback currently uses `break` on exception which
leaves `speedArray` empty and later causes a division-by-zero when dividing by
`speedArray.length`; replace the `break` with an immediate return from the
callback so execution stops after invoking stopService, and additionally add a
guard after the loop that checks if `speedArray` is empty (or length == 0) and
returns/avoids the average calculation to prevent division by zero.
Taraba campaign changes
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
apps/health_campaign_field_worker_app/lib/pages/home.dart (6)
369-416: Guard navigation with context.mounted after awaits; consider dedup of schema parsing.After awaiting SharedPreferences and JSON work, add a mounted check before using context. Also, the schema→templates→singleton setup duplicated in distribution can be extracted to a helper.
Apply this diff to add the safety guard:
RegistrationDeliverySingleton() .setHouseholdType(HouseholdType.family); - await context.router.push(const RegistrationDeliveryWrapperRoute()); + if (!context.mounted) return; + await context.router.push(const RegistrationDeliveryWrapperRoute());Note: Local try/catch for JSON is intentionally omitted per centralized validation. Based on learnings
424-466: Repeat the mounted check and consider extracting shared logic.Mirror the mounted check here. The JSON→templates→singleton code is near-identical to the beneficiary handler; extracting a small helper would reduce duplication.
Apply this diff to guard navigation:
RegistrationDeliverySingleton() .setHouseholdType(HouseholdType.family); - await context.router.push(const RegistrationDeliveryWrapperRoute()); + if (!context.mounted) return; + await context.router.push(const RegistrationDeliveryWrapperRoute());
618-618: Remove stray semicolons.These no-op semicolons can trigger analyzer lints.
Apply this diff:
- ; +- ; +Also applies to: 660-660
709-709: Duplicate key in homeItemsShowcaseMap (i18.home.db).The map defines i18.home.db twice; the latter overrides the former. Remove one to avoid confusion.
Apply this diff to remove the earlier duplicate:
- i18.home.db: homeShowcaseData.db.showcaseKey,Also applies to: 717-717
651-664: Duplicate dashboard entry in homeItemsMap.i18.home.dashboard is declared twice in the map; the second overrides the first. Keep a single entry.
Apply this diff to remove the duplicate block:
- i18.home.dashboard: homeShowcaseData.dashBoard.buildWith( - child: HomeItemCard( - icon: Icons.bar_chart_sharp, - label: i18.home.dashboard, - onPressed: () { - if (isTriggerLocalisation) { - triggerLocalization(); - isTriggerLocalisation = false; - } - ; - context.router.push(const UserDashboardRoute()); - }, - ), - ),
769-805: Reduce duplication in triggerLocalization and avoid "null" string modules.Compute the module string once and reuse in both branches; current interpolation can yield a literal "null".
Apply this diff:
void triggerLocalization({String? module, bool? loadOnline}) { context.read<AppInitializationBloc>().state.maybeWhen( orElse: () {}, initialized: ( AppConfiguration appConfiguration, _, __, ) { final appConfig = appConfiguration; final localizationModulesList = appConfiguration.backendInterface; final selectedLocale = AppSharedPreferences().getSelectedLocale; LocalizationParams() .setCode(LeastLevelBoundarySingleton().boundary); - if (loadOnline == true) { + final resolvedModule = module ?? + localizationModulesList?.interfaces + .where((element) => element.type == Modules.localizationModule) + .map((e) => e.name.toString()) + .join(','); + if (loadOnline == true) { context .read<LocalizationBloc>() .add(LocalizationEvent.onRemoteLoadLocalization( - module: module ?? - "${localizationModulesList?.interfaces.where((element) => element.type == Modules.localizationModule).map((e) => e.name.toString()).join(',')}", + module: resolvedModule, tenantId: envConfig.variables.tenantId, locale: selectedLocale!, path: Constants.localizationApiPath, )); } else { context .read<LocalizationBloc>() .add(LocalizationEvent.onLoadLocalization( - module: module ?? - "${localizationModulesList?.interfaces.where((element) => element.type == Modules.localizationModule).map((e) => e.name.toString()).join(',')}", + module: resolvedModule, tenantId: envConfig.variables.tenantId, locale: selectedLocale!, path: Constants.localizationApiPath, )); } }, ); }Also confirm callers set loadOnline=true where remote refresh is intended.
packages/digit_forms_engine/lib/blocs/forms/forms.freezed.dart (1)
141-164: Potential performance issue with DeepCollectionEquality for list comparisons.Using
DeepCollectionEqualityfor list equality checks can be expensive for large lists. Consider usingListEqualityfrompackage:collectionwhich is more optimized for list comparisons.However, since this is generated code from Freezed, this would need to be addressed in the source
.dartfile by configuring Freezed's equality generation or implementing custom equality methods.packages/digit_forms_engine/lib/helper/visibility_manager.dart (2)
52-97: Simplify return statements and add explicit null handling.The
getDefaultValuemethod has redundant null checks and could be simplified. Also, some paths don't handle all edge cases clearly.Consider simplifying the logic:
dynamic getDefaultValue( PropertySchema schema, dynamic value, dynamic rawValue) { final format = schema.format; + + // Early return if value is already set + if (value != null) return value; switch (schema.type) { case PropertySchemaType.integer: - return parseIntValue(value) ?? parseIntValue(rawValue); + return parseIntValue(rawValue); case PropertySchemaType.boolean: - return value ?? parseBoolValue(rawValue); + return parseBoolValue(rawValue); case PropertySchemaType.string: if (format == PropertySchemaFormat.date) { - return parseDateValue(value) ?? - (schema.systemDate == true - ? DateTime.now() - : parseDateValue(rawValue)); + if (schema.systemDate == true) return DateTime.now(); + return parseDateValue(rawValue); } else if (format == PropertySchemaFormat.idPopulator) { final availableIDs = rawValue?['availableIDs']; final selectedLabel = availableIDs?['UNIQUE_BENEFICIARY_ID'] != null ? 'UNIQUE_BENEFICIARY_ID' : 'DEFAULT'; final selectedId = availableIDs?[selectedLabel]?.toString(); return schema.hidden == true ? '$selectedLabel, $selectedId' : rawValue?.toString(); - } else if (format == PropertySchemaFormat.latLng || - format == PropertySchemaFormat.locality) { - return value ?? rawValue?.toString(); - } else if (format == PropertySchemaFormat.numeric) { - return parseIntValue(value) ?? parseIntValue(rawValue); + } else if (format == PropertySchemaFormat.numeric) { + return parseIntValue(rawValue); } else { - return value ?? - (rawValue?.toString().isEmpty ?? true - ? null - : rawValue.toString()); + final strValue = rawValue?.toString(); + return (strValue?.isEmpty ?? true) ? null : strValue; } case PropertySchemaType.dynamic: - return value; + return rawValue; default: - return value ?? - (rawValue?.toString().isEmpty ?? true ? null : rawValue.toString()); + final strValue = rawValue?.toString(); + return (strValue?.isEmpty ?? true) ? null : strValue; } }
99-122: Consider extracting validator building to avoid redundant calls.The
buildValidatorsfunction is called every time visibility is toggled to true. If validators don't change frequently, consider caching them to improve performance.You could cache validators in the VisibilityManager:
class VisibilityManager { final Map<String, PropertySchema> schemaMap; final FormGroup form; final Map<String, dynamic> formData; + final Map<String, List<Map<String, Object?>>> _validatorCache = {}; void toggleControlVisibility( String key, bool isVisible, PropertySchema schema, ) { final control = form.control(key); if (isVisible) { - control.setValidators(buildValidators(schema)); + _validatorCache[key] ??= buildValidators(schema); + control.setValidators(_validatorCache[key]!); } else {packages/digit_forms_engine/lib/models/schema_object/schema_object.freezed.dart (1)
864-865: Remove unnecessary comment on its own line.Line 864 has a standalone comment that should be on the same line as the field declaration.
This appears to be generated code, but if you can modify the source:
@override final String? description; -// optional @override final String primaryActionLabel;packages/digit_forms_engine/lib/models/property_schema/property_schema.freezed.dart (1)
1614-1615: Remove inline comments from generated code.The inline comments on lines 1614 and 1661 appear to be documentation that leaked into the generated code. These should be in the source file as doc comments, not in the generated output.
These comments should be removed from the generated code or moved to proper documentation in the source
.dartfile:@override final String type; -// "template" or "form" @override final String name;Also applies to: 1661-1662
packages/digit_forms_engine/lib/models/property_schema/property_schema.dart (3)
98-107: Consider typing NavigateToConfig.type as an enum.Using String invites typos; an enum like NavigateToType { template, form } would improve safety. Regenerate JSON after the change.
20-22: Decide if tri-state booleans are needed; otherwise default them.If null isn’t meaningful for readOnly/displayOnly/hidden/systemDate/charCount/isMultiSelect/includeInForm/includeInSummary, prefer non-nullable with defaults (e.g., @default(false)). This reduces downstream null checks. Requires minor breaking change.
Also applies to: 26-28, 45-46, 53-54
152-153: Optional: avoid the enum value named “dynamic”.It’s legal but confusing given the Dart type. Rename and preserve wire format with @jsonvalue.
Apply this diff and regenerate:
-enum PropertySchemaType { object, string, integer, boolean, dynamic } +enum PropertySchemaType { + object, + string, + integer, + boolean, + @JsonValue('dynamic') dynamicType, +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
apps/health_campaign_field_worker_app/pubspec.lockis excluded by!**/*.lock,!**/*.lockapps/health_campaign_field_worker_app/pubspec.yamlis excluded by!**/*.yamlpackages/digit_forms_engine/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/digit_forms_engine/pubspec.yamlis excluded by!**/*.yamlpackages/registration_delivery/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/registration_delivery/pubspec.yamlis excluded by!**/*.yaml
📒 Files selected for processing (13)
apps/health_campaign_field_worker_app/lib/pages/home.dart(12 hunks)apps/health_campaign_field_worker_app/lib/utils/i18_key_constants.dart(4 hunks)apps/health_campaign_field_worker_app/lib/utils/utils.dart(5 hunks)apps/health_campaign_field_worker_app/lib/widgets/showcase/config/home_showcase.dart(2 hunks)packages/digit_forms_engine/lib/blocs/forms/forms.freezed.dart(1 hunks)packages/digit_forms_engine/lib/helper/visibility_manager.dart(1 hunks)packages/digit_forms_engine/lib/json_forms.dart(1 hunks)packages/digit_forms_engine/lib/models/property_schema/property_schema.dart(1 hunks)packages/digit_forms_engine/lib/models/property_schema/property_schema.freezed.dart(1 hunks)packages/digit_forms_engine/lib/models/property_schema/property_schema.g.dart(1 hunks)packages/digit_forms_engine/lib/models/schema_object/schema_object.dart(4 hunks)packages/digit_forms_engine/lib/models/schema_object/schema_object.freezed.dart(17 hunks)packages/digit_forms_engine/lib/models/schema_object/schema_object.g.dart(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/health_campaign_field_worker_app/lib/utils/i18_key_constants.dart
- apps/health_campaign_field_worker_app/lib/widgets/showcase/config/home_showcase.dart
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-07-17T11:57:08.025Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#760
File: apps/health_campaign_field_worker_app/lib/pages/home.dart:369-424
Timestamp: 2025-07-17T11:57:08.025Z
Learning: In apps/health_campaign_field_worker_app/lib/pages/home.dart, the JSON schema loading and parsing logic doesn't require local error handling because validation and error handling are implemented at a global level during the JSON fetching process, before the data is stored in SharedPreferences. This follows the architectural pattern of centralized error handling rather than defensive programming at individual access points.
Applied to files:
packages/digit_forms_engine/lib/json_forms.dartapps/health_campaign_field_worker_app/lib/pages/home.dart
📚 Learning: 2025-07-14T09:01:52.682Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#752
File: apps/health_campaign_field_worker_app/lib/pages/boundary_selection.dart:312-312
Timestamp: 2025-07-14T09:01:52.682Z
Learning: In the health campaign field worker app, when displaying boundary information in the UI, use `result.locality` instead of `result.boundaryName` because `locality` contains localization keys that get translated via `localizations.translate()` to display human-readable, localized boundary names, while `boundaryName` contains raw/untranslated boundary names.
Applied to files:
apps/health_campaign_field_worker_app/lib/utils/utils.dart
📚 Learning: 2025-07-14T09:34:07.154Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#752
File: apps/health_campaign_field_worker_app/lib/pages/qr_details_page.dart:127-132
Timestamp: 2025-07-14T09:34:07.154Z
Learning: In apps/health_campaign_field_worker_app/lib/pages/qr_details_page.dart, the BlocProvider pattern using `create: (_) => attendanceBloc!` where attendanceBloc is initialized in initState is correct implementation. This returns the existing bloc instance rather than creating a new one, avoiding state loss and unnecessary recreations on rebuild.
Applied to files:
apps/health_campaign_field_worker_app/lib/utils/utils.dart
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#433
File: packages/digit_data_model/lib/models/pgr_complaints/pgr_complaints.mapper.dart:305-305
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The file `packages/digit_data_model/lib/models/pgr_complaints/pgr_complaints.mapper.dart` is a generated file, and direct modifications are not sustainable. Issues should be addressed in the generator's configuration or the template files.
Applied to files:
apps/health_campaign_field_worker_app/lib/utils/utils.dartapps/health_campaign_field_worker_app/lib/pages/home.dart
📚 Learning: 2024-10-17T11:06:00.380Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#582
File: packages/attendance_management/lib/utils/utils.dart:1-1
Timestamp: 2024-10-17T11:06:00.380Z
Learning: `BoundaryModel` is defined in `data_model.dart` and can be safely imported from there.
Applied to files:
apps/health_campaign_field_worker_app/lib/utils/utils.dart
📚 Learning: 2025-07-14T09:26:11.654Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#752
File: apps/health_campaign_field_worker_app/lib/pages/qr_details_page.dart:215-232
Timestamp: 2025-07-14T09:26:11.654Z
Learning: In apps/health_campaign_field_worker_app/lib/pages/qr_details_page.dart, the attendance QR generation code within the `registerLoaded` state handler doesn't require additional null/empty checks for registers or individualList because validation occurs at a higher level in the attendance system before reaching this UI code.
Applied to files:
apps/health_campaign_field_worker_app/lib/pages/home.dart
📚 Learning: 2024-10-17T11:06:47.513Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#582
File: apps/health_campaign_field_worker_app/lib/pages/home.dart:477-477
Timestamp: 2024-10-17T11:06:47.513Z
Learning: Access to the `DriftDbViewer` in `apps/health_campaign_field_worker_app/lib/pages/home.dart` is controlled by role action mapping, ensuring it does not appear in production.
Applied to files:
apps/health_campaign_field_worker_app/lib/pages/home.dart
📚 Learning: 2025-07-17T11:48:37.682Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#760
File: apps/health_campaign_field_worker_app/lib/pages/peer_to_peer/data_transfer.dart:0-0
Timestamp: 2025-07-17T11:48:37.682Z
Learning: In apps/health_campaign_field_worker_app/lib/pages/peer_to_peer/data_transfer.dart, the DataTransferPage can safely access context.selectedProject.id without null checks because the navigation flow prevents users from reaching this page if the project ID is null. This follows the architectural pattern of boundary validation rather than defensive programming within internal components.
Applied to files:
apps/health_campaign_field_worker_app/lib/pages/home.dart
📚 Learning: 2025-07-14T08:48:53.732Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#752
File: apps/health_campaign_field_worker_app/lib/pages/profile.dart:208-210
Timestamp: 2025-07-14T08:48:53.732Z
Learning: In apps/health_campaign_field_worker_app/lib/pages/profile.dart, the gender field is mandatory and should always have a value selected, so handling empty string cases for gender conversion to null is not necessary.
Applied to files:
apps/health_campaign_field_worker_app/lib/pages/home.dart
📚 Learning: 2025-07-14T10:36:05.469Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#752
File: packages/attendance_management/lib/pages/manage_attendance.dart:303-305
Timestamp: 2025-07-14T10:36:05.469Z
Learning: In packages/attendance_management/lib/pages/manage_attendance.dart, the RegisterCard widgets are created from the attendanceRegisters list, and each card receives a registerId from the same register used to create it. The UI logic ensures that when the button is pressed, the firstWhere search will always find a matching register because the data comes from the same source, making additional error handling unnecessary.
Applied to files:
apps/health_campaign_field_worker_app/lib/pages/home.dart
🔇 Additional comments (14)
apps/health_campaign_field_worker_app/lib/pages/home.dart (4)
2-2: LGTM: Required imports added.json decoding, template model, and SharedPreferences imports are appropriate.
Also applies to: 13-13, 34-34
559-561: Confirm manual sync enablement in UAT.You’ve added UAT to always-allow manual sync (like dev/qa). Verify this aligns with product expectations for that environment.
718-720: LGTM: Showcase entry for distribution added.The distribution showcase key mapping looks correct.
740-741: LGTM: Distribution label added to the home items list.Ordering and inclusion look fine.
packages/digit_forms_engine/lib/blocs/forms/forms.freezed.dart (1)
1-16: LGTM! Standard Freezed generated code.This is correctly generated boilerplate code by the Freezed package. The file header appropriately disables linting and coverage for generated code.
packages/digit_forms_engine/lib/models/schema_object/schema_object.freezed.dart (1)
341-932: LGTM! Well-structured Freezed model additions.The additions of
SummaryItemandShowAlertPopUpmodels are properly implemented with all necessary Freezed boilerplate. The JSON serialization hooks and copyWith methods are correctly generated.packages/digit_forms_engine/lib/models/property_schema/property_schema.freezed.dart (1)
1-16: LGTM! Properly configured Freezed generated file.The file is correctly set up as generated code with appropriate lint suppressions and coverage ignore directives.
packages/digit_forms_engine/lib/helper/visibility_manager.dart (1)
28-35: Add null safety checks for nested property access.The expression extraction at line 28-29 accesses nested properties without proper null checks. This could throw if
conditionsexists but'visibilityCondition'is missing.Apply this fix:
- final expression = - schema.conditions?['visibilityCondition']?['expression']; + final visibilityCondition = schema.conditions?['visibilityCondition']; + if (visibilityCondition == null || visibilityCondition is! Map) continue; + final expression = visibilityCondition['expression']; if (expression is! String) continue;Likely an incorrect or invalid review comment.
packages/digit_forms_engine/lib/models/property_schema/property_schema.g.dart (3)
9-61: Deserialization looks solid (nested maps, lists, enums).Covers all fields with correct null-safety and helper converters. No issues spotted.
63-111: toJson with writeNotNull is correct and efficient.Properly omits nulls and delegates to nested toJsons.
30-33: Confirm intent: numeric coercion to int floors values.minValue/maxValue/minLength/maxLength use (num?).toInt(), which truncates decimals. Verify the API never sends fractional values here.
packages/digit_forms_engine/lib/models/property_schema/property_schema.dart (3)
10-14: Correct JsonSerializable config.explicitToJson + includeIfNull=false is appropriate for these nested models.
33-35: Verify startDate/endDate are always strings.The fromJson casts only String; numbers/DateTimes will be coerced to null. Confirm upstream data is string-formatted dates.
119-127: VisibilityCondition converter is safe and defensive.Handles null/empty maps gracefully. LGTM.
| .read<LocalizationBloc>() | ||
| .add(LocalizationEvent.onRemoteLoadLocalization( | ||
| module: fullModuleString, | ||
| tenantId: envConfig.variables.tenantId, | ||
| locale: AppSharedPreferences().getSelectedLocale!, | ||
| path: Constants.localizationApiPath, | ||
| )); |
There was a problem hiding this comment.
Use the passed locale instead of force-unwrapping shared preferences.
We made locale a required argument, but still ignore it and force-unwrap AppSharedPreferences().getSelectedLocale!. This breaks two ways: (a) we can crash when the preference is unset (first-launch/clear-data scenarios), and (b) we may load the wrong bundle if the caller asks for a different locale than the stored one. Wire the event up with the provided locale and only fall back to the stored value if you explicitly need a default.
context
.read<LocalizationBloc>()
.add(LocalizationEvent.onRemoteLoadLocalization(
module: fullModuleString,
tenantId: envConfig.variables.tenantId,
- locale: AppSharedPreferences().getSelectedLocale!,
+ locale: locale,
path: Constants.localizationApiPath,
));📝 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.
| .read<LocalizationBloc>() | |
| .add(LocalizationEvent.onRemoteLoadLocalization( | |
| module: fullModuleString, | |
| tenantId: envConfig.variables.tenantId, | |
| locale: AppSharedPreferences().getSelectedLocale!, | |
| path: Constants.localizationApiPath, | |
| )); | |
| context | |
| .read<LocalizationBloc>() | |
| .add(LocalizationEvent.onRemoteLoadLocalization( | |
| module: fullModuleString, | |
| tenantId: envConfig.variables.tenantId, | |
| locale: locale, | |
| path: Constants.localizationApiPath, | |
| )); |
🤖 Prompt for AI Agents
In apps/health_campaign_field_worker_app/lib/utils/utils.dart around lines 709
to 715, the code currently force-unwraps
AppSharedPreferences().getSelectedLocale! and ignores the required locale
argument; replace the force-unwrapped call with the passed-in locale parameter
(e.g., locale) when constructing LocalizationEvent.onRemoteLoadLocalization, and
only fall back to AppSharedPreferences().getSelectedLocale (without
force-unwrapping) or a sensible default if the passed locale is null/empty;
ensure no force-unwraps remain and that the event uses the caller-provided
locale by preference.
| static MapEntry<String, dynamic>? _getParsedValues( | ||
| FormGroup form, | ||
| String name, | ||
| PropertySchema schema, | ||
| ) { | ||
| if (schema.type == PropertySchemaType.object) { | ||
| final results = schema.properties!.entries.map((e) { | ||
| return _getParsedValues(form, e.key, e.value); | ||
| }); | ||
| return MapEntry( | ||
| name, | ||
| Map.fromEntries(results.whereType<MapEntry<String, dynamic>>()), | ||
| ); | ||
| } else { | ||
| final value = form.control(name).value; | ||
| if (value == null) return null; | ||
| return MapEntry(name, value); | ||
| } |
There was a problem hiding this comment.
Fix nested object value parsing
When a property is an object, we must recurse into its FormGroup. Right now, the recursive call keeps using the root FormGroup, so form.control(childName) fails for nested controls like address.city. Grab the child group with form.control(name) as FormGroup and pass that into the recursion before reading values.
Apply this diff:
static MapEntry<String, dynamic>? _getParsedValues(
FormGroup form,
String name,
PropertySchema schema,
) {
if (schema.type == PropertySchemaType.object) {
- final results = schema.properties!.entries.map((e) {
- return _getParsedValues(form, e.key, e.value);
+ final nestedGroup = form.control(name) as FormGroup;
+ final results = schema.properties!.entries.map((e) {
+ return _getParsedValues(nestedGroup, e.key, e.value);
});
return MapEntry(
name,
Map.fromEntries(results.whereType<MapEntry<String, dynamic>>()),
);📝 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.
| static MapEntry<String, dynamic>? _getParsedValues( | |
| FormGroup form, | |
| String name, | |
| PropertySchema schema, | |
| ) { | |
| if (schema.type == PropertySchemaType.object) { | |
| final results = schema.properties!.entries.map((e) { | |
| return _getParsedValues(form, e.key, e.value); | |
| }); | |
| return MapEntry( | |
| name, | |
| Map.fromEntries(results.whereType<MapEntry<String, dynamic>>()), | |
| ); | |
| } else { | |
| final value = form.control(name).value; | |
| if (value == null) return null; | |
| return MapEntry(name, value); | |
| } | |
| static MapEntry<String, dynamic>? _getParsedValues( | |
| FormGroup form, | |
| String name, | |
| PropertySchema schema, | |
| ) { | |
| if (schema.type == PropertySchemaType.object) { | |
| final nestedGroup = form.control(name) as FormGroup; | |
| final results = schema.properties!.entries.map((e) { | |
| return _getParsedValues(nestedGroup, e.key, e.value); | |
| }); | |
| return MapEntry( | |
| name, | |
| Map.fromEntries(results.whereType<MapEntry<String, dynamic>>()), | |
| ); | |
| } else { | |
| final value = form.control(name).value; | |
| if (value == null) return null; | |
| return MapEntry(name, value); | |
| } |
🤖 Prompt for AI Agents
In packages/digit_forms_engine/lib/json_forms.dart around lines 63 to 80, the
recursion for object properties keeps passing the root FormGroup so nested
controls like "address.city" are looked up on the wrong group; modify the object
branch to obtain the child FormGroup for the current property (e.g., final
childForm = form.control(name) as FormGroup) and pass that childForm into the
recursive _getParsedValues calls for each property, then build the MapEntry from
the returned child entries; ensure you handle nulls/absent controls the same way
as the leaf branch.
| @freezed | ||
| class DisplayBehavior with _$DisplayBehavior { | ||
| const factory DisplayBehavior({ | ||
| required FormulaBehavior behavior, | ||
| List<String>? oneOf, | ||
| List<String>? allOf, | ||
| }) = _DisplayBehavior; | ||
|
|
||
| factory DisplayBehavior.fromJson(Map<String, dynamic> json) => | ||
| _$DisplayBehaviorFromJson(json); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add unknownEnumValue for DisplayBehavior.behavior to prevent runtime throws.
Backend changes to behavior would currently crash deserialization.
Apply this diff:
class DisplayBehavior with _$DisplayBehavior {
const factory DisplayBehavior({
- required FormulaBehavior behavior,
+ @JsonKey(unknownEnumValue: FormulaBehavior.show)
+ required FormulaBehavior behavior,
List<String>? oneOf,
List<String>? allOf,
}) = _DisplayBehavior;📝 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.
| @freezed | |
| class DisplayBehavior with _$DisplayBehavior { | |
| const factory DisplayBehavior({ | |
| required FormulaBehavior behavior, | |
| List<String>? oneOf, | |
| List<String>? allOf, | |
| }) = _DisplayBehavior; | |
| factory DisplayBehavior.fromJson(Map<String, dynamic> json) => | |
| _$DisplayBehaviorFromJson(json); | |
| } | |
| @freezed | |
| class DisplayBehavior with _$DisplayBehavior { | |
| const factory DisplayBehavior({ | |
| @JsonKey(unknownEnumValue: FormulaBehavior.show) | |
| required FormulaBehavior behavior, | |
| List<String>? oneOf, | |
| List<String>? allOf, | |
| }) = _DisplayBehavior; | |
| factory DisplayBehavior.fromJson(Map<String, dynamic> json) => | |
| _$DisplayBehaviorFromJson(json); | |
| } |
🤖 Prompt for AI Agents
In packages/digit_forms_engine/lib/models/property_schema/property_schema.dart
around lines 86 to 96, add a JsonKey annotation to the DisplayBehavior.behavior
field to provide an unknownEnumValue so deserialization won't throw if the
backend sends an unexpected enum value; annotate the behavior parameter with
@JsonKey(unknownEnumValue: FormulaBehavior.unknown) (or the correct sentinel
enum member defined on FormulaBehavior), import json_annotation if not already
imported, then run code generation (build_runner) to regenerate the freezed/JSON
code.
| behavior: $enumDecode(_$FormulaBehaviorEnumMap, json['behavior']), | ||
| oneOf: | ||
| (json['oneOf'] as List<dynamic>?)?.map((e) => e as String).toList(), | ||
| allOf: | ||
| (json['allOf'] as List<dynamic>?)?.map((e) => e as String).toList(), | ||
| ); |
There was a problem hiding this comment.
Guard against unknown enum values for DisplayBehavior.behavior.
$enumDecode without unknownEnumValue will throw if backend introduces a new behavior. Add unknownEnumValue on the source field and regenerate.
Apply in property_schema.dart:
class DisplayBehavior with _$DisplayBehavior {
const factory DisplayBehavior({
- required FormulaBehavior behavior,
+ @JsonKey(unknownEnumValue: FormulaBehavior.show)
+ required FormulaBehavior behavior,
List<String>? oneOf,
List<String>? allOf,
}) = _DisplayBehavior;📝 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.
| behavior: $enumDecode(_$FormulaBehaviorEnumMap, json['behavior']), | |
| oneOf: | |
| (json['oneOf'] as List<dynamic>?)?.map((e) => e as String).toList(), | |
| allOf: | |
| (json['allOf'] as List<dynamic>?)?.map((e) => e as String).toList(), | |
| ); | |
| class DisplayBehavior with _$DisplayBehavior { | |
| const factory DisplayBehavior({ | |
| @JsonKey(unknownEnumValue: FormulaBehavior.show) | |
| required FormulaBehavior behavior, | |
| List<String>? oneOf, | |
| List<String>? allOf, | |
| }) = _DisplayBehavior; | |
| } |
🤖 Prompt for AI Agents
packages/digit_forms_engine/lib/models/property_schema/property_schema.g.dart
lines 167-172: the generated code uses $enumDecode which will throw if the
backend returns a new/unknown FormulaBehavior value; fix by updating the source
model in property_schema.dart to annotate the behavior field with a JsonKey
specifying unknownEnumValue (pointing to an appropriate fallback enum member on
the DisplayBehavior/FormulaBehavior enum), then re-run code generation
(build_runner) to regenerate property_schema.g.dart so the decoder uses the
unknownEnumValue fallback.
| ShowAlertPopUp? _showAlertOrNull(dynamic value) { | ||
| if (value is Map && value.isNotEmpty) { | ||
| return ShowAlertPopUp.fromJson(Map<String, dynamic>.from(value)); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Missing empty-map guard for alert config deserialization
_showAlertOrNull currently instantiates ShowAlertPopUp whenever the JSON value is a map, even if the map is empty. That yields a runtime CheckedFromJsonException because ShowAlertPopUp requires four non-null fields. Add an isEmpty check similar to _visibilityConditionOrNull so we return null for {} payloads instead of throwing.
Apply this diff:
ShowAlertPopUp? _showAlertOrNull(dynamic value) {
- if (value is Map && value.isNotEmpty) {
- return ShowAlertPopUp.fromJson(Map<String, dynamic>.from(value));
- }
+ if (value is Map) {
+ if (value.isEmpty) return null;
+ return ShowAlertPopUp.fromJson(Map<String, dynamic>.from(value));
+ }
return null;
}📝 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.
| ShowAlertPopUp? _showAlertOrNull(dynamic value) { | |
| if (value is Map && value.isNotEmpty) { | |
| return ShowAlertPopUp.fromJson(Map<String, dynamic>.from(value)); | |
| } | |
| return null; | |
| } | |
| ShowAlertPopUp? _showAlertOrNull(dynamic value) { | |
| if (value is Map) { | |
| if (value.isEmpty) return null; | |
| return ShowAlertPopUp.fromJson(Map<String, dynamic>.from(value)); | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In packages/digit_forms_engine/lib/models/schema_object/schema_object.dart
around lines 89 to 94, _showAlertOrNull currently constructs ShowAlertPopUp
whenever the value is a Map even if it's empty, causing a runtime
CheckedFromJsonException; update the guard to check both value is Map and
value.isNotEmpty (mirror the _visibilityConditionOrNull pattern) and return null
for empty maps so ShowAlertPopUp.fromJson is only called with non-empty maps.
| showAlertPopUp: _showAlertOrNull(json['showAlertPopUp']), | ||
| actionSchema: (json['actionSchema'] as List<dynamic>?) |
There was a problem hiding this comment.
Mirror the empty-map guard in generated JSON
_showAlertOrNull currently instantiates ShowAlertPopUp even when the JSON object is empty, which then explodes because all fields are required. Modify _showAlertOrNull to return null for empty maps, matching the diff shared on schema_object.dart, so the generator stops producing runtime exceptions for {} payloads.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/digit_forms_engine/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/digit_forms_engine/pubspec.yamlis excluded by!**/*.yamlpackages/digit_formula_parser/pubspec.yamlis excluded by!**/*.yamlpackages/registration_delivery/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/registration_delivery/pubspec.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
packages/digit_forms_engine/CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
packages/digit_forms_engine/CHANGELOG.md
49-49: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
| - Field types supported: | ||
| - `text`, `dropdown`, `latLng`, `locality`, etc. | ||
| - Validation support using `reactive_forms` |
There was a problem hiding this comment.
Fix nested list indentation to satisfy markdownlint
The nested bullet under “Field types supported” is indented with four spaces; markdownlint (MD007/ul-indent) requires two. Please reduce the indentation so CI stops flagging this file.
-- Field types supported:
- - `text`, `dropdown`, `latLng`, `locality`, etc.
+- Field types supported:
+ - `text`, `dropdown`, `latLng`, `locality`, etc.Based on static analysis hints
📝 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.
| - Field types supported: | |
| - `text`, `dropdown`, `latLng`, `locality`, etc. | |
| - Validation support using `reactive_forms` | |
| - Field types supported: | |
| - `text`, `dropdown`, `latLng`, `locality`, etc. | |
| - Validation support using `reactive_forms` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
49-49: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In packages/digit_forms_engine/CHANGELOG.md around lines 48 to 50, the nested
list under "Field types supported" is indented with four spaces which violates
markdownlint MD007/ul-indent; reduce the indentation of the nested bullet(s)
from four spaces to two spaces (so the hyphen for `text`, `dropdown`, etc. is
preceded by two spaces instead of four) to correct the nested list indentation
and satisfy CI.
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/complaints/example/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/complaints/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/complaints/pubspec.yamlis excluded by!**/*.yamlpackages/registration_delivery/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/registration_delivery/pubspec.yamlis excluded by!**/*.yaml
📒 Files selected for processing (26)
packages/complaints/lib/blocs/complaint_wrapper/complaint_wrapper_bloc.dart(1 hunks)packages/complaints/lib/blocs/complaints_inbox/complaints_inbox.dart(0 hunks)packages/complaints/lib/blocs/complaints_inbox/complaints_inbox.freezed.dart(0 hunks)packages/complaints/lib/blocs/complaints_registration/complaints_registration.dart(0 hunks)packages/complaints/lib/blocs/complaints_registration/complaints_registration.freezed.dart(0 hunks)packages/complaints/lib/complaints.dart(1 hunks)packages/complaints/lib/data/digit_crud_service.dart(1 hunks)packages/complaints/lib/data/digit_data_converter.dart(1 hunks)packages/complaints/lib/data/transformer_config.dart(1 hunks)packages/complaints/lib/pages/complaints_acknowledgement.dart(2 hunks)packages/complaints/lib/pages/inbox/complaints_details_view.dart(6 hunks)packages/complaints/lib/pages/inbox/complaints_inbox.dart(3 hunks)packages/complaints/lib/pages/inbox/complaints_inbox_dialog.dart(1 hunks)packages/complaints/lib/pages/inbox/complaints_inbox_filter.dart(0 hunks)packages/complaints/lib/pages/inbox/complaints_inbox_search.dart(0 hunks)packages/complaints/lib/pages/inbox/complaints_inbox_sort.dart(0 hunks)packages/complaints/lib/pages/inbox/complaints_inbox_wrapper.dart(2 hunks)packages/complaints/lib/pages/registration/complaint_type.dart(0 hunks)packages/complaints/lib/pages/registration/complaints_details.dart(0 hunks)packages/complaints/lib/pages/registration/complaints_location.dart(0 hunks)packages/complaints/lib/pages/registration/complaints_registration_wrapper.dart(0 hunks)packages/complaints/lib/router/complaints_router.dart(1 hunks)packages/complaints/lib/router/complaints_router.gm.dart(10 hunks)packages/complaints/lib/utils/constants.dart(1 hunks)packages/complaints/lib/utils/extensions/extensions.dart(1 hunks)packages/complaints/lib/utils/utils.dart(5 hunks)
💤 Files with no reviewable changes (11)
- packages/complaints/lib/pages/registration/complaints_details.dart
- packages/complaints/lib/pages/inbox/complaints_inbox_search.dart
- packages/complaints/lib/pages/inbox/complaints_inbox_sort.dart
- packages/complaints/lib/pages/registration/complaint_type.dart
- packages/complaints/lib/blocs/complaints_registration/complaints_registration.freezed.dart
- packages/complaints/lib/blocs/complaints_inbox/complaints_inbox.dart
- packages/complaints/lib/blocs/complaints_registration/complaints_registration.dart
- packages/complaints/lib/pages/inbox/complaints_inbox_filter.dart
- packages/complaints/lib/pages/registration/complaints_location.dart
- packages/complaints/lib/blocs/complaints_inbox/complaints_inbox.freezed.dart
- packages/complaints/lib/pages/registration/complaints_registration_wrapper.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: naveenr-egov
PR: egovernments/health-campaign-field-worker-app#433
File: packages/digit_data_model/lib/models/pgr_complaints/pgr_complaints.mapper.dart:305-305
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The file `packages/digit_data_model/lib/models/pgr_complaints/pgr_complaints.mapper.dart` is a generated file, and direct modifications are not sustainable. Issues should be addressed in the generator's configuration or the template files.
Applied to files:
packages/complaints/lib/data/digit_data_converter.dartpackages/complaints/lib/pages/inbox/complaints_details_view.dartpackages/complaints/lib/data/transformer_config.dartpackages/complaints/lib/pages/complaints_acknowledgement.dartpackages/complaints/lib/utils/utils.dartpackages/complaints/lib/pages/inbox/complaints_inbox.dartpackages/complaints/lib/complaints.dartpackages/complaints/lib/data/digit_crud_service.dart
🔇 Additional comments (6)
packages/complaints/lib/utils/utils.dart (3)
1-4: Inconsistency between AI summary and code.The AI summary claims this file adds imports for
localization.dart,environment_config.dart, andsurvey_form/models/entities/service.dart, and adds utility functionstransformJsonandtriggerLocalizationIfUpdated. However, these imports and functions are not present in the provided code. This suggests either:
- The summary is incorrect or out of sync with the actual changes
- The code annotations are incomplete
Please verify the actual changes in this file.
34-36: LGTM: Template configuration storage added.The addition of
templateConfigsandcomplaintConfigproperties to the singleton follows the established pattern correctly with proper nullable types, setters, and getters.Also applies to: 66-72, 84-84, 86-86
118-120: routerMap definition and usage verified
ComplaintsAcknowledgementRouteis defined incomplaints_router.gm.dartandrouterMapis consumed inpackages/complaints/lib/pages/inbox/complaints_inbox.dart, so no further changes needed.packages/complaints/lib/utils/constants.dart (1)
15-15: Double-check filter dialog keyLine [15] returns
'filter'whereas the surrounding keys keep the'...Complaints'suffix. If the localization bundles expect'filterComplaints'(to mirror'searchComplaints'/'sortComplaints'), the filter dialog will fall back to a missing translation. Please confirm the intended key or update it for consistency.packages/complaints/lib/utils/extensions/extensions.dart (1)
18-22: Confirm default date format changeSwitching the default format from
'dd-MM-yyyy'to'dd MMM yyyy'will alter every invocation that relied on the old default. Please verify all call sites can absorb the new display style (and locale-aware month text) or keep the previous default to avoid unintended UI regressions.packages/complaints/lib/pages/inbox/complaints_details_view.dart (1)
94-99: Locale-aware timestamp formatting looks great.Deriving the locale from context before calling
getFormattedDatekeeps these audit timestamps aligned with the user’s language settings. Nice touch.
| subscription = globalCrudBloc.stream.listen((globalState) { | ||
| if (globalState is CrudStateLoaded) { | ||
| subscription.cancel(); | ||
| if (!completer.isCompleted) completer.complete(globalState); | ||
| } else if (globalState is CrudStateError) { | ||
| subscription.cancel(); | ||
| if (!completer.isCompleted) | ||
| completer.completeError(globalState.message ?? 'Unknown error'); | ||
| } | ||
| }); | ||
|
|
||
| // Trigger global search via CrudBloc | ||
| globalCrudBloc.add(CrudEventSearch(searchParams)); | ||
|
|
||
| try { | ||
| final globalState = await completer.future; | ||
|
|
||
| // Extract PgrServiceModel list from results (key used by your global CRUD store) | ||
| final allComplaints = (globalState.results['pgrService'] ?? []) | ||
| .whereType<PgrServiceModel>() | ||
| .toList(); | ||
|
|
||
| // Map to wrappers | ||
| final wrappers = allComplaints.map((c) { | ||
| return ComplaintWrapper( | ||
| complaint: c, | ||
| complainant: c.user, | ||
| address: c.address, | ||
| ); | ||
| }).toList(); | ||
|
|
||
| emit(state.copyWith( | ||
| loading: false, | ||
| complaints: wrappers, | ||
| filteredComplaints: [], | ||
| isFiltered: false, | ||
| totalCount: globalState.totalCount ?? wrappers.length, | ||
| searchKeys: null, | ||
| filters: null, | ||
| sortOrder: null, | ||
| )); | ||
| } catch (e) { | ||
| emit(state.copyWith(loading: false, error: e.toString(), complaints: [])); | ||
| } |
There was a problem hiding this comment.
Correlate CrudBloc responses to the dispatched search.
Line 63 wires a plain listener to globalCrudBloc.stream and completes the completer on the first CrudStateLoaded / CrudStateError. Because the bloc is shared, any other feature that fires a CRUD/search in parallel can satisfy this listener first, so we end up rendering unrelated data (or propagating someone else’s error) as the outcome of this search. Please scope the listener to the request you just dispatched—e.g., include a request identifier in the event and filter the stream on it, or use a request-scoped callback from the CRUD layer—before completing the future. Otherwise results become nondeterministic once another module touches the global bloc.
| FutureOr<void> _handleCreate( | ||
| Create event, ComplaintWrapperEmitter emit) async { | ||
| emit(state.copyWith(loading: true)); | ||
| try { | ||
| globalCrudBloc.add(CrudEventCreate(entities: event.entities)); | ||
| emit(state.copyWith( | ||
| loading: false, lastAction: ComplaintWrapperActionType.created)); | ||
| } catch (e) { | ||
| emit(state.copyWith(loading: false, error: e.toString())); | ||
| } | ||
| } | ||
|
|
||
| FutureOr<void> _handleUpdate( | ||
| Update event, ComplaintWrapperEmitter emit) async { | ||
| emit(state.copyWith(loading: true)); | ||
|
|
||
| try { | ||
| final updatedEntities = event.entities.map((entity) { | ||
| final clientAudit = entity.clientAuditDetails; | ||
| final updatedClientAudit = clientAudit?.copyWith( | ||
| lastModifiedBy: ComplaintsSingleton().loggedInUserUuid, | ||
| lastModifiedTime: DateTime.now().millisecondsSinceEpoch, | ||
| ); | ||
| return entity.copyWith(clientAuditDetails: updatedClientAudit); | ||
| }).toList(); | ||
|
|
||
| globalCrudBloc.add(CrudEventUpdate(entities: updatedEntities)); | ||
| emit(state.copyWith( | ||
| loading: false, lastAction: ComplaintWrapperActionType.updated)); | ||
| } catch (e) { | ||
| emit(state.copyWith(loading: false, error: e.toString())); | ||
| } | ||
| } | ||
|
|
||
| FutureOr<void> _handleDelete( | ||
| Delete event, ComplaintWrapperEmitter emit) async { | ||
| emit(state.copyWith(loading: true)); | ||
| try { | ||
| globalCrudBloc.add(CrudEventDelete(entities: event.entities)); | ||
| emit(state.copyWith( | ||
| loading: false, lastAction: ComplaintWrapperActionType.deleted)); | ||
| } catch (e) { | ||
| emit(state.copyWith(loading: false, error: e.toString())); | ||
| } | ||
| } | ||
|
|
||
| FutureOr<void> _handleCreateAndUpdate( | ||
| CreateAndUpdate event, ComplaintWrapperEmitter emit) async { | ||
| emit(state.copyWith(loading: true)); | ||
| try { | ||
| if (event.entitiesToCreate.isNotEmpty) { | ||
| globalCrudBloc.add(CrudEventCreate(entities: event.entitiesToCreate)); | ||
| } | ||
| if (event.entitiesToUpdate.isNotEmpty) { | ||
| final updatedEntities = event.entitiesToUpdate.map((entity) { | ||
| final clientAudit = entity.clientAuditDetails; | ||
| final updatedClientAudit = clientAudit?.copyWith( | ||
| lastModifiedBy: ComplaintsSingleton().loggedInUserUuid, | ||
| lastModifiedTime: DateTime.now().millisecondsSinceEpoch, | ||
| ); | ||
| return entity.copyWith(clientAuditDetails: updatedClientAudit); | ||
| }).toList(); | ||
| globalCrudBloc.add(CrudEventUpdate(entities: updatedEntities)); | ||
| } | ||
|
|
||
| emit(state.copyWith( | ||
| loading: false, | ||
| lastAction: ComplaintWrapperActionType.createAndUpdate)); | ||
| } catch (e) { | ||
| emit(state.copyWith(loading: false, error: e.toString())); | ||
| } |
There was a problem hiding this comment.
Wait for the CRUD result before flipping loading/lastAction.
Lines 109‑179 set loading: false and mark a successful lastAction immediately after calling globalCrudBloc.add(...). add is synchronous, so the try/catch never observes transport failures, and any asynchronous error emitted by the CRUD bloc will arrive later—after we already told the UI that the operation succeeded. That leaves users stuck with stale complaints lists and no way to surface the actual failure. Please mirror the request/response pattern from _handleLoadFromGlobal: watch the global bloc until the matching success/error state arrives, then emit success or propagate the error accordingly. Without waiting for completion, create/update/delete flows report success even when the underlying mutation fails.
| return context | ||
| .repository<PgrServiceModel, PgrServiceSearchModel>(context); | ||
| } else { | ||
| //// FIXME: please add all the required model type, as this is a fallback | ||
| return context | ||
| .repository<PgrServiceModel, PgrServiceSearchModel>(context); | ||
| } |
There was a problem hiding this comment.
Fix fallback repository selection before release
Every non-PgrServiceModel entity still routes through the PGR repository. That breaks CRUD for any other model and will surface as type and runtime mismatches the first time a different entity hits this service. Replace the fallback with a repository that actually matches the incoming entity (or fail fast) before shipping. For example:
- if (entity is PgrServiceModel) {
- return context
- .repository<PgrServiceModel, PgrServiceSearchModel>(context);
- } else {
- //// FIXME: please add all the required model type, as this is a fallback
- return context
- .repository<PgrServiceModel, PgrServiceSearchModel>(context);
- }
+ if (entity is PgrServiceModel) {
+ return context
+ .repository<PgrServiceModel, PgrServiceSearchModel>(context);
+ }
+
+ return searchEntityRepository;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/complaints/lib/data/digit_crud_service.dart around lines 19 to 25,
the fallback branch always returns the PgrService repository which causes
type/runtime mismatches for non-PgrServiceModel entities; instead update the
fallback to either resolve the correct repository for the incoming entity (use
the incoming model/entity type to call context.repository<CorrectModel,
CorrectSearchModel>(context)) or fail fast by throwing a clear
ArgumentError/StateError indicating an unsupported model type; ensure every
branch returns a repository whose generic types match the entity handled or
explicitly error when no matching repository can be determined.
| "supervisorName": "complaintDetails.supervisorName", | ||
| "supervisorContactNumber": | ||
| "complaintDetails.supervisorContactNumber ", | ||
| "otherComplaintDescription": "complaintType.otherReason" |
There was a problem hiding this comment.
Fix the trailing whitespace in the field path
The source path for supervisorContactNumber ends with a space, so the transformer never resolves the value. The supervisor’s contact number will always be missing in the mapped payload. Please drop the trailing space.
- "supervisorContactNumber":
- "complaintDetails.supervisorContactNumber ",
+ "supervisorContactNumber":
+ "complaintDetails.supervisorContactNumber",📝 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.
| "supervisorName": "complaintDetails.supervisorName", | |
| "supervisorContactNumber": | |
| "complaintDetails.supervisorContactNumber ", | |
| "otherComplaintDescription": "complaintType.otherReason" | |
| "supervisorName": "complaintDetails.supervisorName", | |
| "supervisorContactNumber": | |
| "complaintDetails.supervisorContactNumber", | |
| "otherComplaintDescription": "complaintType.otherReason" |
🤖 Prompt for AI Agents
In packages/complaints/lib/data/transformer_config.dart around lines 48 to 51
the source path string for "supervisorContactNumber" contains a trailing space
("complaintDetails.supervisorContactNumber "), which prevents the transformer
from resolving the field; remove the trailing space so the value becomes
"complaintDetails.supervisorContactNumber" ensuring the mapped payload includes
the supervisor contact number.
| static Map<String, dynamic>? requiredMin( | ||
| AbstractControl<dynamic> control, | ||
| ) { | ||
| AbstractControl<dynamic> control, | ||
| ) { | ||
| return control.value == null || | ||
| control.value.toString().length >= 2 || | ||
| control.value.toString().trim().isEmpty | ||
| control.value.toString().length >= 2 || | ||
| control.value.toString().trim().isEmpty | ||
| ? null | ||
| : {'required': true}; | ||
| } |
There was a problem hiding this comment.
Fix validation logic error.
The validation logic contains a critical flaw. The method returns null (valid) when:
control.value == nullORcontrol.value.toString().length >= 2ORcontrol.value.toString().trim().isEmpty
This means empty or null values pass validation, contradicting the "requiredMin" semantics. The OR logic allows empty strings to be considered valid.
For a "required minimum" validator, the logic should be:
Apply this diff to fix the validation logic:
static Map<String, dynamic>? requiredMin(
AbstractControl<dynamic> control,
) {
- return control.value == null ||
- control.value.toString().length >= 2 ||
- control.value.toString().trim().isEmpty
- ? null
- : {'required': true};
+ if (control.value == null || control.value.toString().trim().isEmpty) {
+ return {'required': true};
+ }
+ if (control.value.toString().length < 2) {
+ return {'required': true};
+ }
+ return null;
}📝 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.
| static Map<String, dynamic>? requiredMin( | |
| AbstractControl<dynamic> control, | |
| ) { | |
| AbstractControl<dynamic> control, | |
| ) { | |
| return control.value == null || | |
| control.value.toString().length >= 2 || | |
| control.value.toString().trim().isEmpty | |
| control.value.toString().length >= 2 || | |
| control.value.toString().trim().isEmpty | |
| ? null | |
| : {'required': true}; | |
| } | |
| static Map<String, dynamic>? requiredMin( | |
| AbstractControl<dynamic> control, | |
| ) { | |
| if (control.value == null || control.value.toString().trim().isEmpty) { | |
| return {'required': true}; | |
| } | |
| if (control.value.toString().length < 2) { | |
| return {'required': true}; | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In packages/complaints/lib/utils/utils.dart around lines 91 to 99, the
requiredMin validator currently treats null or empty values as valid due to OR
logic; change it so that it returns an error when the value is null OR the
trimmed value is empty OR the string length is less than the minimum (2), and
return null otherwise — i.e., compute a boolean invalid = control.value == null
|| control.value.toString().trim().isEmpty || control.value.toString().length <
2 and return {'required': true} when invalid, else null.
6 participants
No description provided.