Skip to content

ES-2914#190

Merged
Prafulrakhade merged 5 commits intomosip:developfrom
ase-101:develop
Mar 12, 2026
Merged

ES-2914#190
Prafulrakhade merged 5 commits intomosip:developfrom
ase-101:develop

Conversation

@ase-101
Copy link
Contributor

@ase-101 ase-101 commented Mar 11, 2026

Summary by CodeRabbit

  • Refactor

    • UI spec retrieval now sources allowed values and document metadata from external master-data services via configurable endpoints and pointers, streamlining and simplifying UI-spec augmentation.
    • Defaults added for missing allowed values, max upload size (5MB) and reset-password challenge fields for more robust UI behavior.
  • Tests

    • Unit tests updated to align with master-data-driven fetching and new UI-spec flow.
  • Chores

    • Fixed a configuration value typo in application properties.

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

Warning

Rate limit exceeded

@ase-101 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 31 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0e674b96-a5a7-489f-ae0a-01dc01b8bd0a

📥 Commits

Reviewing files that changed from the base of the PR and between 5490e86 and 29607ab.

📒 Files selected for processing (1)
  • mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java

Walkthrough

Replaces in-method UI-spec parsing with configurable UI-spec retrieval and JSON-pointer extraction; adds master-data-driven allowed-values aggregation via new endpoints; introduces config properties (uiSpecUrl, uiSpecJsonPointer, dynamicFieldsBaseUrl, docTypesAndCategoryBaseUrl); exposes fetchAllowedValuesFromMasterDataService(); updates tests and fixes a properties string.

Changes

Cohort / File(s) Summary
Service Implementation
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java
Removed direct JSONPath-based UI parsing; added config-driven uiSpecUrl + uiSpecJsonPointer extraction, default augmentation (language, maxUploadFileSize, resetPasswordChallengeFields), and master-data-driven allowed-values logic. Added public fetchAllowedValuesFromMasterDataService() and helpers that call dynamicFieldsBaseUrl and docTypesAndCategoryBaseUrl.
Test Suite
mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java
Updated tests for new flow: initialized uiSpecJsonPointer, removed old JSONPath inits, renamed tests to reflect master-data fetch semantics, and added helpers getDynamicFieldsEndpointResponse() and getDocumentTypesAndCategoriesEndpointResponse() to mock new endpoints.
Configuration
mosip-identity-plugin/src/main/resources/application.properties
Fixed a stray trailing double-quote in mosip.signup.mosipid.dynamic-fields.endpoint value (cleaned URL).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Service as IdrepoProfileRegistryPluginImpl
    participant UISpecAPI as UI_Spec_Endpoint
    participant DynamicFieldsAPI as Dynamic_Fields_Service
    participant DocTypesAPI as Doc_Types_Categories_Service

    Client->>Service: getUISpecification()
    Service->>UISpecAPI: GET uiSpecUrl
    UISpecAPI-->>Service: uiSpec JSON
    Service->>Service: extract node via uiSpecJsonPointer
    Service->>Service: apply defaults (language, maxUploadFileSize, resetPasswordChallengeFields)
    Service->>DynamicFieldsAPI: GET dynamicFieldsBaseUrl
    DynamicFieldsAPI-->>Service: dynamic fields JSON
    Service->>DocTypesAPI: GET docTypesAndCategoryBaseUrl
    DocTypesAPI-->>Service: doc types/categories JSON
    Service->>Service: fetchAllowedValuesFromMasterDataService() aggregate allowed values
    Service-->>Client: enriched UI spec JSON
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I chased URLs through branch and tree,

Poked pointers where the JSONs be,
Gathered fields and docs in tow,
Mixed them neat so forms can grow,
Hooray — the plugin hops to show!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'ES-2914' is only a ticket number and does not convey any meaningful information about the changeset, which involves significant refactoring of UI-spec parsing, master-data-driven allowed values generation, and API changes. Use a descriptive title that summarizes the main change, such as 'Refactor UI-spec parsing to use master-data service for allowed values' or similar that clearly indicates the core modification.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java (1)

627-636: ⚠️ Potential issue | 🟡 Minor

This test never exercises an inactive document type.

activeCategoryWithInactiveDocType is marked inactive at Line 628, so the code skips the whole category before it ever reaches documentTypes. The test still passes even if inactive document types are incorrectly included. Make the category active and mark the nested doc type inactive instead.

