[OSDEV-2371] Implemented "General Information" section with contribution hierarchy logic#911
Open
vlad-shapik wants to merge 10 commits intomainfrom
Conversation
- ProductionLocationDetailsGeneralFields: single visible list from getVisibleItems(data, showAdditionalIdentifiers); connect to Redux for feature flag - Add getVisibleItems in utils.js; remove fullConfigs/configsWithoutAdditional useMemos and FeatureFlag wrapper - NavBar: replace @material-ui/icons/SummarizeOutlined with custom Overview icon (MUI Summarize-style SVG) - Add Overview.jsx and MapPointer.jsx icons; update GeneralInformation export Made-with: Cursor
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 15. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 44.42%Your code coverage diff: 0.95% ▴ ✅ All code changes are covered |
vlad-shapik
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 |
vlad-shapik
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 |
vlad-shapik
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 |
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Django App | Unittest test suite - Code coverage reportTotal: 81.98%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
…tus tooltip - Contributions drawer: single always-mounted drawer with open prop for slide animation; keep content visible until close transition ends (onExited) - Align name/extended field logic with non-redesigned page (deduplicate by primary+secondary so data source counts match) - Add getSelectedDrawerItem util; consolidate getContributionsCount in ContributionsDrawer/utils (remove DataPoint/utils) - DataPoint: remove renderDrawer, use onOpenDrawer only; accept node for tooltipText (status tooltip with Learn more link) - Status tooltip: add Learn more link (constants.jsx, LearnMoreLink); ProductionLocationDetailsGeneralFields: use getSelectedDrawerItem Made-with: Cursor
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Made-with: Cursor
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
vlad-shapik
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vlad-shapik
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
…nt meta row - Move useDrawerState to ProductionLocation/hooks.js; use in GeneralFields and Map - Add ProductionLocation/utils.js with getSelectedDrawerField; remove from GeneralFields utils - Map uses getSelectedDrawerField with mapDrawerFields; remove getSelectedDrawerContent from Map utils - DataPoint: meta row columnGap only; simplify separator via meta segments array Made-with: Cursor
…tion-with-contribution-hierarchy-logic
- Add ProductionLocationDetailsGeneralFields.test.js (8 cases, data-testid only) - Add utils.getVisibleFields.tests.js with anonymized fixture and behavior snapshots - Add data-testid to ProductionLocationDetailsGeneralFields root - Fix ProductionLocationDetailsMap test: Geographic Information title case Made-with: Cursor
…What's new) Made-with: Cursor
|
📝 WalkthroughWalkthroughThis PR implements a new General Information section on the Production Location details page that displays core facility identifying fields (name, sector, extended fields, status) as interactive data points with contribution tracking and drawer interactions. It refactors the contributions drawer and data point components with performance optimizations, introduces centralized field configuration management, and unifies drawer state across multiple sections. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as General Information<br/>Section
participant Field as DataPoint<br/>Component
participant Drawer as Contributions<br/>Drawer
participant Redux as Redux State<br/>(Feature Flags)
User->>UI: Load Production Location
activate UI
UI->>Redux: Fetch activeFeatureFlags
activate Redux
Redux-->>UI: Return feature flags
deactivate Redux
UI->>UI: getVisibleFields(data,<br/>includeAdditionalIdentifiers)
UI->>UI: Render DataPoint for each field
deactivate UI
User->>Field: Click sources button<br/>on DataPoint
activate Field
Field->>Field: onSourcesClick(fieldKey)
Field->>UI: openDrawer(fieldKey)
deactivate Field
activate UI
UI->>Drawer: Render with drawerData
deactivate UI
activate Drawer
Drawer->>Drawer: Display promotedContribution
Drawer->>Drawer: List all contributions
User->>Drawer: Review data sources
User->>Drawer: Click close
Drawer->>UI: closeDrawer()
deactivate Drawer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
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/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx (1)
53-58:⚠️ Potential issue | 🟠 MajorAdd the standard Grid overflow/mobile guards with this spacing change.
spacing={16}brings back MUI Grid's negative margins, and the child items still don't declare anxsbreakpoint. That's the same pattern that has already caused horizontal scroll and unstable stacking in these Production Location layouts.Suggested fix
- <Grid container className={classes.containerItem} spacing={16}> - <Grid item sm={12} md={7}> + <Grid + container + className={classes.containerItem} + spacing={16} + style={{ margin: 0, width: '100%' }} + > + <Grid item xs={12} md={7}> <GeneralFields data={data} /> </Grid> - <Grid item sm={12} md={5}> + <Grid item xs={12} md={5}> <DetailsMap /> </Grid> </Grid>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, and child Grid items should still define their ownxsbreakpoints for proper responsive behavior.🤖 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 53 - 58, The Grid container using spacing={16} introduces negative margins and child items lack xs breakpoints; update the container Grid (the one with className={classes.containerItem}) to include inline style={{ margin: 0, width: '100%' }} to neutralize negative margins, and add xs={12} to both child Grid items that render <GeneralFields data={data}/> and <DetailsMap/> (currently declared as Grid item sm={12} md={7} and sm={12} md={5}) so they declare explicit xs breakpoints for proper mobile/responsive stacking.
🤖 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__/components/ProductionLocationDetailsGeneralFields.test.js`:
- Around line 115-140: The test "drawer has title and close control when opened"
currently short-circuits with if (sourcesButtons.length > 0) so it won't fail if
the trigger disappears; replace the guard with a strict assertion that the
trigger exists (e.g., assert sourcesButtons.length > 0 or use
expect(sourcesButtons.length).toBeGreaterThan(0)), then click sourcesButtons[0]
(using renderComponent and the 'data-point-sources-button' test id) and assert
the presence of 'contributions-drawer-title' and 'contributions-drawer-close'
via getByTestId so the test fails if the trigger or drawer regress.
In `@src/react/src/components/ProductionLocation/constants.jsx`:
- Around line 18-20: Update the tooltipText string in the ProductionLocation
constants (the tooltipText property in
src/react/src/components/ProductionLocation/constants.jsx) to use source-neutral
copy that does not assert Google as the origin; change it to something like "The
geographic coordinates (latitude, longitude) of this production location; source
may be a contributor or a geocoding service" so it remains accurate when
coordinates are contributed manually or via different services.
In `@src/react/src/components/ProductionLocation/ContributionsDrawer/styles.js`:
- Around line 5-10: classes.drawerContent applies to the inner div and its
padding increases the rendered drawer width; update the drawerContent style (the
Object.freeze block named drawerContent) to make the width inclusive of
padding—either add boxSizing: 'border-box' to drawerContent or reduce width to
`calc(390px - 48px)` (i.e., subtract 2 * padding) so the final rendered drawer
stays at 390px.
In `@src/react/src/components/ProductionLocation/ContributionsDrawer/utils.js`:
- Around line 15-23: The current extraction builds sourceNames from
contributions but only filters empty strings without normalizing spacing, so
de-duplication via new Set(sourceNames) will treat "Source A" and " Source A "
as different; update the pipeline that creates sourceNames (the mapping over
contributions and the variable sourceNames) to trim each
contribution?.sourceName (e.g., String(...).trim()) before filtering and before
adding to the Set so the Set deduplicates normalized names, then compute
uniqueCount from that Set.
In `@src/react/src/components/ProductionLocation/DataPoint/DataPoint.jsx`:
- Around line 40-47: showSourcesButton currently uses sourcesCount (from
getContributionsCount) which excludes a single promoted contribution, so the
drawer trigger is hidden when drawerData.contributions contains exactly one
promoted item; change the visibility check to use the actual contributions array
presence/length instead: compute showSourcesButton as
Boolean(drawerData?.contributions?.length) && onOpenDrawer (or similarly check
drawerData.contributions.length > 0) so the drawer is available whenever
drawerData.contributions has any entries; apply the same fix to the other
occurrence mentioned (the block around lines 160-172) and keep
getContributionsCount as-is for counts elsewhere.
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/utils.js`:
- Around line 106-121: The current logic removes every rawNameValues whose
formatExtendedField(...).primary equals coreName, dropping legitimate alternate
submissions; change the flow so we no longer blanket-filter out entries with
formatted.primary === coreName. Instead, produce contributions by deduplicating
on a key that preserves alternate submissions (e.g., include contributor/date or
keep duplicates beyond the first) when building rawNameValuesFilteredLikeOther:
stop filtering by formatted.primary !== coreName and use uniqBy (or a grouping +
slice(1) approach) keyed on formatted.primary + formatted.secondary plus
contributor/date so toDrawerContribution still receives alternate canonical
submissions; update references in rawNameValuesFilteredLikeOther, rawNameValues,
formatExtendedField, coreName, and toDrawerContribution accordingly.
- Around line 252-263: The current logic builds contributions by flatMapping
every item in each grouped ISIC (restGroups.flatMap(...)) which produces one
drawer row per raw item; change it to emit one contribution per grouped ISIC
instead: iterate restGroups with map (or forEach push) using the same index to
look up the corresponding groupedContributions[groupIndex + 1] and create a
single drawer contribution for that group (calling toDrawerContribution once per
group, passing the group's representative/formattedGroup.primary), guarding for
absent group/items so you don't produce undefined entries.
---
Outside diff comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 53-58: The Grid container using spacing={16} introduces negative
margins and child items lack xs breakpoints; update the container Grid (the one
with className={classes.containerItem}) to include inline style={{ margin: 0,
width: '100%' }} to neutralize negative margins, and add xs={12} to both child
Grid items that render <GeneralFields data={data}/> and <DetailsMap/> (currently
declared as Grid item sm={12} md={7} and sm={12} md={5}) so they declare
explicit xs breakpoints for proper mobile/responsive stacking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 67d61438-da53-4fd4-8787-6c063703d649
📒 Files selected for processing (30)
doc/release/RELEASE-NOTES.mdsrc/react/src/__tests__/components/DataPoint.test.jsxsrc/react/src/__tests__/components/ProductionLocationDetailsGeneralFields.test.jssrc/react/src/__tests__/components/ProductionLocationDetailsMap.test.jsxsrc/react/src/__tests__/utils/ProductionLocationDetailsGeneralFieldsUtils.test.jssrc/react/src/components/Icons/GeneralInformation.jsxsrc/react/src/components/Icons/MapPointer.jsxsrc/react/src/components/Icons/Overview.jsxsrc/react/src/components/ProductionLocation/ContributionsDrawer/ContributionCard/ContributionCard.jsxsrc/react/src/components/ProductionLocation/ContributionsDrawer/ContributionsDrawer.jsxsrc/react/src/components/ProductionLocation/ContributionsDrawer/styles.jssrc/react/src/components/ProductionLocation/ContributionsDrawer/utils.jssrc/react/src/components/ProductionLocation/DataPoint/DataPoint.jsxsrc/react/src/components/ProductionLocation/DataPoint/styles.jssrc/react/src/components/ProductionLocation/DataPoint/utils.jssrc/react/src/components/ProductionLocation/Heading/LocationTitle/LocationTitle.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/ProductionLocationDetailsGeneralFields.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/styles.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/utils.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/ProductionLocationDetailsMap.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/constants.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/styles.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.jssrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/ContributeFields.jsxsrc/react/src/components/ProductionLocation/Sidebar/ContributeFields/styles.jssrc/react/src/components/ProductionLocation/Sidebar/NavBar/NavBar.jsxsrc/react/src/components/ProductionLocation/constants.jsxsrc/react/src/components/ProductionLocation/hooks.jssrc/react/src/components/ProductionLocation/utils.js
💤 Files with no reviewable changes (4)
- src/react/src/components/ProductionLocation/Sidebar/ContributeFields/styles.js
- src/react/src/tests/components/DataPoint.test.jsx
- src/react/src/components/ProductionLocation/DataPoint/utils.js
- src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/constants.js
Comment on lines
+115
to
+140
| test('renders the contributions drawer when opened via sources button', () => { | ||
| renderComponent(); | ||
| const sourcesButtons = screen.queryAllByTestId('data-point-sources-button'); | ||
| expect(sourcesButtons.length).toBeGreaterThan(0); | ||
| sourcesButtons[0].click(); | ||
| expect(screen.getByTestId('contributions-drawer')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('drawer has title and close control when opened', () => { | ||
| renderComponent(); | ||
| const sourcesButtons = screen.queryAllByTestId('data-point-sources-button'); | ||
| if (sourcesButtons.length > 0) { | ||
| sourcesButtons[0].click(); | ||
| expect(screen.getByTestId('contributions-drawer-title')).toBeInTheDocument(); | ||
| expect(screen.getByTestId('contributions-drawer-close')).toBeInTheDocument(); | ||
| } | ||
| }); | ||
|
|
||
| test('opening a field with sources opens the drawer', () => { | ||
| renderComponent(); | ||
| const sourcesButtons = screen.queryAllByTestId('data-point-sources-button'); | ||
| expect(sourcesButtons.length).toBeGreaterThan(0); | ||
| sourcesButtons[0].click(); | ||
| const drawer = screen.getByTestId('contributions-drawer'); | ||
| expect(drawer).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
Make the drawer assertions fail when the trigger disappears.
The if (sourcesButtons.length > 0) guard turns this into a no-op, so the suite can stay green even if the sources button regresses. It also overlaps with the adjacent drawer-open smoke test; one strict test is enough here.
💡 Proposed fix
- test('drawer has title and close control when opened', () => {
- renderComponent();
- const sourcesButtons = screen.queryAllByTestId('data-point-sources-button');
- if (sourcesButtons.length > 0) {
- sourcesButtons[0].click();
- expect(screen.getByTestId('contributions-drawer-title')).toBeInTheDocument();
- expect(screen.getByTestId('contributions-drawer-close')).toBeInTheDocument();
- }
- });
-
- test('opening a field with sources opens the drawer', () => {
+ test('opens the drawer and renders its chrome', () => {
renderComponent();
- const sourcesButtons = screen.queryAllByTestId('data-point-sources-button');
- expect(sourcesButtons.length).toBeGreaterThan(0);
+ const sourcesButtons = screen.getAllByTestId('data-point-sources-button');
sourcesButtons[0].click();
- const drawer = screen.getByTestId('contributions-drawer');
- expect(drawer).toBeInTheDocument();
+ expect(screen.getByTestId('contributions-drawer')).toBeInTheDocument();
+ expect(
+ screen.getByTestId('contributions-drawer-title'),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByTestId('contributions-drawer-close'),
+ ).toBeInTheDocument();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test('renders the contributions drawer when opened via sources button', () => { | |
| renderComponent(); | |
| const sourcesButtons = screen.queryAllByTestId('data-point-sources-button'); | |
| expect(sourcesButtons.length).toBeGreaterThan(0); | |
| sourcesButtons[0].click(); | |
| expect(screen.getByTestId('contributions-drawer')).toBeInTheDocument(); | |
| }); | |
| test('drawer has title and close control when opened', () => { | |
| renderComponent(); | |
| const sourcesButtons = screen.queryAllByTestId('data-point-sources-button'); | |
| if (sourcesButtons.length > 0) { | |
| sourcesButtons[0].click(); | |
| expect(screen.getByTestId('contributions-drawer-title')).toBeInTheDocument(); | |
| expect(screen.getByTestId('contributions-drawer-close')).toBeInTheDocument(); | |
| } | |
| }); | |
| test('opening a field with sources opens the drawer', () => { | |
| renderComponent(); | |
| const sourcesButtons = screen.queryAllByTestId('data-point-sources-button'); | |
| expect(sourcesButtons.length).toBeGreaterThan(0); | |
| sourcesButtons[0].click(); | |
| const drawer = screen.getByTestId('contributions-drawer'); | |
| expect(drawer).toBeInTheDocument(); | |
| }); | |
| test('renders the contributions drawer when opened via sources button', () => { | |
| renderComponent(); | |
| const sourcesButtons = screen.queryAllByTestId('data-point-sources-button'); | |
| expect(sourcesButtons.length).toBeGreaterThan(0); | |
| sourcesButtons[0].click(); | |
| expect(screen.getByTestId('contributions-drawer')).toBeInTheDocument(); | |
| }); | |
| test('opens the drawer and renders its chrome', () => { | |
| renderComponent(); | |
| const sourcesButtons = screen.getAllByTestId('data-point-sources-button'); | |
| sourcesButtons[0].click(); | |
| expect(screen.getByTestId('contributions-drawer')).toBeInTheDocument(); | |
| expect( | |
| screen.getByTestId('contributions-drawer-title'), | |
| ).toBeInTheDocument(); | |
| expect( | |
| screen.getByTestId('contributions-drawer-close'), | |
| ).toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/react/src/__tests__/components/ProductionLocationDetailsGeneralFields.test.js`
around lines 115 - 140, The test "drawer has title and close control when
opened" currently short-circuits with if (sourcesButtons.length > 0) so it won't
fail if the trigger disappears; replace the guard with a strict assertion that
the trigger exists (e.g., assert sourcesButtons.length > 0 or use
expect(sourcesButtons.length).toBeGreaterThan(0)), then click sourcesButtons[0]
(using renderComponent and the 'data-point-sources-button' test id) and assert
the presence of 'contributions-drawer-title' and 'contributions-drawer-close'
via getByTestId so the test fails if the trigger or drawer regress.
Comment on lines
+18
to
+20
| tooltipText: | ||
| "The geographic coordinates (latitude, longitude) of this production location generated with Google's geocoding API.", | ||
| }), |
There was a problem hiding this comment.
Use source-neutral copy for contributed coordinates.
This section now shows contributor provenance and drawer history for coordinates, so saying the value is always “generated with Google’s geocoding API” will be wrong for manually contributed lat/lng.
Suggested copy
tooltipText:
- "The geographic coordinates (latitude, longitude) of this production location generated with Google's geocoding API.",
+ 'The geographic coordinates (latitude, longitude) associated with this production location.',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| tooltipText: | |
| "The geographic coordinates (latitude, longitude) of this production location generated with Google's geocoding API.", | |
| }), | |
| tooltipText: | |
| 'The geographic coordinates (latitude, longitude) associated with this production location.', | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/src/components/ProductionLocation/constants.jsx` around lines 18 -
20, Update the tooltipText string in the ProductionLocation constants (the
tooltipText property in
src/react/src/components/ProductionLocation/constants.jsx) to use source-neutral
copy that does not assert Google as the origin; change it to something like "The
geographic coordinates (latitude, longitude) of this production location; source
may be a contributor or a geocoding service" so it remains accurate when
coordinates are contributed manually or via different services.
Comment on lines
5
to
10
| drawerContent: Object.freeze({ | ||
| padding: '24px', | ||
| width: '390px', | ||
| overflowY: 'auto', | ||
| height: '100%', | ||
| }), |
There was a problem hiding this comment.
Keep the drawer width inclusive of padding.
classes.drawerContent is attached to the inner <div>, not the Drawer paper. With Line 7 plus padding: '24px', the rendered drawer grows to 438px, so content starts clipping on narrow viewports.
Suggested fix
drawerContent: Object.freeze({
padding: '24px',
width: '390px',
+ boxSizing: 'border-box',
overflowY: 'auto',
height: '100%',
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| drawerContent: Object.freeze({ | |
| padding: '24px', | |
| width: '390px', | |
| overflowY: 'auto', | |
| height: '100%', | |
| }), | |
| drawerContent: Object.freeze({ | |
| padding: '24px', | |
| width: '390px', | |
| boxSizing: 'border-box', | |
| overflowY: 'auto', | |
| height: '100%', | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/src/components/ProductionLocation/ContributionsDrawer/styles.js`
around lines 5 - 10, classes.drawerContent applies to the inner div and its
padding increases the rendered drawer width; update the drawerContent style (the
Object.freeze block named drawerContent) to make the width inclusive of
padding—either add boxSizing: 'border-box' to drawerContent or reduce width to
`calc(390px - 48px)` (i.e., subtract 2 * padding) so the final rendered drawer
stays at 390px.
Comment on lines
+15
to
+23
| const sourceNames = contributions | ||
| .map(contribution => contribution?.sourceName) | ||
| .filter( | ||
| sourceName => | ||
| sourceName != null && String(sourceName).trim() !== '', | ||
| ); | ||
|
|
||
| const uniqueCount = new Set(definedIds).size; | ||
| const anonymousCount = contributions.length - definedIds.length; | ||
| return uniqueCount + anonymousCount; | ||
| const uniqueCount = new Set(sourceNames).size; | ||
| return uniqueCount; |
There was a problem hiding this comment.
Trim sourceName before de-duplicating.
filter(...trim() !== '') only removes blank values; it doesn't normalize the values you actually put into the Set. "Source A" and " Source A " will be counted as two organizations here.
💡 Proposed fix
const sourceNames = contributions
.map(contribution => contribution?.sourceName)
- .filter(
- sourceName =>
- sourceName != null && String(sourceName).trim() !== '',
- );
+ .filter(sourceName => sourceName != null)
+ .map(sourceName => String(sourceName).trim())
+ .filter(Boolean);
const uniqueCount = new Set(sourceNames).size;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/src/components/ProductionLocation/ContributionsDrawer/utils.js`
around lines 15 - 23, The current extraction builds sourceNames from
contributions but only filters empty strings without normalizing spacing, so
de-duplication via new Set(sourceNames) will treat "Source A" and " Source A "
as different; update the pipeline that creates sourceNames (the mapping over
contributions and the variable sourceNames) to trim each
contribution?.sourceName (e.g., String(...).trim()) before filtering and before
adding to the Set so the Set deduplicates normalized names, then compute
uniqueCount from that Set.
Comment on lines
+40
to
+47
| const sourcesCount = useMemo( | ||
| () => getContributionsCount(drawerData?.contributions), | ||
| [drawerData?.contributions], | ||
| ); | ||
| const showSourcesButton = useMemo(() => sourcesCount > 0 && onOpenDrawer, [ | ||
| sourcesCount, | ||
| onOpenDrawer, | ||
| ]); |
There was a problem hiding this comment.
Don't hide the drawer trigger for single-source fields.
showSourcesButton is now derived only from drawerData.contributions.length. For fields that have a promoted contribution but no secondary contributions, the drawer still has content but there is no way to open it. The new minimal getVisibleFields case already produces that shape, so this drops the data-sources affordance for valid fields.
💡 Proposed fix
- const sourcesCount = useMemo(
- () => getContributionsCount(drawerData?.contributions),
- [drawerData?.contributions],
- );
- const showSourcesButton = useMemo(() => sourcesCount > 0 && onOpenDrawer, [
- sourcesCount,
- onOpenDrawer,
- ]);
+ const sourcesCount = useMemo(
+ () =>
+ getContributionsCount(drawerData?.contributions) +
+ (drawerData?.promotedContribution ? 1 : 0),
+ [drawerData],
+ );
+ const showSourcesButton = useMemo(
+ () => sourcesCount > 0 && Boolean(onOpenDrawer),
+ [sourcesCount, onOpenDrawer],
+ );Also applies to: 160-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/src/components/ProductionLocation/DataPoint/DataPoint.jsx` around
lines 40 - 47, showSourcesButton currently uses sourcesCount (from
getContributionsCount) which excludes a single promoted contribution, so the
drawer trigger is hidden when drawerData.contributions contains exactly one
promoted item; change the visibility check to use the actual contributions array
presence/length instead: compute showSourcesButton as
Boolean(drawerData?.contributions?.length) && onOpenDrawer (or similarly check
drawerData.contributions.length > 0) so the drawer is available whenever
drawerData.contributions has any entries; apply the same fix to the other
occurrence mentioned (the block around lines 160-172) and keep
getContributionsCount as-is for counts elsewhere.
Comment on lines
+106
to
+121
| const rawNameValuesFilteredLikeOther = uniqBy( | ||
| rawNameValues.filter( | ||
| rawItem => | ||
| formatExtendedField(rawItem).primary !== coreName, | ||
| ), | ||
| rawItem => { | ||
| const formatted = formatExtendedField(rawItem); | ||
| return formatted.primary + formatted.secondary; | ||
| }, | ||
| ); | ||
| const contributions = rawNameValuesFilteredLikeOther.map(rawItem => | ||
| toDrawerContribution( | ||
| rawItem, | ||
| formatExtendedField(rawItem).primary, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Preserve alternate submissions of the promoted name.
Line 107 filters out every rawNameValues entry whose formatted primary equals coreName. That drops legitimate same-value submissions from different contributors/dates, so the drawer and +N data sources count become incomplete. The address logic in src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js keeps those extra canonical matches via canonicalFields.slice(1); the name branch should do the same.
Suggested fix
- const rawNameValuesFilteredLikeOther = uniqBy(
- rawNameValues.filter(
- rawItem =>
- formatExtendedField(rawItem).primary !== coreName,
- ),
- rawItem => {
- const formatted = formatExtendedField(rawItem);
- return formatted.primary + formatted.secondary;
- },
- );
- const contributions = rawNameValuesFilteredLikeOther.map(rawItem =>
+ const [matchingNameValues, otherNameValues] = partition(
+ rawNameValues,
+ rawItem =>
+ formatExtendedField(rawItem).primary === coreName,
+ );
+ const remainingNameValues = uniqBy(
+ [...matchingNameValues.slice(1), ...otherNameValues],
+ rawItem => {
+ const formatted = formatExtendedField(rawItem);
+ return formatted.primary + formatted.secondary;
+ },
+ );
+ const contributions = remainingNameValues.map(rawItem =>
toDrawerContribution(
rawItem,
formatExtendedField(rawItem).primary,
),
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/utils.js`
around lines 106 - 121, The current logic removes every rawNameValues whose
formatExtendedField(...).primary equals coreName, dropping legitimate alternate
submissions; change the flow so we no longer blanket-filter out entries with
formatted.primary === coreName. Instead, produce contributions by deduplicating
on a key that preserves alternate submissions (e.g., include contributor/date or
keep duplicates beyond the first) when building rawNameValuesFilteredLikeOther:
stop filtering by formatted.primary !== coreName and use uniqBy (or a grouping +
slice(1) approach) keyed on formatted.primary + formatted.secondary plus
contributor/date so toDrawerContribution still receives alternate canonical
submissions; update references in rawNameValuesFilteredLikeOther, rawNameValues,
formatExtendedField, coreName, and toDrawerContribution accordingly.
Comment on lines
+252
to
+263
| const restGroups = formattedGroups.slice(1); | ||
| const contributions = restGroups.flatMap( | ||
| (formattedGroup, groupIndex) => { | ||
| const group = groupedContributions[groupIndex + 1]; | ||
| return (group?.items || []).map(contributionItem => | ||
| toDrawerContribution( | ||
| contributionItem, | ||
| formattedGroup.primary, | ||
| ), | ||
| ); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Emit one drawer row per grouped ISIC submission.
Lines 253-260 map every raw item in a grouped isic_4 contribution to the same formattedGroup.primary. If one submission expands to multiple ISIC rows, the drawer will render identical cards once per row and overcount the sources. This needs to emit one contribution per group, not per item.
Suggested fix
- const restGroups = formattedGroups.slice(1);
- const contributions = restGroups.flatMap(
- (formattedGroup, groupIndex) => {
- const group = groupedContributions[groupIndex + 1];
- return (group?.items || []).map(contributionItem =>
- toDrawerContribution(
- contributionItem,
- formattedGroup.primary,
- ),
- );
- },
- );
+ const restGroups = formattedGroups.slice(1);
+ const contributions = restGroups
+ .map((formattedGroup, groupIndex) => {
+ const group = groupedContributions[groupIndex + 1];
+ const representativeItem = group?.items?.[0];
+
+ return representativeItem
+ ? toDrawerContribution(
+ representativeItem,
+ formattedGroup.primary,
+ )
+ : null;
+ })
+ .filter(Boolean);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const restGroups = formattedGroups.slice(1); | |
| const contributions = restGroups.flatMap( | |
| (formattedGroup, groupIndex) => { | |
| const group = groupedContributions[groupIndex + 1]; | |
| return (group?.items || []).map(contributionItem => | |
| toDrawerContribution( | |
| contributionItem, | |
| formattedGroup.primary, | |
| ), | |
| ); | |
| }, | |
| ); | |
| const restGroups = formattedGroups.slice(1); | |
| const contributions = restGroups | |
| .map((formattedGroup, groupIndex) => { | |
| const group = groupedContributions[groupIndex + 1]; | |
| const representativeItem = group?.items?.[0]; | |
| return representativeItem | |
| ? toDrawerContribution( | |
| representativeItem, | |
| formattedGroup.primary, | |
| ) | |
| : null; | |
| }) | |
| .filter(Boolean); |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 256-256: Refactor this code to not nest functions more than 4 levels deep.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsGeneralFields/utils.js`
around lines 252 - 263, The current logic builds contributions by flatMapping
every item in each grouped ISIC (restGroups.flatMap(...)) which produces one
drawer row per raw item; change it to emit one contribution per grouped ISIC
instead: iterate restGroups with map (or forEach push) using the same index to
look up the corresponding groupedContributions[groupIndex + 1] and create a
single drawer contribution for that group (calling toDrawerContribution once per
group, passing the group's representative/formattedGroup.primary), guarding for
absent group/items so you don't produce undefined entries.
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
1 participant
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.
OSDEV-2371
Implemented the General Information section on the new Production Location page: