[OSDEV-2368] Integration of the Partner Field Groups section#909
Merged
protsack-stephan merged 21 commits intomainfrom Mar 11, 2026
Merged
[OSDEV-2368] Integration of the Partner Field Groups section#909protsack-stephan merged 21 commits intomainfrom
protsack-stephan merged 21 commits intomainfrom
Conversation
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 42.74%Your code coverage diff: 0.37% ▴ ✅ All code changes are covered |
protsack-stephan
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 |
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
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 |
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Django App | Unittest test suite - Code coverage reportTotal: 81.98%Your code coverage diff: 0.01% ▴ ✅ All code changes are covered |
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
protsack-stephan
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Free Tier Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Every group section displays all partner fields
- Added filtering in PartnerDataContainer to pass only group-specific fields to each PartnerSectionItem based on group.partner_fields.
- ✅ Fixed:
shouldSkipPropertydoesn't handle newurlformat- Updated shouldSkipProperty to check for both FORMAT_TYPES.URI and FORMAT_TYPES.URL to skip _text companion properties.
- ✅ Fixed: Duplicate dead file accidentally committed in ParentSectionItem
- Deleted the duplicate ParentSectionItem/PartnerSectionItem.jsx file that was accidentally committed during refactoring.
- ✅ Fixed:
UrlPropertyduplicates existingUriPropertycomponent entirely- Removed UrlProperty component and updated FORMAT_COMPONENTS to use UriProperty for both URI and URL format types.
Or push these changes by commenting:
@cursor push e1317f7626
Preview (e1317f7626)
diff --git a/src/react/src/components/PartnerFields/FormatComponents/UrlProperty.jsx b/src/react/src/components/PartnerFields/FormatComponents/UrlProperty.jsx
deleted file mode 100644
--- a/src/react/src/components/PartnerFields/FormatComponents/UrlProperty.jsx
+++ /dev/null
@@ -1,43 +1,0 @@
-import React from 'react';
-import { string, object } from 'prop-types';
-import { withStyles } from '@material-ui/core/styles';
-import { getTitleFromSchema, getLinkTextFromSchema } from '../utils';
-import { commonPropertyStyles } from '../styles';
-
-const UrlProperty = ({ propertyKey, value, schemaProperties, classes }) => {
- const title = getTitleFromSchema(propertyKey, schemaProperties);
- const propertyValue = value[propertyKey];
-
- if (!propertyValue) {
- return null;
- }
-
- const linkText = getLinkTextFromSchema(
- propertyKey,
- value,
- schemaProperties,
- );
-
- return (
- <div className={classes.container}>
- {title && `${title}: `}
- <a
- key={`${propertyKey}-url-${propertyValue}`}
- href={propertyValue}
- target="_blank"
- rel="noopener noreferrer"
- >
- {linkText}
- </a>
- </div>
- );
-};
-
-UrlProperty.propTypes = {
- propertyKey: string.isRequired,
- value: object.isRequired,
- schemaProperties: object.isRequired,
- classes: object.isRequired,
-};
-
-export default withStyles(commonPropertyStyles)(UrlProperty);
\ No newline at end of file
diff --git a/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/constants.js b/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/constants.js
--- a/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/constants.js
+++ b/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/constants.js
@@ -5,7 +5,6 @@
import IntegerProperty from '../TypeComponents/IntegerProperty';
import NestedObjectProperty from '../TypeComponents/NestedObjectProperty/NestedObjectProperty';
import DefaultProperty from '../TypeComponents/DefaultProperty/DefaultProperty';
-import UrlProperty from '../FormatComponents/UrlProperty';
export const FORMAT_TYPES = {
URI: 'uri',
@@ -20,7 +19,7 @@
[FORMAT_TYPES.URI_REFERENCE]: UriReferenceProperty,
[FORMAT_TYPES.DATE]: DateProperty,
[FORMAT_TYPES.DATE_TIME]: DateTimeProperty,
- [FORMAT_TYPES.URL]: UrlProperty,
+ [FORMAT_TYPES.URL]: UriProperty,
};
export const TYPE_COMPONENTS = {
diff --git a/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx b/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx
--- a/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx
+++ b/src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx
@@ -23,7 +23,7 @@
const baseSchema = schemaProperties[baseKey];
const baseFormat = getFormatFromSchema(baseSchema);
- return baseFormat === FORMAT_TYPES.URI;
+ return baseFormat === FORMAT_TYPES.URI || baseFormat === FORMAT_TYPES.URL;
};
const isNestedObject = (propertySchema, propertyValue) =>
diff --git a/src/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/PartnerSectionItem.jsx b/src/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/PartnerSectionItem.jsx
deleted file mode 100644
--- a/src/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/PartnerSectionItem.jsx
+++ /dev/null
@@ -1,189 +1,0 @@
-import React, { useMemo, useEffect, useRef } from 'react';
-import { connect } from 'react-redux';
-import Typography from '@material-ui/core/Typography';
-import { withStyles } from '@material-ui/core/styles';
-import Grid from '@material-ui/core/Grid';
-import Switch from '@material-ui/core/Switch';
-import Collapse from '@material-ui/core/Collapse';
-import InfoOutlined from '@material-ui/icons/InfoOutlined';
-import IconComponent from '../../../Shared/IconComponent/IconComponent.jsx';
-import getIconURL from '../../Sidebar/NavBar/utils.js';
-import {
- clearScrollTargetSection,
- toggleSectionOpen,
-} from '../../../../actions/partnerFieldGroups.js';
-import { HEADER_HEIGHT } from '../../../../util/constants.jsx';
-import parentSectionItemStyles from './styles.js';
-import PartnerFieldItem from './PartnerFieldItem.jsx';
-
-const transitionDurationMs = 300;
-
-const PartnerSectionItem = ({
- classes,
- group,
- partnerFields,
- facilityData,
- isOpen,
- scrollTargetId,
- dispatch,
-}) => {
- const containerRef = useRef(null);
-
- useEffect(() => {
- if (scrollTargetId === group.uuid) {
- dispatch(clearScrollTargetSection());
- // Wait for the collapse to finish transitioning before scrolling.
- // Alternative would be complex logic to track the collapse state.
- setTimeout(() => {
- if (containerRef.current) {
- const top =
- containerRef.current.getBoundingClientRect().top +
- window.scrollY -
- HEADER_HEIGHT;
- window.scrollTo({ top, behavior: 'smooth' });
- }
- }, transitionDurationMs);
- }
- }, [scrollTargetId, group.uuid, dispatch]);
-
- const columns = useMemo(() => {
- if (!isOpen || !partnerFields) return { left: [], right: [] };
- const fields = partnerFields.filter(field => field);
- const mid = Math.ceil(fields.length / 2);
- return {
- left: fields.slice(0, mid),
- right: fields.slice(mid),
- };
- }, [isOpen, partnerFields]);
-
- const handleToggle = () => dispatch(toggleSectionOpen(group.uuid));
-
- const handleKeyDown = event => {
- if (event.key === 'Enter' || event.key === ' ') {
- event.preventDefault();
- handleToggle();
- }
- };
-
- return (
- <div className={classes.container} ref={containerRef}>
- <div
- className={`${classes.header}${
- isOpen ? ` ${classes.headerOpen}` : ''
- }`}
- role="button"
- tabIndex={0}
- onClick={handleToggle}
- onKeyDown={handleKeyDown}
- >
- <div className={classes.headerLeft}>
- {group.icon_file && (
- <img
- src={getIconURL(group.icon_file)}
- width={20}
- height={20}
- alt={group.name}
- className={classes.iconImage}
- />
- )}
- <Typography
- variant="title"
- className={classes.title}
- component="h3"
- >
- {group.name}
- </Typography>
- <div
- onClick={event => event.stopPropagation()}
- onKeyDown={event => event.stopPropagation()}
- role="presentation"
- >
- <IconComponent
- title={
- <span
- // eslint-disable-next-line react/no-danger
- dangerouslySetInnerHTML={{
- __html: group.helper_text,
- }}
- />
- }
- icon={InfoOutlined}
- className={classes.infoIcon}
- />
- </div>
- </div>
- <div className={classes.headerRight}>
- <Typography className={classes.toggleLabel}>
- {isOpen ? 'Close' : 'Open'}
- </Typography>
- <Switch
- color="primary"
- checked={isOpen}
- onChange={handleToggle}
- onClick={event => event.stopPropagation()}
- className={classes.switchWrapper}
- />
- </div>
- </div>
- <Collapse in={isOpen} timeout={transitionDurationMs}>
- <div className={classes.contentArea}>
- {(columns.left.length > 0 || columns.right.length > 0) && (
- <Grid container spacing={4} alignItems="flex-start">
- <Grid item xs={12} sm={6}>
- {columns.left.map(field => (
- <div
- className={classes.fieldItem}
- key={field.fieldName}
- >
- <PartnerFieldItem
- field={field}
- facilityData={facilityData}
- />
- </div>
- ))}
- </Grid>
- <Grid item xs={12} sm={6}>
- {columns.right.map(field => (
- <div
- className={classes.fieldItem}
- key={field.fieldName}
- >
- <PartnerFieldItem
- field={field}
- facilityData={facilityData}
- />
- </div>
- ))}
- </Grid>
- </Grid>
- )}
- {group.description && (
- <div className={classes.disclaimer}>
- <Typography
- component="div"
- className={classes.disclaimerText}
- >
- <span
- // eslint-disable-next-line react/no-danger
- dangerouslySetInnerHTML={{
- __html: group.description,
- }}
- />
- </Typography>
- </div>
- )}
- </div>
- </Collapse>
- </div>
- );
-};
-
-const mapStateToProps = (state, ownProps) => ({
- facilityData: state.facilities.singleFacility.data,
- scrollTargetId: state.partnerFieldGroups.scrollTargetId,
- isOpen: !!state.partnerFieldGroups.openSectionIds[ownProps.group.uuid],
-});
-
-export default connect(mapStateToProps)(
- withStyles(parentSectionItemStyles)(PartnerSectionItem),
-);
\ No newline at end of file
diff --git a/src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx b/src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx
--- a/src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx
+++ b/src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx
@@ -76,14 +76,19 @@
)}
</Grid>
{!fetching &&
- partnerGroups.map(group => (
- <Grid item xs={12} key={group.uuid} id={group.uuid}>
- <PartnerSectionItem
- group={group}
- partnerFields={partnerFields}
- />
- </Grid>
- ))}
+ partnerGroups.map(group => {
+ const groupFields = partnerFields.filter(field =>
+ group.partner_fields.includes(field.fieldName),
+ );
+ return (
+ <Grid item xs={12} key={group.uuid} id={group.uuid}>
+ <PartnerSectionItem
+ group={group}
+ partnerFields={groupFields}
+ />
+ </Grid>
+ );
+ })}
</Grid>
</>
);This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
...c/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx
Show resolved
Hide resolved
src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx
Show resolved
Hide resolved
...ct/src/components/ProductionLocation/PartnerSection/ParentSectionItem/PartnerSectionItem.jsx
Outdated
Show resolved
Hide resolved
📝 WalkthroughWalkthroughIntroduces a custom Django JSONTextField using PostgreSQL json (preserving key order) and migrates PartnerField.json_schema. Adds Redux-driven, collapsible Partner Section UI in React with auto-scroll, new field format components, utilities, icons, tests, and removes the legacy ParentSectionItem. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NavBar as NavBar Component
participant Redux as Redux Store
participant PartnerSectionItem as PartnerSectionItem<br/>Component
participant DOM as Browser DOM
User->>NavBar: Click group in sidebar
alt Is group UUID?
NavBar->>Redux: dispatch setScrollTargetSection(groupId)
Redux->>Redux: Update scrollTargetId & mark<br/>openSectionIds[groupId]=true
PartnerSectionItem->>Redux: Subscribe to state changes
Redux-->>PartnerSectionItem: groupId is now open
PartnerSectionItem->>DOM: Render section content<br/>(Collapse opens)
Note over PartnerSectionItem: Wait for collapse transition
PartnerSectionItem->>PartnerSectionItem: scrollTargetId matches<br/>group UUID?
alt Target matches
PartnerSectionItem->>DOM: window.scrollTo(x, y - HEADER_HEIGHT)
PartnerSectionItem->>Redux: dispatch clearScrollTargetSection()
Redux->>Redux: Reset scrollTargetId to null
end
else Not a group UUID
NavBar->>DOM: window.scrollTo with offset
end
sequenceDiagram
participant User
participant PartnerSectionItem as PartnerSectionItem Header
participant Redux as Redux Store
participant State as React State<br/>(openSectionIds)
User->>PartnerSectionItem: Click/Enter/Space on header
PartnerSectionItem->>Redux: dispatch toggleSectionOpen(uuid)
Redux->>State: Toggle openSectionIds[uuid]
State-->>PartnerSectionItem: isOpen updated
PartnerSectionItem->>PartnerSectionItem: Collapse/Expand transition
Note over PartnerSectionItem: Switch label changes<br/>Open ↔ Close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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: 6
🧹 Nitpick comments (2)
src/react/src/__tests__/components/NavBar.test.js (1)
97-104: Assert the scroll arguments, not just that a call happened.
toHaveBeenCalled()is too loose for the new behavior. If the fixed-header offset orbehavior: 'smooth'regresses, this test still passes. Please mock the target position and assert thewindow.scrollTopayload so the contract added in this PR stays covered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/__tests__/components/NavBar.test.js` around lines 97 - 104, The test currently only asserts window.scrollTo was called; instead mock the target element's position (override target.getBoundingClientRect to return a known top), then trigger the click (fireEvent.click on screen.getByText('Overview')) and assert window.scrollTo was called with the expected payload (including the computed top offset accounting for the fixed-header and the behavior: 'smooth' option). Use the existing target (id 'overview') and window.scrollTo mock to verify the exact arguments rather than just toHaveBeenCalled().src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx (1)
131-158: Add the repo’s overflow guard to this spaced Grid container.
spacing={4}adds negative margins, so this container can reintroduce horizontal scroll on narrow viewports unless it is constrained the same way as the other MUI Grid fixes in this repo.📐 Proposed fix
- <Grid container spacing={4} alignItems="flex-start"> + <Grid + container + spacing={4} + alignItems="flex-start" + style={{ margin: 0, width: '100%' }} + >Based on learnings: In React components that use Material-UI Grid, when a Grid container with spacing causes horizontal scroll, apply
style={{ margin: 0, width: '100%' }}to the container instead of adding breakpoint props to the container.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx` around lines 131 - 158, The Grid container in PartnerSectionItem (the Grid with spacing={4}) can reintroduce horizontal scroll due to negative margins; update that Grid container element in PartnerSectionItem.jsx to include the repo’s overflow guard by setting its inline style to margin: 0 and width: '100%' (i.e., add style={{ margin: 0, width: '100%' }} on the Grid container with spacing={4}) so the spacing negative margins are constrained like other MUI Grid fixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/react/src/components/PartnerFields/FormatComponents/UrlProperty.jsx`:
- Around line 24-29: Whitelist schemes for links in UrlProperty.jsx by
validating propertyValue before using it as an anchor href: parse propertyValue
(e.g., with the URL constructor or a regex), allow only safe schemes like
"http", "https", and optionally "mailto", and if the scheme is not on the
allowlist or parsing fails render the value as plain text (or a non-clickable
element) instead of creating an <a> tag; update the anchor to use a safeHref
variable derived from propertyValue and keep the key using propertyKey so the
rest of the component logic remains unchanged.
In `@src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx`:
- Around line 92-99: Update shouldSkipProperty to also treat `_text` companion
fields as metadata when the base property's format is FORMAT_TYPES.URL in
addition to FORMAT_TYPES.URI: inside shouldSkipProperty, when checking
propertyKey that endsWith('_text'), look up the base key
(propertyKey.replace(/_text$/, '')) in schemaProperties and skip if that base
property's format is either FORMAT_TYPES.URI or FORMAT_TYPES.URL; ensure you
reference schemaProperties and propertyKey so the reduce in
PartnerFieldSchemaValue/utils.jsx no longer renders both UrlProperty and
separate url_text fields.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx`:
- Around line 22-28: The hasPartnerData memo currently only checks values[0],
causing false negatives when later entries in a partner_fields array contain
data; update the predicate in the Object.values(fields).some(...) call inside
hasPartnerData to scan the whole array (e.g., use Array.prototype.some on values
to check any truthy element) so any non-empty entry in
facilityData?.properties?.partner_fields marks hasPartnerData true; keep the
memo dependency on facilityData and ensure you reference partner_fields and
hasPartnerData when making the change so getPartnerFieldsAndGroups behavior
remains consistent.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/utils.js`:
- Around line 14-18: partnerGroups is being kept visible when partner field keys
exist but have no populated values; after calling
preparePartnerFields(facilityData) (the partnerFields variable), filter out
fields that have no actual data before building availableFieldNames. Update the
partnerFields array (from preparePartnerFields) to only include entries whose
value(s) are populated (e.g., value != null, or for arrays value.length > 0 /
value.some(Boolean)) so availableFieldNames only contains fields with real
content, then compute partnerGroups via groups.filter as before; reference
preparePartnerFields, partnerFields, availableFieldNames, partnerGroups, and
PartnerFieldItem to locate where to apply the filter.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx`:
- Around line 32-47: The effect that scrolls when scrollTargetId === group.uuid
uses setTimeout but never clears the timer, causing stale timers to fire; modify
the useEffect in PartnerSectionItem.jsx to capture the timeout id when calling
setTimeout, clear any existing timeout before creating a new one, and return a
cleanup function that calls clearTimeout on that id (so when scrollTargetId,
group.uuid, or the component unmounts the pending timer is cancelled). Ensure
you reference the existing symbols: useEffect, scrollTargetId, group.uuid,
clearScrollTargetSection, containerRef, transitionDurationMs, and dispatch when
implementing the timeout management and cleanup.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/styles.js`:
- Around line 22-24: The focus style for the collapsible header in
PartnerSectionItem (styles.js) currently removes the focus outline via the
'&:focus' rule, so restore an accessible visible focus indicator: update the
'&:focus' selector in the PartnerSectionItem styles to remove outline: 'none'
and instead apply a clear visible focus style (for example a focused box-shadow
or border color and/or background change) that matches the component theme;
ensure the style applies to the collapsible header element used by the component
(the header class or root of the collapsible button) so keyboard users can see
focus when tabbing.
---
Nitpick comments:
In `@src/react/src/__tests__/components/NavBar.test.js`:
- Around line 97-104: The test currently only asserts window.scrollTo was
called; instead mock the target element's position (override
target.getBoundingClientRect to return a known top), then trigger the click
(fireEvent.click on screen.getByText('Overview')) and assert window.scrollTo was
called with the expected payload (including the computed top offset accounting
for the fixed-header and the behavior: 'smooth' option). Use the existing target
(id 'overview') and window.scrollTo mock to verify the exact arguments rather
than just toHaveBeenCalled().
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx`:
- Around line 131-158: The Grid container in PartnerSectionItem (the Grid with
spacing={4}) can reintroduce horizontal scroll due to negative margins; update
that Grid container element in PartnerSectionItem.jsx to include the repo’s
overflow guard by setting its inline style to margin: 0 and width: '100%' (i.e.,
add style={{ margin: 0, width: '100%' }} on the Grid container with spacing={4})
so the spacing negative margins are constrained like other MUI Grid fixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8c71eeca-cbe4-4592-b3af-25506246a5eb
📒 Files selected for processing (27)
doc/release/RELEASE-NOTES.mdsrc/django/api/fields.pysrc/django/api/migrations/0202_add_alter_partnerfield_to_use_json.pysrc/django/api/models/partner_field.pysrc/react/src/__tests__/components/NavBar.test.jssrc/react/src/__tests__/components/PartnerDataContainer.test.jssrc/react/src/__tests__/components/PartnerFieldItem.test.jssrc/react/src/__tests__/components/PartnerSectionItem.test.jssrc/react/src/__tests__/components/UrlProperty.test.jssrc/react/src/__tests__/utils/PartnerFieldsUtils.test.jssrc/react/src/actions/partnerFieldGroups.jssrc/react/src/components/Icons/Partnership.jsxsrc/react/src/components/PartnerFields/FormatComponents/UrlProperty.jsxsrc/react/src/components/PartnerFields/PartnerFieldSchemaValue/constants.jssrc/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsxsrc/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/ParentSectionItem.jsxsrc/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/PartnerSectionItem.jsxsrc/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/styles.jssrc/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/styles.jssrc/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/utils.jssrc/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerFieldItem.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/styles.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsxsrc/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsxsrc/react/src/reducers/PartnerFieldGroupsReducer.js
💤 Files with no reviewable changes (3)
- src/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/styles.js
- src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
- src/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/ParentSectionItem.jsx
src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx
Show resolved
Hide resolved
...c/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/utils.js
Outdated
Show resolved
Hide resolved
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/styles.js
Show resolved
Hide resolved
VadimKovalenkoSNF
previously approved these changes
Mar 11, 2026
Contributor
VadimKovalenkoSNF
left a comment
VadimKovalenkoSNF
left a comment
There was a problem hiding this comment.
Good work. I proposed a few imporovements though.
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx
Outdated
Show resolved
Hide resolved
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx
Outdated
Show resolved
Hide resolved
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx
Outdated
Show resolved
Hide resolved
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx (1)
32-47:⚠️ Potential issue | 🟠 MajorClear the delayed scroll timer on rerender/unmount.
Each matching target schedules a new timeout, but the old one is never cancelled. If the user switches targets quickly, a stale timer can still fire and scroll to the wrong section after state has already moved on.
Proposed fix
useEffect(() => { - if (scrollTargetId === group.uuid) { - dispatch(clearScrollTargetSection()); - // Wait for the collapse to finish transitioning before scrolling. - // Alternative would be complex logic to track the collapse state. - setTimeout(() => { - if (containerRef.current) { - const top = - containerRef.current.getBoundingClientRect().top + - window.scrollY - - HEADER_HEIGHT; - window.scrollTo({ top, behavior: 'smooth' }); - } - }, transitionDurationMs); - } + if (scrollTargetId !== group.uuid) { + return undefined; + } + + dispatch(clearScrollTargetSection()); + // Wait for the collapse to finish transitioning before scrolling. + // Alternative would be complex logic to track the collapse state. + const timeoutId = window.setTimeout(() => { + if (containerRef.current) { + const top = + containerRef.current.getBoundingClientRect().top + + window.scrollY - + HEADER_HEIGHT; + window.scrollTo({ top, behavior: 'smooth' }); + } + }, transitionDurationMs); + + return () => window.clearTimeout(timeoutId); }, [scrollTargetId, group.uuid, dispatch]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx` around lines 32 - 47, The effect schedules a timeout that is never cleared which can cause stale scrolls; update the useEffect in PartnerSectionItem to capture the timeout id (e.g., let timerId = setTimeout(...)) and return a cleanup that calls clearTimeout(timerId) so pending timers are cancelled on rerender/unmount; ensure this change is applied inside the same useEffect that references scrollTargetId, group.uuid, containerRef, transitionDurationMs and keeps the existing dispatch(clearScrollTargetSection()) behavior.src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx (1)
23-31:⚠️ Potential issue | 🟠 MajorScan the whole partner-field array before hiding the section.
This gate still only checks
values[0]. If the first slot is empty but a later entry is populated,hasPartnerDatastays false and Line 31 suppresses the entire Partner Data section.Proposed fix
const hasPartnerData = useMemo(() => { const fields = facilityData?.properties?.partner_fields; if (!fields) return false; return Object.values(fields).some( - values => Array.isArray(values) && values.length > 0 && values[0], + values => Array.isArray(values) && values.some(Boolean), ); }, [facilityData]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx` around lines 23 - 31, The current hasPartnerData computation in PartnerDataContainer.jsx only checks values[0], which misses later populated entries; update the check to scan the entire inner arrays by replacing the predicate so each entry uses Array.isArray(values) && values.some(v => !!v) (or equivalent truthy check) when calling Object.values(fields).some(...), ensuring the section is only hidden if every partner_fields array is empty or contains no truthy values.src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/utils.js (1)
13-20:⚠️ Potential issue | 🟠 MajorFilter out groups with no populated partner fields here.
preparePartnerFields(...)is key-based, so this code still returns groups whose configured fields exist inproperties.partner_fieldsbut have no usable entries. Those groups flow downstream withpartnerFields: []and render as empty sections, which conflicts with the PR goal of hiding groups without facility data.Proposed fix
export default function getPartnerGroupsWithFields(facilityData, groups) { - const partnerFields = preparePartnerFields(facilityData) ?? []; - return groups.map(group => ({ - ...group, - partnerFields: partnerFields.filter(field => - group.partner_fields.includes(field.fieldName), - ), - })); + const partnerFields = (preparePartnerFields(facilityData) ?? []).filter( + ({ fieldName }) => { + const values = + facilityData?.properties?.partner_fields?.[fieldName] ?? []; + return Array.isArray(values) && values.some(Boolean); + }, + ); + + return (groups ?? []) + .map(group => ({ + ...group, + partnerFields: partnerFields.filter(field => + (group.partner_fields ?? []).includes(field.fieldName), + ), + })) + .filter(group => group.partnerFields.length > 0); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/utils.js` around lines 13 - 20, getPartnerGroupsWithFields currently maps all groups even when preparePartnerFields(facilityData) yields no usable entries, causing empty sections to render; change getPartnerGroupsWithFields to compute partnerFields via preparePartnerFields(facilityData) and then only include groups whose computed partnerFields array is non-empty (i.e., filter out groups where partnerFields.length === 0) before returning the result so groups with configured keys but no populated data are excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsx`:
- Around line 63-72: The partner-group icon wrapper ignores incoming className
so styles like menuImage and menuImageActive never apply; update the image and
fallback renderers used where group.icon_file is evaluated (the getIconURL(...)
<img .../> branch and the Tab fallback) to accept and forward the className prop
to the rendered element (ensure the <img> receives className and the Tab
component is rendered with the className prop) so menuImage/menuImageActive
passed from the parent NavBar are applied to partner-group icons.
---
Duplicate comments:
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx`:
- Around line 23-31: The current hasPartnerData computation in
PartnerDataContainer.jsx only checks values[0], which misses later populated
entries; update the check to scan the entire inner arrays by replacing the
predicate so each entry uses Array.isArray(values) && values.some(v => !!v) (or
equivalent truthy check) when calling Object.values(fields).some(...), ensuring
the section is only hidden if every partner_fields array is empty or contains no
truthy values.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/utils.js`:
- Around line 13-20: getPartnerGroupsWithFields currently maps all groups even
when preparePartnerFields(facilityData) yields no usable entries, causing empty
sections to render; change getPartnerGroupsWithFields to compute partnerFields
via preparePartnerFields(facilityData) and then only include groups whose
computed partnerFields array is non-empty (i.e., filter out groups where
partnerFields.length === 0) before returning the result so groups with
configured keys but no populated data are excluded.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx`:
- Around line 32-47: The effect schedules a timeout that is never cleared which
can cause stale scrolls; update the useEffect in PartnerSectionItem to capture
the timeout id (e.g., let timerId = setTimeout(...)) and return a cleanup that
calls clearTimeout(timerId) so pending timers are cancelled on rerender/unmount;
ensure this change is applied inside the same useEffect that references
scrollTargetId, group.uuid, containerRef, transitionDurationMs and keeps the
existing dispatch(clearScrollTargetSection()) behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 05c82b4b-cdf9-46c1-b72c-9ab06919f42f
📒 Files selected for processing (5)
src/react/src/__tests__/utils/getPartnerGroupsWithFields.test.jssrc/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/utils.jssrc/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsxsrc/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsx
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/useScrollToSection.jsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/react/src/__tests__/utils/PartnerFieldSchemaValueUtils.test.js (1)
89-146: Add one order-sensitiverenderPropertiescase.This suite locks in the skip logic, but not the new schema-order contract from this PR. If
renderPropertiesaccidentally iteratesvaluekeys again, every assertion here still passes. Please add a case wherevalueis ordered differently fromschemaPropertiesand assert the rendered order follows the schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/__tests__/utils/PartnerFieldSchemaValueUtils.test.js` around lines 89 - 146, Add a new order-sensitive test for renderProperties: create a schemaProperties object with two properties in a specific order (e.g., first "firstProp" then "secondProp"), create a value object with the same keys but deliberately reversed order, call renderProperties(value, schemaProperties, {}), and assert that the returned result preserves the schemaProperties order (check the rendered elements correspond to "firstProp" then "secondProp" via titles, keys, or index positions) so the test fails if renderProperties iterates value keys instead of schema order.src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx (1)
34-41: Narrow useMemo dependency togroup.partnerFields.Depending on the entire
groupobject may cause unnecessary recalculations when unrelated properties (likeicon_file) change. Consider depending only on the fields actually used.♻️ Suggested refinement
const columns = useMemo(() => { - if (!isOpen || !group.partnerFields) return { left: [], right: [] }; - const mid = Math.ceil(group.partnerFields.length / 2); + const fields = group.partnerFields; + if (!isOpen || !fields) return { left: [], right: [] }; + const mid = Math.ceil(fields.length / 2); return { - left: group.partnerFields.slice(0, mid), - right: group.partnerFields.slice(mid), + left: fields.slice(0, mid), + right: fields.slice(mid), }; - }, [isOpen, group]); + }, [isOpen, group.partnerFields]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx` around lines 34 - 41, The useMemo for columns currently depends on the whole group object which causes unnecessary recomputation when unrelated group properties change; update the dependency array to only include isOpen and group.partnerFields (e.g., change [isOpen, group] to [isOpen, group.partnerFields]) and ensure the memo callback safely handles undefined partnerFields as it does now (still returning { left: [], right: [] } when partnerFields is falsy) so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/react/src/__tests__/utils/PartnerFieldSchemaValueUtils.test.js`:
- Around line 99-100: The forEach callback currently uses a concise arrow body
which implicitly returns the matcher (violating useIterableCallbackReturn);
change result.forEach(el => expect(React.isValidElement(el)).toBe(true)) to use
a block-bodied arrow (e.g. result.forEach(el => {
expect(React.isValidElement(el)).toBe(true); })) so the callback does not return
a value; update the test in PartnerFieldSchemaValueUtils.test.js where
result.forEach and React.isValidElement are used.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx`:
- Around line 95-101: The helper_text and description HTML are rendered via
dangerouslySetInnerHTML in PartnerSectionItem and must be sanitized client-side;
import and use a trusted sanitizer (e.g., DOMPurify) to sanitize
group.helper_text and group.description before passing them into
dangerouslySetInnerHTML (replace raw group.helper_text/group.description with
DOMPurify.sanitize(...)), and ensure you apply the same change for both places
where dangerouslySetInnerHTML is used; alternatively, if you prefer server-side,
sanitize these fields in the API serializer (e.g., using django-bleach) so the
component receives safe HTML.
- Around line 123-150: The Grid container in PartnerSectionItem (the <Grid
container spacing={8} alignItems="flex-start">) produces negative gutters that
cause overflow; fix by constraining the container — either add inline style={{
margin: 0, width: '100%' }} to that Grid container or change it to be both
container and item (e.g., add item xs={12}) so the layout is properly
constrained while keeping the child Grid items (xs={12} sm={6}) unchanged;
update the JSX in PartnerSectionItem accordingly.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/useScrollToSection.jsx`:
- Around line 10-25: The effect in useScrollToSection.jsx sets a setTimeout
without clearing it, so store the timer ID (e.g., in a ref like
scrollTimeoutRef) before calling setTimeout inside the useEffect for
scrollTargetId === groupUuid, and return a cleanup function that clears the
timeout via clearTimeout(scrollTimeoutRef.current) to prevent stale timers
firing on unmount or when dependencies (scrollTargetId, groupUuid) change;
ensure you also clear any existing timeout before creating a new one and keep
references to containerRef and transitionDurationMs as used.
---
Nitpick comments:
In `@src/react/src/__tests__/utils/PartnerFieldSchemaValueUtils.test.js`:
- Around line 89-146: Add a new order-sensitive test for renderProperties:
create a schemaProperties object with two properties in a specific order (e.g.,
first "firstProp" then "secondProp"), create a value object with the same keys
but deliberately reversed order, call renderProperties(value, schemaProperties,
{}), and assert that the returned result preserves the schemaProperties order
(check the rendered elements correspond to "firstProp" then "secondProp" via
titles, keys, or index positions) so the test fails if renderProperties iterates
value keys instead of schema order.
In
`@src/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx`:
- Around line 34-41: The useMemo for columns currently depends on the whole
group object which causes unnecessary recomputation when unrelated group
properties change; update the dependency array to only include isOpen and
group.partnerFields (e.g., change [isOpen, group] to [isOpen,
group.partnerFields]) and ensure the memo callback safely handles undefined
partnerFields as it does now (still returning { left: [], right: [] } when
partnerFields is falsy) so behavior remains identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 09fe3650-680c-441b-9ad7-405bed4193e3
📒 Files selected for processing (5)
src/react/src/__tests__/utils/PartnerFieldSchemaValueUtils.test.jssrc/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/useScrollToSection.jsxsrc/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/components/PartnerFields/PartnerFieldSchemaValue/utils.jsx
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx
Show resolved
Hide resolved
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/PartnerSectionItem.jsx
Show resolved
Hide resolved
...t/src/components/ProductionLocation/PartnerSection/PartnerSectionItem/useScrollToSection.jsx
Show resolved
Hide resolved
vlad-shapik
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebuilt the Partner Data section on the Production Location page. Replaced the static placeholder, now it fetches partner field groups, filters out groups without data for the current facility, and renders each group as a collapsible section with a two-column field layout.
Added scroll-to-section navigation from the sidebar NavBar. Clicking a partner group link in the sidebar dispatches a Redux action that auto-opens the target section and smoothly scrolls it into view (accounting for the fixed header offset), synchronized with the Material UI
Collapsetransition.Preserved JSON key ordering by introducing a custom
JSONTextField(PostgreSQLjsoninstead ofjsonb) forPartnerField.json_schema, preventing key reordering on round-trip through the database (to preserve ordering of the fields).Added
UrlPropertyformat component* for rendering URL-type partner fields with optional link text derived from the schema (_textconvention), and fixedrenderPropertiesto iterate over schema keys instead of value keys so field order matches the schema definition.Added unit tests for
PartnerDataContainer,PartnerFieldItem,PartnerSectionItem,UrlProperty, and partner field utility functions (getTitleFromSchema,getLinkTextFromSchema,getFormattedDateValue).