[OSDEV-2373] Add geographic information section#904
Conversation
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.98%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.js
Outdated
Show resolved
Hide resolved
React App | Jest test suite - Code coverage reportTotal: 43.3%Your code coverage diff: 0.56% ▴ ✅ All code changes are covered |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.js (1)
28-30: Prefer explicit UTC parsing for claim dates.Evidence confirms
approved_atandcreated_attimestamps from the API are consistently timezone-qualified (UTC withZsuffix), so the current implementation works correctly in practice. However, usingmoment.utc(date)instead ofmoment(date).utc()makes the intent explicit and guards against future edge cases with ambiguous date formats.Suggested change
export const formatClaimDate = date => { if (!date) return null; - const parsedDate = moment(date); + const parsedDate = moment.utc(date); if (!parsedDate.isValid()) return null; - return parsedDate.utc().format('LL'); + return parsedDate.format('LL'); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.js` around lines 28 - 30, The date parsing in utils.js currently uses moment(date).utc(); update it to use moment.utc(date) to make UTC intent explicit—locate the code that defines parsedDate (const parsedDate = moment(date);) and replace the construction so parsedDate is created via moment.utc(date), keeping the subsequent isValid check and parsedDate.utc().format('LL') behavior as-is or remove the extra .utc() call if you prefer since moment.utc already returns a UTC moment.src/react/src/setupTests.js (2)
19-31: Make the setup helper synchronous.Line 19 returns a Promise that Line 31 ignores. Today the body is synchronous, but this makes any future awaited work race with setup and turns thrown errors into unhandled rejections instead of immediate setup failures.
♻️ Proposed fix
-const customJestEnvironment = async () => { +const customJestEnvironment = () => { const jsdom = new JSDOM('<!doctype html><html><body></body></html>', { url: 'http://localhost', }); @@ }; customJestEnvironment();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/setupTests.js` around lines 19 - 31, The setup helper customJestEnvironment is declared async which makes it return a Promise that is ignored and turns future awaited work into race conditions and unhandled rejections; change customJestEnvironment to be a synchronous function (remove the async) so the DOM setup runs and throws synchronously, and leave the direct invocation customJestEnvironment() as-is so any errors surface as immediate test setup failures.
11-12: PreferglobalThisfor the injected test globals.
globalThisis the standard global object here and removes the environment-specificglobalwarnings in this file.♻️ Proposed fix
-global.TextEncoder = global.TextEncoder || NodeTextEncoder; -global.TextDecoder = global.TextDecoder || NodeTextDecoder; +globalThis.TextEncoder = globalThis.TextEncoder || NodeTextEncoder; +globalThis.TextDecoder = globalThis.TextDecoder || NodeTextDecoder; @@ - global.window = jsdom.window; - global.document = jsdom.window.document; - global.navigator = jsdom.window.navigator; - global.btoa = str => Buffer.from(str, 'binary').toString('base64'); - global.atob = str => Buffer.from(str, 'base64').toString('binary'); + globalThis.window = jsdom.window; + globalThis.document = jsdom.window.document; + globalThis.navigator = jsdom.window.navigator; + globalThis.btoa = str => Buffer.from(str, 'binary').toString('base64'); + globalThis.atob = str => Buffer.from(str, 'base64').toString('binary');Also applies to: 24-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/setupTests.js` around lines 11 - 12, Replace use of the environment-specific global object with the standard globalThis for injected test globals: update the assignments that currently reference global.TextEncoder/global.TextDecoder (and the other occurrences around the second block at lines 24-28) to use globalThis.TextEncoder and globalThis.TextDecoder so the test setup uses the standard globalThis and avoids environment-specific warnings; locate the assignments in setupTests.js (the blocks that set TextEncoder/TextDecoder and the additional block at 24-28) and change them to reference globalThis instead of global.src/react/src/__tests__/components/ProductionLocationDetailsContainer.test.js (1)
28-32: MakefetchFacilitiesa spyable mock and assert the map-fetch options.This stub accepts every call shape, so the suite will still pass if the container stops passing the expected
{ pushNewRoute, activateFacilitiesTab: false }options for the new map-context fetch. Ajest.fnthunk mock here would let the test lock that contract down.📌 Tighten the mock and assertion
+const mockFetchFacilities = jest.fn(() => () => {}); + jest.mock('../../actions/facilities', () => ({ fetchSingleFacility: () => ({ type: 'noop' }), resetSingleFacility: () => ({ type: 'RESET_SINGLE_FACILITY' }), - fetchFacilities: () => () => {}, + fetchFacilities: (...args) => mockFetchFacilities(...args), }));+expect(mockFetchFacilities).toHaveBeenCalledWith( + expect.objectContaining({ + activateFacilitiesTab: false, + pushNewRoute: expect.any(Function), + }), +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/__tests__/components/ProductionLocationDetailsContainer.test.js` around lines 28 - 32, The current mock for fetchFacilities is an unspyable stub; replace it with a jest.fn that returns a thunk so tests can assert how the container calls it (e.g., fetchFacilities = jest.fn(() => (opts) => {/* thunk */})); update the test to expect fetchFacilities to have been called with the map-fetch options object ({ pushNewRoute, activateFacilitiesTab: false }) when the container triggers the map-context fetch, using the mock's call/args to lock that contract down; ensure you reference the mocked fetchFacilities in the test assertions (and reset/clear the mock between tests).src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js (1)
25-205: SplitgetFieldContributorInfoby field type before more provenance rules land here.This switch already carries separate dedupe, canonicalization, and fallback rules for addresses vs. coordinates, and it has crossed the current complexity threshold. Extracting
getAddressContributorInfoandgetCoordinatesContributorInfowould make future fixes much safer.♻️ Refactor sketch
+const EMPTY_CONTRIBUTOR_INFO = { + contributorName: '', + userId: null, + date: '', + status: null, + drawerData: { promotedContribution: null, contributions: [] }, +}; + +const getAddressContributorInfo = singleFacilityData => { + // current ADDRESS branch +}; + +const getCoordinatesContributorInfo = singleFacilityData => { + // current COORDINATES branch +}; + export const getFieldContributorInfo = (singleFacilityData, fieldType) => { switch (fieldType) { - case FIELD_TYPE.ADDRESS: { - ... - } - case FIELD_TYPE.COORDINATES: { - ... - } + case FIELD_TYPE.ADDRESS: + return getAddressContributorInfo(singleFacilityData); + case FIELD_TYPE.COORDINATES: + return getCoordinatesContributorInfo(singleFacilityData); default: - return { - contributorName: '', - userId: null, - date: '', - status: null, - drawerData: { promotedContribution: null, contributions: [] }, - }; + return EMPTY_CONTRIBUTOR_INFO; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js` around lines 25 - 205, The getFieldContributorInfo switch has grown complex and should be split into two focused helpers: extract the ADDRESS case logic into getAddressContributorInfo(singleFacilityData) and the COORDINATES case logic into getCoordinatesContributorInfo(singleFacilityData), move all dedupe/canonicalization/fallback logic and construction of drawerData/promotedContribution into those helpers, and have getFieldContributorInfo simply dispatch to the new functions (e.g., case FIELD_TYPE.ADDRESS: return getAddressContributorInfo(singleFacilityData);). Ensure the helpers use the same utility functions and return shape ({ contributorName, userId, date, status, drawerData }) so existing callers/tests remain unchanged.
🤖 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/ProductionLocationDetailsMap/utils.js`:
- Around line 60-76: The row-level date variable (date) only reads created_at
while promotedContribution.date uses created_at || updated_at || '', causing
inconsistent provenance; update the date assignment (where date is derived from
canonicalField) to use the same fallback as promotedContribution (created_at ||
updated_at || '') so both the address row and the promoted drawer entry show the
same date for the same canonicalField (refer to the date variable and
promotedContribution.date in this file).
---
Nitpick comments:
In
`@src/react/src/__tests__/components/ProductionLocationDetailsContainer.test.js`:
- Around line 28-32: The current mock for fetchFacilities is an unspyable stub;
replace it with a jest.fn that returns a thunk so tests can assert how the
container calls it (e.g., fetchFacilities = jest.fn(() => (opts) => {/* thunk
*/})); update the test to expect fetchFacilities to have been called with the
map-fetch options object ({ pushNewRoute, activateFacilitiesTab: false }) when
the container triggers the map-context fetch, using the mock's call/args to lock
that contract down; ensure you reference the mocked fetchFacilities in the test
assertions (and reset/clear the mock between tests).
In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.js`:
- Around line 28-30: The date parsing in utils.js currently uses
moment(date).utc(); update it to use moment.utc(date) to make UTC intent
explicit—locate the code that defines parsedDate (const parsedDate =
moment(date);) and replace the construction so parsedDate is created via
moment.utc(date), keeping the subsequent isValid check and
parsedDate.utc().format('LL') behavior as-is or remove the extra .utc() call if
you prefer since moment.utc already returns a UTC moment.
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js`:
- Around line 25-205: The getFieldContributorInfo switch has grown complex and
should be split into two focused helpers: extract the ADDRESS case logic into
getAddressContributorInfo(singleFacilityData) and the COORDINATES case logic
into getCoordinatesContributorInfo(singleFacilityData), move all
dedupe/canonicalization/fallback logic and construction of
drawerData/promotedContribution into those helpers, and have
getFieldContributorInfo simply dispatch to the new functions (e.g., case
FIELD_TYPE.ADDRESS: return getAddressContributorInfo(singleFacilityData);).
Ensure the helpers use the same utility functions and return shape ({
contributorName, userId, date, status, drawerData }) so existing callers/tests
remain unchanged.
In `@src/react/src/setupTests.js`:
- Around line 19-31: The setup helper customJestEnvironment is declared async
which makes it return a Promise that is ignored and turns future awaited work
into race conditions and unhandled rejections; change customJestEnvironment to
be a synchronous function (remove the async) so the DOM setup runs and throws
synchronously, and leave the direct invocation customJestEnvironment() as-is so
any errors surface as immediate test setup failures.
- Around line 11-12: Replace use of the environment-specific global object with
the standard globalThis for injected test globals: update the assignments that
currently reference global.TextEncoder/global.TextDecoder (and the other
occurrences around the second block at lines 24-28) to use
globalThis.TextEncoder and globalThis.TextDecoder so the test setup uses the
standard globalThis and avoids environment-specific warnings; locate the
assignments in setupTests.js (the blocks that set TextEncoder/TextDecoder and
the additional block at 24-28) and change them to reference globalThis instead
of global.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d27e3380-13a4-4c1f-9892-837b0550cc78
📒 Files selected for processing (6)
src/react/src/__tests__/components/ProductionLocationDetailsContainer.test.jssrc/react/src/__tests__/components/ProductionLocationDetailsMapUtils.test.jssrc/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/constants.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.jssrc/react/src/setupTests.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/constants.js
src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js (2)
25-211: Consider extracting branch handlers to reduce cognitive complexity.Static analysis flags cognitive complexity of 16 (allowed: 15). While the function is well-organized, extracting the ADDRESS and COORDINATES handlers into separate functions would improve testability and bring complexity within bounds.
♻️ Suggested structure
const getAddressContributorInfo = (singleFacilityData) => { // Lines 28-97 logic here }; const getCoordinatesContributorInfo = (singleFacilityData) => { // Lines 101-200 logic here }; export const getFieldContributorInfo = (singleFacilityData, fieldType) => { switch (fieldType) { case FIELD_TYPE.ADDRESS: return getAddressContributorInfo(singleFacilityData); case FIELD_TYPE.COORDINATES: return getCoordinatesContributorInfo(singleFacilityData); default: return { contributorName: '', userId: null, date: '', status: null, drawerData: { promotedContribution: null, contributions: [] }, }; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js` around lines 25 - 211, Extract the ADDRESS and COORDINATES case bodies from getFieldContributorInfo into two new helper functions named getAddressContributorInfo(singleFacilityData) and getCoordinatesContributorInfo(singleFacilityData), move the exact logic currently inside the FIELD_TYPE.ADDRESS and FIELD_TYPE.COORDINATES cases into those functions (preserving all variable names and behavior such as COORD_EPSILON, promotedContribution, drawerData, getContributorStatus usage, etc.), then simplify getFieldContributorInfo to switch on FIELD_TYPE.ADDRESS and FIELD_TYPE.COORDINATES and return the result of calling the corresponding helper; keep the existing default return object unchanged and ensure any exports or tests that reference the behavior still pass.
7-9: Consider usingexport...fromsyntax for cleaner re-export.As flagged by static analysis, the import-then-export pattern can be simplified.
♻️ Suggested refactor
-import { FIELD_TYPE } from './constants'; - -export { FIELD_TYPE }; +export { FIELD_TYPE } from './constants';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js` around lines 7 - 9, Replace the import-then-export pattern for FIELD_TYPE with a direct re-export: remove the existing import of FIELD_TYPE and the separate export statement and use the "export ... from" syntax to re-export FIELD_TYPE from the module where it's defined (i.e., change the import+export for FIELD_TYPE into a single export { FIELD_TYPE } from './constants' style re-export).
🤖 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 56-59: The effect using useEffect currently only depends on
normalizedOsID so fetchFacility(normalizedOsID, contributors) won't re-run when
contributors is hydrated; restore contributors to the dependency array so the
effect re-executes after hydrateFiltersFromQueryString updates
filters.contributors. Locate the useEffect that calls fetchFacility and change
its dependency list to include both normalizedOsID and contributors (i.e.,
[normalizedOsID, contributors]) to reinstate the intended initial empty fetch
and the subsequent contributor-scoped re-fetch.
---
Nitpick comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js`:
- Around line 25-211: Extract the ADDRESS and COORDINATES case bodies from
getFieldContributorInfo into two new helper functions named
getAddressContributorInfo(singleFacilityData) and
getCoordinatesContributorInfo(singleFacilityData), move the exact logic
currently inside the FIELD_TYPE.ADDRESS and FIELD_TYPE.COORDINATES cases into
those functions (preserving all variable names and behavior such as
COORD_EPSILON, promotedContribution, drawerData, getContributorStatus usage,
etc.), then simplify getFieldContributorInfo to switch on FIELD_TYPE.ADDRESS and
FIELD_TYPE.COORDINATES and return the result of calling the corresponding
helper; keep the existing default return object unchanged and ensure any exports
or tests that reference the behavior still pass.
- Around line 7-9: Replace the import-then-export pattern for FIELD_TYPE with a
direct re-export: remove the existing import of FIELD_TYPE and the separate
export statement and use the "export ... from" syntax to re-export FIELD_TYPE
from the module where it's defined (i.e., change the import+export for
FIELD_TYPE into a single export { FIELD_TYPE } from './constants' style
re-export).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4519ac19-b3b9-47ed-aaa8-9816c33ce24a
📒 Files selected for processing (3)
src/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.js
...ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js
Show resolved
Hide resolved
protsack-stephan
left a comment
There was a problem hiding this comment.
Great work! One quick note: could you look over the PR comments and make sure they answer the Why question? Some feel a bit redundant because they just describe what the code is already doing.
src/react/src/__tests__/components/ProductionLocationDetailsMap.test.jsx
Outdated
Show resolved
Hide resolved
src/react/src/__tests__/components/ProductionLocationDetailsMapUtils.test.js
Outdated
Show resolved
Hide resolved
...ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx
Outdated
Show resolved
Hide resolved
...ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx
Show resolved
Hide resolved
...ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/ProductionLocationDetailsMap/utils.js
Show resolved
Hide resolved
...ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx
Show resolved
Hide resolved
...ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsx
Show resolved
Hide resolved
|
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.



Implements OSDEV-2373
A new Geographic Information section has been added to the Production Location details page.
It consists of:
A satellite map (ProductionLocationDetailsMap) rendered in the right column alongside the General Fields section. The map shows:
Address & Coordinates data points rendered below the map, each showing:
Where the data comes from
VectorTileFacilitiesLayerandVectorTileFacilityGridLayercomponents but with the satellite view.properties.address(the canonical address on the facility) andproperties.extended_fields.address(all historical contributions).geometry.coordinates(GeoJSON point — the authoritative lat/lng rendered on the map) andproperties.other_locations(all submitted coordinate contributions).Note
Medium Risk
Touches production-location detail routing/state hydration and adds a new Leaflet-based map with non-trivial contributor attribution logic; issues could affect navigation, map rendering, or displayed provenance but are mostly UI-scoped and covered by tests.
Overview
Adds a new Geographic information section to the Production Location details page, replacing the prior embedded search map with a Leaflet satellite map that reuses vector-tile facility layers, adds custom zoom/center controls, grid legend, and an “Open in Google Maps” action.
Introduces provenance logic for Address and Coordinates via new utilities that select/promote canonical contributions, deduplicate/validate submissions, and drive
DataPointattribution plusContributionsDrawercontents. Updates routing and the details container to preload map data by hydrating/resetting filters from the query string and dispatchingfetchFacilitieswithout pushing navigation.Includes small correctness/testability fixes: ignore
nullpromotedContributionin contributor counts, tighten claim date parsing, makeVectorTileGridLegendlabel configurable, add Jest polyfills forTextEncoder/TextDecoder, and add comprehensive unit tests for the new map + attribution behavior.Written by Cursor Bugbot for commit 7025124. This will update automatically on new commits. Configure here.