fix(breadcrumbs): adjust LogViewer, Build Details and Test Details breadcrumb for new URL structure

[danieldiasbarbo]

Description

This PR fixes the breadcrumb links for the BuildDetails, TestDetails and LogViewer pages to accommodate the updated URL structure.
Previously, breadcrumbs were not resolving correctly due to changes in the route parameters.
With this fix, the breadcrumb now accurately reflects the current path

Changes

• Remanaged the routerState in pages like LogViewer, BuildDetails, and TestDetails
• Updated breadcrumb logic to consistently resolve to the correct TreeDetails url

How to test

1. Navigate to a TreeDetails page via TreeListing or directly via URL
2. From there, open related pages such as LogViewer, BuildDetails and TestDetails
3. Verify that the breadcrumb displays correctly in each case and accurately reflects the full path back to the TreeDetails
4. Ensure no breadcrumb segment is broken or missing

Closes #1314

[giubrocchi]

You can rename the variable directly on the destruct:

const { treeName, branch, id: treeId } = useRouterState({
    select: s => s.location.state,
  });;

[danieldiasbarbo]

Done.

[giubrocchi]

Use treeId instead of id to be consistent

[danieldiasbarbo]

Done

[giubrocchi]

Nit: you can extract this and the params conditional to outside the component props for better readability (and add a memo on the params object)

[danieldiasbarbo]

Done.

[MarceloRobert]

I don't see the change addressing the comment. Also, IMO the BreadcrumbLink could be extracted entirely to a useMemo.

[danieldiasbarbo]

Pinged you on Slack. I have some questions on int.

[giubrocchi]

Ditto about destruct - and you can remove the historyState.id from this file to use id directly

[danieldiasbarbo]

Done.

[giubrocchi]

Ditto

[danieldiasbarbo]

I did not got the intent here. Is to refactor to const { treeName, branch, id } = useRouterState(...)? This historyState is only used here.

[giubrocchi]

Yes, jest to keep the pattern between the files

[MarceloRobert]

Mostly working, but needs improvement

[giubrocchi]

nit: can destruct like the other files:

const { treeName, branch, id: treeId } = useRouterState({
    select: s => s.location.state,
  });

[danieldiasbarbo]

Good catch. Resolved

[giubrocchi]

You don't need to type the treeParams if you add as const to the objects

Suggested change

	const treeParams: {
	to: '/tree/$treeName/$branch/$hash' | '/tree/$treeId';
	params:
	| { treeName?: string; branch?: string; hash?: string }
	| { treeId?: string };
	} = useMemo(
	() =>
	canGoDirect
	? {
	to: '/tree/$treeName/$branch/$hash',
	params: { treeName: treeName, branch: branch, hash: treeId },
	}
	: { to: '/tree/$treeId', params: { treeId: treeId } },
	[canGoDirect, treeName, branch, treeId],
	);
	const treeParams = useMemo(
	() =>
	canGoDirect
	? {
	to: '/tree/$treeName/$branch/$hash',
	params: { treeName: treeName, branch: branch, hash: treeId },
	} as const
	: { to: '/tree/$treeId', params: { treeId: treeId } } as const,
	[canGoDirect, treeName, branch, treeId],
	);

[danieldiasbarbo]

Nice, i didn't knew about this TS feature.

[MarceloRobert]

All working on my tests, LGTM