Skip to content

fix(CatalogDesignTable): pass design ID as third argument to handleCopyUrl for backward compatibility#1477

Open
rishiraj38 wants to merge 2 commits intolayer5io:masterfrom
rishiraj38:fix-copy-link-url
Open

fix(CatalogDesignTable): pass design ID as third argument to handleCopyUrl for backward compatibility#1477
rishiraj38 wants to merge 2 commits intolayer5io:masterfrom
rishiraj38:fix-copy-link-url

Conversation

@rishiraj38
Copy link
Copy Markdown
Member

Description

Fixes the "Copy Link" action in the catalog table view generating a URL with catalog-design=undefined instead of the actual design ID.

Root Cause

The handleCopyUrl callback in createDesignColumns was called with two arguments:
handleCopyUrl(rowData.id, rowData.name)

However, the extension consumer's handler expects the design ID in the third parameter position (a legacy signature from when the call included a source type as the first argument). Since only two arguments were passed, the third parameter evaluated to undefined, resulting in:
https://playground.meshery.io/extension/meshmap?catalog-design=undefined

Fix

Passed rowData.id in both the first and third argument positions to maintain backward compatibility with the extension handler:
handleCopyUrl(rowData.id, rowData.name, rowData.id)

The third parameter is made optional (id?: string) in the ColumnConfigProps interface so existing consumers using only two parameters remain unaffected.

Screenshots / Videos

  • Before
Screen.Recording.2026-05-04.at.2.07.59.PM.mov
  • After
Screen.Recording.2026-05-04.at.2.06.17.PM.mov

Signed-off-by: Rishi Raj rishiraj@users.noreply.github.com

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Rishi Raj <rishiraj438gt@gmail.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the handleCopyUrl function signature and its implementation to include an optional third parameter. Feedback was provided regarding the parameter naming in the interface, suggesting a rename to better distinguish between the source and the design ID for improved readability and maintainability.

handleShowDetails: (design: Pattern) => void;
handleClone: (designId: string, name: string) => void;
handleCopyUrl: (designId: string, name: string) => void;
handleCopyUrl: (designId: string, name: string, id?: string) => void;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The parameter naming (designId: string, name: string, id?: string) is confusing as it uses two different names for the same entity to support legacy signatures. Renaming them to reflect their actual roles (e.g., idOrSource for the first and designId for the third) would improve code readability and maintainability.

Suggested change
handleCopyUrl: (designId: string, name: string, id?: string) => void;
handleCopyUrl: (idOrSource: string, name: string, designId?: string) => void;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rishiraj38 can you check this?

handleShowDetails: (design: Pattern) => void;
handleClone: (designId: string, name: string) => void;
handleCopyUrl: (designId: string, name: string) => void;
handleCopyUrl: (designId: string, name: string, id?: string) => void;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rishiraj38 can you check this?

{
title: 'Copy Link',
onClick: () => handleCopyUrl(rowData.id, rowData.name),
onClick: () => handleCopyUrl(rowData.id, rowData.name, rowData.id),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rishiraj38 why two time row.Data.id ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The MeshMap extension is still using an older signature that expects the ID as the 3rd argument (type, name, id). Because Sistent was only passing (id, name), that 3rd argument was ending up as undefined, which caused the bug. Passing the ID in both the 1st and 3rd spots fixes the bug for the extension while keeping it backward compatible with anything else that relies on the 1st argument.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The ideal way to fix this would be updating the MeshMap extension directly to just read the ID from the 1st argument. But since the extension lives in a separate, private repo, doing the fix here in Sistent makes sure it works right away without waiting on another PR. Plus, if the extension ever gets updated to use the 1st argument instead of the 3rd, this change handles both cases, so it won't break again.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @rishiraj38 lgtm!!

@saurabhraghuvanshii saurabhraghuvanshii dismissed their stale review May 4, 2026 09:00

Change requestef

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