Skip to content

[OSDEV-2352] - Add feature flag for production location page FE routing#875

Merged
VadimKovalenkoSNF merged 13 commits intomainfrom
OSDEV-2352-feature-flag-fe-production-locations
Feb 23, 2026
Merged

[OSDEV-2352] - Add feature flag for production location page FE routing#875
VadimKovalenkoSNF merged 13 commits intomainfrom
OSDEV-2352-feature-flag-fe-production-locations

Conversation

@VadimKovalenkoSNF
Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF commented Feb 5, 2026

Implementation of OSDEV-2352

Add feature flag for production location page for the FE routing and proper links while mailing.

components/ProductionLocation/ProductionLocationDetails.jsx and /components/ProductionLocation/ProductionLocationDetailsContent.jsx are placeholder components for new design.

@VadimKovalenkoSNF VadimKovalenkoSNF self-assigned this Feb 5, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/react/src/components/FacilityDetailsContent.jsx`:
- Around line 194-208: The redirect currently puts the full URL (including query
string) into the location object's pathname which prevents React Router v5 from
parsing search; in FacilityDetailsContent.jsx change the Redirect usage that
returns the object form to pass the URL string directly by calling
makeFacilityDetailLinkOnRedirect(data.id, location.search,
useProductionLocationPage) as the to prop (instead of to={{ pathname: ... }}),
so the pathname and query (search) are correctly split by React Router v5 when
data.id !== normalizedOsID.
🧹 Nitpick comments (1)
src/react/src/util/util.js (1)

257-273: Dual return format (?-prefixed string) used in two different contexts — verify React Router compatibility.

getFilteredSearchForEmbed returns either '' or '?key=val' (with ? prefix). This works correctly when concatenated in makeFacilityDetailLinkOnRedirect (line 283), but it's also used as to.search in FilterSidebarFacilitiesTab.jsx (line 465) and Facilities.jsx (line 48). In React Router v5, to.search accepts both formats (?foo=bar and foo=bar), so this should work — but the mixed usage patterns are worth noting.

Consider documenting or standardizing the return contract — e.g., always returning without the ? prefix and letting callers decide how to attach it — to avoid subtle inconsistencies in future usage. Not blocking.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/react/src/components/FacilityDetailsContent.jsx`:
- Around line 122-130: The effect depends on normalizedOsID but fetchFacility
(from mapDispatchToProps) currently closes over match.params.osID, causing
stale/incorrect fetches; update mapDispatchToProps so fetchFacility accepts an
osID parameter (e.g., fetchFacility = (id, contributors) => dispatch(...use
id...)) instead of reading match.params.osID, then call
fetchFacility(normalizedOsID, contributors) inside the useEffect (and update any
other callers to pass an explicit id) so the dispatched fetch always uses the
resolved normalizedOsID rather than the closed-over match.params.osID.
🧹 Nitpick comments (1)
src/react/src/__tests__/utils.tests.js (1)

2866-2903: Good test coverage for the new utilities.

Tests correctly validate the feature flag lookup, embed parameter filtering, and redirect link construction. One optional gap: there's no test case for makeFacilityDetailLinkOnRedirect when useProductionLocationPage is true but the search string lacks embed (i.e., verifying it produces /production-locations/OS123 without a trailing ?). This would confirm the interaction between the two helpers end-to-end.

vlad-shapik
vlad-shapik previously approved these changes Feb 10, 2026
Copy link
Contributor

@vlad-shapik vlad-shapik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@protsack-stephan protsack-stephan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Couple of comments from me.

@sonarqubecloud
Copy link

Copy link
Collaborator

@protsack-stephan protsack-stephan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants