[OSDEV-2268] Add base_url and display_text for object partner field#819
[OSDEV-2268] Add base_url and display_text for object partner field#819VadimKovalenkoSNF merged 21 commits intomainfrom
Conversation
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 39.39%Your code coverage diff: 0.23% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
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 |
VadimKovalenkoSNF
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 |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.75%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Error
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin UI (browser)
participant Backend as Django API
participant DB as Database
participant Frontend as React client
Admin->>Backend: Create/Update PartnerField (name,type,base_url,display_text,json_schema)
Backend->>DB: Persist PartnerField (base_url, display_text)
DB-->>Backend: Ack persisted
Frontend->>Backend: GET facility index (includes partner_fields)
Backend->>DB: Query facility + partner field metadata
DB-->>Backend: Return partner field rows (base_url, display_text, json_schema)
Backend->>Frontend: Respond with serialized facility (extended-field + partnerConfigFields)
Frontend->>Frontend: PartnerFieldSchemaValue reads jsonSchema + partnerConfigFields
Frontend->>Frontend: UriReferenceProperty constructs href (constructUrlFromPartnerField) and renders link using display_text or fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/django/api/serializers/facility/facility_index_serializer.py (1)
110-111: Consider direct attribute access instead of getattr.Since
base_urlanddisplay_textare defined model fields onPartnerField(added in migration 0189), usinggetattrwith a default is overly defensive. Direct attribute access would be cleaner and consistent with how other fields likesource_by,unit, andlabelare accessed on lines 107-109.- base_url = getattr(field, 'base_url', None) - display_text = getattr(field, 'display_text', None) + base_url = field.base_url + display_text = field.display_textsrc/react/src/__tests__/components/PartnerFieldSchemaValue.test.js (1)
121-203: New uri-reference tests look solid; consider one more edge caseThe new cases nicely cover partnerConfigFields usage, loss of
uri-referenceformat, and primitive fallback. To make this even more robust, consider adding a scenario wherevalue_textis present and differs fromvalue, to assert that the URL is still constructed from the underlying value while the link text comes from config/text. That will guard against regressions in theUriReferencePropertyURL-building logic.src/django/api/models/partner_field.py (1)
52-60: New PartnerField attributes are well-shaped; consider brief help_text
base_urlanddisplay_textlook appropriately typed and optional. If admin users may be unsure when to use them (e.g., only for object/uri-reference fields), you might consider addinghelp_textto each field to document that intent directly in the admin, but the current model definition is otherwise fine.src/react/src/__tests__/components/UriReferenceProperty.test.js (1)
15-129: Good coverage of UriReferenceProperty; one optional extra caseThese tests exercise the key behaviors (description rendering, base URL handling, display text fallback, and empty values) and will be very helpful when evolving the component.
After updating
UriReferencePropertyto always construct URLs from the raw field value, you might add a case wherevalue_textdiffers fromvaluewhilebaseUrlis set, asserting that:
- the link text is still what you expect (config or
value_text), and- the
hrefis built from the underlying value, not the label.That would lock in the intended contract.
src/react/src/components/PartnerFields/utils.js (1)
19-35: *Skip _text fields for uri-reference as well to avoid duplicate display
shouldSkipPropertyalready hides companion_textfields foruriformats. Foruri-referenceschemas that follow the same pattern (a primary field plus a<field>_text), leaving_textvisible will likely cause it to be rendered twice: once as part of the link and once via the default renderer.You can align behaviors and avoid duplication like this:
export const shouldSkipProperty = (propertyKey, schemaProperties) => { @@ if (propertyKey.endsWith('_text')) { const baseKey = propertyKey.slice(0, -5); const baseSchema = schemaProperties[baseKey]; const baseFormat = getFormatFromSchema(baseSchema); - if (baseFormat === FORMAT_TYPES.URI) { + if ( + baseFormat === FORMAT_TYPES.URI || + baseFormat === FORMAT_TYPES.URI_REFERENCE + ) { return true; } }The new helpers
showFieldDefaultDisplayTextandconstructUrlFromPartnerFieldlook good and are safe with the current call sites.Also applies to: 37-46, 48-51
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
54-86: Add PropTypes validation for component props.The component logic correctly threads
partnerConfigFieldsthrough torenderProperty. However, static analysis correctly identifies that PropTypes validation is missing forvalue,jsonSchema, andpartnerConfigFields.Add PropTypes at the end of the file:
import PropTypes from 'prop-types'; // ... existing code ... PartnerFieldSchemaValue.propTypes = { value: PropTypes.oneOfType([ PropTypes.object, PropTypes.string, PropTypes.number, ]), jsonSchema: PropTypes.shape({ properties: PropTypes.object, }), partnerConfigFields: PropTypes.shape({ baseUrl: PropTypes.string, displayText: PropTypes.string, }), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
doc/release/RELEASE-NOTES.md(1 hunks)src/django/api/admin.py(1 hunks)src/django/api/migrations/0189_add_base_url_and_text_field_to_partner_field.py(1 hunks)src/django/api/models/partner_field.py(1 hunks)src/django/api/serializers/facility/facility_index_extended_field_list_serializer.py(2 hunks)src/django/api/serializers/facility/facility_index_serializer.py(2 hunks)src/django/api/static/admin/js/partner_field_admin.js(1 hunks)src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js(1 hunks)src/react/src/__tests__/components/UriReferenceProperty.test.js(1 hunks)src/react/src/components/FacilityDetailsDetail.jsx(2 hunks)src/react/src/components/FacilityDetailsGeneralFields.jsx(5 hunks)src/react/src/components/FacilityDetailsItem.jsx(3 hunks)src/react/src/components/PartnerFields/DefaultProperty.jsx(1 hunks)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx(5 hunks)src/react/src/components/PartnerFields/UriReferenceProperty.jsx(1 hunks)src/react/src/components/PartnerFields/utils.js(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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/PartnerFields/DefaultProperty.jsx
📚 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: 2024-11-28T06:36:47.122Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 420
File: doc/release/RELEASE-NOTES.md:47-54
Timestamp: 2024-11-28T06:36:47.122Z
Learning: Endpoints that have not been enabled to end users do not require migration documentation or old vs new format examples in the release notes.
Applied to files:
doc/release/RELEASE-NOTES.md
📚 Learning: 2024-11-26T04:59:12.296Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 420
File: doc/release/RELEASE-NOTES.md:38-38
Timestamp: 2024-11-26T04:59:12.296Z
Learning: For endpoints that haven't been released to end users, it's acceptable to document API changes under the 'Bugfix' section in the release notes.
Applied to files:
doc/release/RELEASE-NOTES.md
📚 Learning: 2024-11-25T13:28:23.090Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 420
File: doc/release/RELEASE-NOTES.md:37-37
Timestamp: 2024-11-25T13:28:23.090Z
Learning: When modifying unreleased API endpoints, such as refactoring the `search_after` parameter into `search_after_value` and `search_after_id` in the OpenSearch implementation, it's acceptable to leave the "What's New" section empty since the changes haven't been released to end users.
Applied to files:
doc/release/RELEASE-NOTES.md
📚 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/FacilityDetailsGeneralFields.jsx
🧬 Code graph analysis (9)
src/react/src/components/FacilityDetailsDetail.jsx (1)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (1)
partnerConfigFields(38-38)
src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
src/react/src/components/PartnerFields/utils.js (2)
showFieldDefaultDisplayText(37-46)showFieldDefaultDisplayText(37-46)
src/react/src/__tests__/components/PartnerFieldSchemaValue.test.js (2)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (1)
partnerConfigFields(38-38)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
PartnerFieldSchemaValue(54-87)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (4)
src/react/src/components/PartnerFields/DefaultProperty.jsx (2)
propertyValue(8-8)displayText(9-13)src/react/src/__tests__/components/UriReferenceProperty.test.js (1)
schemaProperties(16-25)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
schemaProperties(68-68)src/react/src/components/PartnerFields/utils.js (4)
constructUrlFromPartnerField(48-51)constructUrlFromPartnerField(48-51)showFieldDefaultDisplayText(37-46)showFieldDefaultDisplayText(37-46)
src/react/src/components/PartnerFields/utils.js (4)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (2)
schemaProperties(29-31)propertyValue(21-21)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (2)
schemaProperties(68-68)propertySchema(36-36)src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
propertyValue(8-8)src/react/src/components/PartnerFields/UriProperty.jsx (1)
propertyValue(7-7)
src/react/src/__tests__/components/UriReferenceProperty.test.js (1)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (3)
UriReferenceProperty(14-68)schemaProperties(29-31)partnerConfigFields(38-38)
src/react/src/components/FacilityDetailsGeneralFields.jsx (4)
src/react/src/util/renderUtils.jsx (1)
values(21-21)src/react/src/util/util.js (3)
data(125-125)formatPartnerFieldValue(1906-1926)formatPartnerFieldValue(1906-1926)src/react/src/components/PartnerFields/UriReferenceProperty.jsx (1)
partnerConfigFields(38-38)src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
displayText(9-13)
src/django/api/serializers/facility/facility_index_serializer.py (1)
src/django/api/static/admin/js/partner_field_admin.js (1)
field(5-5)
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (2)
src/react/src/components/PartnerFields/utils.js (2)
FORMAT_TYPES(5-8)FORMAT_TYPES(5-8)src/react/src/components/PartnerFields/UriReferenceProperty.jsx (3)
UriReferenceProperty(14-68)schemaProperties(29-31)partnerConfigFields(38-38)
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/FacilityDetailsDetail.jsx
[warning] 71-71: 'partnerConfigFields' is missing in props validation
src/react/src/components/FacilityDetailsItem.jsx
[warning] 50-50: 'partnerConfigFields' is missing in props validation
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
[warning] 48-48: 'classes.primaryText' is missing in props validation
[warning] 15-15: 'propertyKey' is missing in props validation
[warning] 27-27: Prefer using an optional chain expression instead, as it's more concise and easier to read.
[warning] 17-17: 'schemaProperties' is missing in props validation
[warning] 38-38: 'partnerConfigFields.displayText' is missing in props validation
[warning] 30-30: 'schemaProperties.value' is missing in props validation
[warning] 38-38: 'partnerConfigFields.baseUrl' is missing in props validation
[warning] 16-16: 'value' is missing in props validation
[warning] 18-18: 'partnerConfigFields' is missing in props validation
[warning] 19-19: 'classes' is missing in props validation
[warning] 30-30: 'schemaProperties.value.description' is missing in props validation
src/react/src/components/PartnerFields/utils.js
[warning] 44-44: Unexpected negated condition.
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx
[warning] 57-57: 'partnerConfigFields' is missing in props validation
[warning] 56-56: 'jsonSchema' is missing in props validation
[warning] 55-55: 'value' is missing in props validation
🪛 Ruff (0.14.7)
src/django/api/admin.py
265-274: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
src/django/api/migrations/0189_add_base_url_and_text_field_to_partner_field.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-23: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
🔇 Additional comments (15)
doc/release/RELEASE-NOTES.md (1)
32-32: LGTM!The release note accurately describes the new URI-Reference format support for partner fields. The entry is appropriately placed in the "What's new" section and aligns with the PR objectives.
src/django/api/serializers/facility/facility_index_serializer.py (1)
126-128: LGTM!The context propagation of
base_urlanddisplay_texttoFacilityIndexExtendedFieldListSerializerfollows the established pattern used for other partner field metadata likesource_by,unit, andlabel.src/react/src/components/FacilityDetailsItem.jsx (1)
72-72: LGTM!The
partnerConfigFieldsprop is correctly propagated to allFacilityDetailsDetailinstances (primary, drawer, and additional content items), ensuring consistent rendering behavior throughout the component.src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
2-13: LGTM!Good refactoring to centralize the display text logic in
showFieldDefaultDisplayText. This aligns with the PR's approach of using shared utilities and removes duplicated logic from the component.src/django/api/migrations/0189_add_base_url_and_text_field_to_partner_field.py (1)
12-23: Inconsistent null handling betweenbase_urlanddisplay_text.
base_urlhasnull=Truewhiledisplay_textdoes not. This means:
base_urlcan beNoneor empty stringdisplay_textcan only be empty string (defaults to''when not provided)This is likely intentional (URL fields commonly allow null, while text fields default to empty string), but verify this aligns with your intended behavior. If both should allow
None, addnull=Truetodisplay_text.The static analysis warnings about
ClassVarannotations are false positives for Django migration files - this is the standard Django migration pattern.src/react/src/components/FacilityDetailsDetail.jsx (1)
61-72: Partner config wiring into JSON-schema rendering looks correctPassing
partnerConfigFieldsthroughFacilityDetailsDetailintoPartnerFieldSchemaValueonly on thejsonSchemapath is consistent with the rest of the component and doesn’t change non-schema rendering behavior.Also applies to: 90-95
src/django/api/serializers/facility/facility_index_extended_field_list_serializer.py (1)
18-24: Context override for base_url/display_text is consistent with existing patternAdding
base_urlanddisplay_texttofieldsandcontext_overridescleanly mirrors howsource_by,unit,label, andjson_schemaare already handled. Serialization still prefers context when provided and otherwise falls back toextended_field.get(field), so this should be safe for existing consumers.Also applies to: 41-48
src/django/api/admin.py (1)
263-275: Admin form now exposes base_url/display_text correctlyIncluding
base_urlanddisplay_textinPartnerFieldAdminForm.Meta.fieldskeeps the admin UI in sync with the model and serializer changes. Placement beforejson_schemais consistent and should work as expected.src/react/src/components/FacilityDetailsGeneralFields.jsx (3)
98-98: LGTM - Good defensive guard.The early return prevents rendering when no valid extended field values exist, improving robustness.
235-236: LGTM - Consistent guard for partner fields.The early return is consistent with the extended fields guard and prevents rendering invalid partner field data.
227-232: All call sites correctly passpartnerConfigFieldstorenderPartnerField.The function signature properly includes the
partnerConfigFieldsparameter and it is correctly forwarded toFacilityDetailsItemat line 252. Both invocations at lines 327 and 330 use.map(renderPartnerField)on thepartnerFieldsarray, where each element is constructed with all required parameters includingpartnerConfigFieldsat line 308.src/django/api/static/admin/js/partner_field_admin.js (2)
4-24: LGTM - Well-structured helper with fallback logic.The
findFieldRowfunction correctly handles field lookup with graceful fallback to class selector, returning an empty jQuery object when nothing is found. This is more flexible than the previous implementation.
26-77: LGTM - Toggle logic correctly extended for new fields.The function properly handles all three fields (
json_schema,base_url,display_text), with appropriate guards and event wiring. The forEach iteration correctly skips rows that don't exist.src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (2)
3-3: LGTM - URI-Reference component properly registered.The import and registry entry correctly wire
UriReferencePropertyinto the format component system.Also applies to: 13-13
30-49: LGTM - partnerConfigFields correctly propagated.The
renderPropertyfunction properly accepts and forwardspartnerConfigFieldsto the format components, enabling URI-reference rendering.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/react/src/components/FacilityDetailsGeneralFields.jsx (1)
303-324: Fix unsafe destructuring when buildingpartnerFields(can crash on empty arrays)Here you destructure from the result of:
get(data, `properties.partner_fields.${fieldName}[0]`)If a partner field exists but its list is empty (or the first element is missing),
getreturnsundefined, and destructuring fromundefinedthrows aTypeErrorbeforerenderPartnerField(and its guard) are ever called. This will break the facility details view for such data.Use a safe default object so missing metadata simply yields
undefinedconfig values instead of a crash:const { base_url: baseUrl, // eslint-disable-line camelcase display_text: displayText, // eslint-disable-line camelcase - } = get(data, `properties.partner_fields.${fieldName}[0]`); + } = get( + data, + `properties.partner_fields.${fieldName}[0]`, + {}, + );This preserves behavior while handling empty/malformed partner‑field arrays safely.
Please verify with a facility that has a
partner_fieldsentry whose value list is empty or missing the first element to confirm the page no longer throws.
🧹 Nitpick comments (1)
src/react/src/components/FacilityDetailsGeneralFields.jsx (1)
269-287:renderEmbedFieldsbehavior remains focused on extended + contributor fieldsThe embed path correctly filters
embedConfig.embed_fieldsbyvisibleand tries extended fields first, then contributor fields, without affecting the new partner‑field handling in the non‑embed path. This looks fine given the current requirements; if partner fields ever need to appear in embeds, this is the place to extend.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
doc/release/RELEASE-NOTES.md(1 hunks)src/react/src/components/FacilityDetailsGeneralFields.jsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- doc/release/RELEASE-NOTES.md
🧰 Additional context used
🧠 Learnings (1)
📚 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/FacilityDetailsGeneralFields.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-contricleaner-code-quality
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-flake8-linter
- GitHub Check: run-dd-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-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-integration-test-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (2)
src/react/src/components/FacilityDetailsGeneralFields.jsx (2)
96-102: Early guard inrenderExtendedFieldis appropriateThe null/empty check before formatting avoids calling
formatExtendedFieldwith missing data and aligns with the later usage pattern. No issues from my side here.
233-242: Partner config wiring intoFacilityDetailsItemlooks correctThe updated
renderPartnerFieldsignature, early return on empty values, and passingpartnerConfigFieldsthrough toFacilityDetailsItemare coherent and consistent with the extended‑field rendering pattern.Also applies to: 252-259
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/react/src/components/FacilityDetailsGeneralFields.jsx(5 hunks)src/react/src/components/FacilityDetailsItem.jsx(5 hunks)src/react/src/components/PartnerFields/UriReferenceProperty.jsx(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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/FacilityDetailsGeneralFields.jsxsrc/react/src/components/FacilityDetailsItem.jsx
📚 Learning: 2024-11-22T12:24:03.226Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/ContributeForm.jsx:184-190
Timestamp: 2024-11-22T12:24:03.226Z
Learning: In the Open Supply Hub project, code quality improvements like adding missing prop type validations can be deferred to future improvements tasks.
Applied to files:
src/react/src/components/FacilityDetailsItem.jsxsrc/react/src/components/PartnerFields/UriReferenceProperty.jsx
📚 Learning: 2025-01-23T11:10:17.929Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:680-692
Timestamp: 2025-01-23T11:10:17.929Z
Learning: The ProductionLocationInfo component in Open Supply Hub implements field-level validation for name, address and country fields using Material-UI's TextField validation patterns and touch states, providing real-time feedback to users.
Applied to files:
src/react/src/components/FacilityDetailsItem.jsx
📚 Learning: 2024-11-22T12:22:59.701Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/Button.jsx:43-43
Timestamp: 2024-11-22T12:22:59.701Z
Learning: In the Open Supply Hub codebase, the Button component in `src/react/src/components/Button.jsx` is designed to have an optional `onClick` prop to accommodate different cases.
Applied to files:
src/react/src/components/FacilityDetailsItem.jsx
📚 Learning: 2024-11-20T23:08:05.475Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/ContributeForm.jsx:175-206
Timestamp: 2024-11-20T23:08:05.475Z
Learning: In the `ContributeForm.jsx` React component, it's acceptable to include an empty `onClick={() => {}}` handler on a disabled `<Button>` to prevent console warnings.
Applied to files:
src/react/src/components/FacilityDetailsItem.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/FacilityDetailsItem.jsx
🧬 Code graph analysis (2)
src/react/src/components/FacilityDetailsGeneralFields.jsx (4)
src/react/src/util/renderUtils.jsx (1)
values(21-21)src/react/src/util/util.js (3)
data(125-125)formatPartnerFieldValue(1906-1926)formatPartnerFieldValue(1906-1926)src/react/src/components/PartnerFields/UriReferenceProperty.jsx (2)
partnerConfigFields(27-27)partnerConfigFields(38-38)src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
displayText(9-13)
src/react/src/components/FacilityDetailsItem.jsx (1)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (2)
partnerConfigFields(27-27)partnerConfigFields(38-38)
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
[warning] 18-18: 'partnerConfigFields' is missing in props validation
[warning] 19-19: 'classes' is missing in props validation
[warning] 17-17: 'schemaProperties' is missing in props validation
[warning] 16-16: 'value' is missing in props validation
[warning] 47-47: 'classes.primaryText' is missing in props validation
[warning] 33-33: Remove the declaration of the unused 'linkText' variable.
[warning] 15-15: 'propertyKey' is missing in props validation
[warning] 33-33: Remove this useless assignment to variable "linkText".
🔇 Additional comments (5)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (1)
26-27: LGTM! Past review concerns addressed.The component now correctly:
- Guards optional props with
|| {}defaults (lines 26-27)- Builds URLs from the raw
propertyValue(line 41) instead of display textThis ensures the URL path is always correct even when display text differs from the underlying value.
Also applies to: 40-41
src/react/src/components/FacilityDetailsItem.jsx (1)
2-2: LGTM! PropTypes and prop threading implemented correctly.The changes properly:
- Add comprehensive PropTypes validation including the new
partnerConfigFieldsprop with the correct shape- Thread
partnerConfigFieldsthrough all rendering paths (main detail, drawer detail, and mapped additional content)- Provide an appropriate
nulldefault indefaultPropsThis addresses the past review comment about missing PropTypes validation.
Also applies to: 51-51, 73-73, 110-110, 116-119, 128-172
src/react/src/components/FacilityDetailsGeneralFields.jsx (3)
98-98: LGTM! Enhanced guard prevents edge cases.The updated guard
!values.length || !values[0]correctly handles both empty arrays and arrays with falsy first elements, preventing potential issues downstream.
233-262: LGTM! Partner field rendering correctly extended.The
renderPartnerFieldsignature update and implementation properly:
- Accept and thread
partnerConfigFieldsthrough toFacilityDetailsItem(line 258)- Add consistent early-return guard matching
renderExtendedField(line 241)- Maintain compatibility with the calling site at line 335
303-326: LGTM! Safe destructuring prevents TypeError.The partner fields construction now safely handles missing data:
- Line 310 uses
|| {}to defaultfirstEntrywhen the path doesn't exist- Destructuring
base_urlanddisplay_textfrom an empty object safely yieldsundefinedvalues- No TypeError can occur even when
properties.partner_fields[fieldName]is empty or missingThis addresses the critical concern from the past review about unsafe destructuring.
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (2)
9-9: Consider definingprimaryTextstyle or removing the empty styles.The
stylesfunction returns an empty object, but line 46 referencesclasses.primaryText. Either define theprimaryTextstyle in the styles object or remove the withStyles wrapper if no styling is needed.
14-20: PropTypes can be added in a future improvement.Static analysis flagged missing prop validations for all props. Based on learnings, prop type validations can be deferred to future improvements tasks in this project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/react/src/__tests__/components/UriReferenceProperty.test.js(1 hunks)src/react/src/components/PartnerFields/UriReferenceProperty.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/tests/components/UriReferenceProperty.test.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-22T12:24:03.226Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/ContributeForm.jsx:184-190
Timestamp: 2024-11-22T12:24:03.226Z
Learning: In the Open Supply Hub project, code quality improvements like adding missing prop type validations can be deferred to future improvements tasks.
Applied to files:
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
🧬 Code graph analysis (1)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (4)
src/react/src/components/PartnerFields/DefaultProperty.jsx (2)
propertyValue(8-8)displayText(9-13)src/react/src/__tests__/components/UriReferenceProperty.test.js (1)
schemaProperties(16-25)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
schemaProperties(68-68)src/react/src/components/PartnerFields/utils.js (4)
constructUrlFromPartnerField(48-51)constructUrlFromPartnerField(48-51)showFieldDefaultDisplayText(37-46)showFieldDefaultDisplayText(37-46)
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
[warning] 46-46: 'classes.primaryText' is missing in props validation
[warning] 17-17: 'schemaProperties' is missing in props validation
[warning] 18-18: 'partnerConfigFields' is missing in props validation
[warning] 15-15: 'propertyKey' is missing in props validation
[warning] 19-19: 'classes' is missing in props validation
[warning] 16-16: 'value' 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-eslint-linter-and-prettier-formatter
- GitHub Check: run-flake8-linter
- GitHub Check: run-django-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-contricleaner-cov
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (4)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (4)
21-24: LGTM!Good defensive pattern returning null when the property value is absent.
26-27: LGTM!Properly guards optional props with defaults, as noted in previous review feedback.
32-35: LGTM!Despite a past comment suggesting
linkTextwas unused, it's correctly used on line 57 as part of the fallback chain:displayText || linkText || absoluteURI. This provides a sensible display text hierarchy.
39-41: LGTM!Correctly constructs the URL from the raw
propertyValuerather than display text, as noted in previous review.
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (1)
54-67: Conditionally render link only whenabsoluteURIexists.When
baseUrlis absent,absoluteURIisundefined(line 41), resulting in a non-functional link withhref={undefined}. This creates a clickable element that doesn't navigate anywhere, degrading the user experience.As discussed in previous review threads, the correct approach is to render a link only when a valid URL exists, otherwise display plain text.
Apply this diff to conditionally render:
- <a - key={`${propertyKey}-uri-${propertyValue}`} - href={absoluteURI} - target="_blank" - rel="noopener noreferrer" - > - {baseUrl - ? displayText || linkText || absoluteURI - : showFieldDefaultDisplayText( - schemaProperties, - propertyValue, - propertyKey, - )} - </a> + {absoluteURI ? ( + <a + key={`${propertyKey}-uri-${propertyValue}`} + href={absoluteURI} + target="_blank" + rel="noopener noreferrer" + > + {displayText || linkText || absoluteURI} + </a> + ) : ( + <span> + {showFieldDefaultDisplayText( + schemaProperties, + propertyValue, + propertyKey, + )} + </span> + )}
🧹 Nitpick comments (1)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (1)
14-20: Consider adding PropTypes validation.The component is missing prop type definitions for
propertyKey,value,schemaProperties,partnerConfigFields, andclasses. While this doesn't affect runtime behavior, adding PropTypes would improve developer experience and catch potential issues earlier.Based on learnings, this can be deferred to a future improvements task.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx(1 hunks)src/react/src/components/PartnerFields/utils.js(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-22T12:24:03.226Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/ContributeForm.jsx:184-190
Timestamp: 2024-11-22T12:24:03.226Z
Learning: In the Open Supply Hub project, code quality improvements like adding missing prop type validations can be deferred to future improvements tasks.
Applied to files:
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
📚 Learning: 2024-12-10T09:00:29.628Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 436
File: src/react/src/util/styles.js:567-571
Timestamp: 2024-12-10T09:00:29.628Z
Learning: In the Open Supply Hub project, when defining font sizes in styles (e.g., in `src/react/src/util/styles.js`), prefer using `px` units directly rather than using `theme.typography.pxToRem()` function, as `px` units are used consistently throughout the project.
Applied to files:
src/react/src/components/PartnerFields/UriReferenceProperty.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/components/PartnerFields/utils.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/components/PartnerFields/utils.js
🧬 Code graph analysis (2)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (4)
src/react/src/components/PartnerFields/DefaultProperty.jsx (2)
propertyValue(8-8)displayText(9-13)src/react/src/__tests__/components/UriReferenceProperty.test.js (1)
schemaProperties(16-25)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
schemaProperties(68-68)src/react/src/components/PartnerFields/utils.js (4)
constructUrlFromPartnerField(48-51)constructUrlFromPartnerField(48-51)showFieldDefaultDisplayText(37-46)showFieldDefaultDisplayText(37-46)
src/react/src/components/PartnerFields/utils.js (5)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (3)
schemaProperties(26-26)schemaProperties(30-30)propertyValue(21-21)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (2)
schemaProperties(68-68)propertySchema(36-36)src/react/src/components/PartnerFields/DefaultProperty.jsx (1)
propertyValue(8-8)src/react/src/components/PartnerFields/UriProperty.jsx (1)
propertyValue(7-7)src/react/src/__tests__/utils.tests.js (2)
stringValue(1078-1078)value(771-771)
🪛 GitHub Check: SonarCloud Code Analysis
src/react/src/components/PartnerFields/UriReferenceProperty.jsx
[warning] 19-19: 'classes' is missing in props validation
[warning] 16-16: 'value' is missing in props validation
[warning] 47-47: 'classes.primaryText' is missing in props validation
[warning] 18-18: 'partnerConfigFields' is missing in props validation
[warning] 17-17: 'schemaProperties' is missing in props validation
[warning] 15-15: 'propertyKey' 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-eslint-linter-and-prettier-formatter
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-django-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (7)
src/react/src/components/PartnerFields/utils.js (3)
1-1: LGTM!The
endsWithimport is appropriate for checking trailing slashes in URL construction.
7-7: LGTM!The
URI_REFERENCEconstant follows the existing pattern and aligns with JSON Schema format standards.
37-46: LGTM!The function correctly handles null/undefined values and provides appropriate fallback display text formatting.
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (4)
1-24: LGTM!The component setup and early return logic for missing values are appropriate.
26-30: LGTM!The defensive defaults for incoming props prevent potential crashes when schema or config is missing.
32-35: Dismiss the past review comment about unusedlinkText.The past review comment claiming
linkTextis unused is incorrect. The variable is clearly used on line 61 as a fallback:displayText || linkText || absoluteURI. This is not dead code.
37-41: LGTM!The conditional URL construction correctly guards the call to
constructUrlFromPartnerField, only invoking it whenbaseUrlis present.
roman-stolar
left a comment
roman-stolar
left a comment
There was a problem hiding this comment.
In general LGTM, please check coderabbit/sonarqube comments
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/react/src/components/FacilityDetailsItem.jsx (1)
49-50: Avoid double‑defining defaults for additionalContentText propsYou’re assigning defaults for
additionalContentTextandadditionalContentTextPluralboth in the parameter list and again indefaultProps. That’s redundant and may confuse future readers.You can drop one of the mechanisms (I’d keep
defaultPropsfor consistency with the rest of the component API).Also applies to: 151-152, 173-174
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
90-104: Consider centralizing the partnerConfigFields PropTypes shapeThe
partnerConfigFieldsPropTypes shape ({ baseUrl, displayText }) here matches the one inFacilityDetailsItem.jsx. Duplicating this structure in multiple files risks drift if the config ever grows.You could extract a shared
partnerConfigFieldsShapeinto a small helper module and reuse it across components that accept this prop.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/react/src/components/FacilityDetailsDetail.jsx(4 hunks)src/react/src/components/FacilityDetailsItem.jsx(5 hunks)src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx(6 hunks)src/react/src/components/PartnerFields/UriReferenceProperty.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/react/src/components/FacilityDetailsDetail.jsx
- src/react/src/components/PartnerFields/UriReferenceProperty.jsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-11-22T12:24:03.226Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/ContributeForm.jsx:184-190
Timestamp: 2024-11-22T12:24:03.226Z
Learning: In the Open Supply Hub project, code quality improvements like adding missing prop type validations can be deferred to future improvements tasks.
Applied to files:
src/react/src/components/FacilityDetailsItem.jsx
📚 Learning: 2025-01-23T11:10:17.929Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:680-692
Timestamp: 2025-01-23T11:10:17.929Z
Learning: The ProductionLocationInfo component in Open Supply Hub implements field-level validation for name, address and country fields using Material-UI's TextField validation patterns and touch states, providing real-time feedback to users.
Applied to files:
src/react/src/components/FacilityDetailsItem.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.jsx
📚 Learning: 2024-11-22T12:22:59.701Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/Button.jsx:43-43
Timestamp: 2024-11-22T12:22:59.701Z
Learning: In the Open Supply Hub codebase, the Button component in `src/react/src/components/Button.jsx` is designed to have an optional `onClick` prop to accommodate different cases.
Applied to files:
src/react/src/components/FacilityDetailsItem.jsx
📚 Learning: 2024-11-20T23:08:05.475Z
Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 415
File: src/react/src/components/ContributeForm.jsx:175-206
Timestamp: 2024-11-20T23:08:05.475Z
Learning: In the `ContributeForm.jsx` React component, it's acceptable to include an empty `onClick={() => {}}` handler on a disabled `<Button>` to prevent console warnings.
Applied to files:
src/react/src/components/FacilityDetailsItem.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/FacilityDetailsItem.jsx
🧬 Code graph analysis (2)
src/react/src/components/FacilityDetailsItem.jsx (2)
src/react/src/components/PartnerFields/UriReferenceProperty.jsx (2)
partnerConfigFields(28-28)partnerConfigFields(38-38)src/react/src/components/FacilityDetailsDetail.jsx (1)
FacilityDetailsDetail(62-116)
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (2)
src/react/src/components/PartnerFields/utils.js (2)
FORMAT_TYPES(5-8)FORMAT_TYPES(5-8)src/react/src/components/PartnerFields/UriReferenceProperty.jsx (5)
UriReferenceProperty(15-71)schemaProperties(27-27)schemaProperties(31-31)partnerConfigFields(28-28)partnerConfigFields(38-38)
⏰ 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: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (3)
src/react/src/components/FacilityDetailsItem.jsx (2)
51-51: partnerConfigFields threading through all FacilityDetailsDetail usages looks correct
partnerConfigFieldsis consistently passed fromFacilityDetailsIteminto everyFacilityDetailsDetailinstance (main, drawer, and additionalContent items), and explicitpartnerConfigFields={partnerConfigFields}correctly overrides any value that might be present onitem. This matches the UriReferenceProperty contract ({ baseUrl, displayText }) and keeps behavior predictable.Also applies to: 73-73, 110-119
2-2: PropTypes/defaultProps for new partnerConfigFields look aligned with downstream usageThe added
PropTypes.shape({ baseUrl, displayText })andpartnerConfigFields: nulldefault are consistent withUriReferenceProperty’s expectations (it safely falls back withincomingPartnerConfigFields || {}). The broader PropTypes/defaultProps set for this component also matches existing usage patterns and keeps the public surface explicit.Also applies to: 128-157, 159-176
src/react/src/components/PartnerFields/PartnerFieldSchemaValue.jsx (1)
12-16: URI_REFERENCE wiring and partnerConfigFields propagation are consistentThe new
FORMAT_TYPES.URI_REFERENCE→UriReferencePropertymapping and the extendedrenderPropertysignature correctly ensure thatpartnerConfigFieldsflows fromPartnerFieldSchemaValuedown into the format component. Existing format components safely receive the extra prop and ignore it, whileUriReferencePropertycan now use{ baseUrl, displayText }as intended.Also applies to: 31-48, 55-59, 71-80
src/django/api/serializers/facility/facility_index_serializer.py
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/django/api/migrations/0189_add_base_url_and_text_field_to_partner_field.py (1)
12-23: Confirmblank=Truewithoutnull=Truematches the intended API semanticsBoth
base_urlanddisplay_textare added withblank=Truebut withoutnull=True, so existing rows will use empty strings rather thanNULLwhen these fields are “unset”. That’s fine if the rest of the stack treats empty strings appropriately, but if the API is expected to emit JSONnullfor unset values, you’ll want to double‑check that the serializers (or model helpers) normalize''→None, or considernull=Truehere instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/django/api/migrations/0189_add_base_url_and_text_field_to_partner_field.py(1 hunks)src/django/api/models/partner_field.py(1 hunks)src/django/api/serializers/facility/facility_index_serializer.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/django/api/models/partner_field.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/django/api/migrations/0189_add_base_url_and_text_field_to_partner_field.py (1)
src/django/api/static/admin/js/partner_field_admin.js (1)
field(5-5)
🪛 Ruff (0.14.7)
src/django/api/migrations/0189_add_base_url_and_text_field_to_partner_field.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-23: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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-contricleaner-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-countries-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: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-flake8-linter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (1)
src/django/api/serializers/facility/facility_index_serializer.py (1)
95-146: Passingbase_urlanddisplay_textthrough the partner-field serializer context looks correctReading
base_url/display_textfrom eachPartnerFieldand threading them through theFacilityIndexExtendedFieldListSerializercontext is consistent with howsource_by,unit,label, andjson_schemaare already handled. Given the migration/model changes, this should safely expose the new config per field without altering existing behavior for facilities that lack data for a given partner field.
3 participants
Feature for OSDEV-2268
base_urlanddisplay_textfor object partner fields.format: uri-referenceobject partner fields.