1.8 Dev #717
1.8 Dev #717
Conversation
* wayBillNumber key change from waybillNumber return transaction type logic change in stock_details and report_details bloc * validation for wayBillNumber * Delivery action label and administered_success status update based on the project * Revert "Delivery action label and administered_success status update based on the project" This reverts commit 8673a48. * published digit_components - privacy notice component
status map fix
updated sort at task search query
Updated beneficiary_type variable in project type entity and table to BeneficiaryType enum
TODO: link the unique generated id to identifier in individual
updated digit_data_model and registration_delivery packages
Published new version
updated packages to latest versions
Added loader in sidenav for language change - HCMPRE-2285 checking formula parser for errors and displaying toast, restricting navigation to delivery page
modified formula parser error to dialog from toast
added check for IEC for redirection after submitting checklist
Added check for additional controller based on the reference id from widget params
…ed earlier in household_details.dart and individual_details.dart
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces comprehensive enhancements across the data model, repositories, and business logic, particularly focusing on beneficiary registration, unique ID management, and household member relationships. Key changes include new tables, enums, and data models for unique ID pools and household member relationships, expanded repository and bloc logic to support these entities, and integration of service checklist data into search and overview flows. Numerous schema migrations, enum extensions, and API adjustments are also included to support these new features and improve data integrity and search capabilities. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant UniqueIdBloc
participant UniqueIdPoolLocalRepo
participant UniqueIdPoolRemoteRepo
UI->>UniqueIdBloc: Fetch unique IDs (event)
UniqueIdBloc->>UniqueIdPoolLocalRepo: Get count of unassigned IDs
UniqueIdPoolLocalRepo-->>UniqueIdBloc: Return count
UniqueIdBloc->>UniqueIdPoolRemoteRepo: Fetch batch of unique IDs (if needed)
UniqueIdPoolRemoteRepo-->>UniqueIdBloc: Return list of unique IDs
UniqueIdBloc->>UniqueIdPoolLocalRepo: Store fetched IDs
UniqueIdBloc-->>UI: Emit state with updated ID count/list
sequenceDiagram
participant UI
participant BeneficiaryRegistrationBloc
participant HouseholdMemberLocalRepo
participant UniqueIdPoolLocalRepo
UI->>BeneficiaryRegistrationBloc: Add member event (with parentClientReferenceId, relationshipType)
BeneficiaryRegistrationBloc->>HouseholdMemberLocalRepo: Create household member
BeneficiaryRegistrationBloc->>HouseholdMemberLocalRepo: Create relationship entity (if parentClientReferenceId provided)
BeneficiaryRegistrationBloc->>UniqueIdPoolLocalRepo: Mark unique ID as assigned (if used)
BeneficiaryRegistrationBloc-->>UI: Emit state with new member and relationship info
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
resetting form value of idNumber to null to assign the new uniqueBeneficiaryId
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 52
🔭 Outside diff range comments (12)
packages/digit_data_model/lib/models/entities/target.dart (1)
37-75:⚠️ Potential issueLoss of type-safety:
beneficiaryTypedowngraded from enum to StringChanging from
BeneficiaryType?toString?:
- Removes compile-time guarantees – any typo will now reach the DB/API layer.
- Breaks exhaustive
switchstatements that rely on the enum.Unless you need to accept unknown/forward-compatible values, keep the enum and use a custom mapper that serialises to string for persistence:
- final String? beneficiaryType; + final BeneficiaryType? beneficiaryType; ... - beneficiaryType: Value(beneficiaryType), + beneficiaryType: Value(beneficiaryType?.name),You still store a string in Drift but keep strong typing in Dart.
packages/complaints/lib/data/repositories/local/pgr_service.dart (1)
55-90:⚠️ Potential issue
dataOperationnot forwarded – compilation error
super.updatenow expects the newdataOperationargument. Omitting it will fail to compile.- await super.update( - entity, - createOpLog: createOpLog, - ); + await super.update( + entity, + createOpLog: createOpLog, + dataOperation: dataOperation, + );apps/health_campaign_field_worker_app/lib/data/local_store/downsync/downsync.dart (1)
30-47:⚠️ Potential issueMissing
dataOperationpropagation mirrors the issue aboveSame problem here – pass the parameter to preserve API contract.
- await super.update(entity, createOpLog: false); + await super.update( + entity, + createOpLog: false, + dataOperation: dataOperation, + );packages/inventory_management/lib/data/repositories/local/stock.dart (1)
113-131: 🛠️ Refactor suggestion
dataOperationparameter is never used nor forwarded
dataOperationwas added to the signature but the implementation still calls
super.update(entity, createOpLog: createOpLog)without forwarding it.
IfLocalRepository.updatenow accepts/relies on this parameter (as suggested by similar changes across the code-base), the current call will:
- Compile-fail if the parameter became required.
- Silently ignore the caller’s intent if the parameter is optional but meaningful.
Proposed fix:
- return super.update(entity, createOpLog: createOpLog); + return super.update( + entity, + createOpLog: createOpLog, + dataOperation: dataOperation, + );Also consider actually branching on
dataOperationinside this method (e.g. choosing betweeninsertOrReplacevsupdate) so the extra argument delivers value.packages/inventory_management/lib/data/repositories/local/stock_reconciliation.dart (1)
90-109: 🛠️ Refactor suggestionForward the new
dataOperationargument to the superclassSame issue as in
StockLocalRepository: the freshly addeddataOperationis not propagated tosuper.update, potentially breaking the intended behaviour.- return super.update(entity, createOpLog: createOpLog); + return super.update( + entity, + createOpLog: createOpLog, + dataOperation: dataOperation, + );packages/registration_delivery/lib/data/repositories/local/side_effect.dart (1)
147-167: 🛠️ Refactor suggestionUnused
dataOperationargument
dataOperationis accepted but:
- Not forwarded to
super.update.- Not referenced elsewhere in the method.
Please align with the rest of the repository implementations:
- await super.update(entity, createOpLog: createOpLog); + await super.update( + entity, + createOpLog: createOpLog, + dataOperation: dataOperation, + );packages/registration_delivery/lib/data/repositories/local/household.dart (1)
227-264: 🛠️ Refactor suggestionParameter added, behaviour unchanged
dataOperationis introduced but ignored when delegating tosuper.update.
Forward it to ensure consistency with the new sync semantics:- await super.update(entity, createOpLog: createOpLog); + await super.update( + entity, + createOpLog: createOpLog, + dataOperation: dataOperation, + );Long-term: consider switching between full/bulk update vs
singleUpdatebased on this flag to leverage the new enum value.packages/registration_delivery/lib/models/entities/household_member.dart (1)
74-85: 💡 Verification agent🧩 Analysis chain
memberRelationshipsis not persisted nor included in copy/JSON – verify intentThe new
memberRelationshipsfield is added to the model & constructor, yet it is:
- Not reflected in
companion, therefore never written to Drift.- Not referenced elsewhere in this file (e.g. equality / copy-with / mapper).
If relationships are stored in a separate table this is fine, but in that case the field should probably be marked with
@ignorein the mapper to avoid serialisation noise.
Please double-check that:/// mapper file @MappableField(ignore: true) // or similar final List<HouseholdMemberRelationShipModel>? memberRelationships;or, if you do intend to persist it, extend
companion, mapper and repository code accordingly.
🏁 Script executed:
#!/bin/bash set -e echo "=== Occurrences of memberRelationships ===" rg -n "memberRelationships" echo echo "=== Searching for copyWith in household_member.dart ===" rg -n "copyWith" packages/registration_delivery/lib/models/entities/household_member.dart || true echo echo "=== Searching for toJson in household_member.dart ===" rg -n "toJson" packages/registration_delivery/lib/models/entities/household_member.dart || true echo echo "=== Occurrences of HouseholdMemberCompanion ===" rg -n "HouseholdMemberCompanion" -A 5Length of output: 15592
memberRelationships – align serialization with persistence strategy
The
memberRelationshipsfield is never included in the Drift companion (you handle it via a separate table), but it’s still picked up by your JSON mapper. To avoid unwanted JSON noise, mark it as ignored—or if you actually intended to store it as a JSON column, wire it into the companion too.Points of action:
- File:
packages/registration_delivery/lib/models/entities/household_member.dart, around line 74- Add the mapper annotation to skip serialization:
@MappableField(ignore: true) final List<HouseholdMemberRelationShipModel>? memberRelationships;
- Or, if you meant to persist it in the same table, extend the
companiongetter:HouseholdMemberCompanion get companion => HouseholdMemberCompanion( … memberRelationships: Value(memberRelationships?.map((e) => jsonEncode(e)).toList()), … );packages/registration_delivery/lib/blocs/search_households/individual_global_search.dart (1)
131-133:⚠️ Potential issueNull-safety check is inverted
filteris dereferenced before verifying it’s non-null:else if (event.globalSearchParams.filter!.isNotEmpty && event.globalSearchParams.filter != null)Swap the null-check order or drop the redundant second clause.
apps/health_campaign_field_worker_app/lib/blocs/project/project.dart (1)
646-651:⚠️ Potential issueSuccess state emits empty
projectsAfter a successful selection you emit
projects: []. Down-stream widgets depending on the collection (e.g., project switcher) will fail. Emit the earlier list or leave it unchanged.packages/digit_data_model/lib/data/data_repository.dart (1)
320-324:⚠️ Potential issueDuplicate
else ifbranch makessingleUpdateunreachableBoth branches test
operation == DataOperation.singleCreate.
The second one is clearly intended forsingleUpdate, therefore the code path for dumping errors ofsingleUpdateoperations is never hit.- } else if (operation == DataOperation.singleCreate) { + } else if (operation == DataOperation.singleUpdate) { url = searchPath;apps/health_campaign_field_worker_app/lib/data/sync_service_mapper.dart (1)
35-86:⚠️ Potential issueAdd
breakafter everycaseto prevent illegal fall-throughAll case blocks call
await local.bulkCreate(entity)but then flow straight into the nextcase.
Dart forbids fall-through unless you explicitlycontinue,return, orthrow, so this will not compile (or if suppressed, will bulk-insert into the wrong tables).await local.bulkCreate(entity); +break;Add the above after every
await local.bulkCreate(entity);line in this switch.
♻️ Duplicate comments (2)
packages/referral_reconciliation/lib/pages/create_referral/record_referral_details.dart (1)
506-515: 🛠️ Refactor suggestionDuplicate issue – same empty-string list is built here.
Apply the same guard as suggested above to prevent
['']from being sent.packages/registration_delivery/lib/blocs/parent_overview/parent_overview.dart (1)
208-218: Same null-safety issue as HouseholdOverviewBlocThe attribute sort is missing here, but later operations still assume non-null
clientAuditDetails. Apply the same null checks as flagged inHouseholdOverviewBloc.
🧹 Nitpick comments (51)
packages/inventory_management/CHANGELOG.md (1)
1-3: Changelog entry formatting
Please align the new version header with the existing convention. For example, previous dev releases usedev.xwithout additional labels. Consider renaming1.0.6-dev.1-enumerationto1.0.6-dev.1or documenting the meaning of-enumeration. Also, refine the bullet to read something like “Update data model changes” or “Update data model schema” for clarity.apps/health_campaign_field_worker_app/lib/blocs/auth/auth.dart (1)
98-98: Remove redundantsetBoundaryRefetchcall
The second invocation oflocalSecureStore.setBoundaryRefetch(true)duplicates the one at line 89 and can be removed to avoid unnecessary side-effects.apps/health_campaign_field_worker_app/lib/pages/acknowledgement.dart (1)
51-52: Consider handling text overflow
You’ve limited the label to three lines; addingoverflow: TextOverflow.ellipsiswill prevent layout breakage when text exceeds the limit.packages/digit_data_model/lib/utils/constants.dart (1)
61-63: Consider handling both singular and plural keys for ServiceUp-stream callers might supply either
"Service"or"Services". As only the plural is mapped, the singular variant would silently fall through to the default (householdMember). Adding the singular keeps the API symmetrical with other branches such as Task(s), Referral(s), etc.case 'HFReferrals': return DataModelType.hFReferral; + case 'Service': + return DataModelType.service; case 'Services': return DataModelType.service;apps/health_campaign_field_worker_app/lib/blocs/localization/app_localization.dart (1)
35-36: Boolean return can be simplifiedThe ternary is redundant—
_localizedStrings.isNotEmptyalready yields a boolean.- return _localizedStrings.isNotEmpty ? true : false; + return _localizedStrings.isNotEmpty;packages/referral_reconciliation/CHANGELOG.md (1)
7-8: Wording nit – tighten phrasing“Updated service and data model with respect to enumeration changes” is verbose. Consider:
* Updated service and data model to reflect enumeration changes🧰 Tools
🪛 LanguageTool
[style] ~7-~7: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ation * Updated service and data model with respect to enumeration changes * Updated reference...(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
apps/health_campaign_field_worker_app/lib/utils/constants.dart (1)
94-97: Prefer raw-string (r'') for un-escaped asset/API pathsAlthough nothing is functionally wrong, declaring string literals that never require interpolation or escape sequences as raw-strings improves readability and protects against future escaping bugs.
- static const String dashboardAnalyticsPath = - '/dashboard-analytics/dashboard/getChartV2'; + static const String dashboardAnalyticsPath = + r'/dashboard-analytics/dashboard/getChartV2';packages/digit_data_model/lib/data/local_store/sql_store/tables/attributes.dart (1)
29-31: Column re-ordering is benign but triggers unnecessary migrationRe-ordering
additionalDetailsandadditionalFieldsdoes not alter the schema, yet Drift will emit a migration that rewrites the table – costly on large datasets. Consider reverting or adding an explicit@DriftDatabase(dontWarnAboutSchemaChanges: true)override for this table to avoid a full rebuild.packages/digit_scanner/lib/utils/scanner_utils.dart (1)
135-143: Potential O(N²) duplicate check
bloc.state.barCodes.any(...)inside the scanning loop runs on each frame; with hundreds of scans this turns into quadratic time.
Store barcodes in aSet<String>keyed by the GTIN once and usecontains()for O(1) lookups.apps/health_campaign_field_worker_app/lib/pages/profile.dart (1)
207-210: Minor readability nitMultiline
genderextraction could be inlined for brevity, but functionality is fine.packages/digit_data_model/lib/models/entities/beneficiary_type.dart (1)
14-21: Enum naming consistency – consider more descriptive identifiers
sgProduct/rcProductare not self-explanatory at first glance (contrast withspecialGroups,refugeeCamps). A more explicit pair such asspecialGroupProduct/refugeeCampProductwould make the intent clear and avoid future misunderstandings.packages/registration_delivery/lib/blocs/search_households/tag_by_search.dart (1)
26-28: Parameter name typo:houseHoldGlobalSearchRepositoryEverywhere else in the codebase the repository is spelled
householdGlobalSearchRepository(lower-case “h” after “house”). Keeping the same spelling avoids confusion and enables IDE symbol search.- required super.houseHoldGlobalSearchRepository, + required super.householdGlobalSearchRepository,packages/inventory_management/lib/pages/record_stock/stock_details.dart (1)
969-978: Length validators added but translations still contain placeholdersThe newly-added messages insert raw
{}which will be shown to users if the i18n JSON is not updated. Ensure the corresponding keys (common.maxCharsRequired,common.min2CharsRequired) are parametrised or pre-formatted.packages/digit_data_model/lib/models/entities/identifier_types.dart (1)
6-12: Enum member casing is inconsistent with Dart style guidelines
Dart‐style guidelines recommendcamelCasefor enum values.
uniqueBeneficiaryIDanddefaultIDmix camel-case with a trailing all-caps acronym.
For readability and consistency with the rest of the codebase consider:- uniqueBeneficiaryID, - defaultID, + uniqueBeneficiaryId, + defaultId,The mapper annotation will still serialise them as the desired
UNIQUE_BENEFICIARY_ID/DEFAULTstrings, so no wire-format is affected.packages/registration_delivery/lib/models/entities/beneficiary_checklist_enums.dart (1)
6-15: Minor naming nit – avoid the redundant “Enums” suffixThe enum is already an enum; naming it
BeneficiaryChecklistEnumorBeneficiaryChecklistTypeimproves readability:-enum BeneficiaryChecklistEnums { +enum BeneficiaryChecklistType {No functional impact, purely stylistic.
apps/health_campaign_field_worker_app/lib/data/repositories/remote/mdms.dart (1)
326-341: Assignment ok but consider immutabilityProperties are appended directly to the mutable
appConfigurationinstance; ensure this object is not shared across isolates or cached unsafely. Otherwise looks fine.apps/health_campaign_field_worker_app/lib/pages/project_selection.dart (2)
90-116: Inconsistent use ofcontextvscxtintroduces fragility in retry actionsWithin the two lambdas passed to
DigitDialogActions.actionyou alternate between the outer-scopecontextand the parametercxtwhen dispatching bloc events.
Although both currently refer to the sameBuildContext, using different instances is error-prone (e.g. if the outercontextbecomes unmounted after route removal).- context + cxt .read<ProjectBloc>() .add(const ProjectInitializeEvent());Apply the same substitution in the first branch to keep the code self-contained and future-proof.
34-40:_selectedProjectis now write-onlyAfter switching the retry logic to
state.selectedProject, the private field_selectedProjectis no longer read anywhere in this widget. Consider removing it (and the accompanying comments) to avoid dead state.apps/health_campaign_field_worker_app/lib/pages/authenticated.dart (1)
281-408: Potential over-pop:Navigator.pop()runs unconditionally
BlocListenerblindly callsNavigator.pop()when localization loading completes. If the loader dialog was never shown (e.g. user re-opens drawer quickly), this pops the drawer itself or the current route.Guard with
if (Navigator.of(context, rootNavigator: true).canPop())before popping.packages/digit_data_model/lib/data/local_store/sql_store/tables/package_tables/household_member_relationship.dart (1)
24-26: Consider adding secondary indexes for frequent look-upsIf queries will join on
selfId,relativeId, or filter onrelationshipType, declareindex()on those columns now to avoid full-table scans later.TextColumn get selfId => text().nullable().indexed()(); TextColumn get relativeId => text().nullable().indexed()(); TextColumn get relationshipType => text().nullable().indexed()();Also applies to: 41-43
packages/registration_delivery/CHANGELOG.md (1)
1-41: Fix Markdown lint errors & duplicate headingIndentation is off (
MD007) and## 1.0.7-dev.1-idGenappears twice (MD024). Quick tidy-up:-## 1.0.7-dev.1-idGen - -* IdGen: +## 1.0.7-dev.1-idGen + +### IdGenAnd align bullet indents to two spaces throughout the section.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
8-8: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
35-35: Multiple headings with the same content
null(MD024, no-duplicate-heading)
38-38: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
41-41: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
packages/registration_delivery/lib/blocs/search_households/individual_global_search.dart (2)
334-346: Heavy per-household queries hurt performance
serviceDataRepository.searchis executed for every household inside the loop, resulting in N×network/database calls.
Batch fetch all checklist data for the collected IDs before the loop and reuse the in-memory maps.
188-197: UseaddAllinstead of repeatedforEachMinor: replace the two loops with direct
taskList.addAll(finalResults)/referralsList.addAll(finalResults)for clarity and speed.packages/digit_data_model/lib/data/repositories/remote/project_resource.dart (1)
18-100: Minor API inconsistencies
- Parameter
offSet(camel-case mix) differs from the widely-usedoffset; harmonise to avoid confusion.- Response-key handling treats
ServiceDefinitionspecially but notService; confirm the backend key forServiceresponses, or include it here to preventInvalidApiResponseException.apps/health_campaign_field_worker_app/lib/blocs/projects_beneficiary_downsync/project_beneficiaries_downsync.dart (1)
209-219: Repository order fine – but keep the list DRYNice to see
serviceLocalRepositoryincluded in the batch write.
If more entities get added, this ever-growing positional list will become error-prone. Consider passing aList<LocalRepository>built once (e.g. in the BLoC ctor) or exposing arepositoriesgetter to keep this section maintenance-free.packages/digit_data_model/lib/data/repositories/remote/product_variant.dart (2)
16-22: Parameter naming – stick tooffset/limitfor consistencyThe method signature uses
offSet(camel-case capital S). All other repos useoffset. Mixing the two invites typos downstream.-FutureOr<List<ProductVariantModel>> search( - ProductVariantSearchModel query, { - int? offSet, - int? limit, - }) +FutureOr<List<ProductVariantModel>> search( + ProductVariantSearchModel query, { + int? offset, + int? limit, + })Don’t forget to update the
queryParametersmap accordingly.
30-34: ExposeincludeDeletedonly when trueCurrently
query.isDeleted ?? falseis re-inserted directly, soincludeDeletedis sent even whenfalse. Some APIs reject boolean parameters with a falsey value.-if (query.isDeleted ?? false) 'includeDeleted': query.isDeleted, +if (query.isDeleted == true) 'includeDeleted': true,Minor, but avoids unnecessary payload noise.
apps/health_campaign_field_worker_app/lib/models/app_config/app_config_model.dart (2)
85-87: Spelling / naming inconsistency
relationShipTypeOptions→relationshipTypeOptionsfor consistency with common naming and to match the enum valuerelationShipTypeOptionsintroduced elsewhere.
208-218: Class name casing
RelationShipTypeOptionsuses mid-word capital “S”. PreferRelationshipTypeOptionsto stay idiomatic and avoid future import pain.apps/health_campaign_field_worker_app/lib/blocs/project/project.dart (2)
193-197: Clearingprojectslist loses contextOn remote fetch failure you now emit
projects: []. The UI can no longer show previously cached projects, degrading offline UX. Consider retaining the last known list and showing an error banner instead.
259-266: Same concern for facilities failureClearing
projectson a facility-sync error removes already-synced data. Preserve the list and rely on thesyncErrorflag to indicate partial failure.packages/registration_delivery/lib/blocs/search_households/search_households.dart (1)
121-131: Duplicate network callsTwo separate searches for household and member checklists can be merged into a single
ServiceSearchModelwith combinedreferenceIds.packages/registration_delivery/lib/blocs/search_households/search_households.freezed.dart (2)
2698-2714: New checklist & member collections – watch for payload bloat
householdMembers,householdChecklists, andindividualChecklistsare now part of each wrapper.
If these lists can grow large, equality/hash operations (which run through DeepCollectionEquality) may become expensive when the Bloc state updates.Consider:
@unfreezed class HouseholdMemberWrapper { … @override bool operator == … }or holding only IDs and lazy-loading heavy lists when actually needed.
1-5: Generated file committed – ensure CI step keeps it up-to-dateNothing wrong here, but remember that committing generated Freezed files requires all contributors to regenerate them on every model change.
If this repo already follows that workflow, feel free to ignore.packages/digit_data_model/lib/data/data_repository.dart (1)
82-117: Payload construction has become deeply nested and brittleThe ternary cascade that handles
Service,ServiceDefinition,Downsync, pluralisation, etc. is difficult to read and error-prone.
Consider extracting this into a dedicated helper that returnsMap<String, dynamic> buildSearchBody(...).This will:
- Improve readability & testability.
- Reduce the risk of silent regressions when another special-case entity is added.
packages/registration_delivery/lib/data/repositories/local/unique_id_pool.dart (1)
84-90: Redundant null-check – typo hides potential bugadditionalFields: uniqueIdPool.additionalFields != null ? UniqueIdPoolAdditionalFieldsMapper.fromJson( uniqueIdPool.additionalFields.toString()) : null,
uniqueIdPool.additionalFieldsis already aString?; callingtoString()on a non-null string is a no-op but will stringifynullas'null'when the column isNULL, defeating the guard.
Use the value directly:- uniqueIdPool.additionalFields.toString()) + uniqueIdPool.additionalFields!)packages/digit_data_model/lib/data/repositories/local/individual.dart (1)
329-339: Address TODO: Fetch identifier audit details from server responseThe TODO comment indicates that identifier audit details should be fetched from the server response. Using
auditDetailsasclientAuditDetailsis a workaround that could lead to incorrect audit trail.Would you like me to help implement the proper fetching of identifier audit details from the server response or create an issue to track this?
packages/registration_delivery/lib/blocs/household_overview/household_overview.dart (1)
270-281: Checklist data missing when filters/search are used
householdChecklistsandindividualChecklistsare merged only in the first branch; the filtered/search paths never touch these fields, so UI may show stale/empty checklist data after a search.Synchronise the logic across branches or move the merge to a helper used by all branches.
packages/registration_delivery/lib/blocs/parent_overview/parent_overview.dart (1)
262-274: Duplicate business logic – consider extracting a reusable helper
ParentOverviewBlocrepeats ~400 lines of code already present inHouseholdOverviewBloc, differing only in minor details.
Maintaining two near-identical flows will quickly diverge and introduce bugs.Extract shared loading logic into a service or mixin and keep the blocs thin.
packages/digit_data_model/CHANGELOG.md (1)
26-30: Fix unordered-list indentation to satisfy markdownlintCurrent indentation triggers
MD007. Align sub-items by two spaces.🧰 Tools
🪛 LanguageTool
[uncategorized] ~30-~30: You might be missing the article “the” here.
Context: ...o insert all the identifiers instead of first index ## 1.0.7-dev.1-idGen * Added ne...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
26-26: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
packages/registration_delivery/lib/blocs/beneficiary_registration/beneficiary_registration.dart (2)
382-399: Avoid generating multiple unrelated IDs for the same entity
IdGen.i.identifieris invoked twice: once for theHouseholdMemberModel
and again for itsmemberRelationships. If both objects are meant to
represent the same household-member instance, reuse the already-generated
memberClientReferenceIdto maintain referential integrity.Also applies to: 760-770
840-878: Use a typed enum forrelationshipTypeUsing
String? relationshipTypescatters magic strings throughout the codebase
and bypasses the compile-time checks you already get forBeneficiaryType
andStatus. Define a smallenum RelationshipType { father, mother, … }
(or reuse an existing one) and let Freezed give you exhaustivewhen/map
support.Also applies to: 904-978, 970-978
packages/registration_delivery/lib/data/repositories/local/household_member.dart (2)
82-118: Filter out soft-deleted relationships
rescurrently contains every relationship row, including those withisDeleted == true.
Add a.where((r) => r.isDeleted != true)before mapping (or after) to keep the API symmetric with the member-level filter applied later.
293-317: Batch updates can be groupedInside
delete()a separatesql.batchis opened for every relationship, incurring multiple SQLite transactions.
Accumulate thebatch.updatecalls for all relationships inside a single batch to improve I/O efficiency.apps/health_campaign_field_worker_app/lib/data/local_store/no_sql/schema/app_configuration.g.dart (4)
38-43: Consider storing BeneficiaryIdConfig as a single embedded object instead of a list
BENEFICIARY_ID_CONFIGis modelled asIsarType.objectList, but the domain sounds like a singleton configuration (batch size / min-count).
Using anobjectrather than anobjectListwould simplify callers and avoid accidental multiple-item states.
98-103: Property name typo –RelationShipvsRelationshipThe new property key and target class use the spelling
RelationShipTypeOptions.
For consistency with existing naming (e.g.HouseholdMemberDeletionReasonOptions) and to aid grep-ability, rename toRelationshipTypeOptions.
6176-6200: Class name spelling
RelationShipTypeOptionsSchemarepeats theRelationShipspelling.
Renaming the generated model toRelationshipTypeOptionskeeps terminology consistent and prevents future typos propagating across layers.
7180-7192:MIN_COUNTstored asdouble
MIN_COUNTrepresents a count; storing it asdoublemay introduce precision/rounding issues and wastes 8 bytes vs 4.
If fractional values are not expected, change the field toIsarType.longand the Dart type toint.apps/health_campaign_field_worker_app/lib/pages/home.dart (3)
462-477: Guard manual-sync toast against null snapshot
snapshot.data?['enablesManualSync']can benull.
When that happens we fall into the sync-in-progress toast even if the service isn’t running, which may confuse users. Consider:-} else { - if (snapshot.data?['enablesManualSync'] == true) { +} else if (snapshot.data?['enablesManualSync'] == true) { _attemptSyncUp(context); - } else { +} else { Toast.showToast(/* … */); - } }
552-567: Tile looks good – mind duplicate map keys
homeItemsMapalready contains adashboardkey twice; adding newbeneficiaryIdLabelis fine, but repeated keys make intent unclear and complicate maintenance.
628-630: Mutating afinallist is allowed but brittle
filteredLabelsis declaredfinaland later mutated withremove.
Prefer building the list with the condition up-front to avoid side effects:final filteredLabels = homeItemsLabel .where(/* … */) .whereNot((e) => envConfig.variables.envType == EnvType.demo && kReleaseMode && e == i18.home.db) .toList();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (19)
apps/health_campaign_field_worker_app/android/app/src/main/AndroidManifest.xmlis excluded by!**/*.xmlapps/health_campaign_field_worker_app/assets/animated_json/download_animation.jsonis excluded by!**/*.jsonapps/health_campaign_field_worker_app/assets/animated_json/download_success.jsonis excluded by!**/*.jsonapps/health_campaign_field_worker_app/assets/icons/svg/beneficiary_ids.svgis excluded by!**/*.svg,!**/*.svgapps/health_campaign_field_worker_app/pubspec.lockis excluded by!**/*.lock,!**/*.lockapps/health_campaign_field_worker_app/pubspec.yamlis excluded by!**/*.yamlpackages/complaints/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/complaints/pubspec.yamlis excluded by!**/*.yamlpackages/digit_data_model/pubspec.yamlis excluded by!**/*.yamlpackages/inventory_management/pubspec.yamlis excluded by!**/*.yamlpackages/referral_reconciliation/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/referral_reconciliation/pubspec.yamlis excluded by!**/*.yamlpackages/registration_delivery/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/registration_delivery/pubspec.yamlis excluded by!**/*.yamlpackages/survey_form/build.yamlis excluded by!**/*.yamlpackages/survey_form/example/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/survey_form/pubspec.lockis excluded by!**/*.lock,!**/*.lockpackages/survey_form/pubspec.yamlis excluded by!**/*.yamlpackages/sync_service/pubspec.yamlis excluded by!**/*.yaml
📒 Files selected for processing (107)
apps/health_campaign_field_worker_app/lib/blocs/app_initialization/app_initialization.dart(1 hunks)apps/health_campaign_field_worker_app/lib/blocs/auth/auth.dart(1 hunks)apps/health_campaign_field_worker_app/lib/blocs/localization/app_localization.dart(1 hunks)apps/health_campaign_field_worker_app/lib/blocs/project/project.dart(7 hunks)apps/health_campaign_field_worker_app/lib/blocs/projects_beneficiary_downsync/project_beneficiaries_downsync.dart(6 hunks)apps/health_campaign_field_worker_app/lib/data/local_store/downsync/downsync.dart(1 hunks)apps/health_campaign_field_worker_app/lib/data/local_store/no_sql/schema/app_configuration.dart(3 hunks)apps/health_campaign_field_worker_app/lib/data/local_store/no_sql/schema/app_configuration.g.dart(28 hunks)apps/health_campaign_field_worker_app/lib/data/remote_client.dart(2 hunks)apps/health_campaign_field_worker_app/lib/data/repositories/remote/auth.dart(1 hunks)apps/health_campaign_field_worker_app/lib/data/repositories/remote/mdms.dart(4 hunks)apps/health_campaign_field_worker_app/lib/data/sync_registry.dart(1 hunks)apps/health_campaign_field_worker_app/lib/data/sync_service_mapper.dart(10 hunks)apps/health_campaign_field_worker_app/lib/models/app_config/app_config_model.dart(5 hunks)apps/health_campaign_field_worker_app/lib/models/app_config/app_config_model.freezed.dart(27 hunks)apps/health_campaign_field_worker_app/lib/models/app_config/app_config_model.g.dart(6 hunks)apps/health_campaign_field_worker_app/lib/models/entities/mdms_master_enums.dart(2 hunks)apps/health_campaign_field_worker_app/lib/models/entities/mdms_master_enums.mapper.dart(4 hunks)apps/health_campaign_field_worker_app/lib/pages/acknowledgement.dart(1 hunks)apps/health_campaign_field_worker_app/lib/pages/authenticated.dart(5 hunks)apps/health_campaign_field_worker_app/lib/pages/home.dart(12 hunks)apps/health_campaign_field_worker_app/lib/pages/login.dart(3 hunks)apps/health_campaign_field_worker_app/lib/pages/profile.dart(2 hunks)apps/health_campaign_field_worker_app/lib/pages/project_selection.dart(3 hunks)apps/health_campaign_field_worker_app/lib/utils/constants.dart(2 hunks)apps/health_campaign_field_worker_app/lib/utils/i18_key_constants.dart(12 hunks)apps/health_campaign_field_worker_app/lib/widgets/network_manager_provider_wrapper.dart(3 hunks)apps/health_campaign_field_worker_app/lib/widgets/showcase/config/home_showcase.dart(1 hunks)packages/complaints/CHANGELOG.md(1 hunks)packages/complaints/lib/data/repositories/local/pgr_service.dart(1 hunks)packages/digit_data_model/CHANGELOG.md(2 hunks)packages/digit_data_model/lib/data/data_repository.dart(6 hunks)packages/digit_data_model/lib/data/local_store/no_sql/schema/oplog.g.dart(2 hunks)packages/digit_data_model/lib/data/local_store/sql_store/sql_store.dart(5 hunks)packages/digit_data_model/lib/data/local_store/sql_store/tables/attributes.dart(1 hunks)packages/digit_data_model/lib/data/local_store/sql_store/tables/identifier.dart(1 hunks)packages/digit_data_model/lib/data/local_store/sql_store/tables/package_tables/household_member_relationship.dart(1 hunks)packages/digit_data_model/lib/data/local_store/sql_store/tables/service.dart(1 hunks)packages/digit_data_model/lib/data/local_store/sql_store/tables/service_attributes.dart(1 hunks)packages/digit_data_model/lib/data/local_store/sql_store/tables/target.dart(1 hunks)packages/digit_data_model/lib/data/local_store/sql_store/tables/unique_id_pool.dart(1 hunks)packages/digit_data_model/lib/data/repositories/local/individual.dart(7 hunks)packages/digit_data_model/lib/data/repositories/oplog/oplog.dart(4 hunks)packages/digit_data_model/lib/data/repositories/remote/facility.dart(1 hunks)packages/digit_data_model/lib/data/repositories/remote/product_variant.dart(2 hunks)packages/digit_data_model/lib/data/repositories/remote/project_facility.dart(1 hunks)packages/digit_data_model/lib/data/repositories/remote/project_resource.dart(2 hunks)packages/digit_data_model/lib/data_model.dart(3 hunks)packages/digit_data_model/lib/data_model.init.dart(2 hunks)packages/digit_data_model/lib/models/entities/attributes.dart(1 hunks)packages/digit_data_model/lib/models/entities/beneficiary_type.dart(1 hunks)packages/digit_data_model/lib/models/entities/beneficiary_type.mapper.dart(2 hunks)packages/digit_data_model/lib/models/entities/identifier.dart(5 hunks)packages/digit_data_model/lib/models/entities/identifier.mapper.dart(7 hunks)packages/digit_data_model/lib/models/entities/identifier_types.dart(1 hunks)packages/digit_data_model/lib/models/entities/identifier_types.mapper.dart(1 hunks)packages/digit_data_model/lib/models/entities/target.dart(1 hunks)packages/digit_data_model/lib/models/entities/target.mapper.dart(2 hunks)packages/digit_data_model/lib/models/oplog/oplog_entry.dart(2 hunks)packages/digit_data_model/lib/models/oplog/oplog_entry.mapper.dart(4 hunks)packages/digit_data_model/lib/utils/constants.dart(1 hunks)packages/digit_scanner/lib/utils/i18_key_constants.dart(1 hunks)packages/digit_scanner/lib/utils/scanner_utils.dart(1 hunks)packages/inventory_management/CHANGELOG.md(1 hunks)packages/inventory_management/lib/data/repositories/local/stock.dart(1 hunks)packages/inventory_management/lib/data/repositories/local/stock_reconciliation.dart(1 hunks)packages/inventory_management/lib/pages/record_stock/stock_details.dart(3 hunks)packages/referral_reconciliation/CHANGELOG.md(1 hunks)packages/referral_reconciliation/lib/data/repositories/local/hf_referral.dart(1 hunks)packages/referral_reconciliation/lib/pages/create_referral/create_hf_referral_wrapper.dart(1 hunks)packages/referral_reconciliation/lib/pages/create_referral/record_reason_checklist.dart(1 hunks)packages/referral_reconciliation/lib/pages/create_referral/record_referral_details.dart(2 hunks)packages/referral_reconciliation/lib/pages/search_referral_reconciliations.dart(1 hunks)packages/registration_delivery/CHANGELOG.md(2 hunks)packages/registration_delivery/lib/blocs/beneficiary_registration/beneficiary_registration.dart(16 hunks)packages/registration_delivery/lib/blocs/household_overview/household_overview.dart(9 hunks)packages/registration_delivery/lib/blocs/parent_overview/parent_overview.dart(1 hunks)packages/registration_delivery/lib/blocs/parent_overview/parent_overview.freezed.dart(1 hunks)packages/registration_delivery/lib/blocs/search_households/household_global_seach.dart(7 hunks)packages/registration_delivery/lib/blocs/search_households/individual_global_search.dart(8 hunks)packages/registration_delivery/lib/blocs/search_households/search_households.dart(8 hunks)packages/registration_delivery/lib/blocs/search_households/search_households.freezed.dart(32 hunks)packages/registration_delivery/lib/blocs/search_households/tag_by_search.dart(1 hunks)packages/registration_delivery/lib/blocs/unique_id/unique_id.dart(1 hunks)packages/registration_delivery/lib/blocs/unique_id/unique_id.freezed.dart(1 hunks)packages/registration_delivery/lib/data/repositories/local/household.dart(1 hunks)packages/registration_delivery/lib/data/repositories/local/household_global_search.dart(6 hunks)packages/registration_delivery/lib/data/repositories/local/household_member.dart(7 hunks)packages/registration_delivery/lib/data/repositories/local/individual_global_search.dart(14 hunks)packages/registration_delivery/lib/data/repositories/local/project_beneficiary.dart(1 hunks)packages/registration_delivery/lib/data/repositories/local/referral.dart(1 hunks)packages/registration_delivery/lib/data/repositories/local/side_effect.dart(1 hunks)packages/registration_delivery/lib/data/repositories/local/task.dart(1 hunks)packages/registration_delivery/lib/data/repositories/local/unique_id_pool.dart(1 hunks)packages/registration_delivery/lib/data/repositories/oplog/oplog.dart(8 hunks)packages/registration_delivery/lib/data/repositories/remote/unique_id_pool.dart(1 hunks)packages/registration_delivery/lib/models/entities/additional_fields_type.dart(1 hunks)packages/registration_delivery/lib/models/entities/additional_fields_type.mapper.dart(2 hunks)packages/registration_delivery/lib/models/entities/beneficiary_checklist_enums.dart(1 hunks)packages/registration_delivery/lib/models/entities/beneficiary_checklist_enums.mapper.dart(1 hunks)packages/registration_delivery/lib/models/entities/household_member.dart(3 hunks)packages/registration_delivery/lib/models/entities/household_member.mapper.dart(9 hunks)packages/registration_delivery/lib/models/entities/household_member_relationship.dart(1 hunks)packages/registration_delivery/lib/models/entities/household_member_relationship.mapper.dart(1 hunks)packages/registration_delivery/lib/models/entities/id_status.dart(1 hunks)packages/registration_delivery/lib/models/entities/id_status.mapper.dart(1 hunks)packages/registration_delivery/lib/models/entities/status.dart(1 hunks)
⛔ Files not processed due to max files limit (18)
- packages/registration_delivery/lib/models/entities/status.mapper.dart
- packages/registration_delivery/lib/models/entities/unique_id_pool.dart
- packages/registration_delivery/lib/models/entities/unique_id_pool.mapper.dart
- packages/registration_delivery/lib/pages/beneficiary/beneficiary_checklist.dart
- packages/registration_delivery/lib/pages/beneficiary/beneficiary_details.dart
- packages/registration_delivery/lib/pages/beneficiary/beneficiary_id_downsync.dart
- packages/registration_delivery/lib/pages/beneficiary/beneficiary_wrapper.dart
- packages/registration_delivery/lib/pages/beneficiary/deliver_intervention.dart
- packages/registration_delivery/lib/pages/beneficiary/dose_administered.dart
- packages/registration_delivery/lib/pages/beneficiary/household_overview.dart
- packages/registration_delivery/lib/pages/beneficiary/parent_overview.dart
- packages/registration_delivery/lib/pages/beneficiary/refer_beneficiary.dart
- packages/registration_delivery/lib/pages/beneficiary/widgets/past_delivery.dart
- packages/registration_delivery/lib/pages/beneficiary_registration/beneficiary_acknowledgement.dart
- packages/registration_delivery/lib/pages/beneficiary_registration/beneficiary_registration_wrapper.dart
- packages/registration_delivery/lib/pages/beneficiary_registration/house_details.dart
- packages/registration_delivery/lib/pages/beneficiary_registration/household_details.dart
- packages/registration_delivery/lib/pages/beneficiary_registration/household_location.dart
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/registration_delivery/CHANGELOG.md
8-8: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
35-35: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
109-109: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
packages/digit_data_model/CHANGELOG.md
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
36-36: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
40-40: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
111-111: Multiple headings with the same content
null
(MD024, no-duplicate-heading)
🪛 LanguageTool
packages/referral_reconciliation/CHANGELOG.md
[style] ~7-~7: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ation * Updated service and data model with respect to enumeration changes * Updated reference...
(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
packages/digit_data_model/CHANGELOG.md
[uncategorized] ~30-~30: You might be missing the article “the” here.
Context: ...o insert all the identifiers instead of first index ## 1.0.7-dev.1-idGen * Added ne...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~34-~34: You might be missing the article “a” here.
Context: ...st index ## 1.0.7-dev.1-idGen * Added new table for storing unique Id's ## 1.0.6...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🔇 Additional comments (57)
apps/health_campaign_field_worker_app/lib/data/repositories/remote/auth.dart (1)
21-21: No-op formatting changeThe inserted blank line is purely cosmetic and does not affect functionality or readability significantly.
packages/digit_data_model/lib/models/entities/attributes.dart (1)
6-6: Import reorder is benign
The adjustment of import order introduces no functional change.packages/complaints/CHANGELOG.md (1)
1-4: Changelog entry added correctly
The new version entry for1.0.3-dev.1-enumerationaligns with upstream schema and enumeration enhancements.packages/digit_scanner/lib/utils/i18_key_constants.dart (1)
126-127: Ensure localization entries exist for new key
Verify thatSCANNED_QTY_EXCEED_THAN_REQUIREDis added to all locale resource files so translations are available at runtime.packages/registration_delivery/lib/models/entities/status.dart (1)
18-19:INELIGIBLEstatus added – looks goodEnum extension is consistent with the existing mapping style. Verify that any switch‐statements or UI filters over
Statushave a default branch or are updated to avoidUnhandled enumerrors at runtime.packages/referral_reconciliation/lib/pages/create_referral/record_reason_checklist.dart (1)
242-258:❌ Incorrect review comment
Inconsistent key names between model and additionalFields
You switched the model property to
referenceId(👍) but still push anAdditionalField('relatedClientReferenceId', …).-AdditionalField('relatedClientReferenceId', widget.referralClientRefId), +AdditionalField('referenceId', widget.referralClientRefId),Maintaining two different keys will confuse downstream mapping and analytics. Align the additional-field key with the new column name unless an API contract mandates the legacy key.
🏁 Script executed:
#!/bin/bash # Locate AdditionalField and ServiceAdditionalFields definitions rg -A5 "class AdditionalField" --max-count 1 rg -A5 "class ServiceAdditionalFields" --max-count 1 # Search for usages of the legacy key 'relatedClientReferenceId' rg "relatedClientReferenceId"Length of output: 23947
Keep the existing
relatedClientReferenceIdkey in additionalFields
The top‐levelreferenceIdproperty on the service model is distinct from the additional‐fields key. The API contract and downstream analytics expect the legacy"relatedClientReferenceId"inadditionalFields, so swapping it to"referenceId"would break existing mappings.Likely an incorrect or invalid review comment.
packages/registration_delivery/lib/models/entities/additional_fields_type.dart (1)
52-55: Enum additions look good – remember code-genThe new
heightandweightvalues are correctly annotated. Ensuredart run build_runner build(ordartx build) is executed so thatadditional_fields_type.mapper.dartis regenerated, otherwise runtime decoding will fail.apps/health_campaign_field_worker_app/lib/utils/constants.dart (1)
191-193: Missing l10n / unit-test guard for new asset
beneficiaryIdDownloadis added but:
- No unit test (golden or otherwise) asserts the asset exists – an easy source of runtime crashes on misspelling.
- No localisation key is introduced that references this constant; ensure the icon is wired to an i18n string and added to
pubspec.yaml.Please verify via an asset test or at least
flutter test --update-goldens.packages/referral_reconciliation/lib/pages/search_referral_reconciliations.dart (1)
178-181:✅ Verification successful
Constructor argument changed – double-check API consistency
ServiceSearchModel(referenceIds: [i.clientReferenceId])assumes:
referenceIdsexists and is aList<String?>.- Back-end APIs and repository mappers have migrated from the previous single field (
relatedClientReferenceId).If any downstream call still expects the old field, searches will silently return empty results.
Run a quick grep to confirm no lingering writes/reads of the old property:
🏁 Script executed:
rg -n "relatedClientReferenceId"Length of output: 20055
🏁 Script executed:
#!/bin/bash # Find where ServiceSearchModel is defined rg -n "class ServiceSearchModel" -n . # Find usage of ServiceSearchModel in referral_reconciliation package rg -n "ServiceSearchModel" -n packages/referral_reconciliation/lib/pages/search_referral_reconciliations.dartLength of output: 706
🏁 Script executed:
#!/bin/bash # Display the ServiceSearchModel definition to inspect its constructor sed -n '1,200p' packages/survey_form/lib/models/entities/service.dartLength of output: 3726
🏁 Script executed:
#!/bin/bash # Inspect mapping of referenceIds in the generated mapper rg -n "referenceIds" -n packages/survey_form/lib/models/entities/service.mapper.dartLength of output: 794
🏁 Script executed:
#!/bin/bash # Show context for ServiceSearchModel usage in the search referral reconciliations page sed -n '1,120p' packages/referral_reconciliation/lib/pages/search_referral_reconciliations.dart echo '==== bottom context ====' sed -n '140,200p' packages/referral_reconciliation/lib/pages/search_referral_reconciliations.dartLength of output: 9290
All clear: ServiceSearchModel migration confirmed
I’ve verified that
ServiceSearchModelnow definesreferenceIdsasList<String>?and its generated mapper handles onlyreferenceIds(no lingeringrelatedClientReferenceId). Insearch_referral_reconciliations.dart, all calls use the newreferenceIdsfield, and there are no remaining references to the old single-field property in that context.apps/health_campaign_field_worker_app/lib/widgets/showcase/config/home_showcase.dart (1)
106-108: Sanity-check the new i18n keyEnsure
i18.home.beneficiaryIdLabelexists in all supported locale JSON/ARB files, otherwise the showcase will render the raw key.packages/digit_data_model/lib/models/oplog/oplog_entry.dart (2)
111-118: Enum extension looks good; verify downstream switch/when exhaustivenessAdding
singleUpdateis fine here, but everyswitch/whenonDataOperationorApiOperationmust now include the new case to avoid runtimeFallThroughErrors and analysis warnings.
130-132: 👍 API enum keeps snake_case mapping consistencyNo issues detected with the added value – mapper will emit
single_updateas expected.apps/health_campaign_field_worker_app/lib/pages/profile.dart (1)
61-64: Required validator added for name – good catchName is now mandatory in the form. Implementation looks correct.
apps/health_campaign_field_worker_app/lib/data/remote_client.dart (1)
31-37: Interceptor order deliberately protects credentials – double-checkPlacing
CurlLoggerDioInterceptorbeforeAuthTokenInterceptorprevents auth tokens from being logged, which is good. Verify that no other interceptor later injects sensitive headers before this logger is hit ononRequest.packages/registration_delivery/lib/models/entities/id_status.dart (1)
6-14: Enum value naming is inconsistent with Dart style & peer enums
unAssignedmixes lower- and upper-case mid-word.
Dart enum identifiers conventionally use lowerCamelCase with no internal caps unless required (e.g.unassigned). Every other enum in the codebase appears to follow that rule.- @MappableValue("UN_ASSIGNED") - unAssigned + @MappableValue("UN_ASSIGNED") + unassignedRenaming early avoids ripple-on refactors later and keeps API surface tidy.
[ suggest_nitpick ]apps/health_campaign_field_worker_app/lib/data/sync_registry.dart (1)
41-49: Silent failure obscures sync-up errors
singleUpdateswallows every exception withdebugPrint, mirroring other helpers.
For update operations this can silently mark an entity as synced while the server rejected it, leaving the local store stale.Consider returning a boolean / result or re-throwing after logging so callers (or oplog manager) can react and retry.
- } catch (e) { - debugPrint('$e'); + } catch (e, st) { + AppLogger.instance.error( + title: 'SyncRegistry : singleUpdate', + message: e.toString(), + stackTrace: st, + ); + rethrow; // propagate so higher layer can flag failure }Aligning with
AppLoggeralso stays consistent with the detailed error capture used in the bulkcreatepath below.
[ suggest_essential_refactor ]apps/health_campaign_field_worker_app/lib/blocs/app_initialization/app_initialization.dart (1)
90-105: New master enums added – verify local fallback parity
beneficiaryIdConfig(l. 97) andrelationShipTypeOptions(l. 104) are now part of the remote MDMS fetch list.
Ensure the same two enums are present in any offline bootstrap JSON used by_loadOfflineData; otherwise first-run without connectivity will still throw on missing config.No code change needed here if the offline bundle already contains them – just double-check assets.
[ request_verification ]packages/registration_delivery/lib/models/entities/additional_fields_type.mapper.dart (1)
70-76: Manual edits in generated mapper will be lost
*.mapper.dartfiles are generated; adding"height"/"weight"branches directly will be overwritten the next timebuild_runneris run.Add the two values to the source enum (
additional_fields_type.dart) and regenerate instead.dart run build_runner build --delete-conflicting-outputsThis guarantees mapper, copy-with, and other generated helpers stay in sync.
[ raise_critical_issue ]Also applies to: 126-132
apps/health_campaign_field_worker_app/lib/models/entities/mdms_master_enums.dart (2)
18-22: 👍 Enum constant added forBENEFICIARY_ID_CONFIGFits existing pattern and keeps remote string in sync.
[ approve_code_changes ]
34-36: Confirm downstream handling ofrelationShipTypeOptionsMapper/regenerator should already pick this up, but ensure any switch/when statements over
MasterEnumshave a default branch or are updated, otherwise the compiler will warn once code referring to the new case is added.
[ request_verification ]packages/digit_data_model/lib/data_model.dart (1)
231-267: Verify allswitch/whenstatements that exhaustively handleDataModelType.
userLocationanduniqueIdhave been appended to the enum. Any previously-exhaustiveswitch/whenoverDataModelTypewithout adefaultwill now fail to compile; those with adefaultmay silently ignore the new cases.
Please run a quick search and extend the handling logic where appropriate.packages/digit_data_model/lib/data_model.init.dart (1)
18-20: Mapper initialisation looks good.
IdentifierTypesMapperis correctly registered; generated file is consistent with new import aliases.packages/digit_data_model/lib/models/entities/beneficiary_type.mapper.dart (1)
34-41: Enum extension correctly mapped.New beneficiary types are handled in both
decodeandencode; no issues spotted.packages/inventory_management/lib/pages/record_stock/stock_details.dart (1)
988-993: Waybill number expects numeric keyboard but validator allows any stringIf the field must be numeric, add
Validators.number()to_waybillNumberKey; otherwise keep the alphanumeric keyboard to avoid frustrating users.packages/digit_data_model/lib/models/entities/target.mapper.dart (1)
186-188: Type downgraded from strongly-typed enum toString?– validate upstream usage
SwitchingbeneficiaryTypefromBeneficiaryType?toString?removes
compile-time guarantees and auto-completion support.
If the change is intentional, ensure:
- All call-sites now validate / restrict the string to accepted values.
- Database migrations include NOT NULL / CHECK constraints (or equivalent)
so invalid values cannot be inserted.- UI widgets that previously used the enum are updated to avoid runtime
switchfall-throughs.If the loss of type-safety is accidental, revert to the enum and add the new
values toBeneficiaryType.Also applies to: 310-311
apps/health_campaign_field_worker_app/lib/models/entities/mdms_master_enums.mapper.dart (1)
40-56: Mappings added – looks correct
Decode/encode branches for the two new enum constants are symmetrical and
string keys match MDMS payloads. No further action required.Also applies to: 102-118
packages/digit_data_model/lib/models/oplog/oplog_entry.mapper.dart (1)
38-60: Enum mapping extended correctly
singleUpdatesupport is added symmetrically to both encode & decode branches – good catch.Also applies to: 108-110, 136-138
apps/health_campaign_field_worker_app/lib/pages/authenticated.dart (1)
341-347:⚠️ Potential issueCompile-time error:
ConnectivityResultis not iterable
Connectivity().checkConnectivity()returns a singleConnectivityResult, not an iterable.
UsingfirstOrNullwill not compile.- final connectivityResult = await (Connectivity().checkConnectivity()); - final isOnline = connectivityResult.firstOrNull == ConnectivityResult.wifi || - connectivityResult.firstOrNull == ConnectivityResult.mobile; + final connectivityResult = await Connectivity().checkConnectivity(); + final isOnline = connectivityResult == ConnectivityResult.wifi || + connectivityResult == ConnectivityResult.mobile;Please update the condition (and remove the now-unused
collectionimport if it was added solely forfirstOrNull).⛔ Skipped due to learnings
Learnt from: naveenr-egov PR: egovernments/health-campaign-field-worker-app#684 File: apps/health_campaign_field_worker_app/lib/utils/utils.dart:0-0 Timestamp: 2025-02-14T10:03:34.143Z Learning: The connectivity_plus package's checkConnectivity() method returns List<ConnectivityResult>, so firstOrNull should be used to get the first connectivity state.packages/digit_data_model/lib/data/local_store/sql_store/tables/service_attributes.dart (1)
37-41: Schema-migration reminderChanging
clientReferenceIdto nullable and adding new columns (id,serviceClientReferenceId) requires a forward migration path; otherwise existing installations will crash on opening the database.
Make sure a Drift migration step is added that usesALTER TABLEto:
- Add the two new columns with default
NULL- Modify
clientReferenceIdto nullable (requires table rebuild)packages/registration_delivery/lib/models/entities/beneficiary_checklist_enums.mapper.dart (1)
58-64: Enum mapper looks good – no concernsAuto-generated mapper correctly covers all enum values and provides
toValue()extension.packages/digit_data_model/lib/data/local_store/no_sql/schema/oplog.g.dart (1)
294-347: Generated file – looks fineThe enum maps now include
uniqueIdandsingleUpdate; this aligns with the new data model and operation.
No manual action required.packages/digit_data_model/lib/models/entities/identifier_types.mapper.dart (1)
9-45: Generated mapper – no issuesEnum ↔ string mapping appears correct; nothing to change.
packages/registration_delivery/lib/models/entities/id_status.mapper.dart (1)
9-53: Generated mapper looks fineNo issues spotted; mapping covers all enum values and throws on unknowns.
packages/digit_data_model/lib/data/local_store/sql_store/tables/unique_id_pool.dart (1)
5-30: Schema definition LGTMTable is well-defined with explicit PK; no correctness issues identified.
packages/registration_delivery/lib/blocs/search_households/individual_global_search.dart (1)
19-34: Ensure callers supply the newserviceDataRepositoryConstructor signature changed; verify all bloc instantiations are updated, otherwise DI will fail at runtime.
apps/health_campaign_field_worker_app/lib/blocs/projects_beneficiary_downsync/project_beneficiaries_downsync.dart (2)
43-44: AddServiceModelrepository – verify wiring & mapper supportThe field + constructor-param look good, but please double-check two things:
ServiceModelmust be registered withSyncServiceSingleton().entityMapper; otherwisewriteToEntityDBwill silently ignore the repo.- Ensure the DI graph actually passes a non-null instance when this BLoC is created, otherwise runtime NoSuchMethod errors will surface when the down-sync starts.
Also applies to: 56-57
125-129:limit: 0may break server contractChanging the initial request limit from
1to0relies on the server treating0as “return nothing but only meta-data”.
Many REST back-ends instead interpret0as unbounded (i.e. “give me everything”), which could download the entire dataset in one call and defeat the batching logic.Please confirm the API semantics; if meta-data-only is required, consider using an explicit flag or a constant like
metaDataOnlyLimitto avoid confusion.apps/health_campaign_field_worker_app/lib/widgets/network_manager_provider_wrapper.dart (1)
242-248: Local provider looks correctThe
UniqueIdPoolLocalRepositoryis properly registered with its OpLog manager – LGTM.
No further action required.packages/digit_data_model/lib/models/entities/identifier.dart (1)
42-45: New nullable fields added – remember search & uniqueness constraintsAdding
individualIdandindividualClientReferenceIdwidens the model – good.
Ensure:
- Corresponding DB indices / constraints exist, otherwise searches by these fields will be slow.
- Any uniqueness expectation (e.g. one identifier per individual) is enforced at either DB or repo level.
apps/health_campaign_field_worker_app/lib/models/app_config/app_config_model.g.dart (1)
90-103: Generated file – no manual changes requiredThe diff purely reflects code-generator output for new config types. Nothing to review.
Also applies to: 125-130, 170-176, 183-184, 314-329, 346-359
apps/health_campaign_field_worker_app/lib/models/app_config/app_config_model.dart (1)
232-241: Data type ofminCount
MIN_COUNTlooks like a whole number but is modelled asdouble. Useintunless fractional counts are genuinely expected.apps/health_campaign_field_worker_app/lib/blocs/project/project.dart (1)
620-629: Early return on empty boundary leaves local cache untouchedThe branch emits an error but never repopulates
projects, thereby erasing existing data. Double-check user flow here.packages/registration_delivery/lib/blocs/search_households/search_households.freezed.dart (1)
2403-2410:beneficiaryIdQueryadded – confirm model & JSON generation are in syncFreezed output looks consistent (copyWith, equality, hashCode are all updated).
Please double-check that the source@freezedmodel (search_households.dart) contains the same field with the correct@JsonKeyannotation; otherwise (de)serialization will silently drop this value.No action if the source was regenerated via
dart run build_runner build.packages/registration_delivery/lib/models/entities/household_member.mapper.dart (1)
397-404: Mapper change looks correct
memberRelationshipsfield is wired end-to-end (field descriptor, instantiate, copyWith).
No issues spotted.packages/digit_data_model/lib/models/entities/identifier.mapper.dart (1)
1-7: Generated file - no manual changes neededThis is an auto-generated mapper file. Any manual changes will be overwritten on the next code generation.
packages/digit_data_model/lib/data/repositories/local/individual.dart (1)
468-474: Good change: Individual inserts prevent data lossSwitching from
replaceAllto individualinsertoperations is a good improvement. This prevents accidental deletion of identifiers that aren't included in the update.packages/digit_data_model/lib/data/local_store/sql_store/sql_store.dart (1)
209-267: Well-implemented migration for beneficiaryType conversionThe migration logic properly converts the beneficiaryType column from integer enum to text values with appropriate mapping for all enum cases.
packages/registration_delivery/lib/data/repositories/local/household_global_search.dart (2)
307-341: Well-implemented identifier search functionalityThe new
searchByIdentifierIdmethod properly handles all cases:
- Returns early if no identifier provided
- Creates appropriate joins for household member and household tables
- Integrates seamlessly with existing search flow
513-513: Appropriate nullable handling for householdMaking household nullable is correct since the new join structure with identifier search may not always have a corresponding household record.
apps/health_campaign_field_worker_app/lib/data/sync_service_mapper.dart (1)
780-799: Verify identifier used for ServiceModel oplog update
clientReferenceIdis passed asentity.clientId. Everywhere else the model key isreferenceId.
Please confirm the correct field; a mismatch will stop the upstream rows from being resolved on the next sync.apps/health_campaign_field_worker_app/lib/models/app_config/app_config_model.freezed.dart (2)
1331-1348: Hard-required new lists will break deserialization if MDMS doesn’t ship them
beneficiaryIdConfigandrelationShipTypeOptionsare markedrequired, which meansHCMWrapperModel.fromJson()will throw if the MDMS payload for any tenant is missing these masters.
Unless back-end contracts guarantee their presence for all environments, consider making the lists optional with sensible defaults (e.g. empty list) to avoid runtime crashes.- @JsonKey(name: 'BENEFICIARY_ID_CONFIG') - required final List<BeneficiaryIdConfig> beneficiaryIdConfig, + @JsonKey(name: 'BENEFICIARY_ID_CONFIG') + final List<BeneficiaryIdConfig> beneficiaryIdConfig = const [],Apply the same to
relationShipTypeOptionsand regenerate code.
1455-1465: Generated file noticeThis file is auto-generated by
freezed/json_serializable. Manual edits will be lost the next time code generation runs. Keep all changes in the source models (*.dart) and rerun the generator instead of touching the*.freezed.dartdirectly.packages/registration_delivery/lib/blocs/parent_overview/parent_overview.freezed.dart (1)
1-740: Generated code – no manual issues to review
parent_overview.freezed.dartis auto-generated and should not be edited manually. Skipping detailed review unless you hit code-generation problems.packages/registration_delivery/lib/blocs/unique_id/unique_id.freezed.dart (1)
1-1868: Generated code – no manual issues to review
unique_id.freezed.dartis produced by the build system; no action required here.apps/health_campaign_field_worker_app/lib/pages/home.dart (1)
776-783: Potential crash whenbeneficiaryIdConfigis empty
appConfiguration.beneficiaryIdConfig?.firstthrowsStateErroron an empty list.
Safeguard withisNotEmptyor default values:final minCount = appConfiguration.beneficiaryIdConfig?.firstOrNull?.minCount; final batchSize = appConfiguration.beneficiaryIdConfig?.firstOrNull?.batchSize;packages/registration_delivery/lib/models/entities/household_member_relationship.mapper.dart (1)
1-731: Generated file – no manual changes requiredLooks auto-generated; no review comments.
apps/health_campaign_field_worker_app/lib/utils/i18_key_constants.dart (1)
575-576: New i18n key acknowledged
HOME_BENEFICIARY_ID_LABELadded – no issues.
| rethrow; | ||
| break; // Break out of the loop if an error occurs | ||
| } |
There was a problem hiding this comment.
Unreachable break after rethrow
rethrow exits the current stack frame; code that follows is never executed and the analyzer will flag it. Remove the break (and the accompanying comment) or move logging before rethrow.
} catch (error) {
- rethrow;
- break; // Break out of the loop if an error occurs
+ rethrow; // propagate to caller
}📝 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.
| rethrow; | |
| break; // Break out of the loop if an error occurs | |
| } | |
| } catch (error) { | |
| - rethrow; | |
| - break; // Break out of the loop if an error occurs | |
| + rethrow; // propagate to caller | |
| } |
🤖 Prompt for AI Agents
In packages/digit_data_model/lib/data/repositories/remote/project_facility.dart
around lines 61 to 63, there is a `break` statement immediately following a
`rethrow`, which is unreachable code. Remove the `break` statement and its
comment entirely, or if you want to keep the comment, move any logging or
actions before the `rethrow` so that the code after `rethrow` is not executed.
| rethrow; | ||
| break; // Break out of the loop if an error occurs | ||
| } |
There was a problem hiding this comment.
Same unreachable break pattern as in project_facility.dart
See previous comment; the break will never execute.
} catch (error) {
- rethrow;
- break; // Break out of the loop if an error occurs
+ rethrow;
}📝 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.
| rethrow; | |
| break; // Break out of the loop if an error occurs | |
| } | |
| } catch (error) { | |
| rethrow; | |
| } |
🤖 Prompt for AI Agents
In packages/digit_data_model/lib/data/repositories/remote/facility.dart at lines
58 to 60, remove the unreachable break statement that follows the rethrow; since
rethrow exits the current function, the break will never execute and is
redundant.
| @@ -9,7 +9,7 @@ class Service extends Table { | |||
|
|
|||
| TextColumn get serviceDefId => text().nullable()(); | |||
|
|
|||
| TextColumn get relatedClientReferenceId => text().nullable()(); | |||
| TextColumn get referenceId => text().nullable()(); | |||
There was a problem hiding this comment.
Column rename requires an explicit Drift migration
relatedClientReferenceId → referenceId is a breaking change for existing installations. Drift will drop the old column unless a custom migration is supplied, causing data loss and failing reads of historical rows.
Add a MigrationStrategy that:
- Creates the new column.
- Copies data from the old column.
- Drops the old column after verification (optional).
Without this, the app will crash on production databases that already contain relatedClientReferenceId.
🤖 Prompt for AI Agents
In packages/digit_data_model/lib/data/local_store/sql_store/tables/service.dart
at line 12, the column rename from relatedClientReferenceId to referenceId is a
breaking change that will cause data loss without a proper migration. To fix
this, add a MigrationStrategy in your Drift database setup that first adds the
new referenceId column, then copies data from relatedClientReferenceId to
referenceId, and finally drops the old relatedClientReferenceId column after
verifying the data transfer. This ensures existing data is preserved and
prevents crashes on production databases.
| referenceId: | ||
| widget | ||
| .referralClientRefId, | ||
| additionalFields: |
There was a problem hiding this comment.
Nullable value may violate server contract
referenceId: widget.referralClientRefId passes a nullable field straight through. If the backend expects a non-null reference for reconciliation, submitting a null here will create invalid service rows.
Add a null-check (or generate a UUID fallback) before constructing the ServiceModel.
🤖 Prompt for AI Agents
In
packages/referral_reconciliation/lib/pages/create_referral/record_reason_checklist.dart
around lines 242 to 245, the field referenceId is assigned from
widget.referralClientRefId which may be null, potentially violating the backend
contract that expects a non-null reference. To fix this, add a null-check for
widget.referralClientRefId before passing it to ServiceModel; if it is null,
generate and use a UUID fallback value to ensure referenceId is never null when
constructing the ServiceModel.
| initialValue: form.control(_userId).value, | ||
| errorMessage: field.errorText, |
There was a problem hiding this comment.
Hard-coded credentials in source control – remove immediately
Committing real or dummy credentials ('USR-260862', 'eGov@123') poses a security risk and encourages unsafe practices. It also prevents other users from logging in with their own accounts because the fields are pre-populated every build.
Quick fix:
- value: 'USR-260862',
+ value: '',
...
- value: 'eGov@123',
+ value: '',Optionally wrap dev defaults behind kDebugMode:
value: kDebugMode ? 'USR-260862' : '',Please purge the history if these are real credentials.
Also applies to: 129-130, 234-242
🤖 Prompt for AI Agents
In apps/health_campaign_field_worker_app/lib/pages/login.dart around lines
106-107, remove the hard-coded credentials used as initial values for the login
form fields to avoid security risks and prevent pre-populating fields with fixed
data. Replace these with empty strings or conditionally assign them only in
debug mode using a check like kDebugMode. Apply the same fix to lines 129-130
and 234-242 to ensure no credentials are hard-coded anywhere in the file.
| if (rawEntities is! List) { | ||
| if (responseMap['Errors'] is List) { | ||
| final errors = responseMap['Errors'] as List; | ||
| if (errors.isNotEmpty && errors.first['message'] != null) { | ||
| final errorMessage = errors.first['message'] as String; | ||
| if (errorMessage.contains('ID generation limit exceeded')) { | ||
| throw UniqueIdLimitExceededException(errorMessage); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Limit-exceeded detection is fragile
You rely on errors.first['message'] containing the substring 'ID generation limit exceeded'.
Any wording change on the server side will break this path silently. Prefer a structured error code if the API provides one, or at least match on a dedicated field such as errors.first['code'].
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/data/repositories/remote/unique_id_pool.dart
around lines 61 to 70, the current code checks for the error condition by
matching a substring in errors.first['message'], which is fragile. To fix this,
update the condition to check for a structured error code or a dedicated field
like errors.first['code'] if available, instead of relying on the message text.
This will make the detection more robust against server-side wording changes.
| await super.update(entity, createOpLog: createOpLog); | ||
| }); |
There was a problem hiding this comment.
Same super-call compilation error as in RemoteRepository
super.update originates from DataRepository.update and does not accept createOpLog.
Remove the named arg or widen the base method.
- await super.update(entity, createOpLog: createOpLog);
+ await super.update(entity);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await super.update(entity, createOpLog: createOpLog); | |
| }); | |
| await super.update(entity); | |
| }); |
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/data/repositories/local/unique_id_pool.dart
at lines 123-124, the call to super.update incorrectly includes the named
argument createOpLog, which is not accepted by the base method
DataRepository.update. To fix this, remove the createOpLog named argument from
the super.update call or modify the base DataRepository.update method to accept
this argument if appropriate.
| TextColumn get auditCreatedBy => text().nullable()(); | ||
|
|
||
| BoolColumn get nonRecoverableError => | ||
| boolean().nullable().withDefault(const Constant(false))(); | ||
|
|
There was a problem hiding this comment.
Primary-key column declared nullable – Drift will reject this schema
auditCreatedBy participates in the composite primary key yet is declared text().nullable().
Primary-key columns must be non-null. Either:
- Make the column non-nullable:
- TextColumn get auditCreatedBy => text().nullable()();
+ TextColumn get auditCreatedBy => text()();- Or drop it from the primary key.
Currently migrations will fail at runtime.
Also applies to: 45-48
🤖 Prompt for AI Agents
In packages/digit_data_model/lib/data/local_store/sql_store/tables/target.dart
around lines 14 to 18 and 45 to 48, the column auditCreatedBy is declared
nullable but is part of the composite primary key, which Drift does not allow.
To fix this, either make auditCreatedBy non-nullable by removing .nullable() or
remove it from the primary key definition to ensure the schema is valid and
migrations succeed.
| @MappableClass(ignoreNull: true, discriminatorValue: MappableClass.useAsDefault) | ||
| class HouseholdMemberRelationShipModel extends EntityModel | ||
| with HouseholdMemberRelationShipModelMappable { | ||
| static const schemaName = 'HouseholdMemberRelationShip'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update schema name and related references to match corrected naming
After fixing the class names, also update the schema name and other references.
- static const schemaName = 'HouseholdMemberRelationShip';
+ static const schemaName = 'HouseholdMemberRelationship';- final HouseholdMemberRelationShipAdditionalFields? additionalFields;
+ final HouseholdMemberRelationshipAdditionalFields? additionalFields;- HouseholdMemberRelationShipCompanion get companion {
- return HouseholdMemberRelationShipCompanion(
+ HouseholdMemberRelationshipCompanion get companion {
+ return HouseholdMemberRelationshipCompanion(- HouseholdMemberRelationShipAdditionalFields({
- super.schema = 'HouseholdMemberRelationShip',
+ HouseholdMemberRelationshipAdditionalFields({
+ super.schema = 'HouseholdMemberRelationship',Also applies to: 58-58, 77-78, 105-106
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/models/entities/household_member_relationship.dart
at lines 47, 58, 77-78, and 105-106, the schema name and related references
still use the old or incorrect naming. Update the static schemaName constant and
all other references in these lines to match the corrected class name, ensuring
consistent and accurate naming throughout the file.
| class HouseholdMemberRelationShipSearchModel extends EntitySearchModel | ||
| with HouseholdMemberRelationShipSearchModelMappable { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix inconsistent naming: Use "Relationship" instead of "RelationShip"
The class names use "RelationShip" which is grammatically incorrect. It should be "Relationship" (one word).
Apply this naming fix throughout the file:
-class HouseholdMemberRelationShipSearchModel extends EntitySearchModel
- with HouseholdMemberRelationShipSearchModelMappable {
+class HouseholdMemberRelationshipSearchModel extends EntitySearchModel
+ with HouseholdMemberRelationshipSearchModelMappable {- HouseholdMemberRelationShipSearchModel({
+ HouseholdMemberRelationshipSearchModel({- HouseholdMemberRelationShipSearchModel.ignoreDeleted({
+ HouseholdMemberRelationshipSearchModel.ignoreDeleted({-class HouseholdMemberRelationShipModel extends EntityModel
- with HouseholdMemberRelationShipModelMappable {
+class HouseholdMemberRelationshipModel extends EntityModel
+ with HouseholdMemberRelationshipModelMappable {-class HouseholdMemberRelationShipAdditionalFields extends AdditionalFields
- with HouseholdMemberRelationShipAdditionalFieldsMappable {
+class HouseholdMemberRelationshipAdditionalFields extends AdditionalFields
+ with HouseholdMemberRelationshipAdditionalFieldsMappable {Also applies to: 32-32, 45-46, 102-103
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/models/entities/household_member_relationship.dart
at lines 9-10, 32, 45-46, and 102-103, the class and related identifiers use the
incorrect spelling "RelationShip" instead of the correct "Relationship". Rename
all occurrences of "RelationShip" to "Relationship" consistently throughout
these lines to fix the grammatical error.
| // TODO: need to fetch from server response of identifier | ||
| clientAuditDetails: e | ||
| .clientAuditDetails ?? // NOTE: Using auditDetails as clientAuditDetails as it is null from backend | ||
| (e.auditDetails != null | ||
| ? ClientAuditDetails( | ||
| createdBy: e.auditDetails!.createdBy, | ||
| createdTime: e.auditDetails!.createdTime, | ||
| lastModifiedBy: e.auditDetails!.createdBy, | ||
| lastModifiedTime: | ||
| e.auditDetails!.lastModifiedTime) | ||
| : null)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate TODO: Same audit details workaround as in bulkCreate
This is the same workaround as in the bulkCreate method. Consider extracting this logic to a shared method to maintain consistency.
Create a helper method to handle identifier companion creation:
IdentifierCompanion _createIdentifierCompanion(
IdentifierModel identifier,
String individualId,
String individualClientReferenceId,
ClientAuditDetails? fallbackClientAuditDetails,
) {
return identifier
.copyWith(
individualId: individualId,
individualClientReferenceId: individualClientReferenceId,
clientAuditDetails: identifier.clientAuditDetails ??
(identifier.auditDetails != null
? ClientAuditDetails(
createdBy: identifier.auditDetails!.createdBy,
createdTime: identifier.auditDetails!.createdTime,
lastModifiedBy: identifier.auditDetails!.lastModifiedBy,
lastModifiedTime: identifier.auditDetails!.lastModifiedTime,
)
: fallbackClientAuditDetails),
)
.companion;
}🤖 Prompt for AI Agents
In packages/digit_data_model/lib/data/repositories/local/individual.dart around
lines 432 to 442, the code duplicates the workaround for setting
clientAuditDetails from auditDetails when clientAuditDetails is null, similar to
the bulkCreate method. To fix this, extract this logic into a shared helper
method that takes an IdentifierModel and related parameters, constructs the
clientAuditDetails properly by checking for nulls, and returns the
IdentifierCompanion. Replace the duplicated code with calls to this new helper
method to ensure consistency and reduce redundancy.
| if (kDebugMode) { | ||
| print( | ||
| "Failed to add columns for householdType, attendance - localityCode, and service - relatedClientReferenceId"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in migration logic
The catch blocks only print to debug console. Consider proper error logging or propagating the error to ensure migration failures are properly tracked.
} catch (e) {
- if (kDebugMode) {
- print("Failed to add columns for householdType, attendance - localityCode, and service - relatedClientReferenceId");
- }
+ if (kDebugMode) {
+ print("Failed to add columns: $e");
+ }
+ // Consider logging to a proper error tracking system
+ // or re-throwing the error if migration should fail
+ rethrow;
}Also applies to: 269-273, 274-279
🤖 Prompt for AI Agents
In packages/digit_data_model/lib/data/local_store/sql_store/sql_store.dart
around lines 194-199, 269-273, and 274-279, the catch blocks currently only
print error messages in debug mode, which is insufficient for tracking migration
failures. Replace these print statements with proper error logging using a
logging framework or propagate the caught exceptions so that migration errors
are recorded and can be handled appropriately. This ensures migration failures
are not silently ignored and can be diagnosed effectively.
| @@ -97,11 +99,13 @@ | |||
| HFReferral, | |||
| Household, | |||
| HouseholdMember, | |||
| HouseholdMemberRelationShip, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix table name to match corrected model naming
The table name should be updated to match the corrected model naming convention.
- HouseholdMemberRelationShip,
+ HouseholdMemberRelationship,📝 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.
| HouseholdMemberRelationShip, | |
| HouseholdMemberRelationship, |
🤖 Prompt for AI Agents
In packages/digit_data_model/lib/data/local_store/sql_store/sql_store.dart at
line 102, the table name HouseholdMemberRelationShip does not match the
corrected model naming convention. Update the table name to reflect the
corrected model name exactly, ensuring consistency between the model and the
table name used in the code.
| if (updatedEntity is ServiceModel) { | ||
| final attributeIds = e.additionalIds | ||
| .where((element) => element.idType == serviceAttributesIdKey) | ||
| .map((e) => e.id) | ||
| .toList(); | ||
|
|
||
| updatedEntity = updatedEntity.copyWith( | ||
| attributes: updatedEntity.attributes?.asMap().entries.map((entry) { | ||
| final index = entry.key; | ||
| final attribute = entry.value; | ||
|
|
||
| final attributeId = (index < attributeIds.length) | ||
| ? attributeIds[index] | ||
| : serverGeneratedId; | ||
|
|
||
| return attribute.copyWith( | ||
| id: attribute.id ?? attributeId, | ||
| ); | ||
| }).toList(), | ||
| ); | ||
| return updatedEntity; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Attribute ID fallback looks wrong
When attribute.id is null and there is no matching element in attributeIds, the code substitutes serverGeneratedId (the service id) as the attribute id.
This will store the same id on every attribute.
Consider defaulting to attribute.id (keeps null) or attributeIds.elementAtOrNull(index) instead.
🤖 Prompt for AI Agents
In apps/health_campaign_field_worker_app/lib/data/sync_service_mapper.dart
around lines 920 to 941, the fallback for attribute.id uses serverGeneratedId
when no matching attributeId exists, causing all such attributes to share the
same id. To fix this, change the fallback to use attribute.id itself (which may
be null) or attributeIds.elementAtOrNull(index) to avoid assigning the service
id to multiple attributes. Update the code to select attribute.id if not null,
otherwise attributeIds.elementAtOrNull(index), and only fallback to
serverGeneratedId if absolutely necessary.
| e.identifiers?.sort((a, b) => a.clientAuditDetails!.lastModifiedTime! | ||
| .compareTo(b.clientAuditDetails!.lastModifiedTime!)); | ||
|
|
There was a problem hiding this comment.
Null-safety crash in identifier sort
a.clientAuditDetails!.lastModifiedTime! force-unwraps two nullable levels.
If any identifier lacks audit details or timestamps, this blows up at runtime.
Guard the values or use ?? 0 in the comparator.
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/blocs/household_overview/household_overview.dart
around lines 145 to 147, the sort comparator force-unwraps nullable fields
clientAuditDetails and lastModifiedTime, which can cause runtime crashes if
these are null. Modify the comparator to safely handle nulls by using null-aware
operators and providing a default value like 0 for lastModifiedTime when null,
ensuring the sort does not throw exceptions.
| for (final e in results) { | ||
| final householdMember = e.readTable(sql.householdMember); | ||
|
|
||
| // Fetch relationships based on clientReferenceId | ||
| final val = await (sql.select(sql.householdMemberRelationShip) | ||
| ..where( | ||
| (tbl) => tbl.selfClientReferenceId.equals( | ||
| householdMember.clientReferenceId ?? '', | ||
| ), | ||
| )) | ||
| .get(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid an N + 1 query in search()
Fetching relationships inside the loop issues one SQL call per household member.
Collect the clientReferenceIds first and fetch all relationships with a single WHERE selfClientReferenceId IN (...) to cut DB round-trips and speed up large searches.
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/data/repositories/local/household_member.dart
around lines 70 to 80, the code fetches household member relationships inside a
loop, causing an N+1 query problem. To fix this, first collect all
clientReferenceIds from the household members, then perform a single query to
fetch all relationships where selfClientReferenceId is in that collected list.
Finally, map these relationships back to their respective household members to
avoid multiple database calls and improve performance.
| return HouseholdMemberRelationShipModel( | ||
| selfClientReferenceId: relation.selfClientReferenceId, | ||
| relationshipType: relation.relationshipType, | ||
| relativeClientReferenceId: relation.relativeClientReferenceId, | ||
| tenantId: relation.tenantId, | ||
| isDeleted: relation.isDeleted, | ||
| rowVersion: relation.rowVersion, | ||
| clientReferenceId: relation.clientReferenceId, | ||
| clientAuditDetails: (relation.clientCreatedBy != null && | ||
| relation.clientCreatedTime != null) | ||
| ? ClientAuditDetails( | ||
| createdBy: relation.clientCreatedBy!, | ||
| createdTime: relation.clientCreatedTime!, | ||
| lastModifiedBy: relation.clientModifiedBy, | ||
| lastModifiedTime: relation.clientModifiedTime, | ||
| ) | ||
| : null, | ||
| auditDetails: (relation.auditCreatedBy != null && | ||
| relation.auditCreatedTime != null) | ||
| ? AuditDetails( | ||
| createdBy: relation.auditCreatedBy!, | ||
| createdTime: relation.auditCreatedTime!, | ||
| lastModifiedBy: relation.auditModifiedBy, | ||
| lastModifiedTime: relation.auditModifiedTime, | ||
| ) | ||
| : null, | ||
| additionalFields: relation.additionalFields != null | ||
| ? HouseholdMemberRelationShipAdditionalFieldsMapper.fromJson( | ||
| relation.additionalFields!) | ||
| : null, | ||
| ); | ||
| return null; | ||
| }).whereType<HouseholdMemberRelationShipModel>().toList(); |
There was a problem hiding this comment.
Remove unreachable code and return the built model
return null; after already returning HouseholdMemberRelationShipModel is dead code and will trigger linter warnings.
- );
- 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.
| return HouseholdMemberRelationShipModel( | |
| selfClientReferenceId: relation.selfClientReferenceId, | |
| relationshipType: relation.relationshipType, | |
| relativeClientReferenceId: relation.relativeClientReferenceId, | |
| tenantId: relation.tenantId, | |
| isDeleted: relation.isDeleted, | |
| rowVersion: relation.rowVersion, | |
| clientReferenceId: relation.clientReferenceId, | |
| clientAuditDetails: (relation.clientCreatedBy != null && | |
| relation.clientCreatedTime != null) | |
| ? ClientAuditDetails( | |
| createdBy: relation.clientCreatedBy!, | |
| createdTime: relation.clientCreatedTime!, | |
| lastModifiedBy: relation.clientModifiedBy, | |
| lastModifiedTime: relation.clientModifiedTime, | |
| ) | |
| : null, | |
| auditDetails: (relation.auditCreatedBy != null && | |
| relation.auditCreatedTime != null) | |
| ? AuditDetails( | |
| createdBy: relation.auditCreatedBy!, | |
| createdTime: relation.auditCreatedTime!, | |
| lastModifiedBy: relation.auditModifiedBy, | |
| lastModifiedTime: relation.auditModifiedTime, | |
| ) | |
| : null, | |
| additionalFields: relation.additionalFields != null | |
| ? HouseholdMemberRelationShipAdditionalFieldsMapper.fromJson( | |
| relation.additionalFields!) | |
| : null, | |
| ); | |
| return null; | |
| }).whereType<HouseholdMemberRelationShipModel>().toList(); | |
| return HouseholdMemberRelationShipModel( | |
| selfClientReferenceId: relation.selfClientReferenceId, | |
| relationshipType: relation.relationshipType, | |
| relativeClientReferenceId: relation.relativeClientReferenceId, | |
| tenantId: relation.tenantId, | |
| isDeleted: relation.isDeleted, | |
| rowVersion: relation.rowVersion, | |
| clientReferenceId: relation.clientReferenceId, | |
| clientAuditDetails: (relation.clientCreatedBy != null && | |
| relation.clientCreatedTime != null) | |
| ? ClientAuditDetails( | |
| createdBy: relation.clientCreatedBy!, | |
| createdTime: relation.clientCreatedTime!, | |
| lastModifiedBy: relation.clientModifiedBy, | |
| lastModifiedTime: relation.clientModifiedTime, | |
| ) | |
| : null, | |
| auditDetails: (relation.auditCreatedBy != null && | |
| relation.auditCreatedTime != null) | |
| ? AuditDetails( | |
| createdBy: relation.auditCreatedBy!, | |
| createdTime: relation.auditCreatedTime!, | |
| lastModifiedBy: relation.auditModifiedBy, | |
| lastModifiedTime: relation.auditModifiedTime, | |
| ) | |
| : null, | |
| additionalFields: relation.additionalFields != null | |
| ? HouseholdMemberRelationShipAdditionalFieldsMapper.fromJson( | |
| relation.additionalFields!) | |
| : null, | |
| ); | |
| }).whereType<HouseholdMemberRelationShipModel>().toList(); |
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/data/repositories/local/household_member.dart
between lines 85 and 117, remove the unreachable `return null;` statement that
appears immediately after returning a `HouseholdMemberRelationShipModel`. This
dead code is unnecessary and causes linter warnings. Ensure only the constructed
model is returned within the mapping function.
| LocalRepository<UserActionModel, UserActionSearchModel>>(), | ||
| context | ||
| .read<LocalRepository<ServiceModel, ServiceSearchModel>>(), | ||
| ], |
There was a problem hiding this comment.
Remove duplicate repository entries
ServiceModel (and PgrServiceModel) are pushed twice into both localRepositories and remoteRepositories.
This yields redundant API calls and can break index-based pairing in the sync layer.
- context.read<LocalRepository<ServiceModel, ServiceSearchModel>>(),
...
- context.read<RemoteRepository<ServiceModel, ServiceSearchModel>>(),(keep only one occurrence in each list)
Also applies to: 717-718
🤖 Prompt for AI Agents
In apps/health_campaign_field_worker_app/lib/pages/home.dart around lines 681 to
684 and also lines 717 to 718, the ServiceModel and PgrServiceModel repositories
are duplicated in both localRepositories and remoteRepositories lists. Remove
the duplicate entries so that each repository appears only once in each list to
avoid redundant API calls and maintain correct index-based pairing in the sync
layer.
| selectQuery = selectQuery.join([joinName(sql), joinIdentifier(sql)]); | ||
| selectQuery = searchByName(selectQuery, params, sql); | ||
| } | ||
| } | ||
| return selectQuery; | ||
| } |
There was a problem hiding this comment.
Duplicate identifier JOIN may break the query
searchByIdentifierId() already joins the identifier table; nameSearch() joins it again when selectQuery != null, leading to the same table being joined twice and Drift throwing an ambiguous column error.
Guard with a flag or refactor to add the join only once.
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/data/repositories/local/individual_global_search.dart
around lines 352 to 357, the code joins the 'identifier' table twice due to both
searchByIdentifierId() and nameSearch() adding the join, causing an ambiguous
column error. Fix this by adding a boolean flag or condition to ensure the
'identifier' table is joined only once before returning selectQuery, preventing
duplicate joins and query errors.
| if (params.identifierId == null || params.identifierId!.isEmpty) { | ||
| return selectQuery; | ||
| } else if (params.identifierId != null || | ||
| params.identifierId!.isNotEmpty && selectQuery == null) { | ||
| selectQuery = super.sql.individual.select().join([joinIdentifier(sql)]); | ||
| await searchByBeneficiary(selectQuery, params, sql); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Logical precedence bug – condition always true
if (params.identifierId != null || params.identifierId!.isNotEmpty && selectQuery == null)
Because && binds tighter than ||, the left part (params.identifierId != null) alone makes the whole expression true, so the branch executes even for an empty string. Parenthesise properly:
if (params.identifierId != null &&
params.identifierId!.isNotEmpty &&
selectQuery == null)Same pattern exists in nameSearch.
🤖 Prompt for AI Agents
In
packages/registration_delivery/lib/data/repositories/local/individual_global_search.dart
around lines 398 to 403, the condition using `||` and `&&` operators is
incorrectly grouped, causing the branch to always execute when
`params.identifierId` is not null, even if empty. Fix this by replacing the
condition with properly parenthesized checks using `&&` for all parts: ensure
`params.identifierId` is not null, not empty, and `selectQuery` is null before
executing the branch. Apply the same fix to the similar condition in the
`nameSearch` method.
Removed curllogger
Fixed code rabbit comments Removed extra BeneficiaryTypes
No description provided.