[OSDEV-2353] Connect fetchFacility action wrapper to the ProductionLocationDetailsContent component#888
Merged
VadimKovalenkoSNF merged 7 commits intomainfrom Feb 25, 2026
Conversation
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 40.01%Your code coverage diff: -0.02% ▾ ✅ All code changes are covered |
VadimKovalenkoSNF
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 |
VadimKovalenkoSNF
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds URL ID normalization, triggers facility fetch on mount/update and clears on unmount, shows a loading spinner while fetching, introduces PropTypes/defaultProps, and simplifies mapDispatchToProps to expose Changes
Sequence Diagram(s)sequenceDiagram
participant Component as ProductionLocationDetailsContent
participant Router as Router (location/match)
participant Dispatcher as mapDispatchToProps
participant Action as fetchSingleFacility (thunk)
participant Store as Redux Store / API
Router->>Component: provide pathname / params
Component->>Component: compute normalizedOsID
Component->>Dispatcher: fetchFacility(normalizedOsID, contributors)
Dispatcher->>Action: dispatch fetchSingleFacility(id, contributorId)
Action->>Store: request facility data (API)
Store-->>Action: facility data response
Action-->>Dispatcher: dispatch success/failure
Dispatcher-->>Component: props updated (data/fetching/error)
Component->>Dispatcher: on unmount -> clearFacility()
Dispatcher->>Store: dispatch clear facility action
Store-->>Component: facility cleared
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx (1)
132-149: RenamefetchFacilityparameter for clarity.
contributorIdactually holds the contributors array; renaming reduces future call-site confusion.♻️ Suggested rename
- fetchFacility: (id, contributorId) => { - const contributorValue = get(contributorId, ['0', 'value']); - const contributors = contributorValue ? contributorId : null; + fetchFacility: (id, contributors) => { + const contributorValue = get(contributors, ['0', 'value']); + const normalizedContributors = contributorValue ? contributors : null; - return dispatch(fetchSingleFacility(id, 0, contributors, true)); + return dispatch(fetchSingleFacility(id, 0, normalizedContributors, true)); },🤖 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 132 - 149, Rename the fetchFacility parameter from contributorId to contributors (or contributorsArray) inside mapDispatchToProps to reflect that it holds the contributors array; update the internal references (change get(contributorId, ['0','value']) and the contributors assignment) to use the new parameter name and ensure the dispatch call still passes contributors (not contributorId). Also update any local usages within this file that reference fetchFacility's parameter name so call-sites remain correct.
🤖 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/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 52-57: The useEffect is calling fetchFacility(normalizedOsID, 0,
contributors) but the fetchFacility wrapper expects (id, contributors) so the
contributors argument gets dropped; update the call in
ProductionLocationDetailsContent (the useEffect that currently calls
fetchFacility) to pass only the id and contributors—i.e., call
fetchFacility(normalizedOsID, contributors)—or alternatively adjust the wrapper
signature to accept (id, page, contributors) consistently, ensuring the
contributors value reaches the fetchFacility implementation.
---
Nitpick comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 132-149: Rename the fetchFacility parameter from contributorId to
contributors (or contributorsArray) inside mapDispatchToProps to reflect that it
holds the contributors array; update the internal references (change
get(contributorId, ['0','value']) and the contributors assignment) to use the
new parameter name and ensure the dispatch call still passes contributors (not
contributorId). Also update any local usages within this file that reference
fetchFacility's parameter name so call-sites remain correct.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
...nts/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
Outdated
Show resolved
Hide resolved
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Django App | Unittest test suite - Code coverage reportTotal: 81.9%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 52-55: The useEffect calling fetchFacility(normalizedOsID,
contributors) is missing contributors (and fetchFacility) in its dependency
array, causing stale facility data when contributors change; update the effect
on ProductionLocationDetailsContent by including contributors and fetchFacility
in the dependency array (e.g., [normalizedOsID, contributors, fetchFacility])
and remove the eslint-disable comment so React re-runs fetchFacility when those
values change.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
...nts/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
Outdated
Show resolved
Hide resolved
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx (1)
52-54:⚠️ Potential issue | 🟡 MinorGuard against calling
fetchFacilitywith a falsy ID.If
location.pathnameis empty andosIDis undefined,normalizedOsIDresolves to an empty or falsy value, triggering a fetch with an invalid ID.🛠️ Proposed fix
useEffect(() => { + if (!normalizedOsID) return; fetchFacility(normalizedOsID, contributors); }, [normalizedOsID, contributors, fetchFacility]);🤖 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 52 - 54, The useEffect currently calls fetchFacility(normalizedOsID, contributors) even when normalizedOsID is falsy; update the effect in ProductionLocationDetailsContent (the useEffect that references normalizedOsID, contributors, fetchFacility) to first check that normalizedOsID is a truthy/non-empty value before invoking fetchFacility, so the fetch is skipped when location.pathname/osID is undefined or empty; keep the same dependency array ([normalizedOsID, contributors, fetchFacility]) and only call fetchFacility when the guard passes.
🧹 Nitpick comments (2)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx (2)
156-166: Misleading parameter namecontributorIdand unnecessary intermediate variable.
contributorIdactually receives thecontributorsarray from Redux state (an array of objects with.value). Naming it as a singular ID is confusing. The intermediatecontributorValuevariable can also be inlined.♻️ Proposed cleanup
- fetchFacility: (id, contributorId) => { - const contributorValue = get(contributorId, ['0', 'value']); - const contributors = contributorValue ? contributorId : null; - - return dispatch(fetchSingleFacility(id, 0, contributors, true)); + fetchFacility: (id, contributors) => { + const resolvedContributors = get(contributors, ['0', 'value']) + ? contributors + : null; + + return dispatch(fetchSingleFacility(id, 0, resolvedContributors, true)); },🤖 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 156 - 166, Rename the misleading parameter contributorId to contributors in mapDispatchToProps and inline the intermediate variable: inside fetchFacility use get(contributors, ['0','value']) directly to decide whether to pass the original contributors array or null into dispatch(fetchSingleFacility(...)); keep function names as-is (mapDispatchToProps, fetchFacility, fetchSingleFacility, clearFacility/resetSingleFacility) so callers remain unchanged.
47-50: Remove redundant fallback innormalizedOsIDchain.
osIDcomes frommatch.params.osIDand is already a bare path segment without slashes. CallinggetLastPathParameteron a slash-free string returns it unchanged (confirmed by test case), making the second and third conditions in the||chain equivalent. Simplify to:Suggested change
- const normalizedOsID = - getLastPathParameter(location?.pathname || '') || - getLastPathParameter(osID) || - osID; + const normalizedOsID = + getLastPathParameter(location?.pathname || '') || osID;🤖 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 47 - 50, The fallback chain for normalizedOsID is redundant: remove the middle getLastPathParameter(osID) call and compute normalizedOsID by taking getLastPathParameter(location?.pathname || '') first and falling back to osID (which is already a raw segment from match.params.osID); update the expression that sets normalizedOsID in ProductionLocationDetailsContent.jsx to use getLastPathParameter(location?.pathname || '') || osID so the logic remains correct but avoids the duplicate transformation.
🤖 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/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Line 56: The useEffect in ProductionLocationDetailsContent currently uses a
cleanup arrow without listing clearFacility in the dependency array; update the
effect to include clearFacility (e.g., useEffect(() => () => clearFacility(),
[clearFacility])) or, if you intentionally only want unmount behavior and
clearFacility is a stable Redux dispatch wrapper, add an inline eslint disable
comment (// eslint-disable-next-line react-hooks/exhaustive-deps) immediately
above that useEffect to suppress the lint warning; target the useEffect in the
ProductionLocationDetailsContent component and reference the clearFacility
symbol when making the change.
---
Duplicate comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 52-54: The useEffect currently calls fetchFacility(normalizedOsID,
contributors) even when normalizedOsID is falsy; update the effect in
ProductionLocationDetailsContent (the useEffect that references normalizedOsID,
contributors, fetchFacility) to first check that normalizedOsID is a
truthy/non-empty value before invoking fetchFacility, so the fetch is skipped
when location.pathname/osID is undefined or empty; keep the same dependency
array ([normalizedOsID, contributors, fetchFacility]) and only call
fetchFacility when the guard passes.
---
Nitpick comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 156-166: Rename the misleading parameter contributorId to
contributors in mapDispatchToProps and inline the intermediate variable: inside
fetchFacility use get(contributors, ['0','value']) directly to decide whether to
pass the original contributors array or null into
dispatch(fetchSingleFacility(...)); keep function names as-is
(mapDispatchToProps, fetchFacility, fetchSingleFacility,
clearFacility/resetSingleFacility) so callers remain unchanged.
- Around line 47-50: The fallback chain for normalizedOsID is redundant: remove
the middle getLastPathParameter(osID) call and compute normalizedOsID by taking
getLastPathParameter(location?.pathname || '') first and falling back to osID
(which is already a raw segment from match.params.osID); update the expression
that sets normalizedOsID in ProductionLocationDetailsContent.jsx to use
getLastPathParameter(location?.pathname || '') || osID so the logic remains
correct but avoids the duplicate transformation.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
...nts/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx (2)
56-56:⚠️ Potential issue | 🟡 MinorInclude
clearFacilityin cleanup effect deps (or explicitly suppress lint).Line 56 still captures
clearFacilitywith an empty deps array. This is the same dependency-array concern previously raised.#!/bin/bash # Verify hook lint configuration and the exact cleanup effect usage. rg -n "react-hooks/exhaustive-deps|react-hooks/rules-of-hooks" src/react/.eslintrc rg -n "useEffect\\(\\(\\) => \\(\\) => clearFacility\\(\\), \\[\\]\\)" src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx🤖 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` at line 56, The useEffect cleanup currently references clearFacility but has an empty dependency array; update the effect in ProductionLocationDetailsContent to either include clearFacility in the deps (e.g., useEffect(() => () => clearFacility(), [clearFacility])) or, if clearFacility is stable and you intentionally want an empty deps array, add an explicit lint suppression comment for the react-hooks/exhaustive-deps rule above the effect; locate the useEffect inside the ProductionLocationDetailsContent component and modify it accordingly.
52-54:⚠️ Potential issue | 🟡 MinorGuard the fetch call when ID normalization returns empty.
If
normalizedOsIDis empty/undefined, this still dispatchesfetchSingleFacilitywith an invalid ID.🔧 Proposed fix
useEffect(() => { + if (!normalizedOsID) return; fetchFacility(normalizedOsID, contributors); }, [normalizedOsID, contributors]);🤖 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 52 - 54, The effect currently calls fetchFacility(normalizedOsID, contributors) even when normalizedOsID is empty; update the useEffect in ProductionLocationDetailsContent to first check that normalizedOsID is truthy (non-empty) before dispatching fetchFacility (i.e., only call fetchFacility(normalizedOsID, contributors) when normalizedOsID exists), leaving the dependency array as [normalizedOsID, contributors] and ensuring no fetchSingleFacility is dispatched with an invalid ID.
🤖 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/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 158-160: In fetchFacility, the logic uses contributorValue =
get(contributorId, ['0','value']) and then contributorValue ? contributorId :
null which incorrectly treats valid falsy values (e.g., 0) as missing; change
the presence check to test the contributorId list itself (e.g.,
Array.isArray(contributorId) && contributorId.length > 0 or an equivalent check)
and use that result to set contributors, leaving contributorValue extraction via
get untouched.
---
Duplicate comments:
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Line 56: The useEffect cleanup currently references clearFacility but has an
empty dependency array; update the effect in ProductionLocationDetailsContent to
either include clearFacility in the deps (e.g., useEffect(() => () =>
clearFacility(), [clearFacility])) or, if clearFacility is stable and you
intentionally want an empty deps array, add an explicit lint suppression comment
for the react-hooks/exhaustive-deps rule above the effect; locate the useEffect
inside the ProductionLocationDetailsContent component and modify it accordingly.
- Around line 52-54: The effect currently calls fetchFacility(normalizedOsID,
contributors) even when normalizedOsID is empty; update the useEffect in
ProductionLocationDetailsContent to first check that normalizedOsID is truthy
(non-empty) before dispatching fetchFacility (i.e., only call
fetchFacility(normalizedOsID, contributors) when normalizedOsID exists), leaving
the dependency array as [normalizedOsID, contributors] and ensuring no
fetchSingleFacility is dispatched with an invalid ID.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
...nts/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
Outdated
Show resolved
Hide resolved
|
This was referenced Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up PR for OSDEV-2353
Fetch single facility data in the production location page (
ProductionLocationDetailsContent.jsx)