Added ARC Audit #667
Added ARC Audit #667
Conversation
Signed-off-by: Rakshithasai123 <[email protected]>
WalkthroughAdds widespread audit instrumentation across Android services, DI wiring, and Flutter UI; expands AuditEvent enum with many new event constants; updates PacketAuthenticationApi and PreRegistrationDataSyncService constructors to accept additional dependencies. Changes
Sequence Diagram(s)(Skipped — changes are audit instrumentation spread across many locations and do not introduce a single new multi-component control flow requiring visualization.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
android/app/src/main/java/io/mosip/registration_client/MainActivity.java (1)
339-343: Audit event placement during Flutter engine configuration is problematic.The
NAV_REDIRECT_HOMEevent is logged inconfigureFlutterEngine(), which is a framework lifecycle hook that Android invokes multiple times during the app's lifetime—including during activity recreation from orientation changes or process death—rather than during user-initiated navigation. This creates misleading audit trails.Key issues:
- Multiple invocations:
configureFlutterEngine()fires multiple times per session, causing redundant audit entries for a single app initialization.- Semantic mismatch: The event name suggests user navigation, but this is a framework initialization hook (correctly classified as
SYSTEM_EVENT, but the name is misleading).- No app-entry event: The codebase has no dedicated event for app initialization/entry, and
onCreate()is not currently used for audit logging.Either move this audit to
onCreate()(which fires once per activity instance) with appropriate session tracking, or rename the event to reflect engine initialization rather than user navigation.android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java (1)
48-60: Non-atomic check-and-set on audit flag creates race condition potential.The
isDemoCaptureAuditedflag check (lines 49-53) is not atomic. SinceaddDemographicFieldis called via Pigeon platform channel from Flutter with multiple concurrent field updates (textbox, dropdown, checkbox, date, gender controls), concurrent threads can both observe the flag as false and execute the audit code, resulting in duplicate audit event logging.Use
AtomicBooleanwithcompareAndSet()to safely guard the one-time audit:import java.util.concurrent.atomic.AtomicBoolean; private final AtomicBoolean isDemoCaptureAudited = new AtomicBoolean(false); if (isDemoCaptureAudited.compareAndSet(false, true)) { auditManagerService.audit(AuditEvent.REG_DEMO_CAPTURE, Components.REGISTRATION); Log.i("AUDIT", "Logged REG_DEMO_CAPTURE - Started capturing demographic details"); }android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java (1)
84-95: Fix the missing constructor parameter causing compilation error.The constructor assigns
this.auditManagerServiceon line 94, but theAuditManagerServiceparameter is missing from the constructor signature (lines 85-87). This will cause a compilation error.🐛 Proposed fix
@Inject public PacketServiceImpl(Context context, RegistrationRepository registrationRepository, IPacketCryptoService packetCryptoService, SyncRestService syncRestService, - MasterDataService masterDataService, GlobalParamRepository globalParamRepository) { + MasterDataService masterDataService, GlobalParamRepository globalParamRepository, + AuditManagerService auditManagerService) { this.context = context; this.registrationRepository = registrationRepository; this.packetCryptoService = packetCryptoService; this.syncRestService = syncRestService; this.masterDataService = masterDataService; this.globalParamRepository = globalParamRepository; this.auditManagerService = auditManagerService; }
🤖 Fix all issues with AI agents
In
@android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java:
- Line 35: The isDemoCaptureAudited flag is incorrectly stored as an instance
field in the @Singleton DemographicsDetailsApi; move it into the
per-registration model by adding a boolean field (e.g., isDemoCaptureAudited)
with getter/setter in RegistrationDto, then update DemographicsDetailsApi to
remove its private isDemoCaptureAudited field and instead read/write
registrationDto.isDemoCaptureAudited()/setDemoCaptureAudited(true) at the points
where REG_DEMO_CAPTURE is logged; ensure the flag is initialized false for new
RegistrationDto instances so each registration can be audited independently.
In
@android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java:
- Around line 40-41: PacketAuthenticationApi currently injects and assigns a
userDetailRepository field and imports UserDetail and UserDetailRepository even
though they are never used; remove the unused imports (UserDetailRepository,
UserDetail), delete the userDetailRepository field and its constructor
parameter/assignment in PacketAuthenticationApi, and update any constructor
calls to stop passing that dependency so the class no longer depends on or
references UserDetailRepository/UserDetail.
In
@android/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.java:
- Around line 51-77: The debug logging in RegistrationApi.startRegistration is
inconsistent and overly verbose: change the default-case Log.w message to
reference flowType or flowTypeUpper instead of process, and reduce verbosity by
demoting or removing the many Log.d calls (those logging startRegistration
inputs and each switch match) — keep only the important warning/notice and
optionally a single trimmed info/trace entry for entry/selected flow; update
references to flowType/flowTypeUpper in the messages so they reflect the value
being switched on.
In @android/app/src/main/java/io/mosip/registration_client/HostApiModule.java:
- Line 75: Remove the duplicate import of UserDetailRepository in HostApiModule:
locate the redundant import statement "import
io.mosip.registration.clientmanager.repository.UserDetailRepository;" and delete
it so only the single original import (the one at the earlier import block)
remains, ensuring no other imports or code references are changed.
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java:
- Around line 115-117: The REG_DOC_POE_* audit event entries (REG_DOC_POE_SCAN,
REG_DOC_POE_VIEW, REG_DOC_POE_DELETE) have copy-paste descriptions using "PoB"
instead of the correct "PoE"; update the description strings for these constants
to use "PoE: Click of Scan", "PoE: View", and "PoE: Delete" respectively so the
event descriptions correctly reflect Proof of Exception.
- Around line 25-30: Replace the hardcoded audit ID string passed to
globalProvider.getAudit with the corresponding AuditEvent enum constant;
specifically change the call that currently uses "REG-AUTH-002" to use
AuditEvent.LOGIN_WITH_PASSWORD (and similarly replace "REG-MOD-101" with the
appropriate enum constant if applicable) so the code uses the AuditEvent enum
rather than brittle string literals.
In @lib/ui/process_ui/widgets/document_upload_control.dart:
- Around line 52-148: Rename private method _DocumentAudit to _documentAudit and
update all call sites; fix spacing around = in declarations for eventId and
event (use e.g. String eventId = "" and String event = ""); change the final
conditional in _documentAudit from checking event != null to event.isNotEmpty;
ensure default assignments use a single space after = and consistent formatting
throughout.
In @lib/ui/scanner/custom_scanner.dart:
- Around line 41-43: The _documentScanFailedAudit() function is calling getAudit
with the wrong event id; replace the first argument "REG-EVT-049" with the
correct event code "REG-EVT-050" (DOCUMENT_SCAN_FAILED) in the call to
context.read<GlobalProvider>().getAudit("REG-EVT-049", "REG-MOD-103") so it
becomes getAudit("REG-EVT-050", "REG-MOD-103").
- Around line 204-208: The preview audit is currently set once via
_previewAuditLogged in _uploadImage(), preventing audits on subsequent "Retake"
actions; decide intended behavior and implement it: if each retake should
produce a preview audit, reset _previewAuditLogged to false in the retake
handler (the method invoked by the "Retake" button) before calling
_uploadImage(); if only the first preview should be audited, leave the flag
as-is but add a clarifying comment near _previewAuditLogged and _uploadImage()
to document the intent. Also decide whether getAudit(...) inside _uploadImage()
should be awaited: if it must complete, await the call (or propagate the
future); if it should remain fire-and-forget, explicitly use an unawaited
pattern or comment to make the choice clear.
🧹 Nitpick comments (5)
lib/ui/scanner/custom_scanner.dart (1)
114-114: Remove commented-out code.Since the preview audit logic has been moved to
_uploadImage(lines 204-208), this commented call is no longer needed and should be removed to improve readability.🧹 Proposed cleanup
- //_documentPreviewAudit();lib/ui/onboard/home_page.dart (1)
135-135: Remove commented-out code or document the reason for commenting.The commented audit call should either be removed entirely if no longer needed, or accompanied by a TODO/FIXME comment explaining why it's temporarily disabled. Leaving commented code without explanation makes maintenance harder.
🧹 Proposed cleanup
If the audit call is permanently removed:
- //globalProvider.getAudit("REG-HOME-002", "REG-MOD-102");Or if temporarily disabled, add context:
- //globalProvider.getAudit("REG-HOME-002", "REG-MOD-102"); + // TODO(RCF-1277): Audit moved to native layer - remove this after verificationandroid/app/src/main/java/io/mosip/registration_client/api_services/AuthenticationApi.java (1)
201-202: Consider consolidating redundant audit calls.Both
LOGIN_AUTHENTICATE_USER_IDandLOGIN_WITH_PASSWORDare logged at method entry. If these represent the same authentication attempt, consider consolidating into a single audit event to reduce noise in audit logs.lib/ui/process_ui/generic_process.dart (1)
102-102: Clarify the commented-out audit call.The
_registrationScreenLoadedAudit()call is commented out rather than removed. If this audit event is no longer needed, remove the line entirely. If it's temporarily disabled, add a comment explaining why and when it should be re-enabled.lib/ui/process_ui/widgets/document_upload_control.dart (1)
369-372: Simplify the audit method signature.The
_documentScanClickedAuditmethod is no longerasyncbut still could be simplified since it just calls two other methods synchronously.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
android/app/src/main/java/io/mosip/registration_client/HostApiModule.javaandroid/app/src/main/java/io/mosip/registration_client/MainActivity.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/AuthenticationApi.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/BiometricsDetailsApi.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/DynamicDetailsApi.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.javalib/ui/onboard/home_page.dartlib/ui/process_ui/generic_process.dartlib/ui/process_ui/widgets/document_upload_control.dartlib/ui/scanner/custom_scanner.dart
🧰 Additional context used
🧬 Code graph analysis (3)
android/app/src/main/java/io/mosip/registration_client/HostApiModule.java (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/UserDetailRepository.java (1)
UserDetailRepository(29-238)
android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java (2)
android/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.java (1)
Singleton(34-185)android/app/src/main/java/io/mosip/registration_client/api_services/AuditDetailsApi.java (1)
Singleton(25-54)
android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/UserDetailRepository.java (1)
UserDetailRepository(29-238)
🔇 Additional comments (15)
lib/ui/scanner/custom_scanner.dart (1)
39-39: LGTM: Preview audit guard flag.The
_previewAuditLoggedflag is a clean solution to prevent duplicate preview audits.android/app/src/main/java/io/mosip/registration_client/api_services/BiometricsDetailsApi.java (1)
520-531: LGTM! Appropriate audit placement for operator onboarding.The audit event is correctly placed at the start of the operator onboarding flow, logging the navigation intent before any business logic executes. This follows the auditing pattern established in other API services.
android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java (1)
227-229: LGTM! Appropriate audit placement for master data sync.The audit event is correctly placed at the start of the master data sync operation, providing a clear audit trail of when synchronization is initiated.
android/app/src/main/java/io/mosip/registration_client/api_services/DynamicDetailsApi.java (1)
235-236: LGTM!The audit logging for pre-registration data fetch is correctly implemented with appropriate event and component identifiers.
android/app/src/main/java/io/mosip/registration_client/api_services/AuthenticationApi.java (3)
109-109: Verify the component classification for navigation event.The
NAV_HOMEevent usesComponents.REGISTRATION, but this occurs in the authentication flow withinAuthenticationApi. Confirm whetherComponents.LOGINwould be more appropriate, or if navigation events should always useComponents.REGISTRATION.
179-179: Verify the component classification for navigation event.Same concern as in
doLogin: theNAV_HOMEevent usesComponents.REGISTRATIONin the authentication context. Verify this is the intended component for navigation events after login.
218-218: LGTM!The logout audit is correctly placed after clearing the auth token.
lib/ui/process_ui/generic_process.dart (3)
228-228: Audit event updated.The audit event ID changed to
REG-EVT-069. Ensure this aligns with the updated audit event taxonomy referenced in the PR objectives.
241-247: LGTM!The audit logging for navigating back from the Documents screen is correctly implemented with proper boundary checks.
690-696: LGTM!The audit logging for advancing from the Documents screen is correctly implemented with appropriate event tracking before navigation.
android/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.java (2)
132-132: LGTM!The audit logging for registration submission is appropriately placed at method entry.
178-178: LGTM!The audit logging for geolocation capture is correctly positioned before setting the machine location.
android/app/src/main/java/io/mosip/registration_client/HostApiModule.java (1)
163-164: LGTM! Dependency wiring updated correctly.The
UserDetailRepositoryhas been properly added to the dependency injection configuration and passed to thePacketAuthenticationApiconstructor.android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java (1)
152-152: LGTM! Audit logging correctly instrumented.The audit events are appropriately logged:
NAV_UPLOAD_PACKETSwhen the upload flow beginsPACKET_UPLOADon successful completion of each packet uploadNAV_APPROVE_REGwhen retrieving created registration packetsAlso applies to: 174-174, 221-221
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java (1)
32-124: LGTM! Comprehensive audit taxonomy expansion.The addition of navigation events (NAV_), document events (REG_DOC_), demographic events (REG_DEMO_), and preview events (REG_PREVIEW_) provides comprehensive audit coverage across the registration workflow. This aligns well with the broader PR objective of adding audit logging throughout the application.
android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java
Outdated
Show resolved
Hide resolved
...oid/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java
Show resolved
Hide resolved
android/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.java
Outdated
Show resolved
Hide resolved
android/app/src/main/java/io/mosip/registration_client/HostApiModule.java
Outdated
Show resolved
Hide resolved
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
Signed-off-by: Rakshithasai123 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java:
- Line 38: The AuditEvent enum contains a duplicate event ID: NAV_CORRECTION
currently uses "REG-EVT-007" which is also used by REG_DEMO_CAPTURE; update one
of these to a unique unused ID (replace the placeholder XXX with a new ID) and
correct the REG_DEMO_CAPTURE name field from "REG_DEMO_NEXT" to
"REG_DEMO_CAPTURE"; specifically modify the NAV_CORRECTION and REG_DEMO_CAPTURE
enum entries so each has a distinct code string and the REG_DEMO_CAPTURE entry
has the correct name value.
- Around line 115-117: The enum entries REG_DOC_POE_SCAN, REG_DOC_POE_VIEW, and
REG_DOC_POE_DELETE have copy-paste errors: their name strings and human-readable
descriptions reference "POB" and "PoB" instead of "POE" and "PoE"; update the
second constructor argument values (currently "REG_DOC_POB_SCAN",
"REG_DOC_POB_VIEW", "REG_DOC_POB_DELETE") to "REG_DOC_POE_SCAN",
"REG_DOC_POE_VIEW", "REG_DOC_POE_DELETE" and change the description strings from
"PoB: Click of Scan"/"PoB: View"/"PoB: Delete" to "PoE: Click of Scan"/"PoE:
View"/"PoE: Delete" respectively for the enum constants REG_DOC_POE_SCAN,
REG_DOC_POE_VIEW, and REG_DOC_POE_DELETE.
In @lib/ui/process_ui/widgets/document_upload_control.dart:
- Around line 149-152: The call to getAudit uses an unnecessary null assertion:
remove the `!` on `event` in the call
`context.read<GlobalProvider>().getAudit(event!, "REG-MOD-103")` because `event`
is declared as a non-nullable `String event = ""` and is assigned in the switch,
so it can never be null; update the invocation to pass `event` directly (i.e.,
`getAudit(event, "REG-MOD-103")`) in the DocumentUploadControl logic.
🧹 Nitpick comments (2)
lib/ui/process_ui/widgets/document_upload_control.dart (2)
374-377: Async methods called withoutawait- fire-and-forget pattern.
_documentAuditand_logDocumentAuditare async methods but are called withoutawait. This fire-and-forget pattern means:
- Audit logging might not complete before navigation or state changes
- Errors in audit calls are silently ignored
If audit completion is required before proceeding, consider awaiting these calls. If fire-and-forget is intentional for non-blocking UX, this is acceptable but should be documented.
Option A: Await the calls if audit must complete first
- _documentScanClickedAudit() { - _documentAudit("SCAN"); - _logDocumentAudit("SCAN"); - } + Future<void> _documentScanClickedAudit() async { + await _documentAudit("SCAN"); + await _logDocumentAudit("SCAN"); + }Option B: Use unawaited for explicit fire-and-forget
+ import 'dart:async' show unawaited; ... _documentScanClickedAudit() { - _documentAudit("SCAN"); - _logDocumentAudit("SCAN"); + unawaited(_documentAudit("SCAN")); + unawaited(_logDocumentAudit("SCAN")); }
52-131: Consider using a Map for event ID lookup to reduce complexity.The nested switch statements are verbose and error-prone. A
Map<String, Map<String, String>>could simplify maintenance and reduce the risk of copy-paste errors.Proposed refactor using a Map
static const _docAuditEvents = { "SCAN": {"POA": "REG-EVT-010", "POI": "REG-EVT-013", "POR": "REG-EVT-016", "POB": "REG-EVT-019", "POE": "REG-EVT-022"}, "VIEW": {"POA": "REG-EVT-011", "POI": "REG-EVT-014", "POR": "REG-EVT-017", "POB": "REG-EVT-020", "POE": "REG-EVT-023"}, "DELETE": {"POA": "REG-EVT-012", "POI": "REG-EVT-015", "POR": "REG-EVT-018", "POB": "REG-EVT-021", "POE": "REG-EVT-024"}, }; Future<void> _logDocumentAudit(String action) async { final subType = (widget.field.subType ?? "").toUpperCase(); final eventId = _docAuditEvents[action]?[subType] ?? ""; if (eventId.isNotEmpty) { await context.read<GlobalProvider>().getAudit(eventId, "REG-MOD-103"); } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
android/app/src/main/java/io/mosip/registration_client/HostApiModule.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.javaandroid/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.javalib/ui/process_ui/widgets/document_upload_control.dartlib/ui/scanner/custom_scanner.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- android/app/src/main/java/io/mosip/registration_client/HostApiModule.java
- lib/ui/scanner/custom_scanner.dart
- android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java
🔇 Additional comments (4)
android/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.java (3)
50-72: LGTM - flow-type driven audit implementation looks correct.The switch on
flowTypeUppercorrectly maps registration flow types to corresponding navigation audit events. The warning log for unmatched flow types is a reasonable fallback.Minor: Consider removing the extra blank lines (51-52, 54-55) for cleaner formatting.
126-127: LGTM - Audit correctly placed before submission logic.The
REG_DEMO_NEXTaudit is appropriately placed at the method start to track when demographics submission is initiated.
172-173: LGTM - Geo-location audit added appropriately.android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java (1)
25-48: New navigation and authentication events follow the established pattern correctly.The new
LOGIN_AUTHENTICATE_USER_ID, navigation events (NAV_*), and authentication events at these lines are well-structured with consistent parameter formatting and appropriate event type assignments. No hardcoded references to the event IDs exist elsewhere in the codebase that could create maintenance issues.
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
Signed-off-by: Rakshithasai123 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/config/RoomModule.java:
- Around line 113-120: Restore SQLCipher encryption by re-enabling the
SupportFactory and migration when building the Room database: use the existing
dbPwd (the password generated in the block around lines 99-111) and pass
dbPwd.getBytes() into new SupportFactory(...), re-add
ClientDatabaseMigrations.MIGRATION_1_2 to the Room.databaseBuilder chain, and
keep allowMainThreadQueries() as before; also either call encryptExistingDb()
where appropriate to migrate any plaintext DB to the encrypted DB or remove the
unused encryptExistingDb() method and the SupportFactory import if you
intentionally won't support encryption (preferred fix is to restore encryption).
Ensure clientDatabase is created with SupportFactory and migrations so the
database remains encrypted and no dead code remains.
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java:
- Around line 79-82: The REG_DEMO_CAPTURE enum entry has a copy-paste error: its
name field is set to "REG_DEMO_NEXT" and its description contains a typo
"FvStarted"; update the REG_DEMO_CAPTURE declaration so the second argument for
the name is the correct unique name "REG_DEMO_CAPTURE" (not "REG_DEMO_NEXT") and
fix the description to "Started capturing demographic details" to avoid
duplicate event names and correct the typo.
- Around line 115-117: The AuditEvent enum entries REG_DOC_POE_SCAN,
REG_DOC_POE_VIEW, and REG_DOC_POE_DELETE currently use the wrong
name/description strings referencing "POB"; update those entries so their third
and fourth parameters reflect "REG_DOC_POE_*"/"PoE: ..." respectively (i.e.,
replace "REG_DOC_POB_SCAN"/"PoB: Click of Scan", "REG_DOC_POB_VIEW"/"PoB: View",
and "REG_DOC_POB_DELETE"/"PoB: Delete" with the corresponding
"REG_DOC_POE_SCAN"/"PoE: Click of Scan", "REG_DOC_POE_VIEW"/"PoE: View", and
"REG_DOC_POE_DELETE"/"PoE: Delete") so PoE actions are logged correctly.
🧹 Nitpick comments (2)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java (1)
122-129: Consider leveraging Lombok and improving field placement.The class already uses
@Getterand@Setterat the class level, so Lombok will auto-generateisDemoCaptureAudited()andsetDemoCaptureAudited()for this boolean field. The manual getter/setter can be removed.Additionally, for consistency with the existing class structure, move the field declaration to the field section (around lines 70-91) where other instance fields are grouped.
♻️ Suggested simplification
Move field to the fields section (e.g., after line 91) and remove manual getter/setter:
+ private boolean demoCaptureAudited = false; + public RegistrationDto(@NonNull String rid, @NonNull String flowType, @NonNull String process,Then remove the redundant methods:
- private boolean demoCaptureAudited = false; - - public boolean isDemoCaptureAudited() { - return demoCaptureAudited; - } - public void setDemoCaptureAudited(boolean audited) { - this.demoCaptureAudited = audited; - }Lombok will generate the equivalent methods automatically.
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java (1)
66-77: Minor: Enum constant and name field mismatch.At line 72, the enum constant
BIOMETRIC_CAPTUREhas a name field value of"BIOMETRIC_CAPTURE_INITIATED". Consider aligning these for consistency unless the difference is intentional for audit log clarity.♻️ Suggested alignment
- BIOMETRIC_CAPTURE("REG-EVT-107", USER_EVENT.getCode(), "BIOMETRIC_CAPTURE_INITIATED", "Biometric capture initiated"), + BIOMETRIC_CAPTURE("REG-EVT-107", USER_EVENT.getCode(), "BIOMETRIC_CAPTURE", "Biometric capture initiated"),
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/config/RoomModule.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.javalib/ui/process_ui/widgets/document_upload_control.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ui/process_ui/widgets/document_upload_control.dart
- android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java
🔇 Additional comments (6)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/config/RoomModule.java (2)
358-361: Formatting-only change.
476-480: Formatting-only change.android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java (4)
25-30: LGTM!The authentication events are properly defined with appropriate IDs and descriptions.
32-48: LGTM!Navigation events are well-structured. The use of
SYSTEM_EVENTforNAV_HOMEandNAV_REDIRECT_HOMEappropriately distinguishes system-triggered navigation from user-initiated actions.
91-98: LGTM with minor observation.The system events section is well-defined. Note that
CREATE_PACKET_FAILEDusesUSER_EVENTwhile others in this section useSYSTEM_EVENT- this seems intentional as packet creation is user-initiated, but verify this aligns with your audit categorization requirements.
100-101: LGTM!The packet upload and generic document events are properly defined.
Also applies to: 118-124
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/config/RoomModule.java
Outdated
Show resolved
Hide resolved
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
Signed-off-by: Rakshithasai123 <[email protected]>
Signed-off-by: Rakshithasai123 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/ui/export_packet/widgets/export_button.dart (1)
44-57: Export proceeds even when network is disconnected.The audit call and
exportSelected()execute regardless of network connectivity. When the network check fails, only the snackbar is shown, but the code continues to audit and attempt export.Suggested fix
onPressed: () async { await connectivityProvider.checkNetworkConnection(); if (!connectivityProvider.isConnected) { _showInSnackBar(appLocalizations.network_error); + return; - }else{ + } - await exportPacketsProvider.packetSyncAll(); - } + await exportPacketsProvider.packetSyncAll(); await context.read<GlobalProvider>().getAudit( "REG-EXPT-PKT-001", "REG-MOD-103" ); await exportPacketsProvider.exportSelected(); exportPacketsProvider.searchedList(); },lib/ui/approve_packet/widget/template_bottom_sheet.dart (2)
358-384: Missing audit call for approve action in non-mobile layout.The mobile layout branch (line 147) includes an audit call
getAudit("REG-EVT-067", "REG-MOD-103")for the approve action, but the non-mobile layout branch here does not. Add the audit call for consistency:Suggested fix
onPressed: reviewStatus == ReviewStatus.APPROVED.name ? null : () { + globalProvider.getAudit("REG-EVT-067", "REG-MOD-103"); context .read<ApprovePacketsProvider>() .approvePacket(regCurrent.packetId);
392-426: Missing audit call for reject action in non-mobile layout.The mobile layout branch (line 185) includes an audit call
getAudit("REG-EVT-068", "REG-MOD-103")for the reject action, but the non-mobile layout branch here does not. Add the audit call for consistency:Suggested fix
onPressed: reviewStatus == ReviewStatus.REJECTED.name ? null : () { + globalProvider.getAudit("REG-EVT-068", "REG-MOD-103"); showDialog( context: context,
🤖 Fix all issues with AI agents
In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java`:
- Around line 56-58: The AuditEvent enum contains duplicate audit IDs —
SYNC_PACKET uses "REG-PKT-001" which is already used by PACKET_RETRIVE; update
one of them to a unique ID (and ensure the same correction is applied for the
similar duplicates around the other packet events later in the enum) by editing
the AuditEvent entries (e.g., SYNC_PACKET, PACKET_RETRIVE and the packet-related
entries near the end) so each constant has a distinct audit ID string.
- Around line 147-148: The AuditEvent enum contains duplicate audit ID
"REG-SYNC-014" for SYNC_SERVER and SYNC_GEO_VALIDATE; update one of these enum
entries (either SYNC_SERVER or SYNC_GEO_VALIDATE) to a unique audit ID (e.g.,
"REG-SYNC-015" or another unused code) so IDs no longer collide in logs, and
ensure the change is applied consistently where AuditEvent values are
referenced.
In `@lib/ui/export_packet/widgets/export_button.dart`:
- Around line 8-11: Remove the duplicate import of
package:flutter_gen/gen_l10n/app_localizations.dart at the top of
export_button.dart so it is only imported once; locate the two identical import
lines in the imports block of the ExportButton widget file and delete the
redundant one, then run the formatter/analyzer to ensure no unused-import
warnings remain.
In `@lib/ui/process_ui/widgets/device_settings_tab.dart`:
- Around line 30-31: fetchDeviceDetails() currently calls
context.read<GlobalProvider>().getAudit(...) which is unsafe from initState;
move the audit invocation out of fetchDeviceDetails and trigger it only after
the widget is mounted — either call GlobalProvider.getAudit from
didChangeDependencies (checking a mounted/one-time flag) or schedule it with
WidgetsBinding.instance.addPostFrameCallback in initState; update references in
fetchDeviceDetails, initState and/or didChangeDependencies to remove direct
context access during init and use the chosen post-mount approach.
♻️ Duplicate comments (2)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java (1)
80-80: REG_DEMO_CAPTURE uses the wrong name and has a typo.The name is set to
REG_DEMO_NEXT, and the description starts withFvStarted, which will mislabel audit entries.🐛 Proposed fix
- REG_DEMO_CAPTURE("REG-EVT-110", USER_EVENT.getCode(), "REG_DEMO_NEXT", "FvStarted capturing demographic details"), + REG_DEMO_CAPTURE("REG-EVT-110", USER_EVENT.getCode(), "REG_DEMO_CAPTURE", "Started capturing demographic details"),android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java (1)
40-68: Remove unusedUserDetailRepository/UserDetailinjection.This dependency and import still aren’t used anywhere in this class. Please drop the import, field, and constructor parameter unless a follow-up uses it.
🧹 Nitpick comments (5)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PreRegistrationDataSyncServiceImpl.java (1)
140-140: Consider adding audit events for sync completion and failure.The audit logs only the "started" event. For complete traceability, consider adding corresponding audit events in the success path (around line 158) and failure paths (lines 161-164, 168-170, 173-175, and
onFailureat lines 179-184).Example for success path (around line 158)
Toast.makeText(context, "Application Id Sync Completed", Toast.LENGTH_LONG).show(); auditManager.audit(AuditEvent.SYNC_PRE_REGISTRATION_PACKET, Components.REGISTRATION, "Sync pre-registration data completed successfully"); result = "";lib/provider/global_provider.dart (1)
906-909: Consider awaiting the audit call for consistency.The
performAuditcall is not awaited, making it fire-and-forget. While this is acceptable for non-blocking audit logging, other audit calls in this PR (e.g., inhome_page.dartline 95) useawait. For consistency and to ensure the audit completes before proceeding, consider awaiting:- audit.performAudit( + await audit.performAudit( "REG-NAV-005", "REG-MOD-102", );If fire-and-forget is intentional here to avoid delaying the permission request UX, this is fine as-is.
lib/ui/onboard/home_page.dart (1)
136-136: Remove or enable the commented-out audit call.Commented-out code creates confusion about intent. If this audit is no longer needed, remove the line entirely. If it's temporarily disabled for debugging, add a TODO comment explaining why and when it should be re-enabled.
- //globalProvider.getAudit("REG-HOME-002", "REG-MOD-102");lib/ui/onboard/portrait/operational_tasks.dart (1)
196-201: Avoid title-based checks and hardcoded audit IDs.Using the display title (
"Pending Approval") is brittle for localization and makes the audit call dependent on UI text. Prefer a stable task identifier (e.g., an enum/key inoperationalTasks) and centralize audit IDs in constants to avoid drift.android/app/src/main/java/io/mosip/registration_client/api_services/UserDetailsApi.java (1)
21-23: Remove duplicate imports to reduce noise.
AuditEventandComponentsare imported twice; keep a single set.♻️ Proposed fix
-import io.mosip.registration.clientmanager.constant.AuditEvent; -import io.mosip.registration.clientmanager.constant.Components; @@ -import io.mosip.registration.clientmanager.constant.AuditEvent; -import io.mosip.registration.clientmanager.constant.Components; +import io.mosip.registration.clientmanager.constant.AuditEvent; +import io.mosip.registration.clientmanager.constant.Components;Also applies to: 32-33
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
...oid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Show resolved
Hide resolved
Signed-off-by: Rakshithasai123 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/export_packet/widgets/export_button.dart (1)
43-56: Audit and export execute even when disconnected.The
getAudit()call andexportSelected()execute unconditionally after the network check. When!connectivityProvider.isConnected, the snackbar is shown but the code continues to audit and attempt export anyway. This is likely unintended behavior.🐛 Proposed fix: Add early return on network failure
onPressed: () async { await connectivityProvider.checkNetworkConnection(); if (!connectivityProvider.isConnected) { _showInSnackBar(appLocalizations.network_error); - }else{ + return; + } else { await exportPacketsProvider.packetSyncAll(); } await context.read<GlobalProvider>().getAudit( "REG-EXPT-PKT-001", "REG-MOD-103" ); await exportPacketsProvider.exportSelected(); exportPacketsProvider.searchedList(); },
♻️ Duplicate comments (2)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java (2)
79-83: Fix REG_DEMO_CAPTURE name/description mismatch (Line 80).
REG_DEMO_CAPTUREuses theREG_DEMO_NEXTname and has a typo in the description. This can confuse audit analysis and duplicate event names.🛠️ Proposed fix
- REG_DEMO_CAPTURE("REG-EVT-110", USER_EVENT.getCode(), "REG_DEMO_NEXT", "FvStarted capturing demographic details"), + REG_DEMO_CAPTURE("REG-EVT-110", USER_EVENT.getCode(), "REG_DEMO_CAPTURE", "Started capturing demographic details"),
100-120: POE document audit entries reference POB strings (Line 115-117).
If POE means Proof of Exception, the name/description should be POE-specific; current values log as POB.🛠️ Proposed fix
- REG_DOC_POE_SCAN("REG-EVT-022", USER_EVENT.getCode(), "REG_DOC_POB_SCAN", "PoB: Click of Scan"), - REG_DOC_POE_VIEW("REG-EVT-023", USER_EVENT.getCode(), "REG_DOC_POB_VIEW", "PoB: View"), - REG_DOC_POE_DELETE("REG-EVT-024", USER_EVENT.getCode(), "REG_DOC_POB_DELETE", "PoB: Delete"), + REG_DOC_POE_SCAN("REG-EVT-022", USER_EVENT.getCode(), "REG_DOC_POE_SCAN", "PoE: Click of Scan"), + REG_DOC_POE_VIEW("REG-EVT-023", USER_EVENT.getCode(), "REG_DOC_POE_VIEW", "PoE: View"), + REG_DOC_POE_DELETE("REG-EVT-024", USER_EVENT.getCode(), "REG_DOC_POE_DELETE", "PoE: Delete"),
android/app/src/main/java/io/mosip/registration_client/api_services/AuthenticationApi.java
Show resolved
Hide resolved
android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java
Outdated
Show resolved
Hide resolved
android/app/src/main/java/io/mosip/registration_client/api_services/MachineDetailsApi.java
Outdated
Show resolved
Hide resolved
...oid/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java
Outdated
Show resolved
Hide resolved
android/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.java
Outdated
Show resolved
Hide resolved
android/app/src/main/java/io/mosip/registration_client/api_services/RegistrationApi.java
Show resolved
Hide resolved
...ager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java
Outdated
Show resolved
Hide resolved
...ientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java
Outdated
Show resolved
Hide resolved
...ain/java/io/mosip/registration/clientmanager/service/PreRegistrationDataSyncServiceImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Rakshithasai123 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PreRegistrationDataSyncServiceImpl.java (1)
87-98: Remove unusedauditManagerdependency or add a TODO comment.The
auditManagerServiceis injected into the constructor but never called within this class. Either remove the unused dependency to keep the code clean, or add a TODO comment if audit logging for pre-registration sync is planned for a future implementation.
🤖 Fix all issues with AI agents
In `@lib/ui/process_ui/generic_process.dart`:
- Around line 691-716: The code increments globalProvider.newProcessTabIndex
then immediately indexes process.screens![globalProvider.newProcessTabIndex],
which can be out of range when leaving the last screen; fix by adding a bounds
check (e.g., ensure globalProvider.newProcessTabIndex < process.screens!.length)
before creating currentScreen and running the label/audit logic (or clamp/return
if out of range). Update the logic around globalProvider.newProcessTabIndex,
currentScreen, and the screenLabel check so you only access process.screens!
when the index is valid.
♻️ Duplicate comments (2)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java (2)
80-82: REG_DEMO_CAPTURE name/description mismatch still present.
Name is still"REG_DEMO_NEXT"and description has the typo"FvStarted"; if unintended, align to"REG_DEMO_CAPTURE"and fix the text.
116-118: POE events still reference POB name/description.
If this is not intentional, updateREG_DOC_POE_*name/description strings to POE.
🧹 Nitpick comments (7)
lib/ui/process_ui/generic_process.dart (1)
102-105: Remove the commented‑out audit call or document why it’s disabled.
Leftover commented code adds noise and is easy to forget.♻️ Suggested cleanup
- //_registrationScreenLoadedAudit();android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java (1)
27-28: Unused imports.
AuditEventandComponentsare imported but not used in this file. Based on past review feedback, audit logging was moved to the UI layer. These unused imports should be removed.🧹 Suggested fix
-import io.mosip.registration.clientmanager.constant.AuditEvent; -import io.mosip.registration.clientmanager.constant.Components;lib/ui/onboard/home_page.dart (2)
136-152: Missingawaiton async audit call and leftover commented code.
- Line 136 has commented-out code that should be removed.
- Line 152:
getAuditis an async method but is not awaited. This means the dialog may show before the audit is logged, and any errors would be silently swallowed.- The switch statement initializes
auditeventidto"REG-EVT-002"which duplicates theNEWcase—consider using adefaultclause instead.♻️ Suggested fix
globalProvider.createRegistrationLanguageMap(); - //globalProvider.getAudit("REG-HOME-002", "REG-MOD-102"); - String auditeventid="REG-EVT-002"; + String auditeventid; switch(process.flow){ case "NEW": auditeventid="REG-EVT-002"; break; case "UPDATE": auditeventid="REG-EVT-003"; break; case "LOST": auditeventid="REG-EVT-001"; break; case "CORRECTION": auditeventid="REG-EVT-007"; break; + default: + auditeventid="REG-EVT-002"; } - globalProvider.getAudit(auditeventid, "REG-MOD-102"); + await globalProvider.getAudit(auditeventid, "REG-MOD-102");
256-262: Missingawaiton async audit call.The
getAuditcall on line 257 is not awaited. This could result in the navigation occurring before the audit is recorded, and any errors would be silently ignored.♻️ Suggested fix
- "onTap": (context){ - Provider.of<GlobalProvider>(context, listen: false).getAudit("REG-EVT-005", "REG-MOD-102"); + "onTap": (context) async { + await Provider.of<GlobalProvider>(context, listen: false).getAudit("REG-EVT-005", "REG-MOD-102"); Navigator.push(lib/ui/process_ui/widgets/document_upload_control.dart (3)
499-535: Missingawaiton async audit calls.The
getAuditcalls on lines 520-526 are async but not awaited. This could cause the scanner navigation to occur before audit logging completes, and any errors would be silently lost.♻️ Suggested fix
if (eventId.isNotEmpty) { - context + await context .read<GlobalProvider>() .getAudit(eventId, "REG-MOD-103"); } - context + await context .read<GlobalProvider>() .getAudit("REG-EVT-089", "REG-MOD-103");
621-654: Handler should beasyncwithawaiton audit calls.The
onTaphandler at line 621 is not markedasync, but it calls asyncgetAuditmethods. This means audit calls are fire-and-forget, potentially losing errors and not guaranteeing audit completion before the delete operation.♻️ Suggested fix
- onTap: () { + onTap: () async { String eventId = ""; // ... switch logic ... if (eventId.isNotEmpty) { - context + await context .read<GlobalProvider>() .getAudit(eventId, "REG-MOD-103"); } - context + await context .read<GlobalProvider>() .getAudit("REG-EVT-091", "REG-MOD-103");
849-886: Repeated audit logic and missingasync/awaitin non-mobile handlers.The same subType-to-eventId switch pattern is duplicated across 6 locations (mobile and non-mobile variants of scan, view, and delete). Additionally, the non-mobile handlers at lines 921, 970 have the same missing
async/awaitissue flagged for mobile handlers.While I understand from past feedback that new helper methods should not be introduced, consider at minimum:
- Adding
asyncandawaitto all handlers for correctness- Defining the event ID mappings as constants to reduce hardcoded string repetition
♻️ Suggested fix for non-mobile scan handler (apply similar pattern to view/delete)
onPressed: (documentController.text == "") ? null - : () async { + : () async { String eventId = ""; // ... switch logic ... if (eventId.isNotEmpty) { - context + await context .read<GlobalProvider>() .getAudit(eventId, "REG-MOD-103"); } - context + await context .read<GlobalProvider>() .getAudit("REG-EVT-089", "REG-MOD-103");
Signed-off-by: Rakshithasai123 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/ui/process_ui/generic_process.dart`:
- Around line 705-723: The demographics audit is triggered even when the tab
index didn't advance; capture the current index before calling
_nextButtonClickedAudit() (e.g., previousIndex =
globalProvider.newProcessTabIndex) and only call
globalProvider.getAudit("REG-EVT-110", "REG-MOD-102") if
globalProvider.newProcessTabIndex > previousIndex and the screen label matches
the localized demographics name; update the logic around
_nextButtonClickedAudit(), the conditional using
globalProvider.newProcessTabIndex, process.screens, and getAudit to enforce this
post-increment check.
2 participants
Summary by CodeRabbit
New Features
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.