Skip to content

Conversation

@hornm-knime
Copy link
Contributor

No description provided.

@hornm-knime hornm-knime requested a review from a team as a code owner December 19, 2025 13:32
Copilot AI review requested due to automatic review settings December 19, 2025 13:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enables the "Reveal in space explorer" functionality for linked components after they have been uploaded to a space. The main change involves migrating the getAncestorInfo API call from the desktop namespace to the space namespace, making it available in the browser context when explicitly requested.

Key changes:

  • Added a "Reveal in space explorer" button to the success toast after component upload
  • Migrated getAncestorInfo from DesktopAPI to SpaceService in the backend
  • Updated the frontend to call API.space.getAncestorInfo instead of API.desktop.getAncestorInfo
  • Added a fetchAncestorInfo parameter to control when ancestor information should be fetched from the backend

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
componentInteractions.ts Added "Reveal in space explorer" button to component upload success toast
useRevealInSpaceExplorer.ts Added fetchAncestorInfo parameter and migrated to use API.space.getAncestorInfo
RecentWorkflowContextMenu.test.ts Updated test mocks to reference API.space.getAncestorInfo
AppHeaderContextMenu.test.ts Updated test mocks to reference API.space.getAncestorInfo
generated-api.ts Added AncestorInfo interface and getAncestorInfo method to SpaceService
desktop-api.ts Removed deprecated getAncestorInfo function
custom-types.ts Removed duplicate AncestorInfo type definition
SpaceAPI.java Removed getAncestorInfo method (migrated to SpaceService)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hornm-knime hornm-knime force-pushed the bug/NXT-4251-reveal-in-explorer-after-compon branch from fb6ff8d to 39718b8 Compare December 19, 2025 17:50
Copilot AI review requested due to automatic review settings December 22, 2025 11:59
@hornm-knime hornm-knime force-pushed the bug/NXT-4251-reveal-in-explorer-after-compon branch from 39718b8 to bdc0650 Compare December 22, 2025 11:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

org.knime.ui.js/src/components/spaces/useRevealInSpaceExplorer.ts:1

  • The revealSingleItem function signature appears to have changed to accept a third boolean parameter (true), but the function definition shows it only accepts two parameters (origin and itemName). This mismatch will cause a runtime error. Remove the third argument or update the function signature accordingly.
import { watch } from "vue";

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings January 7, 2026 08:36
@xnhp xnhp force-pushed the bug/NXT-4251-reveal-in-explorer-after-compon branch from 43359ff to 289f3fd Compare January 7, 2026 08:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

org.knime.ui.js/src/components/spaces/useRevealInSpaceExplorer.ts:1

  • Using non-null assertion operator (!) on result.uploadedItem assumes it's always defined. Consider adding a conditional check or guard clause before calling revealSingleItem to handle cases where uploadedItem might be undefined.
import { watch } from "vue";

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings January 7, 2026 08:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 178 to 180
useRevealInSpaceExplorer().revealSingleItem(
result.uploadedItem!
);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Using the non-null assertion operator (!) can lead to runtime errors if result.uploadedItem is actually null or undefined. Consider adding an explicit null check before calling revealSingleItem to ensure the callback only executes when uploadedItem exists.

Suggested change
useRevealInSpaceExplorer().revealSingleItem(
result.uploadedItem!
);
const uploadedItem = result.uploadedItem;
if (!uploadedItem) {
return;
}
useRevealInSpaceExplorer().revealSingleItem(uploadedItem);

Copilot uses AI. Check for mistakes.
@xnhp
Copy link
Contributor

xnhp commented Jan 7, 2026

@hriverahdez this branch appears to have test failures related to WorkflowBreadcrumb.vue, could you diagnose that?

 FAIL  src/components/toolbar/__tests__/WorkflowBreadcrumb.test.ts > WorkflowBreadcrumb.vue > renders dropdown items in BROWSER
AssertionError: expected [ …(2) ] to deeply equal [ ObjectContaining{…} ]

