[OSDEV-2269] [Partner Fields Enhancement] Add URI (JSON Schema) support for Partner Fields#823
Conversation
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 38.37%Your code coverage diff: 0.35% ▴ ✅ All code changes are covered |
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 55.73%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.75%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Django App | Unittest test suite - Code coverage reportTotal: 81.51%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roman-stolar
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
…r-field # Conflicts: # src/react/src/util/renderUtils.jsx
…69-add-uri-format-support-for-partner-field # Conflicts: # doc/release/RELEASE-NOTES.md
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
doc/release/RELEASE-NOTES.md(2 hunks)src/react/src/components/FacilityDetailsGeneralFields.jsx(1 hunks)src/react/src/util/partnerFieldFormatter.jsx(1 hunks)src/react/src/util/renderUtils.jsx(1 hunks)src/react/src/util/util.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/react/src/components/FacilityDetailsGeneralFields.jsx
- doc/release/RELEASE-NOTES.md
- src/react/src/util/util.js
- src/react/src/util/renderUtils.jsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-02-12T11:45:01.562Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 511
File: src/react/src/util/util.js:1436-1447
Timestamp: 2025-02-12T11:45:01.562Z
Learning: The `generateRangeField` utility function in `src/react/src/util/util.js` is meant for data transformation only. Input validation is handled separately on the frontend for specific input fields.
Applied to files:
src/react/src/util/partnerFieldFormatter.jsx
📚 Learning: 2024-12-10T15:07:54.819Z
Learnt from: roman-stolar
Repo: opensupplyhub/open-supply-hub PR: 448
File: src/react/src/util/util.js:1317-1317
Timestamp: 2024-12-10T15:07:54.819Z
Learning: Changes to date handling functions like `formatDate` in `src/react/src/util/util.js` should be made carefully, considering that different parts of the codebase may intentionally use local time instead of UTC, and broad changes to `moment()` usage may not be appropriate without context.
Applied to files:
src/react/src/util/partnerFieldFormatter.jsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-dd-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-flake8-linter
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: run-django-code-quality
🔇 Additional comments (6)
src/react/src/util/partnerFieldFormatter.jsx (6)
1-9: LGTM! Good use of Object.freeze for immutability.The constant structure is clean and extensible for future format types.
11-23: LGTM! Safe property access and appropriate data structure.The use of optional chaining and Set for collecting unique property names is well-implemented.
33-42: LGTM! Clean implementation with safe defaults.The default empty object fallback for
schemaPropertiesprevents runtime errors.
58-70: LGTM! Clean implementation of the _text property pattern.The helper functions correctly identify and retrieve display text for URI fields.
72-96: LGTM! Proper handling of empty URIs and mixed field types.The function correctly filters out empty URI values and _text properties, returning a mixed array of formatted strings and React link elements.
28-31: TheformatValueWithLabelfunction is appropriately designed for its use case.The function correctly handles primitive values using
String()coercion, which is suitable for partner field formatting. Input validation informatPartnerFieldWithSchemaensures only plain objects are processed, and schema-driven extraction provides guidance on expected property types. The existing implementation and comprehensive test coverage confirm that only primitive values are formatted.Likely an incorrect or invalid review comment.
…69-add-uri-format-support-for-partner-field # Conflicts: # doc/release/RELEASE-NOTES.md
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/react/src/components/FacilityDetailsItem.jsx (1)
33-50: jsonSchema prop wiring through FacilityDetailsItem is consistent and safeAdding
jsonSchemato the props and forwarding it to the primary and drawerFacilityDetailsDetailinstances (gated by!embed, likesecondary,sourceBy, andunit) looks correct and keeps embed behavior unchanged. The additional content rows can still carry their ownjsonSchemavia the spread, so no extra work is needed there. If you ever want schema‑aware rendering (e.g., clickable URIs) in embed views too, consider dropping the!embedguard forjsonSchemawhile keeping the others as‑is.Also applies to: 60-71, 99-107
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js(1 hunks)src/react/src/__tests__/utils.tests.js(1 hunks)src/react/src/components/FacilityDetailsDetail.jsx(3 hunks)src/react/src/components/FacilityDetailsGeneralFields.jsx(1 hunks)src/react/src/components/FacilityDetailsItem.jsx(3 hunks)src/react/src/components/PartnerFieldSchemaValue.jsx(1 hunks)src/react/src/util/util.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/react/src/components/FacilityDetailsGeneralFields.jsx
- src/react/src/util/util.js
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-01-17T16:12:18.285Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 483
File: src/react/src/__tests__/components/SearchByNameAndAddressSuccessResult.test.js:0-0
Timestamp: 2025-01-17T16:12:18.285Z
Learning: In the SearchByNameAndAddressSuccessResult component's tests, attempting to test internal navigation logic through mocking useHistory is not feasible, and button presence/click events should be covered in the main rendering test case.
Applied to files:
src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js
📚 Learning: 2025-10-31T08:29:48.108Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 781
File: src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js:23-52
Timestamp: 2025-10-31T08:29:48.108Z
Learning: In src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js, the Redux state field `claimantEmploymentVerificationMethod` stores the label (full sentence, e.g., "Company website showing your name and title (e.g., About Us, Team page)") from EMPLOYMENT_VERIFICATION_OPTIONS, not the option value (e.g., "company-website-address"). The validation schema correctly compares against labels.
Applied to files:
src/react/src/components/PartnerFieldSchemaValue.jsxsrc/react/src/components/FacilityDetailsItem.jsxsrc/react/src/components/FacilityDetailsDetail.jsx
📚 Learning: 2024-12-10T07:09:35.641Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 436
File: src/react/src/components/Contribute/ProductionLocationDialog.jsx:33-181
Timestamp: 2024-12-10T07:09:35.641Z
Learning: In the `ProductionLocationDialog.jsx` component, hardcoded facility data is acceptable for now as per the project's requirements.
Applied to files:
src/react/src/components/FacilityDetailsItem.jsxsrc/react/src/components/FacilityDetailsDetail.jsx
📚 Learning: 2024-12-05T10:43:59.922Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 437
File: src/react/src/components/Filters/StyledSelect.jsx:31-39
Timestamp: 2024-12-05T10:43:59.922Z
Learning: In the `StyledSelect` component (`src/react/src/components/Filters/StyledSelect.jsx`), the `label` prop is optional. The `InputLabel` should be conditionally rendered only when a label is provided to prevent unnecessary DOM elements, extra padding, and browser console warnings.
Applied to files:
src/react/src/components/FacilityDetailsDetail.jsx
📚 Learning: 2024-12-10T15:07:54.819Z
Learnt from: roman-stolar
Repo: opensupplyhub/open-supply-hub PR: 448
File: src/react/src/util/util.js:1317-1317
Timestamp: 2024-12-10T15:07:54.819Z
Learning: Changes to date handling functions like `formatDate` in `src/react/src/util/util.js` should be made carefully, considering that different parts of the codebase may intentionally use local time instead of UTC, and broad changes to `moment()` usage may not be appropriate without context.
Applied to files:
src/react/src/__tests__/utils.tests.js
🧬 Code graph analysis (3)
src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js (1)
src/react/src/components/PartnerFieldSchemaValue.jsx (1)
PartnerFieldSchemaValue(117-151)
src/react/src/components/FacilityDetailsDetail.jsx (2)
src/react/src/components/PartnerFieldSchemaValue.jsx (1)
PartnerFieldSchemaValue(117-151)src/react/src/util/util.js (1)
primary(1792-1795)
src/react/src/__tests__/utils.tests.js (1)
src/react/src/util/util.js (2)
formatPartnerFieldValue(1906-1919)formatPartnerFieldValue(1906-1919)
🔇 Additional comments (3)
src/react/src/components/FacilityDetailsDetail.jsx (1)
9-10: Schema‑aware primary rendering is correct; confirm fallback behavior with jsonSchema presentWiring
jsonSchemathrough and switching the primary rendering toPartnerFieldSchemaValuewhenjsonSchemais truthy is appropriate and aligns with the new schema‑driven partner fields. One subtle change is that whenjsonSchemais present, thelocationLabeledfallback is no longer used—PartnerFieldSchemaValuereturnsprimaryas‑is (includingundefined), so an emptyprimarywon’t fall back tolocationLabeledanymore. That’s fine ifjsonSchemais only set for partner fields that always provide an object value, but it’s worth double‑checking this assumption.Also applies to: 61-71, 88-97
src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js (1)
1-120: PartnerFieldSchemaValue tests comprehensively cover URI and non‑URI casesThe new test suite hits the key behaviors: URI fields with and without
_text, non‑URI fields with titles, and mixed objects while skipping the_textproperty. Assertions onhref,target,rel, and single occurrence of link text match the implementation and should guard future refactors well.src/react/src/__tests__/utils.tests.js (1)
2739-2863: formatPartnerFieldValue tests thoroughly exercise the updated APIThe new
formatPartnerFieldValuesuite does a good job of pinning down behavior across raw_value, raw_values (arrays/objects/strings), mixed types, and the new schema‑driven object case. Passing a second argument keeps tests aligned with the updated(fieldValueData, json_schema)signature while ensuring pre‑existing formatting expectations remain intact when no real schema is supplied.
…69-add-uri-format-support-for-partner-field # Conflicts: # doc/release/RELEASE-NOTES.md
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/react/src/components/PartnerFieldSchemaValue.jsx (3)
23-32: Remove redundantkeyprop inside UriProperty.The
keyprop at line 25 is unnecessary becauserenderPropertyalready provides akeywhen wrappingFormatComponentat line 105. React only needs the outermostkeyfor list rendering.Apply this diff to remove the redundant key:
return ( <a - key={`${propertyKey}-uri-${propertyValue}`} href={propertyValue} target="_blank" rel="noopener noreferrer" > {linkText} </a> );
43-46: Remove redundantkeyprop inside DefaultProperty.Similar to
UriProperty, thekeyprop at line 44 is redundant sincerenderPropertyprovides it at line 105.Apply this diff:
return ( - <span key={`${propertyKey}-default`} style={{ display: 'block' }}> + <span style={{ display: 'block' }}> {displayText} </span> );
94-112: Remove redundant property existence check.The check at lines 97-99 is redundant because
propertyKeycomes fromObject.keys(value)(line 129), which only returns own enumerable properties. The condition will always be true.Apply this diff to simplify the function:
const renderProperty = (propertyKey, propertyValue, propertySchema, value) => { const format = getFormatFromSchema(propertySchema); - - if (!Object.prototype.hasOwnProperty.call(value, propertyKey)) { - return null; - } - const FormatComponent = getFormatComponent(format); return (src/react/src/util/util.js (1)
1906-1924: Partner field raw_values/raw_value handling matches backend semantics; small cleanups possibleThis function correctly reflects the convention that object‑type partner fields use
raw_valueswhile others useraw_value, and it sensibly bypasses stringification when a JSON Schema withpropertiesis present so the schema‑aware renderer can handle URIs. Based on learnings, this aligns with the intended backend behavior.Two small improvements you might consider:
- Use optional chaining for the schema properties check (also aligns with the SonarCloud hint around Line 1911).
- Be explicit that the schema‑driven path is for object‑shaped
raw_values, to avoid accidentally treating non‑object values as schema‑driven in the future.For example:
export const formatPartnerFieldValue = (fieldValueData, json_schema) => { const { raw_values, raw_value } = fieldValueData; if (raw_values !== undefined) { - if ( - json_schema && - json_schema.properties && - Object.keys(json_schema.properties).length > 0 && - !Array.isArray(raw_values) - ) { - return raw_values; - } + if ( + json_schema?.properties && + Object.keys(json_schema.properties).length > 0 && + typeof raw_values === 'object' && + raw_values !== null && + !Array.isArray(raw_values) + ) { + return raw_values; + } return formatRawValues(raw_values); } if (raw_value !== undefined) { return raw_value; } return fieldValueData; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/react/src/components/PartnerFieldSchemaValue.jsx(1 hunks)src/react/src/util/util.js(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-31T08:29:48.108Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 781
File: src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js:23-52
Timestamp: 2025-10-31T08:29:48.108Z
Learning: In src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js, the Redux state field `claimantEmploymentVerificationMethod` stores the label (full sentence, e.g., "Company website showing your name and title (e.g., About Us, Team page)") from EMPLOYMENT_VERIFICATION_OPTIONS, not the option value (e.g., "company-website-address"). The validation schema correctly compares against labels.
Applied to files:
src/react/src/components/PartnerFieldSchemaValue.jsx
📚 Learning: 2024-12-10T15:07:54.819Z
Learnt from: roman-stolar
Repo: opensupplyhub/open-supply-hub PR: 448
File: src/react/src/util/util.js:1317-1317
Timestamp: 2024-12-10T15:07:54.819Z
Learning: Changes to date handling functions like `formatDate` in `src/react/src/util/util.js` should be made carefully, considering that different parts of the codebase may intentionally use local time instead of UTC, and broad changes to `moment()` usage may not be appropriate without context.
Applied to files:
src/react/src/util/util.js
📚 Learning: 2025-02-12T11:45:01.562Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 511
File: src/react/src/util/util.js:1436-1447
Timestamp: 2025-02-12T11:45:01.562Z
Learning: The `generateRangeField` utility function in `src/react/src/util/util.js` is meant for data transformation only. Input validation is handled separately on the frontend for specific input fields.
Applied to files:
src/react/src/util/util.js
📚 Learning: 2025-09-25T10:52:27.455Z
Learnt from: roman-stolar
Repo: opensupplyhub/open-supply-hub PR: 757
File: src/django/api/extended_fields.py:186-207
Timestamp: 2025-09-25T10:52:27.455Z
Learning: In partner extended fields, OBJECT_FIELD_TYPE fields use 'raw_values' while other partner field types use 'raw_value'. This is the correct and intended behavior as confirmed by the implementation team.
Applied to files:
src/react/src/util/util.js
🧬 Code graph analysis (1)
src/react/src/util/util.js (1)
src/react/src/util/renderUtils.jsx (1)
renderUniqueListItems(8-40)
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/PartnerFieldSchemaValue.jsx
[warning] 97-97: Use 'Object.hasOwn()' instead of 'Object.prototype.hasOwnProperty.call()'.
[warning] 19-19: Use 'Object.hasOwn()' instead of 'Object.prototype.hasOwnProperty.call()'.
src/react/src/util/util.js
[warning] 1911-1912: Prefer using an optional chain expression instead, as it's more concise and easier to read.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-flake8-linter
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-django-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: run-fe-code-quality
🔇 Additional comments (2)
src/react/src/components/PartnerFieldSchemaValue.jsx (1)
19-19: Verify target browser support before adoptingObject.hasOwn().
Object.hasOwn()is an ES2022 feature supported in Chrome 93+, Edge 93+, Firefox 92+, Safari 15.4+, but not in Internet Explorer. Since React 16.13.1 targets "modern browsers" without specifying concrete version minimums, many projects still support older environments whereObject.hasOwn()is unavailable. If your target browsers include IE or older evergreen versions, stick withObject.prototype.hasOwnProperty.call()or add a polyfill (core-js/es-shims). Check your project's Browserslist configuration to confirm whether ES2022 is viable.src/react/src/util/util.js (1)
1780-1809: Extended field json_schema wiring and primary value formatting look correctAccepting
json_schema, passing it toformatValue(value, json_schema), and exposing it asjsonSchemain the returned object is coherent with the new schema‑driven rendering and remains backward‑compatible (extra arg toformatValueis ignored by existing single‑param implementations). No changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/react/src/components/PartnerFieldSchemaValue.jsx (3)
19-25: Consider optional chaining for cleaner code.Line 20 could be simplified using optional chaining for better consistency with the rest of the codebase.
Apply this diff:
- const textPropertyDefined = schemaProperties && schemaProperties[textKey]; + const textPropertyDefined = schemaProperties?.[textKey];
13-37: Consider adding PropTypes validation.PropTypes would improve component maintainability and provide runtime validation in development. The same applies to
DefaultPropertybelow.Add at the top of the file:
import PropTypes from 'prop-types';Add after the component definitions:
UriProperty.propTypes = { propertyKey: PropTypes.string.isRequired, value: PropTypes.object.isRequired, schemaProperties: PropTypes.object, }; DefaultProperty.propTypes = { propertyKey: PropTypes.string.isRequired, value: PropTypes.object.isRequired, schemaProperties: PropTypes.object, };
23-23: Consider usingObject.hasOwn()for modern syntax.Lines 23, 74, and 105 use
Object.prototype.hasOwnProperty.call(). The modernObject.hasOwn()(ES2022) is more concise and readable.Before applying this change, verify browser support requirements match your target environments. If ES2022 support is confirmed, apply:
- Object.prototype.hasOwnProperty.call(value, textKey) + Object.hasOwn(value, textKey)- if (!Object.prototype.hasOwnProperty.call(schemaProperties, propertyKey)) { + if (!Object.hasOwn(schemaProperties, propertyKey)) {- if (!Object.prototype.hasOwnProperty.call(value, propertyKey)) { + if (!Object.hasOwn(value, propertyKey)) {Also applies to: 74-74, 105-105
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js(1 hunks)src/react/src/components/PartnerFieldSchemaValue.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/tests/components/PartnerFieldSchemaValue.test.js
🧰 Additional context used
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/PartnerFieldSchemaValue.jsx
[warning] 74-74: Use 'Object.hasOwn()' instead of 'Object.prototype.hasOwnProperty.call()'.
[warning] 42-42: 'value' is missing in props validation
[warning] 42-42: 'propertyKey' is missing in props validation
[warning] 13-13: 'schemaProperties' is missing in props validation
[warning] 20-20: Prefer using an optional chain expression instead, as it's more concise and easier to read.
[warning] 42-42: 'schemaProperties' is missing in props validation
[warning] 23-23: Use 'Object.hasOwn()' instead of 'Object.prototype.hasOwnProperty.call()'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (3)
src/react/src/components/PartnerFieldSchemaValue.jsx (3)
1-8: LGTM!Clean constant definition with proper immutability via
Object.freeze.
60-63: LGTM!Clean, extensible registry pattern that makes it easy to add new format types in the future.
126-148: LGTM!The main component logic is well-structured with appropriate input validation and clean functional composition. The early return pattern for non-object values maintains backward compatibility while enabling schema-driven rendering for object values.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/react/src/components/PartnerFieldSchemaValue.jsx (2)
29-29: Remove redundantkeyprops from component return elements.The
<a>element at line 29 and<span>element at line 50 includekeyprops, butrenderPropertyalready assigns akeyto theFormatComponentwrapper at line 115. Since these elements are not in an array/list context within the component, the inner keys serve no purpose and can be safely removed.Apply this diff:
const UriProperty = ({ propertyKey, value, schemaProperties }) => { // ... return ( <a - key={`${propertyKey}-uri-${propertyValue}`} href={propertyValue} target="_blank" rel="noopener noreferrer" > {linkText} </a> ); };const DefaultProperty = ({ propertyKey, value, schemaProperties }) => { // ... return ( - <span key={`${propertyKey}-default`} style={{ display: 'block' }}> + <span style={{ display: 'block' }}> {displayText} </span> ); };Also applies to: 50-50
13-13: Consider adding PropTypes validation for component props.Both
UriPropertyandDefaultPropertyacceptpropertyKey,value, andschemaPropertiesprops but lack PropTypes validation. While not strictly required in modern React, PropTypes improve maintainability by documenting expected prop shapes and catching type mismatches during development.Add PropTypes at the end of the file:
import PropTypes from 'prop-types'; // ... existing code ... UriProperty.propTypes = { propertyKey: PropTypes.string.isRequired, value: PropTypes.object.isRequired, schemaProperties: PropTypes.object, }; DefaultProperty.propTypes = { propertyKey: PropTypes.string.isRequired, value: PropTypes.object.isRequired, schemaProperties: PropTypes.object, }; PartnerFieldSchemaValue.propTypes = { value: PropTypes.object, jsonSchema: PropTypes.shape({ properties: PropTypes.object, }), };🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 42-42: 'value' is missing in props validation
[warning] 42-42: 'propertyKey' is missing in props validation
[warning] 42-42: 'schemaProperties' is missing in props validation
Also applies to: 42-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/PartnerFieldSchemaValue.jsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-31T08:29:48.108Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 781
File: src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js:23-52
Timestamp: 2025-10-31T08:29:48.108Z
Learning: In src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js, the Redux state field `claimantEmploymentVerificationMethod` stores the label (full sentence, e.g., "Company website showing your name and title (e.g., About Us, Team page)") from EMPLOYMENT_VERIFICATION_OPTIONS, not the option value (e.g., "company-website-address"). The validation schema correctly compares against labels.
Applied to files:
src/react/src/components/PartnerFieldSchemaValue.jsx
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/PartnerFieldSchemaValue.jsx
[warning] 46-46: Unexpected negated condition.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-flake8-linter
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: run-integration-test-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (3)
src/react/src/components/PartnerFieldSchemaValue.jsx (3)
1-8: LGTM!Clean imports and well-structured format constants. Using
Object.freezeprevents accidental mutations.
56-121: LGTM! Well-architected format handling.The helper functions demonstrate solid design:
- Frozen
FORMAT_COMPONENTSregistry prevents accidental mutations and makes extension points clearshouldSkipPropertycorrectly filters_textcompanion fields when their base property uses URI formathasOwnPropertychecks guard against prototype pollution- Optional chaining provides safe property access
- Clean separation of concerns with discrete helper functions
The format-based rendering strategy is extensible and maintainable.
123-148: LGTM! Robust input validation and clean rendering pipeline.The main component demonstrates solid defensive programming:
- Validates that
jsonSchemaandvaluemeet requirements before proceeding- Gracefully degrades by returning the raw value when schema-based rendering isn't applicable
- Uses safe property access with optional chaining and fallback to empty object
- Clean filter/map/filter pipeline produces the rendered items
- Fragment wrapper avoids unnecessary DOM nesting
The component integrates seamlessly with the helper functions to deliver schema-driven rendering.
Note: The static analysis warning about "negated condition" on line 46 is a false positive—the
!= nullpattern is idiomatic JavaScript for checking bothnullandundefined.
VadimKovalenkoSNF
left a comment
VadimKovalenkoSNF
left a comment
There was a problem hiding this comment.
Request few changes, pls check.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/react/src/components/PartnerFields/UriProperty.jsx (2)
12-17: Use optional chaining for cleaner code.As noted by static analysis, the condition on line 13 can be simplified using optional chaining.
- const textPropertyDefined = schemaProperties && schemaProperties[textKey]; + const textPropertyDefined = schemaProperties?.[textKey];
19-27: Consider validating URI protocol to prevent potential XSS.While the
rel="noopener noreferrer"is correctly used,propertyValueis directly used ashref. If malicious data withjavascript:protocol is passed, it could execute scripts. Consider validating that the URI uses an allowed protocol (e.g.,http:,https:).+const isValidUri = uri => { + try { + const url = new URL(uri); + return ['http:', 'https:'].includes(url.protocol); + } catch { + return false; + } +}; + const UriProperty = ({ propertyKey, value, schemaProperties }) => { const propertyValue = value[propertyKey]; - if (!propertyValue) { + if (!propertyValue || !isValidUri(propertyValue)) { return null; }src/react/src/components/PartnerFields/utils.js (1)
14-33: Logic is correct and handles edge cases safely.The function correctly identifies properties to skip based on schema presence and URI format logic. The optional chaining in
getFormatFromSchemaensures safe handling of undefined schemas.Optional: Consider extracting the magic number for clarity.
Line 23 uses
-5to remove the_textsuffix. While this is correct, you could make it more explicit:+const TEXT_SUFFIX = '_text'; + export const shouldSkipProperty = (propertyKey, schemaProperties) => { if (!(propertyKey in schemaProperties)) { return true; } - if (propertyKey.endsWith('_text')) { - const baseKey = propertyKey.slice(0, -5); + if (propertyKey.endsWith(TEXT_SUFFIX)) { + const baseKey = propertyKey.slice(0, -TEXT_SUFFIX.length); const baseSchema = schemaProperties[baseKey]; const baseFormat = getFormatFromSchema(baseSchema); if (baseFormat === FORMAT_TYPES.URI) { return true; } } return false; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js(1 hunks)src/react/src/__tests__/utils.tests.js(1 hunks)src/react/src/components/FacilityDetailsDetail.jsx(3 hunks)src/react/src/components/PartnerFields/DefaultProperty.jsx(1 hunks)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx(1 hunks)src/react/src/components/PartnerFields/UriProperty.jsx(1 hunks)src/react/src/components/PartnerFields/utils.js(1 hunks)src/react/src/util/util.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/react/src/util/util.js
- src/react/src/tests/components/PartnerFieldSchemaValue.test.js
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-02-12T11:45:01.562Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 511
File: src/react/src/util/util.js:1436-1447
Timestamp: 2025-02-12T11:45:01.562Z
Learning: The `generateRangeField` utility function in `src/react/src/util/util.js` is meant for data transformation only. Input validation is handled separately on the frontend for specific input fields.
Applied to files:
src/react/src/components/PartnerFields/utils.jssrc/react/src/__tests__/utils.tests.js
📚 Learning: 2024-12-10T15:07:54.819Z
Learnt from: roman-stolar
Repo: opensupplyhub/open-supply-hub PR: 448
File: src/react/src/util/util.js:1317-1317
Timestamp: 2024-12-10T15:07:54.819Z
Learning: Changes to date handling functions like `formatDate` in `src/react/src/util/util.js` should be made carefully, considering that different parts of the codebase may intentionally use local time instead of UTC, and broad changes to `moment()` usage may not be appropriate without context.
Applied to files:
src/react/src/components/PartnerFields/utils.jssrc/react/src/__tests__/utils.tests.js
📚 Learning: 2025-10-31T08:29:48.108Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 781
File: src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js:23-52
Timestamp: 2025-10-31T08:29:48.108Z
Learning: In src/react/src/components/InitialClaimFlow/ClaimForm/validationSchemas.js, the Redux state field `claimantEmploymentVerificationMethod` stores the label (full sentence, e.g., "Company website showing your name and title (e.g., About Us, Team page)") from EMPLOYMENT_VERIFICATION_OPTIONS, not the option value (e.g., "company-website-address"). The validation schema correctly compares against labels.
Applied to files:
src/react/src/components/FacilityDetailsDetail.jsx
📚 Learning: 2024-12-05T10:43:59.922Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 437
File: src/react/src/components/Filters/StyledSelect.jsx:31-39
Timestamp: 2024-12-05T10:43:59.922Z
Learning: In the `StyledSelect` component (`src/react/src/components/Filters/StyledSelect.jsx`), the `label` prop is optional. The `InputLabel` should be conditionally rendered only when a label is provided to prevent unnecessary DOM elements, extra padding, and browser console warnings.
Applied to files:
src/react/src/components/FacilityDetailsDetail.jsx
🧬 Code graph analysis (5)
src/react/src/components/PartnerFields/UriProperty.jsx (3)
src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
propertyValue(7-7)src/react/src/__tests__/utils.tests.js (1)
value(771-771)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
schemaProperties(56-56)
src/react/src/components/PartnerFields/utils.js (2)
src/react/src/components/PartnerFields/DefaultProperty.jsx (2)
propertySchema(8-8)propertySchema(9-9)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (2)
propertySchema(29-29)schemaProperties(56-56)
src/react/src/components/FacilityDetailsDetail.jsx (2)
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
PartnerFieldSchemaValue(46-70)src/react/src/util/util.js (1)
primary(1792-1795)
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (3)
src/react/src/components/PartnerFields/utils.js (6)
FORMAT_TYPES(4-6)FORMAT_TYPES(4-6)getFormatFromSchema(11-12)getFormatFromSchema(11-12)shouldSkipProperty(17-33)shouldSkipProperty(17-33)src/react/src/components/PartnerFields/UriProperty.jsx (1)
UriProperty(6-29)src/react/src/components/PartnerFields/DefaultProperty.jsx (3)
DefaultProperty(6-18)propertySchema(8-8)propertySchema(9-9)
src/react/src/__tests__/utils.tests.js (1)
src/react/src/util/util.js (2)
formatPartnerFieldValue(1906-1926)formatPartnerFieldValue(1906-1926)
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/PartnerFields/UriProperty.jsx
[warning] 6-6: 'propertyKey' is missing in props validation
[warning] 6-6: 'schemaProperties' is missing in props validation
[warning] 6-6: 'value' is missing in props validation
[warning] 13-13: Prefer using an optional chain expression instead, as it's more concise and easier to read.
src/react/src/components/PartnerFields/DefaultProperty.jsx
[warning] 6-6: 'value' is missing in props validation
[warning] 6-6: 'propertyKey' is missing in props validation
[warning] 10-10: Unexpected negated condition.
[warning] 6-6: 'schemaProperties' is missing in props validation
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx
[warning] 56-56: 'jsonSchema.properties' is missing in props validation
[warning] 46-46: 'value' is missing in props validation
[warning] 46-46: 'jsonSchema' is missing in props validation
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-flake8-linter
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (6)
src/react/src/components/FacilityDetailsDetail.jsx (1)
89-96: LGTM! Conditional rendering correctly delegates to schema-based rendering.The conditional logic properly uses
PartnerFieldSchemaValuewhenjsonSchemais provided, falling back to the originalprimary || locationLabeledbehavior otherwise. ThePartnerFieldSchemaValuecomponent handles falsy values gracefully by returning them directly.src/react/src/__tests__/utils.tests.js (1)
2835-2860: Test may pass for unintended reasons - verify intended behavior.Based on the
formatPartnerFieldValueimplementation in the relevant code snippet, whenraw_value(singular) is present, it's returned directly without consideringjsonSchema. The schema-aware path only applies toraw_values(plural) when it's a non-array object.If the intent is to test schema-aware formatting, consider using
raw_valuesinstead:- const value = { - raw_value: { - url: 'https://example.com/report', - url_text: 'View Report', - }, - }; + const value = { + raw_values: { + url: 'https://example.com/report', + url_text: 'View Report', + }, + };src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
6-17: LGTM! Clean implementation with appropriate fallbacks.The component correctly:
- Handles missing schema with fallback to empty object
- Safely converts values to strings with null/undefined handling
- Conditionally includes title prefix when available
The
keyprop on line 14 is redundant since the parent already provides a key, but it's harmless.src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
58-67: Rendering logic is well-structured.The reduce pattern cleanly aggregates rendered items while respecting the skip logic. The null check on line 63 is defensive, though
renderPropertyalways returns a React element.src/react/src/components/PartnerFields/utils.js (2)
1-6: LGTM!The FORMAT_TYPES constant is well-defined and clearly documents the supported format types.
8-12: LGTM!The function correctly uses optional chaining to safely extract the format from the property schema, returning null when format is not present.
|
3 participants
OSDEV-2269 [Partner Fields Enhancement] Add URI (JSON Schema) support for Partner Fields
"format": "uri"are rendered as clickable links on production location profile pages.