[OSDEV-2369][PL Redesign] Implement Contribute to this profile section#889
Conversation
React App | Jest test suite - Code coverage reportTotal: 40.54%Your code coverage diff: 0.53% ▴ ✅ All code changes are covered |
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 55.73%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 |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Django App | Unittest test suite - Code coverage reportTotal: 81.97%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
📝 WalkthroughWalkthroughThis PR refactors the ProductionLocation feature by migrating single-facility data-loading logic into ProductionLocationDetailsContainer with Redux integration, adding loading and error states, and implementing data consistency redirects. ContributeFields is redesigned with new action items and a ReportFacilityStatusDialog for status reporting. Numerous component style imports are renamed for clarity. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx (1)
89-101:⚠️ Potential issue | 🟡 MinorAdd
margin: 0andwidth: '100%'to therootstyle to prevent horizontal scroll from Grid spacing.The
rootstyle is missing the necessary properties to counteract the negative margins fromspacing={8}on the Grid container. UpdateproductionLocationDetailsContainer/styles.js:root: Object.freeze({ background: theme.palette.background.grey, padding: '48px 5% 120px 5%', margin: 0, width: '100%', }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx` around lines 89 - 101, The Grid container in ProductionLocationDetailsContainer.jsx uses spacing={8} which introduces negative margins; update the root style object in productionLocationDetailsContainer/styles.js (the exported "root" style used by ProductionLocationDetailsContainer) to include margin: 0 and width: '100%' so the container counteracts the Grid spacing and prevents horizontal scrolling — keep the existing background and padding properties and add those two keys to the root Object.freeze configuration.
🧹 Nitpick comments (5)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx (1)
22-29: Consider addingxs={12}to Grid items for complete mobile responsiveness.The Grid items specify
smandmdbreakpoints but omitxs. Withoutxs={12}, the items may not stack to full width on extra-small screens (below thesmbreakpoint of 700px defined in your theme).♻️ Suggested improvement
<Grid container className={classes.containerItem}> - <Grid item sm={12} md={7}> + <Grid item xs={12} sm={12} md={7}> <GeneralFields /> </Grid> - <Grid item sm={12} md={5}> + <Grid item xs={12} sm={12} md={5}> <DetailsMap /> </Grid> </Grid>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx` around lines 22 - 29, The Grid item columns for the details panel aren't specifying xs so they may not stack full width on extra-small screens; update the two Grid item elements that render <GeneralFields /> and <DetailsMap /> (the Grid items inside the container using classes.containerItem) to include xs={12} so they occupy full width on extra-small devices while preserving the existing sm and md props.src/react/src/components/ProductionLocation/Shared/OutlinedButton/styles.js (1)
1-15: Consider using theme parameter for consistency with FilledButton.Unlike
FilledButton/styles.jswhich accepts athemeparameter and usestheme.palette.action.*for colors, this style factory uses hardcoded color values. While functional, this divergence could cause visual inconsistencies if the theme palette changes.♻️ Suggested refactor for theme consistency
-export default () => +export default theme => Object.freeze({ button: Object.freeze({ backgroundColor: 'transparent', - border: '1px solid rgb(13, 17, 40)', + border: `1px solid ${theme.palette.text.primary}`, borderRadius: 0, padding: '8px 16px', fontWeight: 900, boxShadow: 'none', '&:hover': Object.freeze({ - backgroundColor: 'rgba(0, 0, 0, 0.08)', + backgroundColor: theme.palette.action.hover, boxShadow: 'none', }), }), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Shared/OutlinedButton/styles.js` around lines 1 - 15, The styles factory currently hardcodes colors; change the exported factory signature from "() =>" to accept a theme parameter (e.g., "theme") and replace the literal color values in the returned object (the exported default factory and its "button" object) with theme palette values — use theme.palette.action.active (or the appropriate action color used by FilledButton) for the border color and theme.palette.action.hover (or the equivalent hover color) for the "&:hover" backgroundColor so the OutlinedButton matches theme-driven FilledButton styling.src/react/src/components/ProductionLocation/Sidebar/ContributeFields/styles.js (1)
25-25: Empty style object — intentional placeholder?
actionItemWrapperis an empty frozen object. If it's a placeholder for future styling, consider adding a brief comment. Otherwise, it could be removed if no styles are needed for this class.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/styles.js` at line 25, The actionItemWrapper style is an empty frozen object in styles.js; either remove the unused actionItemWrapper property or mark it explicitly as a placeholder by adding a short inline comment (e.g., "placeholder for future styles") so reviewers know it’s intentional; update the Object.freeze({}) entry for actionItemWrapper accordingly in the styles export.src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx (1)
53-58: Consider providing feedback when the reason is empty.
handleSubmitsilently returns whenreasonForReportis empty (line 54). Users may not understand why clicking "Report" does nothing. Consider adding visual validation feedback to the TextField or disabling the submit button when the reason is empty.💡 Option: Disable submit button when reason is empty
<FilledButton label="Report" onClick={handleSubmit} + disabled={!reasonForReport.trim()} data-testid="report-facility-status-dialog-report" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx` around lines 53 - 58, handleSubmit currently returns silently when reasonForReport is empty, causing no user feedback; update the component to validate reasonForReport before submitting by (1) showing visual validation on the TextField (e.g., set an error state/message when reasonForReport is empty) and (2) disabling the Report/submit button whenever reasonForReport.trim().length === 0 so users cannot click it; wire the validation state to the TextField and ensure submitReport and closeDialog are only called when validation passes in handleSubmit.src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsx (1)
101-124: Add Space key handling for accessibility compliance.The interactive
divwithrole="button"only handles the Enter key. Per ARIA button pattern guidelines, elements withrole="button"should also activate on Space key press.♿ Proposed fix for keyboard accessibility
<div role="button" tabIndex={0} onClick={openDialog} - onKeyDown={e => e.key === 'Enter' && openDialog()} + onKeyDown={e => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + openDialog(); + } + }} className={classes.actionItem} data-testid="contribute-report-status" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsx` around lines 101 - 124, The clickable div with role="button" in ContributeFields.jsx only activates on Enter; update its keyboard handler (the onKeyDown for the element that calls openDialog) to also handle Space: when e.key === ' ' or e.key === 'Spacebar' (and/or e.code === 'Space'), call e.preventDefault() and invoke openDialog() so the element follows ARIA button activation behavior; ensure you update the same element that uses classes.actionItem and data-testid="contribute-report-status" and keep existing Enter handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx`:
- Around line 89-101: The Grid container in
ProductionLocationDetailsContainer.jsx uses spacing={8} which introduces
negative margins; update the root style object in
productionLocationDetailsContainer/styles.js (the exported "root" style used by
ProductionLocationDetailsContainer) to include margin: 0 and width: '100%' so
the container counteracts the Grid spacing and prevents horizontal scrolling —
keep the existing background and padding properties and add those two keys to
the root Object.freeze configuration.
---
Nitpick comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 22-29: The Grid item columns for the details panel aren't
specifying xs so they may not stack full width on extra-small screens; update
the two Grid item elements that render <GeneralFields /> and <DetailsMap /> (the
Grid items inside the container using classes.containerItem) to include xs={12}
so they occupy full width on extra-small devices while preserving the existing
sm and md props.
In `@src/react/src/components/ProductionLocation/Shared/OutlinedButton/styles.js`:
- Around line 1-15: The styles factory currently hardcodes colors; change the
exported factory signature from "() =>" to accept a theme parameter (e.g.,
"theme") and replace the literal color values in the returned object (the
exported default factory and its "button" object) with theme palette values —
use theme.palette.action.active (or the appropriate action color used by
FilledButton) for the border color and theme.palette.action.hover (or the
equivalent hover color) for the "&:hover" backgroundColor so the OutlinedButton
matches theme-driven FilledButton styling.
In
`@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsx`:
- Around line 101-124: The clickable div with role="button" in
ContributeFields.jsx only activates on Enter; update its keyboard handler (the
onKeyDown for the element that calls openDialog) to also handle Space: when
e.key === ' ' or e.key === 'Spacebar' (and/or e.code === 'Space'), call
e.preventDefault() and invoke openDialog() so the element follows ARIA button
activation behavior; ensure you update the same element that uses
classes.actionItem and data-testid="contribute-report-status" and keep existing
Enter handling.
In
`@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx`:
- Around line 53-58: handleSubmit currently returns silently when
reasonForReport is empty, causing no user feedback; update the component to
validate reasonForReport before submitting by (1) showing visual validation on
the TextField (e.g., set an error state/message when reasonForReport is empty)
and (2) disabling the Report/submit button whenever
reasonForReport.trim().length === 0 so users cannot click it; wire the
validation state to the TextField and ensure submitReport and closeDialog are
only called when validation passes in handleSubmit.
In
`@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/styles.js`:
- Line 25: The actionItemWrapper style is an empty frozen object in styles.js;
either remove the unused actionItemWrapper property or mark it explicitly as a
placeholder by adding a short inline comment (e.g., "placeholder for future
styles") so reviewers know it’s intentional; update the Object.freeze({}) entry
for actionItemWrapper accordingly in the styles export.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/react/src/components/ProductionLocation/Sidebar/ContributeFields/icons/copy.svgis excluded by!**/*.svgsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/icons/shield-x.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
.gitignoredoc/release/RELEASE-NOTES.mdsrc/react/src/App.jsxsrc/react/src/__tests__/components/ContributeFields.test.jssrc/react/src/components/ProductionLocation/ClaimSection/ClaimDataContainer/ClaimDataContainer.jsxsrc/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsxsrc/react/src/components/ProductionLocation/Heading/ClosureStatus/ClosureStatus.jsxsrc/react/src/components/ProductionLocation/Heading/DataSourcesInfo/DataSourcesInfo.jsxsrc/react/src/components/ProductionLocation/Heading/LocationTitle/LocationTitle.jsxsrc/react/src/components/ProductionLocation/PartnerSection/AssessmentsAndAudits/AssessmentsAndAudits.jsxsrc/react/src/components/ProductionLocation/PartnerSection/Certifications/Certifications.jsxsrc/react/src/components/ProductionLocation/PartnerSection/Emissions/Emissions.jsxsrc/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/ParentSectionItem.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetails/ProductionLocationDetails.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/styles.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/ProductionLocationDetailsGeneralFields.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsLocation/ProductionLocationDetailsLocation.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/ProductionLocationDetailsMap.jsxsrc/react/src/components/ProductionLocation/Shared/FilledButton/FilledButton.jsxsrc/react/src/components/ProductionLocation/Shared/FilledButton/styles.jssrc/react/src/components/ProductionLocation/Shared/OutlinedButton/OutlinedButton.jsxsrc/react/src/components/ProductionLocation/Shared/OutlinedButton/styles.jssrc/react/src/components/ProductionLocation/Sidebar/BackToSearch/BackToSearch.jsxsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsxsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsxsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/hooks.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/styles.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/hooks.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/styles.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/utils.jssrc/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsxsrc/react/src/components/ProductionLocation/Sidebar/SupplyChain/SupplyChain.jsxsrc/react/src/components/ProductionLocation/commonStyles.js
VadimKovalenkoSNF
left a comment
There was a problem hiding this comment.
Approved, but left optional comments.
...act/src/components/ProductionLocation/ClaimSection/ClaimDataContainer/ClaimDataContainer.jsx
Show resolved
Hide resolved
...nts/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
Show resolved
Hide resolved
…-2369 - Add ContributeFields.test.js with data-testid-only selectors - Add data-testid attributes to ContributeFields and ReportFacilityStatusDialog - Add OSDEV-2369 release notes to 2.20.0 (What's new and Code/API changes) Made-with: Cursor
…ation Made-with: Cursor
Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/react/src/__tests__/components/ContributeFields.test.js (2)
174-182: Make the anonymous-user test explicit.This test relies on
USER_DEFAULT_STATEbeing anonymous. SetisAnon: truein preloaded state inside the test so it remains deterministic if defaults change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/__tests__/components/ContributeFields.test.js` around lines 174 - 182, The test "anonymous user sees login prompt in dialog" depends on default anonymity; make it explicit by passing a preloaded state with isAnon: true when calling renderContributeFields in that test. Update the test to call renderContributeFields with a preloadedState (or similar arg accepted by the helper) that spreads USER_DEFAULT_STATE and overrides isAnon: true so the dialog assertion remains deterministic even if defaults change; reference the test name and the renderContributeFields helper to locate where to modify.
207-226: Harden the dialog-close assertion to avoid transition flakes.Using
getByTestIdimmediately after cancel is brittle with modal transitions. PreferqueryBy...+waitFor/waitForElementToBeRemovedfor stable close checks.✅ Suggested test adjustment
-import { screen, within, fireEvent } from '@testing-library/react'; +import { + screen, + within, + fireEvent, + waitForElementToBeRemoved, +} from '@testing-library/react'; ... - test('cancel closes dialog', () => { + test('cancel closes dialog', async () => { ... fireEvent.click(screen.getByTestId('report-facility-status-dialog-cancel')); - - const dialogAfterClose = screen.getByTestId('report-facility-status-dialog'); - expect(dialogAfterClose).not.toBeVisible(); + await waitForElementToBeRemoved(() => + screen.queryByTestId('report-facility-status-dialog'), + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/__tests__/components/ContributeFields.test.js` around lines 207 - 226, The dialog-close assertion is brittle due to transitions in the 'cancel closes dialog' test; update the test that calls renderContributeFields and clicks 'report-facility-status-dialog-cancel' to use a query + async wait instead of immediate getByTestId: use screen.queryByTestId('report-facility-status-dialog') together with await waitForElementToBeRemoved(...) or await waitFor(() => expect(...).not.toBeVisible()) to assert the dialog is removed/hidden after the cancel click; keep the existing fireEvent calls and test ids (e.g., 'contribute-report-status', 'report-facility-status-dialog', 'report-facility-status-dialog-cancel') so the change is localized to the assertion only.
🤖 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/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx`:
- Around line 42-45: The code computes normalizedOsID but then mixes use of
normalizedOsID and the raw osID causing inconsistent behavior; update all places
that reference osID to instead use normalizedOsID — specifically use
normalizedOsID for the fetch call, the redirect existence check, and when
passing the ID into ContributeFields and any sidebar/redirect logic (references:
normalizedOsID, osID, ContributeFields, the fetch/useEffect block, and the
redirect check block) so the same normalized identifier is used everywhere.
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 23-27: The two Material-UI Grid items wrapping GeneralFields and
DetailsMap lack an xs breakpoint, causing incorrect mobile layouts; update the
Grid item elements that render GeneralFields and DetailsMap to include xs={12}
so both columns span full width on extra-small screens (i.e., add xs={12} to the
Grid item containing GeneralFields and to the Grid item containing DetailsMap).
In
`@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsx`:
- Around line 103-107: The custom control with role="button" (the element using
className={classes.actionItem}) only handles Enter in its onKeyDown handler;
update the onKeyDown handler attached to that element to also handle the Space
key by detecting e.key === ' ' or e.key === 'Spacebar'/'Space' (and/or e.code
=== 'Space'), call e.preventDefault() for Space, and then call openDialog() so
keyboard interaction matches native button semantics.
In
`@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx`:
- Around line 118-131: The multiline TextField (id "report-reason", data-testid
"report-facility-status-reason", value reasonForReport, onChange
setReportReason) lacks an accessible label; update the TextField component to
include an explicit label (e.g., label="Reason for report") or a corresponding
aria-labelledby/aria-label so screen readers can identify it, and keep the
existing id and className (classes.dialogTextFieldStyles) to preserve styling
and test selectors.
- Around line 53-57: In handleSubmit, trim reasonForReport before validating and
before passing it to submitReport so whitespace-only input is rejected and the
submitted reason has no surrounding spaces; update the check from using
reasonForReport.length to using trimmedReason.length and pass trimmedReason to
submitReport (keep using data.id and computed closureState from
data.properties.is_closed) and then call closeDialog.
---
Nitpick comments:
In `@src/react/src/__tests__/components/ContributeFields.test.js`:
- Around line 174-182: The test "anonymous user sees login prompt in dialog"
depends on default anonymity; make it explicit by passing a preloaded state with
isAnon: true when calling renderContributeFields in that test. Update the test
to call renderContributeFields with a preloadedState (or similar arg accepted by
the helper) that spreads USER_DEFAULT_STATE and overrides isAnon: true so the
dialog assertion remains deterministic even if defaults change; reference the
test name and the renderContributeFields helper to locate where to modify.
- Around line 207-226: The dialog-close assertion is brittle due to transitions
in the 'cancel closes dialog' test; update the test that calls
renderContributeFields and clicks 'report-facility-status-dialog-cancel' to use
a query + async wait instead of immediate getByTestId: use
screen.queryByTestId('report-facility-status-dialog') together with await
waitForElementToBeRemoved(...) or await waitFor(() =>
expect(...).not.toBeVisible()) to assert the dialog is removed/hidden after the
cancel click; keep the existing fireEvent calls and test ids (e.g.,
'contribute-report-status', 'report-facility-status-dialog',
'report-facility-status-dialog-cancel') so the change is localized to the
assertion only.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/react/src/components/ProductionLocation/Sidebar/ContributeFields/icons/copy.svgis excluded by!**/*.svgsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/icons/shield-x.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
.gitignoredoc/release/RELEASE-NOTES.mdsrc/react/src/App.jsxsrc/react/src/__tests__/components/ContributeFields.test.jssrc/react/src/components/ProductionLocation/ClaimSection/ClaimDataContainer/ClaimDataContainer.jsxsrc/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsxsrc/react/src/components/ProductionLocation/Heading/ClosureStatus/ClosureStatus.jsxsrc/react/src/components/ProductionLocation/Heading/DataSourcesInfo/DataSourcesInfo.jsxsrc/react/src/components/ProductionLocation/Heading/LocationTitle/LocationTitle.jsxsrc/react/src/components/ProductionLocation/PartnerSection/AssessmentsAndAudits/AssessmentsAndAudits.jsxsrc/react/src/components/ProductionLocation/PartnerSection/Certifications/Certifications.jsxsrc/react/src/components/ProductionLocation/PartnerSection/Emissions/Emissions.jsxsrc/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/ParentSectionItem.jsxsrc/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetails/ProductionLocationDetails.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/styles.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/ProductionLocationDetailsGeneralFields.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsLocation/ProductionLocationDetailsLocation.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/ProductionLocationDetailsMap.jsxsrc/react/src/components/ProductionLocation/Shared/FilledButton/FilledButton.jsxsrc/react/src/components/ProductionLocation/Shared/FilledButton/styles.jssrc/react/src/components/ProductionLocation/Shared/OutlinedButton/OutlinedButton.jsxsrc/react/src/components/ProductionLocation/Shared/OutlinedButton/styles.jssrc/react/src/components/ProductionLocation/Sidebar/BackToSearch/BackToSearch.jsxsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsxsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsxsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/hooks.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/styles.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/hooks.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/styles.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/utils.jssrc/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsxsrc/react/src/components/ProductionLocation/Sidebar/SupplyChain/SupplyChain.jsxsrc/react/src/components/ProductionLocation/commonStyles.js
✅ Files skipped from review due to trivial changes (1)
- src/react/src/components/ProductionLocation/ProductionLocationDetails/ProductionLocationDetails.jsx
🚧 Files skipped from review as they are similar to previous changes (21)
- src/react/src/components/ProductionLocation/PartnerSection/PartnerDataContainer/PartnerDataContainer.jsx
- src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx
- src/react/src/components/ProductionLocation/Shared/OutlinedButton/OutlinedButton.jsx
- src/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/ProductionLocationDetailsGeneralFields.jsx
- src/react/src/components/ProductionLocation/ClaimSection/ClaimDataContainer/ClaimDataContainer.jsx
- src/react/src/components/ProductionLocation/PartnerSection/ParentSectionItem/ParentSectionItem.jsx
- src/react/src/components/ProductionLocation/Sidebar/ContributeFields/utils.js
- src/react/src/components/ProductionLocation/Sidebar/ContributeFields/hooks.js
- src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/hooks.js
- .gitignore
- src/react/src/components/ProductionLocation/PartnerSection/Emissions/Emissions.jsx
- src/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/styles.js
- src/react/src/components/ProductionLocation/PartnerSection/Certifications/Certifications.jsx
- src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/ProductionLocationDetailsMap.jsx
- src/react/src/components/ProductionLocation/Shared/FilledButton/styles.js
- src/react/src/components/ProductionLocation/Heading/LocationTitle/LocationTitle.jsx
- src/react/src/App.jsx
- src/react/src/components/ProductionLocation/Sidebar/SupplyChain/SupplyChain.jsx
- src/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsx
- src/react/src/components/ProductionLocation/ProductionLocationDetailsLocation/ProductionLocationDetailsLocation.jsx
- src/react/src/components/ProductionLocation/Shared/OutlinedButton/styles.js
...ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx
Show resolved
Hide resolved
...nts/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsx
Show resolved
Hide resolved
...nLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx
Show resolved
Hide resolved
...nLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx
Show resolved
Hide resolved
protsack-stephan
left a comment
There was a problem hiding this comment.
Great work! One small comment from me.
src/react/src/components/ProductionLocation/Shared/OutlinedButton/OutlinedButton.jsx
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx (2)
52-56:⚠️ Potential issue | 🟡 MinorTrim and validate the reason before submit.
Line 53 currently allows whitespace-only input and Line 55 submits untrimmed text.
Suggested patch
const handleSubmit = () => { - if (!reasonForReport.length) return; + const trimmedReason = reasonForReport.trim(); + if (!trimmedReason.length) return; const closureState = data.properties.is_closed ? 'OPEN' : 'CLOSED'; - submitReport({ id: data.id, reasonForReport, closureState }); + submitReport({ + id: data.id, + reasonForReport: trimmedReason, + closureState, + }); closeDialog(); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx` around lines 52 - 56, In handleSubmit, trim and validate the reasonForReport before checking and submitting: replace the whitespace-only check with a trimmed-length check (e.g., use reasonForReport.trim()) and pass the trimmed string to submitReport instead of the raw reasonForReport; ensure you still call closeDialog() after a successful submit. Target the handleSubmit function and the submitReport call (and adjust any local variable or state usage for reasonForReport if needed) so only non-empty, trimmed text is accepted and sent.
118-131:⚠️ Potential issue | 🟠 MajorAdd an accessible name to the reason input.
The multiline
TextField(Line 118+) has no explicit label/aria-label, which makes this form control ambiguous for assistive tech.Suggested patch
<TextField autoFocus + label="Reason for report" + required margin="dense" id="report-reason" inputProps={{ 'data-testid': 'report-facility-status-reason', }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx` around lines 118 - 131, The multiline TextField for the reason (id "report-reason", data-testid "report-facility-status-reason", value bound to reasonForReport and updater setReportReason) lacks an accessible name; add one by providing either a visible label via the TextField label prop or an explicit aria-label/aria-labelledby on the TextField/inputProps so screen readers can identify it (keep the id for form association if you add a <label> elsewhere or use aria-labelledby referencing that label).
🧹 Nitpick comments (1)
src/react/src/components/ProductionLocation/Shared/Button/Button.jsx (1)
20-23: MakeonClickoptional to match existing button conventions in this repo.Requiring
onClickhere can force unnecessary no-op handlers in valid usage paths.Based on learnings: 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.Proposed change
Button.propTypes = { label: string.isRequired, - onClick: func.isRequired, + onClick: func, variant: oneOf([VARIANT.outlined, VARIANT.filled]), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Shared/Button/Button.jsx` around lines 20 - 23, The Button component's propTypes currently require onClick, which forces callers to pass no-op handlers; update Button.propTypes to make onClick optional (remove .isRequired from the onClick prop declaration) and add a corresponding Button.defaultProps entry (set onClick to undefined or a noop) so consumers can omit the prop safely; locate the propTypes definition for Button (Button.propTypes) and the component declaration to apply these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx`:
- Around line 52-56: In handleSubmit, trim and validate the reasonForReport
before checking and submitting: replace the whitespace-only check with a
trimmed-length check (e.g., use reasonForReport.trim()) and pass the trimmed
string to submitReport instead of the raw reasonForReport; ensure you still call
closeDialog() after a successful submit. Target the handleSubmit function and
the submitReport call (and adjust any local variable or state usage for
reasonForReport if needed) so only non-empty, trimmed text is accepted and sent.
- Around line 118-131: The multiline TextField for the reason (id
"report-reason", data-testid "report-facility-status-reason", value bound to
reasonForReport and updater setReportReason) lacks an accessible name; add one
by providing either a visible label via the TextField label prop or an explicit
aria-label/aria-labelledby on the TextField/inputProps so screen readers can
identify it (keep the id for form association if you add a <label> elsewhere or
use aria-labelledby referencing that label).
---
Nitpick comments:
In `@src/react/src/components/ProductionLocation/Shared/Button/Button.jsx`:
- Around line 20-23: The Button component's propTypes currently require onClick,
which forces callers to pass no-op handlers; update Button.propTypes to make
onClick optional (remove .isRequired from the onClick prop declaration) and add
a corresponding Button.defaultProps entry (set onClick to undefined or a noop)
so consumers can omit the prop safely; locate the propTypes definition for
Button (Button.propTypes) and the component declaration to apply these changes.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/react/src/components/ProductionLocation/Shared/Button/Button.jsxsrc/react/src/components/ProductionLocation/Shared/Button/constants.jssrc/react/src/components/ProductionLocation/Shared/Button/styles.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ReportFacilityStatusDialog/ReportFacilityStatusDialog.jsx



Summary
OSDEV-2369 – As part of the Production Location page redesign, this PR implements the "Contribute to this profile" section in the sidebar.
What's new
Code/API changes