Skip to content

fix(iop): separate route for pathway details#1237

Open
Fewwy wants to merge 1 commit into
theforeman:developfrom
Fewwy:separate-route-for-pathway-details
Open

fix(iop): separate route for pathway details#1237
Fewwy wants to merge 1 commit into
theforeman:developfrom
Fewwy:separate-route-for-pathway-details

Conversation

@Fewwy

@Fewwy Fewwy commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Pathway details and recommendation details were rendered by a single component (IopRecommendationDetails) on one catch-all route, using useRouteMatch internally to switch between views.
When inside the federated PathwayDetailsWrapped module navigated to a recommendation details URL, the route still matched the same catch-all, so the host router never re-mounted the component and useRouteMatch didn't re-evaluate — leaving the user stuck on the Pathway details page despite the URL changing.

Splitting into separate routes with a dedicated IopPathwayDetails component ensures the router treats pathway-to-recommendation navigation as a proper route change, unmounting one view and mounting the other.

How to repeat the bug:
Run the dev environment, open pathway details page, click on Link to the Recommendation details in Recommendations table - you will see URL is changed but you are still on the same page and not navigated to the Recommendation details page

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In IopPathwayDetails, you can drop the useRouteMatch hook and read slug from props.match.params.slug, since the router already passes route params in via the render prop.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `IopPathwayDetails`, you can drop the `useRouteMatch` hook and read `slug` from `props.match.params.slug`, since the router already passes route params in via the `render` prop.

## Individual Comments

### Comment 1
<location path="webpack/IopPathwayDetails/IopPathwayDetails.js" line_range="12-16" />
<code_context>
+const scope = 'advisor';
+const pathwayModule = './PathwayDetailsWrapped';
+
+const IopPathwayDetails = props => {
+  const pathwayMatch = useRouteMatch(
+    '/foreman_rh_cloud/recommendations/pathways/:slug'
+  );
+  const slug = pathwayMatch?.params?.slug;
+
+  return (
</code_context>
<issue_to_address>
**suggestion:** Consider using the route props instead of `useRouteMatch` to avoid redundant matching.

Because this component is rendered via `<Route render={props => <IopPathwayDetails {...props} />}`, you can read the slug directly from `props.match.params.slug` (or the router props you pass down) instead of calling `useRouteMatch` with the same pattern. This avoids duplicate matching and keeps the route definition as the single source of truth if the path changes.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +12 to +16
const IopPathwayDetails = props => {
const pathwayMatch = useRouteMatch(
'/foreman_rh_cloud/recommendations/pathways/:slug'
);
const slug = pathwayMatch?.params?.slug;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Consider using the route props instead of useRouteMatch to avoid redundant matching.

Because this component is rendered via <Route render={props => <IopPathwayDetails {...props} />}, you can read the slug directly from props.match.params.slug (or the router props you pass down) instead of calling useRouteMatch with the same pattern. This avoids duplicate matching and keeps the route definition as the single source of truth if the path changes.

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.

1 participant