Suggested fix
-        activeCategoryWithInactiveDocType.put("isActive", false);
+        activeCategoryWithInactiveDocType.put("isActive", true);
         activeCategoryWithInactiveDocType.put("code", "cat2");
         activeCategoryWithInactiveDocType.put("langCode", "eng");

         ArrayNode docTypesArray = objectMapper.createArrayNode();
         ObjectNode inactiveDocType = objectMapper.createObjectNode();
+        inactiveDocType.put("isActive", false);
         inactiveDocType.put("code", "doc1");
+        inactiveDocType.put("name", "Document Name");
         docTypesArray.add(inactiveDocType);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java`
around lines 627 - 636, The test currently never exercises an inactive document
type because activeCategoryWithInactiveDocType has isActive set to false; change
the category to active and mark the nested document type as inactive instead so
the code iterates the category and skips the inactive docType. Specifically, in
the test setup modify activeCategoryWithInactiveDocType.put("isActive", ...) to
true and add inactiveDocType.put("isActive", false) (the variables
activeCategoryWithInactiveDocType and inactiveDocType in
IdrepoProfileRegistryPluginImplTest) so the category is processed but the nested
document type is treated as inactive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 643-645: The direct RestTemplate calls in
IdrepoProfileRegistryPluginImpl.getUISpecification() (e.g.,
restTemplate.getForEntity(docTypesAndCategoryBaseUrl, JsonNode.class) which
assigns response/responseBody) must be replaced to use the shared request(...)
wrapper so transport failures and non-200 payloads are handled by the plugin’s
error mapping; locate the two occurrences (around docTypesAndCategoryBaseUrl and
the other call at the later block) and invoke the existing request(...) helper
with the same URL and expected JsonNode type, then extract the body from the
returned ResponseEntity and proceed as before, ensuring exceptions and
non-success statuses go through the common error path.
- Around line 665-668: In IdrepoProfileRegistryPluginImpl, inside the loop
iterating documentTypes (the for (JsonNode docType : documentTypes) block), add
a check for the documentType-level active flag and skip inactive entries;
specifically, after extracting docTypeCode/docTypeName, verify
docType.hasNonNull("isActive") && docType.get("isActive").asBoolean() (or treat
missing as inactive) and continue when false so that only active document types
are added to allowedValues.
- Around line 328-330: The code casts the result of
responseJson.at(uiSpecJsonpath) directly to ObjectNode (uiSpec), which will
throw ClassCastException if the JSON pointer resolves to a MissingNode or a
non-object; update the logic in IdrepoProfileRegistryPluginImpl around the
request(...) call and the responseJson/uiSpec extraction to first retrieve
JsonNode node = responseJson.at(uiSpecJsonpath), validate that node is not
MissingNode and node.isObject() (or handle array/scalar cases as required), and
only then cast to ObjectNode (or convert/throw a controlled, descriptive
exception if the node is missing or of the wrong type) so configuration/payload
errors produce a clear, controlled failure instead of a runtime
ClassCastException.
- Around line 735-737: The code in IdrepoProfileRegistryPluginImpl mutates the
shared variable pageSize using remainingItems = totalItems - (pageNumber *
pageSize) and then pageSize = remainingItems, which changes the server-side
offset across subsequent pages and can cause duplicates/skips; instead preserve
the original pageSize and compute a local currentPageSize = Math.min(pageSize,
remainingItems) (or simply remove the assignment and use remainingItems only for
the final-page fetch), then use currentPageSize when calling the paging API or
constructing the request so pageSize remains stable across pageNumber iterations
while still handling the final partial page.

---

Outside diff comments:
In
`@mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java`:
- Around line 627-636: The test currently never exercises an inactive document
type because activeCategoryWithInactiveDocType has isActive set to false; change
the category to active and mark the nested document type as inactive instead so
the code iterates the category and skips the inactive docType. Specifically, in
the test setup modify activeCategoryWithInactiveDocType.put("isActive", ...) to
true and add inactiveDocType.put("isActive", false) (the variables
activeCategoryWithInactiveDocType and inactiveDocType in
IdrepoProfileRegistryPluginImplTest) so the category is processed but the nested
document type is treated as inactive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cc8107df-ab0f-4d8f-a37b-3cd2b191f3f6

📥 Commits

Reviewing files that changed from the base of the PR and between f93800c and e2fa9b1.

📒 Files selected for processing (2)
  • mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java
  • mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java

ase-101 added 2 commits March 12, 2026 00:08
Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>
Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java (1)

622-671: ⚠️ Potential issue | 🟡 Minor

This test never exercises the document-type branch.

activeCategoryWithInactiveDocType is marked inactive on Line 642, so fetchAndProcessDocTypesAndCategories() skips the category before it ever inspects documentTypes. The assertion is still useful for inactive categories, but the "inactive document types" part of the scenario is currently untested/misleading.

Based on learnings, the document types payload does not expose a document-type-level isActive flag; only category-level filtering is expected.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java`
around lines 622 - 671, The test never reaches the document-type branch because
activeCategoryWithInactiveDocType is marked inactive; update the test so the
category is active (set activeCategoryWithInactiveDocType.put("isActive", true))
so fetchAndProcessDocTypesAndCategories() will inspect its "documentTypes" array
and exercise the document-type handling in
fetchAllowedValuesFromMasterDataService(); also remove or update the misleading
"inactive document types" comment (document types don't have an isActive flag)
and keep references to activeCategoryWithInactiveDocType and inactiveDocType so
the payload reflects the intended scenario.
♻️ Duplicate comments (2)
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java (2)

739-741: ⚠️ Potential issue | 🟠 Major

Keep pageSize constant across page-numbered requests.

Shrinking pageSize after incrementing pageNumber changes the next request's offset on a typical pageNumber/pageSize API, so multi-page responses can skip or duplicate records. totalPages already bounds the loop.

Suggested fix
             pageNumber++;
-            int remainingItems = totalItems - (pageNumber * pageSize);
-            if (remainingItems < pageSize) {
-                pageSize = remainingItems;
-            }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`
around lines 739 - 741, In IdrepoProfileRegistryPluginImpl, do not mutate the
shared pageSize when computing the final page; keep pageSize constant across
requests (do not assign to pageSize after incrementing pageNumber). Instead
compute remainingItems = totalItems - (pageNumber * pageSize) and set a local
variable (e.g., currentPageSize or lastPageSize) to min(pageSize,
remainingItems) and use that for the final request; ensure loops bounded by
totalPages continue to use the original pageSize for offsets.

328-335: ⚠️ Potential issue | 🟠 Major

Validate the extracted UI-spec node before casting.

JsonNode.at(...) can return MissingNode or any non-object node. The cast on Line 331 happens before the guard, so a bad JSON pointer/payload still fails here with ClassCastException.

Suggested fix
-        ObjectNode uiSpec = (ObjectNode) responseJson.at(uiSpecJsonpath);
-        if(uiSpec.isMissingNode() || uiSpec.isNull()) {
+        JsonNode extractedUiSpec = responseJson.at(uiSpecJsonpath);
+        if (!extractedUiSpec.isObject()) {
             log.error("UI Spec is missing in the response from {} at json path {}", uiSpecUrl, uiSpecJsonpath);
             return objectMapper.createObjectNode();
         }
+        ObjectNode uiSpec = (ObjectNode) extractedUiSpec;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`
around lines 328 - 335, The code casts the result of
responseJson.at(uiSpecJsonpath) directly to ObjectNode (ObjectNode uiSpec =
(ObjectNode) responseJson.at(...)) which can throw ClassCastException for
MissingNode or non-object nodes; change to first capture as JsonNode (e.g.,
JsonNode uiSpecNode = responseJson.at(uiSpecJsonpath)), validate with
uiSpecNode.isMissingNode() || uiSpecNode.isNull() || !uiSpecNode.isObject(), log
and return objectMapper.createObjectNode() on invalid, and only then cast or
convert to ObjectNode (e.g., (ObjectNode) uiSpecNode or
objectMapper.convertValue) for further use; references:
responseJson.at(uiSpecJsonpath), uiSpecUrl, uiSpecJsonpath, request(...),
ObjectNode uiSpec, objectMapper.createObjectNode().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 75-76: The injected field identifierField in
IdrepoProfileRegistryPluginImpl is annotated with
`@Value`("${mosip.signup.identifier.name}") but the property is not present in
application.properties, causing startup failure; fix by either adding
mosip.signup.identifier.name=<default> to the module's application.properties or
change the annotation to include a default like
`@Value`("${mosip.signup.identifier.name:defaultIdentifier}") so Spring can
instantiate the bean; update the annotation on the identifierField or add the
property to runtime config and remove/adjust any redundant null checks
accordingly.

---

Outside diff comments:
In
`@mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java`:
- Around line 622-671: The test never reaches the document-type branch because
activeCategoryWithInactiveDocType is marked inactive; update the test so the
category is active (set activeCategoryWithInactiveDocType.put("isActive", true))
so fetchAndProcessDocTypesAndCategories() will inspect its "documentTypes" array
and exercise the document-type handling in
fetchAllowedValuesFromMasterDataService(); also remove or update the misleading
"inactive document types" comment (document types don't have an isActive flag)
and keep references to activeCategoryWithInactiveDocType and inactiveDocType so
the payload reflects the intended scenario.

---

Duplicate comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 739-741: In IdrepoProfileRegistryPluginImpl, do not mutate the
shared pageSize when computing the final page; keep pageSize constant across
requests (do not assign to pageSize after incrementing pageNumber). Instead
compute remainingItems = totalItems - (pageNumber * pageSize) and set a local
variable (e.g., currentPageSize or lastPageSize) to min(pageSize,
remainingItems) and use that for the final request; ensure loops bounded by
totalPages continue to use the original pageSize for offsets.
- Around line 328-335: The code casts the result of
responseJson.at(uiSpecJsonpath) directly to ObjectNode (ObjectNode uiSpec =
(ObjectNode) responseJson.at(...)) which can throw ClassCastException for
MissingNode or non-object nodes; change to first capture as JsonNode (e.g.,
JsonNode uiSpecNode = responseJson.at(uiSpecJsonpath)), validate with
uiSpecNode.isMissingNode() || uiSpecNode.isNull() || !uiSpecNode.isObject(), log
and return objectMapper.createObjectNode() on invalid, and only then cast or
convert to ObjectNode (e.g., (ObjectNode) uiSpecNode or
objectMapper.convertValue) for further use; references:
responseJson.at(uiSpecJsonpath), uiSpecUrl, uiSpecJsonpath, request(...),
ObjectNode uiSpec, objectMapper.createObjectNode().

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 573f1313-7d37-4a1d-be8e-04f54c72ac8a

📥 Commits

Reviewing files that changed from the base of the PR and between e2fa9b1 and 2236cb0.

📒 Files selected for processing (3)
  • mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java
  • mosip-identity-plugin/src/main/resources/application.properties
  • mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 693-705: The pagination code in IdrepoProfileRegistryPluginImpl
uses objectMapper.convertValue(responseNode.get("totalPages"), Integer.class)
which can throw on missing/null/malformed fields; replace these calls with
null-safe reads from the JsonNode (e.g.,
responseNode.path("totalPages").asInt(defaultValue) and
responseNode.path("totalItems").asInt(defaultValue)) or check
responseNode.hasNonNull("totalPages") before converting so totalPages is
assigned a safe default when absent; also remove or stop relying on the unused
totalItems variable if not needed. This change should be applied where
totalPages/totalItems are set (the block that currently calls
objectMapper.convertValue) and ensure the while loop still uses the safe default
values returned by buildDynamicFieldsUrl/request/response handling.
- Around line 338-345: The current logic only checks key presence so keys
present with explicit null values (e.g., "language": null or "allowedValues":
null) are not replaced by defaults; update the code that builds uiSpec (the
block using uiSpec.has(...), uiSpec.putIfAbsent(...),
objectMapper.valueToTree(...)) to treat null-valued entries as absent by
checking uiSpec.has("key") && !uiSpec.get("key").isNull() (or using
uiSpec.path("key").isMissingNode()/isNull()) before skipping defaults, and if
the node is missing or null call fetchAllowedValuesFromMasterDataService() or
put the default objectMapper.valueToTree(...) for "language", "allowedValues",
"maxUploadFileSize", and "resetPasswordChallengeFields" so explicit nulls are
replaced with the fallback values.
- Around line 126-127: The property name uiSpecJsonpath is misleading because
the code uses JsonNode.at() (JSON Pointer); rename the property key to something
like mosip.signup.mosipid.uispec.schema-json-pointer and rename the field to
uiSpecJsonPointer (update the `@Value` annotation on uiSpecJsonpath and its
default pointer value accordingly), update any references that read the field,
and change the log message that currently says "json path" to explicitly say
"JSON Pointer (RFC 6901)" so callers know to supply pointer syntax; also add a
brief comment or property description in application.properties clarifying that
JSON Pointer, not JSONPath, is expected.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e2c54a18-ad12-4f70-9fd3-62575f6c8710

📥 Commits

Reviewing files that changed from the base of the PR and between 2236cb0 and a25dce6.

📒 Files selected for processing (1)
  • mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java

Copy link
Contributor

@zesu22 zesu22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

ase-101 added 2 commits March 12, 2026 00:40
Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>
Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java (1)

563-568: Add regression coverage for the new paging and pointer guards.

These tests only exercise totalPages = 1 and a valid JSON Pointer, and the contains("dynamic") stubs will still pass if buildDynamicFieldsUrl(...) regresses. A multi-page dynamic-fields fixture plus a pointer-miss/non-object case, with exact URL assertions for the dynamic-fields request, would lock in the riskiest branches this refactor touched.

Also applies to: 603-606, 631-634, 700-738, 797-844

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java`
around lines 563 - 568, The tests (e.g.,
fetchAllowedValues_FromMasterDataService_withValidDetails_thenPass) only cover
single-page responses and use loose Mockito.contains("dynamic") URL matching, so
add regression tests that (1) mock a multi-page dynamic-fields response
(totalPages > 1) to exercise pagination in
buildDynamicFieldsUrl/IdrepoProfileRegistryPluginImpl paging logic, (2) include
a fixture where the JSON Pointer misses or resolves to a non-object to exercise
the pointer guards, and (3) replace contains("dynamic") with exact URL equality
using the concrete value returned by buildDynamicFieldsUrl(...) (or compute the
expected URL inline) so the test fails if the request URL regresses; apply the
same pattern to the other affected test methods referenced (lines 603-606,
631-634, 700-738, 797-844).
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java (1)

129-130: Build the document-type language query from the configured UI languages.

getUISpecification() now advertises mandatoryLanguages/optionalLanguages, but the document-type fetch still depends on a separately pre-expanded docTypesAndCategoryBaseUrl. If those drift, the UI can expose languages that never get localized document labels. Deriving the languages= params from the configured lists would remove that second source of truth.

Also applies to: 335-336, 647-648

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`
around lines 129 - 130, The code currently relies on a pre-expanded
docTypesAndCategoryBaseUrl for the languages= query, which can drift from the UI
spec; change callers that build the document-type request to derive the
languages= parameter from the getUISpecification() result instead of hardcoding
it. In the IdrepoProfileRegistryPluginImpl places where
docTypesAndCategoryBaseUrl is used (references around docTypesAndCategoryBaseUrl
and the methods that fetch document types), obtain UISpecification uiSpec =
getUISpecification(), collect uiSpec.getMandatoryLanguages() and
uiSpec.getOptionalLanguages(), merge and dedupe them, join with commas,
URL-encode the value and append as languages=<joined> to the base endpoint
(falling back to a sensible default if lists are empty). Update all occurrences
(the spots you flagged including the other uses of docTypesAndCategoryBaseUrl)
to use this constructed languages param so UI languages and document-label
localization stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 129-130: The code currently relies on a pre-expanded
docTypesAndCategoryBaseUrl for the languages= query, which can drift from the UI
spec; change callers that build the document-type request to derive the
languages= parameter from the getUISpecification() result instead of hardcoding
it. In the IdrepoProfileRegistryPluginImpl places where
docTypesAndCategoryBaseUrl is used (references around docTypesAndCategoryBaseUrl
and the methods that fetch document types), obtain UISpecification uiSpec =
getUISpecification(), collect uiSpec.getMandatoryLanguages() and
uiSpec.getOptionalLanguages(), merge and dedupe them, join with commas,
URL-encode the value and append as languages=<joined> to the base endpoint
(falling back to a sensible default if lists are empty). Update all occurrences
(the spots you flagged including the other uses of docTypesAndCategoryBaseUrl)
to use this constructed languages param so UI languages and document-label
localization stay in sync.

In
`@mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java`:
- Around line 563-568: The tests (e.g.,
fetchAllowedValues_FromMasterDataService_withValidDetails_thenPass) only cover
single-page responses and use loose Mockito.contains("dynamic") URL matching, so
add regression tests that (1) mock a multi-page dynamic-fields response
(totalPages > 1) to exercise pagination in
buildDynamicFieldsUrl/IdrepoProfileRegistryPluginImpl paging logic, (2) include
a fixture where the JSON Pointer misses or resolves to a non-object to exercise
the pointer guards, and (3) replace contains("dynamic") with exact URL equality
using the concrete value returned by buildDynamicFieldsUrl(...) (or compute the
expected URL inline) so the test fails if the request URL regresses; apply the
same pattern to the other affected test methods referenced (lines 603-606,
631-634, 700-738, 797-844).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 56a72e0a-ea55-4247-835e-8a5ef66ecf36

📥 Commits

Reviewing files that changed from the base of the PR and between a25dce6 and 5490e86.

📒 Files selected for processing (2)
  • mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java
  • mosip-identity-plugin/src/test/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImplTest.java

@Prafulrakhade Prafulrakhade merged commit da49f0e into mosip:develop Mar 12, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants