RCF-1302 Added config properties #669
Conversation
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
WalkthroughAdds two new global constants and cached accessors, a DTO method to retain configured fields used during pre-reg ZIP parsing, a new PacketService SPI method and implementation to validate pending-registration approval count (enforced in pre-checks), localization entries for a new error, and a UI private variable rename. Changes
Sequence Diagram(s)sequenceDiagram
participant RegSvc as RegistrationServiceImpl
participant PacketSPI as PacketService (SPI)
participant PacketImpl as PacketServiceImpl
participant GPRepo as GlobalParamRepository
participant RegRepo as RegistrationRepository
RegSvc->>PacketSPI: validatingRegisteredPacketNotApproveCount()
PacketSPI->>PacketImpl: invoke implementation
PacketImpl->>GPRepo: getCachedIntRegMaxCountApproveLimit()
GPRepo-->>PacketImpl: maxCount
PacketImpl->>RegRepo: getAllRegistrationByStatus(CREATED)
RegRepo-->>PacketImpl: listOfCreated
PacketImpl-->>PacketSPI: boolean limitReached
PacketSPI-->>RegSvc: boolean limitReached
alt limitReached == true
RegSvc->>RegSvc: throw ClientCheckedException(REG_PKT_APPRVL_CNT_EXCEED)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting 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: 2
🤖 Fix all issues with AI agents
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java:
- Line 132: The constant name FIELDS_TO_RETAIN_ON_PRID_FETCH is misleading
because RegistrationDto.removeConfiguredFields() removes matching fields; rename
the constant to FIELDS_TO_REMOVE_ON_PRID_FETCH in RegistrationConstants and
update all references/usages (e.g., in RegistrationDto.removeConfiguredFields()
and any config lookups) to use the new name, keeping the property value string
if desired; alternatively, if you prefer to keep the current constant name,
change the logic in RegistrationDto.removeConfiguredFields() to retain only the
configured fields instead of removing them—pick one approach and apply the
corresponding updates consistently.
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java:
- Around line 403-418: The removeConfiguredFields method uses a raw
Collections.EMPTY_LIST and does not trim split values; change keysToRemove
initialization to use Collections.emptyList() when config is null and otherwise
build a trimmed list from config by splitting on RegistrationConstants.COMMA
then mapping String::trim and filtering out empty strings (e.g., via
Arrays.stream(config.split(...)).map(String::trim).filter(s ->
!s.isEmpty()).collect(Collectors.toList())). Then use that keysToRemove in the
existing loop that checks and removes from demographics, documents, and
biometrics.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
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-150)
🔇 Additional comments (2)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.java (1)
250-252: LGTM!The new accessor method follows the established pattern of other cached string getters in this class.
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java (1)
248-250: Verify call order:removeConfiguredFieldscalled after clearing demographics and documents.
removeConfiguredFields()is invoked aftergetDocuments().clear()andgetDemographics().clear(). This means the method will only effectively operate on thebiometricsmap since the other two maps are already empty. If the intent is to selectively remove/retain fields from demographics or documents, this call should be placed before the clear operations.
...anager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java
Show resolved
Hide resolved
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java:
- Around line 403-418: The method removeConfiguredFields currently treats the
config list as fields to remove but the config
(mosip.registration.fields.to.retain.post.prid.fetch) actually lists fields to
retain; update removeConfiguredFields to interpret config as keysToRetain:
rename keysToRemove to keysToRetain (or create a new Set/List keysToRetain from
config.split(RegistrationConstants.COMMA) using Collections.emptyList() when
null), then iterate allKeys (from
this.demographics/this.documents/this.biometrics) and remove any key where
!keysToRetain.contains(key) so only configured fields remain; keep references to
removeConfiguredFields, keysToRetain/keysToRemove, this.demographics,
this.documents, this.biometrics and RegistrationConstants.COMMA to locate the
change.
🧹 Nitpick comments (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java (1)
404-404: Use parameterizedCollections.emptyList()and handle empty string edge case.Two issues on this line:
Collections.EMPTY_LISTis a raw type that generates unchecked assignment warnings. UseCollections.emptyList()instead.If
configis an empty string"",split(",")returns[""](array with one empty string), not an empty array. This could cause unintended matching against keys.Proposed fix
- List<String> keysToRemove = config == null ? Collections.EMPTY_LIST : List.of(config.split(RegistrationConstants.COMMA)); + List<String> keysToRemove = (config == null || config.isEmpty()) + ? Collections.emptyList() + : List.of(config.split(RegistrationConstants.COMMA));
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java
🧰 Additional context used
🧬 Code graph analysis (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (1)
RegistrationConstants(9-150)
🔇 Additional comments (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java (1)
411-417: Consider whether observer notification is needed after field removal.Other methods that modify
demographics,documents, orbiometrics(e.g.,addDemographicField,removeDemographicField) callclearAndNotifyAllObservers()to signal state changes. This method modifies all three maps but does not notify observers.If this is intentional (e.g., called as part of a larger batch operation where notification happens elsewhere), consider adding a brief comment to clarify. Otherwise, add the notification call.
...ager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java:
- Around line 454-457: The comparison on Integer maxCount from
getCachedIntRegMaxCountApproveLimit() can NPE through auto-unboxing; update the
PacketServiceImpl block that currently does "Integer maxCount =
globalParamRepository.getCachedIntRegMaxCountApproveLimit(); if (maxCount <= 0)
{ return false; }" to first check for null (e.g., if (maxCount == null) return
false or handle as a configuration error) before performing the numeric
comparison, mirroring the null-check pattern used in
isMaxPacketCountLimitReached() and validatingLastExportDuration(); ensure the
change avoids auto-unboxing when value is null and preserves existing behavior.
🧹 Nitpick comments (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/spi/PacketService.java (1)
95-99: Consider renaming for consistency.The method name
validatingRegisteredPacketNotApproveCount()uses a different naming convention than the similar methodsisRegisteredPacketApprovalTimeBreached()andisMaxPacketCountLimitReached(). Consider renaming toisRegisteredPacketApprovalCountLimitReached()for consistency.That said,
validatingLastExportDuration()(line 87) also uses the "validating..." prefix, so the current naming has precedent.🔧 Optional rename for consistency
- boolean validatingRegisteredPacketNotApproveCount(); + boolean isRegisteredPacketApprovalCountLimitReached();
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/spi/PacketService.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/ui/process_ui/widgets/update_field_selector.dart
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: MadhuMosip
Repo: mosip/android-registration-client PR: 669
File: android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java:132-132
Timestamp: 2026-01-13T11:37:29.220Z
Learning: In the MOSIP registration client codebase, the configuration property `mosip.registration.fields.to.retain.post.prid.fetch` (constant `FIELDS_TO_RETAIN_ON_PRID_FETCH` in RegistrationConstants.java) is the standardized/expected name from the config system. Despite the "retain" naming, the implementation in `RegistrationDto.removeConfiguredFields()` removes the fields specified in this config. This naming is intentional and should not be changed.
📚 Learning: 2026-01-05T05:15:49.771Z
Learnt from: MadhuMosip
Repo: mosip/android-registration-client PR: 661
File: lib/ui/process_ui/widgets/update_field_selector.dart:64-72
Timestamp: 2026-01-05T05:15:49.771Z
Learning: In lib/ui/process_ui/widgets/update_field_selector.dart, ensure that the process configuration contains only one field type (either UIN or VID). Consequently, the existing idSubType determination logic (scanning the fields for the first 'uin' or 'vid' subType) is appropriate for this use case. If future changes introduce multiple field types, revise the idSubType resolution to handle both, with clear validation to prevent mixing types.
Applied to files:
lib/ui/process_ui/widgets/update_field_selector.dart
📚 Learning: 2026-01-13T11:37:29.220Z
Learnt from: MadhuMosip
Repo: mosip/android-registration-client PR: 669
File: android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java:132-132
Timestamp: 2026-01-13T11:37:29.220Z
Learning: In the MOSIP registration client codebase, the configuration property `mosip.registration.fields.to.retain.post.prid.fetch` (constant `FIELDS_TO_RETAIN_ON_PRID_FETCH` in RegistrationConstants.java) is the standardized/expected name from the config system. Despite the "retain" naming, the implementation in `RegistrationDto.removeConfiguredFields()` removes the fields specified in this config. This naming is intentional and should not be changed.
Applied to files:
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java
🔇 Additional comments (12)
lib/ui/process_ui/widgets/update_field_selector.dart (1)
39-39: LGTM!The rename from
inputValidationtoidRegeximproves code clarity by explicitly indicating this regex validates ID (UIN/VID) format. The change is consistent across declaration and usage.Also applies to: 225-225
assets/l10n/app_en.arb (1)
305-305: LGTM!The new
REG_PKT_APPRVL_CNT_EXCEEDerror message is well-structured, follows existing naming conventions, and provides a clear, actionable message for users when the packet approval limit is reached. The ICU select syntax is correct.assets/l10n/app_fr.arb (1)
305-305: LGTM!The French translation accurately conveys the error message meaning and maintains consistency with the English source. The ICU select syntax is correct.
assets/l10n/app_kn.arb (1)
305-305: LGTM!The Kannada localization for
REG_PKT_APPRVL_CNT_EXCEEDis properly added with correct ICU select syntax, maintaining consistency with other locale files.assets/l10n/app_hi.arb (1)
305-305: LGTM!The Hindi localization for
REG_PKT_APPRVL_CNT_EXCEEDis properly added with correct ICU select syntax, maintaining consistency with other locale files.assets/l10n/app_ar.arb (1)
305-305: LGTM!The Arabic localization for
REG_PKT_APPRVL_CNT_EXCEEDis properly added with correct ICU select syntax, maintaining consistency with other locale files.assets/l10n/app_ta.arb (1)
314-314: LGTM!The new
REG_PKT_APPRVL_CNT_EXCEEDerror code is properly added to the ICU select statement with a Tamil translation. This aligns with the new validation check added inRegistrationServiceImpl.doPreChecksBeforeRegistration().android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (2)
132-132: LGTM!The new constant
FIELDS_TO_RETAIN_ON_PRID_FETCHis correctly defined and follows the existing naming conventions. Based on learnings, the naming is intentional and aligns with the standardized config property from the MOSIP config system.
150-150: LGTM!The new constant
REG_PAK_MAX_CNT_APPRV_LIMITfollows the existing pattern (similar toREG_PAK_MAX_TIME_APPRV_LIMITat line 135) and is correctly placed.android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.java (1)
250-256: LGTM!Both accessor methods follow the established patterns in this class:
getCachedStringFieldsToRetainOnPridFetch()mirrors other string accessors likegetCachedStringForgotPassword()getCachedIntRegMaxCountApproveLimit()mirrors other integer accessors likegetCachedIntegerDiskSpaceSize()android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java (1)
673-677: LGTM!The new validation check follows the established pattern of the other pre-checks in this method. The null-safety check on
packetServiceis correctly applied, and the error codeREG_PKT_APPRVL_CNT_EXCEEDmatches the localization entries added in the ARB files.android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java (1)
463-465: This review comment is incorrect.The
NumberFormatExceptioncatch block at lines 463-465 is reachable and valid. The repository methodgetCachedIntegerGlobalParam()callsInteger.parseInt(value)(line 100 in GlobalParamRepository) without handling the exception internally, allowingNumberFormatExceptionto propagate to the caller in PacketServiceImpl. If the global parameter value is not a valid integer string, the exception will be thrown and caught by the existing handler.Likely an incorrect or invalid review comment.
...ientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java:
- Around line 403-424: In retainConfiguredFields (RegistrationDto), replace the
undefined keysToRemove with the declared keysToRetain and use it in the contains
check; specifically, update the lambda so it calls keysToRetain.contains(key)
and removes the key when true (since the config lists fields to be removed per
the intent), leaving demographics/documents/biometrics handling unchanged.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.javaandroid/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java
🚧 Files skipped from review as they are similar to previous changes (1)
- android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: MadhuMosip
Repo: mosip/android-registration-client PR: 669
File: android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java:132-132
Timestamp: 2026-01-13T11:37:37.805Z
Learning: In the MOSIP registration client codebase, the configuration property `mosip.registration.fields.to.retain.post.prid.fetch` (constant `FIELDS_TO_RETAIN_ON_PRID_FETCH` in RegistrationConstants.java) is the standardized/expected name from the config system. Despite the "retain" naming, the implementation in `RegistrationDto.removeConfiguredFields()` removes the fields specified in this config. This naming is intentional and should not be changed.
📚 Learning: 2026-01-13T11:37:37.805Z
Learnt from: MadhuMosip
Repo: mosip/android-registration-client PR: 669
File: android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java:132-132
Timestamp: 2026-01-13T11:37:37.805Z
Learning: In the MOSIP registration client codebase, the configuration property `mosip.registration.fields.to.retain.post.prid.fetch` (constant `FIELDS_TO_RETAIN_ON_PRID_FETCH` in RegistrationConstants.java) is the standardized/expected name from the config system. Despite the "retain" naming, the implementation in `RegistrationDto.removeConfiguredFields()` removes the fields specified in this config. This naming is intentional and should not be changed.
Applied to files:
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java
🧬 Code graph analysis (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (1)
RegistrationConstants(9-151)
...ager/src/main/java/io/mosip/registration/clientmanager/dto/registration/RegistrationDto.java
Show resolved
Hide resolved
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: MadhuMosip
Repo: mosip/android-registration-client PR: 669
File: android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java:132-132
Timestamp: 2026-01-13T11:37:37.805Z
Learning: In the MOSIP registration client codebase, the configuration property `mosip.registration.fields.to.retain.post.prid.fetch` (constant `FIELDS_TO_RETAIN_ON_PRID_FETCH` in RegistrationConstants.java) is the standardized/expected name from the config system. Despite the "retain" naming, the implementation in `RegistrationDto.removeConfiguredFields()` removes the fields specified in this config. This naming is intentional and should not be changed.
...ientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java
Show resolved
Hide resolved
Signed-off-by: Madhuravas reddy <madhu@mosip.io>
2 participants
mosip.registration.fields.to.retain.post.prid.fetch
mosip.registration.reg_pak_max_cnt_apprv_limit
Summary by CodeRabbit
New Features
Chores
Style
✏️ Tip: You can customize this high-level summary in your review settings.