US2 Block Registration Operations When Centre Remapping is In Progress#1084
Open
SachinPremkumar wants to merge 7 commits into
Open
US2 Block Registration Operations When Centre Remapping is In Progress#1084SachinPremkumar wants to merge 7 commits into
SachinPremkumar wants to merge 7 commits into
Conversation
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (3)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (1)
72-73: 💤 Low valueNaming inconsistency: mix of CENTER vs CENTRE spelling.
Line 72 uses
MACHINE_CENTER_CHANGED(American spelling) while line 73 usesCENTRE_REMAP_ERROR_CODE(British spelling). The rest of the codebase consistently uses "Center" (e.g.,RegistrationCenter,regCenterId). For consistency, consider renamingCENTRE_REMAP_ERROR_CODEtoCENTER_REMAP_ERROR_CODE.Proposed fix
public static final String MACHINE_CENTER_CHANGED = "mosip.registration.machinecenterchanged"; -public static final String CENTRE_REMAP_ERROR_CODE = "KER-SNC-149"; +public static final String CENTER_REMAP_ERROR_CODE = "KER-SNC-149";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java` around lines 72 - 73, The constant name CENTRE_REMAP_ERROR_CODE is inconsistent with the project's "Center" spelling; rename CENTRE_REMAP_ERROR_CODE to CENTER_REMAP_ERROR_CODE in RegistrationConstants and update all references/usages accordingly (search for CENTRE_REMAP_ERROR_CODE and replace with CENTER_REMAP_ERROR_CODE), ensuring places that interact with MACHINE_CENTER_CHANGED, RegistrationCenter, regCenterId, etc., use the new identifier so naming is consistent across the codebase.lib/provider/sync_provider.dart (1)
290-297: ⚡ Quick winConsider adding error handling and progress tracking to
centreRemapSync().The method executes a sequence of cleanup operations during centre remap but lacks individual error handling for each step. If any operation fails, the remaining operations won't execute and the caller won't know which steps succeeded. Consider:
- Adding try-catch blocks around individual operations to allow partial success
- Tracking and reporting progress to provide user feedback
- Returning a result indicating which operations completed
This would improve resilience and user experience during the remap cleanup process.
Example error handling pattern
Future<void> centreRemapSync() async { String Function(String) findJobIdByApiName = await _getJobIdFinder(); try { await syncResponseService.syncPacketStatus(findJobIdByApiName("packetSyncStatusJob")); } catch (e) { log('centreRemapSync: syncPacketStatus failed: $e'); } try { await syncResponseService.batchJob(); } catch (e) { log('centreRemapSync: batchJob failed: $e'); } // ... similar pattern for remaining operations }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/provider/sync_provider.dart` around lines 290 - 297, centreRemapSync currently runs multiple cleanup calls sequentially without per-step error handling or progress reporting; update centreRemapSync to wrap each call (syncResponseService.syncPacketStatus, batchJob, deleteRegistrationPackets, getPreRegIds, deletePreRegRecords) in its own try-catch so one failure doesn't stop later steps, use the existing _getJobIdFinder result to resolve job ids for the respective calls, record each step's success/failure (e.g., a Map<String,bool> or a small result DTO keyed by operation name), and return or emit that result so callers can see which operations completed and which failed (also log or include error messages for each caught exception for debugging).lib/ui/onboard/home_page.dart (1)
107-115: ⚡ Quick winDuplicated
_remapBlockedMessagemapping across two widgets.This exact flow→message switch is also defined in
lib/ui/onboard/portrait/registration_tasks.dart(Lines 45-54). The two copies will drift over time (e.g., when a new flow code is added). Consider extracting a single shared helper (e.g., a top-level function or a small util that takesAppLocalizations+ flow) and reusing it in both places.♻️ Example shared helper
// lib/utils/remap_messages.dart import 'package:flutter_gen/gen_l10n/app_localizations.dart'; String remapBlockedMessage(AppLocalizations l, String? flow) { switch (flow) { case 'NEW': return l.remap_blocked_new_registration; case 'UPDATE': return l.remap_blocked_uin_update; case 'LOST': return l.remap_blocked_lost_uin; case 'CORRECTION': return l.remap_blocked_correction; default: return l.centre_remap_notification; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/ui/onboard/home_page.dart` around lines 107 - 115, The duplicate switch mapping in _remapBlockedMessage should be extracted to a single shared helper to avoid drift; create a top-level function (e.g., remapBlockedMessage(AppLocalizations l, String? flow)) that implements the switch and returns the localized string, place it in a small util file (e.g., lib/utils/remap_messages.dart), then replace the local _remapBlockedMessage usage in home_page.dart and the copy in registration_tasks.dart to call the new remapBlockedMessage helper. Ensure you import AppLocalizations where needed and remove the duplicated switch bodies from both widgets so they delegate to the new function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/l10n/app_en.arb`:
- Around line 192-198: The strings use mixed American ("center" in
centre_remap_notification value) and British ("centre" in the other keys/values)
spellings; pick one convention and make all keys and values consistent: either
change the value of "centre_remap_notification" to use "centre" to match the
existing keys, or rename the key to "center_remap_notification" and update all
other messages ("remap_blocked_new_registration", "remap_blocked_uin_update",
"remap_blocked_lost_uin", "remap_blocked_correction",
"remap_blocked_biometric_update", "remap_blocked_onboarding") to use "center" in
their values if you adopt American spelling; also update any references to the
key if you rename it.
In `@lib/provider/sync_provider.dart`:
- Around line 77-84: checkCentreRemapState() is never called so persisted
centre-remap flag isn't applied; call it during SyncProvider startup (e.g., in
the SyncProvider constructor or the init/startPolling method) before any polling
or sync attempts so _onRemapDetected() can set _isCentreRemapped on app start,
or remove the unused method if you prefer; update the SyncProvider
initialization path to invoke checkCentreRemapState().
In `@lib/ui/onboard/portrait/operational_tasks.dart`:
- Around line 82-89: The remap banner Text widget using
AppLocalizations.of(context)!.centre_remap_notification currently sets its color
to Color(0xFFE6A817) which fails WCAG contrast against the light background;
replace that color with a darker amber/brown tone (e.g., a hex around 0xFF8A5B00
or Colors.black87) to raise contrast to at least AA levels while leaving the
border/icon amber (0xFFE6A817) intact, keeping the same Text widget, fontWeight
and size in operational_tasks.dart.
---
Nitpick comments:
In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java`:
- Around line 72-73: The constant name CENTRE_REMAP_ERROR_CODE is inconsistent
with the project's "Center" spelling; rename CENTRE_REMAP_ERROR_CODE to
CENTER_REMAP_ERROR_CODE in RegistrationConstants and update all
references/usages accordingly (search for CENTRE_REMAP_ERROR_CODE and replace
with CENTER_REMAP_ERROR_CODE), ensuring places that interact with
MACHINE_CENTER_CHANGED, RegistrationCenter, regCenterId, etc., use the new
identifier so naming is consistent across the codebase.
In `@lib/provider/sync_provider.dart`:
- Around line 290-297: centreRemapSync currently runs multiple cleanup calls
sequentially without per-step error handling or progress reporting; update
centreRemapSync to wrap each call (syncResponseService.syncPacketStatus,
batchJob, deleteRegistrationPackets, getPreRegIds, deletePreRegRecords) in its
own try-catch so one failure doesn't stop later steps, use the existing
_getJobIdFinder result to resolve job ids for the respective calls, record each
step's success/failure (e.g., a Map<String,bool> or a small result DTO keyed by
operation name), and return or emit that result so callers can see which
operations completed and which failed (also log or include error messages for
each caught exception for debugging).
In `@lib/ui/onboard/home_page.dart`:
- Around line 107-115: The duplicate switch mapping in _remapBlockedMessage
should be extracted to a single shared helper to avoid drift; create a top-level
function (e.g., remapBlockedMessage(AppLocalizations l, String? flow)) that
implements the switch and returns the localized string, place it in a small util
file (e.g., lib/utils/remap_messages.dart), then replace the local
_remapBlockedMessage usage in home_page.dart and the copy in
registration_tasks.dart to call the new remapBlockedMessage helper. Ensure you
import AppLocalizations where needed and remove the duplicated switch bodies
from both widgets so they delegate to the new function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 18edbd37-6fef-40dc-bb91-5572aaebf7b8
📒 Files selected for processing (15)
android/app/src/main/java/io/mosip/registration_client/api_services/GlobalConfigSettingsApi.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/MasterDataServiceImpl.javaassets/l10n/app_ar.arbassets/l10n/app_en.arbassets/l10n/app_fr.arbassets/l10n/app_hi.arbassets/l10n/app_kn.arbassets/l10n/app_ta.arblib/platform_android/global_config_service_impl.dartlib/platform_spi/global_config_service.dartlib/provider/sync_provider.dartlib/ui/onboard/home_page.dartlib/ui/onboard/portrait/operational_tasks.dartlib/ui/onboard/portrait/registration_tasks.dart
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
…/android-registration-client into US2-block-operation # Conflicts: # assets/l10n/app_ar.arb # assets/l10n/app_en.arb # assets/l10n/app_fr.arb # assets/l10n/app_hi.arb # assets/l10n/app_kn.arb # assets/l10n/app_ta.arb # lib/ui/onboard/portrait/operational_tasks.dart # lib/ui/onboard/portrait/registration_tasks.dart
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/ui/onboard/home_page.dart (2)
107-109: ⚡ Quick winRemove unused
flowparameter from_remapBlockedMessage.The
flowparameter is accepted but never used in the method body—it always returns the same localized string regardless of the flow. This suggests either the parameter is vestigial from earlier design iterations or flow-specific messages were planned but not implemented.♻️ Proposed fix
- String _remapBlockedMessage(String? flow) { + String _remapBlockedMessage() { return appLocalizations.remap_operation_blocked; }Then update the call site at line 133:
- _showInSnackBar(_remapBlockedMessage(process.flow)); + _showInSnackBar(_remapBlockedMessage());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/ui/onboard/home_page.dart` around lines 107 - 109, The _remapBlockedMessage function currently accepts an unused parameter "flow"; remove that parameter from the signature (change String _remapBlockedMessage(String? flow) to String _remapBlockedMessage()) and leave the body returning appLocalizations.remap_operation_blocked, then update all call sites that pass a flow (e.g., the call to _remapBlockedMessage(...) near the current call site) to invoke _remapBlockedMessage() with no arguments.
132-135: 🏗️ Heavy liftConsider showing a remap notification banner instead of an empty container.
When
getProcessUIis called during a centre remap, the code shows a transient snackbar then returns an emptyContainer(). This approach has two drawbacks:
- The snackbar disappears after a few seconds, leaving no persistent visual cue.
- An empty container might look like a rendering bug rather than intentional blocking.
Consider rendering a persistent warning banner (similar to the one in
RegistrationTasks) or a disabled card with an explanation, so users continuously see why the process UI is unavailable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/ui/onboard/home_page.dart` around lines 132 - 135, When syncProvider.isCentreRemapped is true, don't return an empty Container() after calling _showInSnackBar; instead render a persistent UI element so users see why the process UI is blocked. Replace the early return in getProcessUI with a call that returns a persistent warning widget (e.g., _buildRemapBanner(process) or a disabled card) that displays the same message produced by _remapBlockedMessage(process.flow); mirror the visual style used by RegistrationTasks' banner (or use a disabled Card with explanatory text and optional action buttons) and keep _showInSnackBar if you still want a transient notice. Add the helper widget (e.g., _buildRemapBanner) in this file and ensure it uses the same identifying text from _remapBlockedMessage and respects layout/spacing of the surrounding UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/ui/onboard/home_page.dart`:
- Around line 107-109: The _remapBlockedMessage function currently accepts an
unused parameter "flow"; remove that parameter from the signature (change String
_remapBlockedMessage(String? flow) to String _remapBlockedMessage()) and leave
the body returning appLocalizations.remap_operation_blocked, then update all
call sites that pass a flow (e.g., the call to _remapBlockedMessage(...) near
the current call site) to invoke _remapBlockedMessage() with no arguments.
- Around line 132-135: When syncProvider.isCentreRemapped is true, don't return
an empty Container() after calling _showInSnackBar; instead render a persistent
UI element so users see why the process UI is blocked. Replace the early return
in getProcessUI with a call that returns a persistent warning widget (e.g.,
_buildRemapBanner(process) or a disabled card) that displays the same message
produced by _remapBlockedMessage(process.flow); mirror the visual style used by
RegistrationTasks' banner (or use a disabled Card with explanatory text and
optional action buttons) and keep _showInSnackBar if you still want a transient
notice. Add the helper widget (e.g., _buildRemapBanner) in this file and ensure
it uses the same identifying text from _remapBlockedMessage and respects
layout/spacing of the surrounding UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e588f332-d0f7-43e7-a1de-4544d752029c
📒 Files selected for processing (15)
android/app/src/main/java/io/mosip/registration_client/api_services/GlobalConfigSettingsApi.javaassets/l10n/app_ar.arbassets/l10n/app_en.arbassets/l10n/app_fr.arbassets/l10n/app_hi.arbassets/l10n/app_kn.arbassets/l10n/app_ta.arblib/platform_android/global_config_service_impl.dartlib/platform_spi/global_config_service.dartlib/provider/sync_provider.dartlib/ui/onboard/home_page.dartlib/ui/onboard/portrait/mobile_home_page.dartlib/ui/onboard/portrait/operational_tasks.dartlib/ui/onboard/portrait/registration_tasks.dartpigeon/global_config_settings.dart
💤 Files with no reviewable changes (1)
- lib/ui/onboard/portrait/operational_tasks.dart
✅ Files skipped from review due to trivial changes (2)
- assets/l10n/app_ta.arb
- assets/l10n/app_kn.arb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#1046
Summary by CodeRabbit
Release Notes