RCF-1283 Added config properties #652
Conversation
…ag is enable Signed-off-by: Madhuravas reddy <madhu@mosip.io>
…ends on configure property Signed-off-by: Madhuravas reddy <madhu@mosip.io>
|
Warning Rate limit exceeded@MadhuMosip has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
WalkthroughAdds an SDK-based biometric quality-check path, a supervisor-approval config flag with auto-approve behavior, Pigeon/Dart bridge for the new flag, UI support to show SDK quality, and localization entries for MDS/SDK quality across languages. Changes
Sequence DiagramsequenceDiagram
participant UI as Flutter Biometric Widget
participant Dart as GlobalConfigService (Dart)
participant Pigeon as GlobalConfigSettingsApi (Pigeon)
participant Repo as GlobalParamRepository
participant BiometricSvc as Biometrics095Service
participant SDK as Match SDK
UI->>Dart: getQualityCheckWithSdk()
Dart->>Pigeon: getQualityCheckWithSdk()
Pigeon->>Repo: getCachedStringQualityCheckWithSdk()
Repo-->>Pigeon: "Y"/"N"/""
Pigeon-->>Dart: flag
Dart-->>UI: enable/disable SDK quality UI
UI->>BiometricSvc: captureBiometric()
BiometricSvc->>Repo: read QUALITY_CHECK_WITH_SDK
alt QUALITY_CHECK_WITH_SDK == "Y"
BiometricSvc->>SDK: getSDKScore(BiometricsDto)
SDK-->>BiometricSvc: sdkScore / error
BiometricSvc->>BiometricSvc: set sdkScore in BiometricsDto
else
BiometricSvc->>BiometricSvc: use device-reported quality
end
BiometricSvc-->>UI: BiometricsDto (contains quality scores)
UI->>UI: compute avg SDK score and render indicators
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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: 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/RegistrationServiceImpl.java (1)
290-294: Pre-existing bug: NPE risk due to incorrect logical operator.This is outside the scope of this PR, but worth noting: the condition uses
||instead of&&. IfcontainerPathisnull, the second operand!containerPath.trim().isEmpty()will throw aNullPointerException.Consider fixing in a follow-up:
- if (containerPath != null || !containerPath.trim().isEmpty()) { + if (containerPath != null && !containerPath.trim().isEmpty()) {
🧹 Nitpick comments (4)
lib/ui/process_ui/widgets_mobile/biometric_capture_scan_block_portrait.dart (2)
41-67: Guard asyncsetStateand add an explicit return typeThe config‑driven SDK flag wiring looks good. Two small robustness tweaks would help:
_getSdkQualityCheckStatuscurrently has an implicitdynamicreturn type; preferFuture<void>for clarity withasync.- After the
awaitcall, add aif (!mounted) return;check beforesetStateto avoid callingsetStateon a disposed widget if the screen is closed quickly.
303-319: Avoid recomputing average SDK score and consider “no score” handling
_getAvgSdkScorecorrectly averages non‑nullsdkScorevalues, but:
- In the UI you call
_getAvgSdkScore()twice (for the text and the color), which re‑loops the list each time. Compute it once into a localfinal avgSdkScore = _getAvgSdkScore();and reuse.- When there are no SDK scores, you return
0, which is rendered as0%and treated as below threshold. If “no SDK score available” should be distinguishable from a genuinely low score, consider returningnulland showing an “N/A” label or hiding the SDK block in that case.Also applies to: 412-460
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/Biometrics095Service.java (2)
147-175: Move flag lookup out of the loop and reconsider hard‑fail on SDK score errorsThe SDK score enrichment is wired correctly, but two details are worth adjusting:
qualityCheckWithSdkis fetched on every iteration of theforloop. Since it’s a global param, read it once before the loop and reuse the value to avoid repeated repository/cache lookups.- When the flag is enabled, any exception in
getSDKScorecauses the entire RCapture to fail withSBI_RCAPTURE_ERROR. That makes SDK scoring a hard dependency; consider instead logging the failure and proceeding withoutsdkScore(or setting a sentinel) if the requirement is to keep capture functional even when the SDK path is misconfigured or temporarily broken.
268-302: SimplifygetSDKScoreuntil a real SDK quality API is availableThis helper currently:
- Derives
biometricType, base64‑decodesbioValue, and builds aBIRviaMatchUtil.buildBir, but- Ultimately just returns
biometricsDto.getQualityScore()and doesn’t use the constructedBIR.Given the comment that the Android
IBioApiV2doesn’t yet expose a quality API, this extra work adds CPU and potential failure points without affecting the result. Until you actually call a quality API that consumes theBIR, you could:
- Drop the BIR construction (and potentially the base64 decode + modality conversion), and
- Implement
getSDKScoreas a simple passthrough to the device score (optionally after basic null/empty checks).This keeps the method cheap and reduces the chance of spurious failures when the SDK scoring feature flag is enabled, while still allowing you to expand it later when the Android quality API is available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
android/app/src/main/java/io/mosip/registration_client/api_services/GlobalConfigSettingsApi.java(1 hunks)android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java(1 hunks)android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.java(1 hunks)android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/Biometrics095Service.java(5 hunks)android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java(2 hunks)assets/l10n/app_ar.arb(1 hunks)assets/l10n/app_en.arb(1 hunks)assets/l10n/app_fr.arb(1 hunks)assets/l10n/app_hi.arb(1 hunks)assets/l10n/app_kn.arb(1 hunks)assets/l10n/app_ta.arb(1 hunks)lib/platform_android/global_config_service_impl.dart(1 hunks)lib/ui/process_ui/widgets_mobile/biometric_capture_scan_block_portrait.dart(4 hunks)pigeon/global_config_settings.dart(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (1)
RegistrationConstants(9-122)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.java (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (1)
RegistrationConstants(9-122)
🔇 Additional comments (11)
assets/l10n/app_hi.arb (1)
335-337: LGTM!The Hindi translations for the new quality-related keys are properly added with correct JSON syntax.
assets/l10n/app_ar.arb (1)
335-337: LGTM!The Arabic translations for the new quality-related keys are properly added with correct JSON syntax.
assets/l10n/app_kn.arb (1)
335-337: LGTM!The Kannada translations for the new quality-related keys are properly added with correct JSON syntax.
assets/l10n/app_fr.arb (1)
335-337: LGTM!The French translations for the new quality-related keys are properly added with correct JSON syntax.
assets/l10n/app_en.arb (1)
335-337: LGTM!The English translations for the new quality-related keys are properly added with correct JSON syntax. These serve as the base translations for all other locales.
assets/l10n/app_ta.arb (1)
344-346: Trailing spaces in "mds_quality" and "sdk_quality" are intentional and consistent across all locales.All six locale files (en, ta, hi, kn, ar, fr) include trailing spaces in both "mds_quality" and "sdk_quality" values. This is a deliberate, consistent design pattern used for UI spacing and does not require changes.
pigeon/global_config_settings.dart (1)
15-16: LGTM!The new
getQualityCheckWithSdk()method follows the established pattern of other async methods in this Pigeon HostApi interface, consistent withgetGpsEnableFlag().android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.java (1)
161-163: LGTM!The new
getCachedStringQualityCheckWithSdk()method follows the established pattern of other cached string getters in this repository (e.g.,getCachedStringGpsDeviceEnableFlag()).android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java (1)
319-325: LGTM!The auto-approval logic is correctly placed after the registration is persisted. The condition appropriately treats "Y" (case-insensitive) as enabling supervisor approval, with any other value (including null/unconfigured) resulting in auto-approval.
android/app/src/main/java/io/mosip/registration_client/api_services/GlobalConfigSettingsApi.java (1)
88-98: LGTM!The new
getQualityCheckWithSdk()method correctly follows the established pattern ofgetGpsEnableFlag(), with appropriate error handling and logging.android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (1)
120-121: New global param keys look consistentThe added constants follow the existing naming and namespacing pattern and are safe to use across services and UI.
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
| // Current IBioApiV2 does not expose a quality API on Android; fall back to | ||
| // the device-reported quality score to keep sdkScore populated. | ||
| return biometricsDto.getQualityScore(); |
There was a problem hiding this comment.
Implementation is not complete!
2 participants
RCF-1283
Added auto approval packets if the supervisor approval config flag is enabled.
Added quality check with sdk in the packet and showing in UI, depending on the configuration property.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.