- Expected
+ Received

  [
    ObjectContaining {
      "text": "Version history",
    },
+   {
+     "icon": {
+       "render": [Function render],
+     },
+     "metadata": {
+       "handler": [Function handler],
+     },
+     "text": "Reveal in space explorer",
+   },
  ]

 ❯ src/components/toolbar/__tests__/WorkflowBreadcrumb.test.ts:136:23
    134|     const menuItems = wrapper.findComponent(SubMenu).props("items");
    135|
    136|     expect(menuItems).toEqual([
       |                       ^
    137|       expect.objectContaining({ text: "Version history" }),
    138|     ]);

 FAIL  src/components/toolbar/__tests__/WorkflowBreadcrumb.test.ts > WorkflowBreadcrumb.vue > doesn't render dropdown in JobViewer mode
AssertionError: expected true to be false // Object.is equality

- Expected
+ Received

- false
+ true

 ❯ src/components/toolbar/__tests__/WorkflowBreadcrumb.test.ts:146:53
    144|     const { wrapper } = doMount({ appMode: AppState.AppModeEnum.JobViewer });
    145|
    146|     expect(wrapper.findComponent(SubMenu).exists()).toBe(false);
       |                                                     ^
    147|   });
    148|

 FAIL  src/components/toolbar/__tests__/WorkflowBreadcrumb.test.ts > WorkflowBreadcrumb.vue > doesn't render dropdown in Playground mode
AssertionError: expected true to be false // Object.is equality

- Expected
+ Received

- false
+ true

 ❯ src/components/toolbar/__tests__/WorkflowBreadcrumb.test.ts:154:53
    152|     const { wrapper } = doMount({ appMode: AppState.AppModeEnum.Playground });
    153|
    154|     expect(wrapper.findComponent(SubMenu).exists()).toBe(false);
       |                                                     ^
    155|   });
    156|

@xnhp xnhp self-requested a review January 7, 2026 08:56
xnhp
xnhp previously requested changes Jan 7, 2026
Copy link
Contributor

@xnhp xnhp left a comment

Choose a reason for hiding this comment

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

failing tests in WorkflowBreadcrumb.ts

@hriverahdez
Copy link
Contributor

The test caught a very interesting case, actually. Which is the presence of this option for jobviewer/playground mode. Because this option used to be completely excluded for browser, but now it's not anymore.

I'll push a commit which adds a check (inside the canReveal function) for the uiControl of whether access to SpaceExplorer is available

Copilot AI review requested due to automatic review settings January 7, 2026 14:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@hornm-knime hornm-knime force-pushed the bug/NXT-4251-reveal-in-explorer-after-compon branch from 92a73fe to 1580140 Compare January 8, 2026 09:56
hornm-knime and others added 2 commits January 8, 2026 10:57
…xplorer

... after component sharing.
* 'getAncestorInfo' endpoint moved from desktop to gateway API
* fetch ancestor-item-ids on demand - but only in case of component
  sharing and not for other 'reveal'-actions

NXT-4251 ('Reveal in explorer' after component sharing in browser does not work)
- depending on uiControl that determines whether the space explorer
  is accessible

NXT-4251 ('Reveal in explorer' after component sharing in browser does not work)
@hornm-knime hornm-knime force-pushed the bug/NXT-4251-reveal-in-explorer-after-compon branch from 1580140 to 3e0b921 Compare January 8, 2026 09:58
@hornm-knime hornm-knime requested a review from xnhp January 8, 2026 10:21
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
64.7% Coverage on New Code (required ≥ 85%)

See analysis details on SonarQube Cloud

@hornm-knime hornm-knime dismissed xnhp’s stale review January 8, 2026 10:53

test has been fixed

@hornm-knime hornm-knime merged commit 3e0b921 into master Jan 8, 2026
5 of 6 checks passed
@hornm-knime hornm-knime deleted the bug/NXT-4251-reveal-in-explorer-after-compon branch January 8, 2026 10:54
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.

4 participants