Skip to content

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

Merged
jeremylenz merged 1 commit into
theforeman:developfrom
Fewwy:separate-route-for-pathway-details
Aug 6, 2026
Merged

fix(iop): separate route for pathway details#1237
jeremylenz merged 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 thread webpack/IopPathwayDetails/IopPathwayDetails.js

@adonispuente adonispuente 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.

works as intended, lgtm!

@jeremylenz jeremylenz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, please fix lint and rebase :)

@Fewwy
Fewwy force-pushed the separate-route-for-pathway-details branch from 6dfc0eb to 5875da3 Compare August 6, 2026 12:46
@jeremylenz
jeremylenz merged commit 57a8ca5 into theforeman:develop Aug 6, 2026
18 checks passed
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