feat: add multi-mode view for Flink Artifacts/UDFs #2313

shouples · 2025-08-08T23:19:06Z

Summary of Changes

Continuation of #2243, but with reusable MultiModeViewProvider and ViewProviderDelegate classes to switch between "Flink Artifacts" and "Flink UDFs" modes in the single confluent-flink-artifacts view.

flink-multi-view.mov

Any additional details or context that should be provided?

The current action-toggling behavior works fine for two view modes, but we'll need to explore better UI/UX if a view needs three modes. Cycling through them would be clunky, but a quickpick may feel like overkill.
I don't like the FlinkArtifactsUDFs* naming pattern, but can't come up with anything better at the moment without it being too vague

Pull request checklist

Please check if your PR fulfills the following (if applicable):

Tests

Added new
Updated existing
Deleted existing

Other

All new disposables (event listeners, views, channels, etc.) collected as for eventual cleanup?

Does anything in this PR need to be mentioned in the user-facing CHANGELOG or README?
Have you validated this change locally by packaging and installing the extension .vsix file?
```
gulp clicktest
```

shouples · 2025-08-09T01:13:11Z

src/commands/flinkUDFs.ts

@Cerchie what do you think about if we moved the uploadUDF command into here as a more generalized command area for UDFs?

that jives with what we have for Flink statements etc... sure!

shouples · 2025-08-09T01:13:31Z

src/viewProviders/flinkArtifacts.ts

I want to rename this file to something else, but I'm not sure what. Can be a future branch.

maybe flinkUDFs to reflect the final state of the resource? Or flinkArtifactsAndUDFs.... the rest of the files in viewProviders seem to be named after the type of resource you're viewing.

the rest of the files in viewProviders seem to be named after the type of resource you're viewing

That's the tricky part here - we're enabling users to view either Artifacts or UDFs, so naming for only one of them doesn't feel quite right. 🤔

shouples · 2025-08-09T01:14:13Z

src/viewProviders/flinkArtifacts.ts

  }
-}
-
-export function triageGetFlinkArtifactsError(


Moved to src/viewProviders/multiViewDelegates/flinkArtifactsDelegate.ts

shouples · 2025-08-09T01:16:11Z

src/viewProviders/flinkArtifacts.test.ts


-describe("FlinkArtifactsViewProvider", () => {


Most of these tests moved to src/viewProviders/multiViewDelegates/flinkArtifactsDelegate.test.ts

src/viewProviders/baseModels/multiViewBase.ts

sonarqube-confluent · 2025-08-09T01:33:21Z

Analysis Details

6 Issues

0 Bugs
0 Vulnerabilities
6 Code Smells

Coverage and Duplications

84.30% Coverage (69.60% Estimated after merge)
No duplication information (0.80% Estimated after merge)

Project ID: vscode

View in SonarQube

Copilot

Pull Request Overview

This PR implements a multi-mode view provider for Flink artifacts and UDFs, allowing users to switch between viewing Flink artifacts and UDFs within a single tree view. The implementation introduces a reusable architecture for multi-mode view providers that can be applied to other views requiring mode switching.

Key changes:

Creates a new MultiModeViewProvider base class for multi-mode tree views
Implements delegate pattern with separate FlinkArtifactsDelegate and FlinkUDFsDelegate classes
Adds commands and UI elements for switching between artifacts and UDFs modes

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/viewProviders/baseModels/multiViewBase.ts`	New base classes for multi-mode view providers and delegates
`src/viewProviders/flinkArtifacts.ts`	Refactored to use multi-mode architecture with `FlinkArtifactsUDFsViewProvider`
`src/viewProviders/multiViewDelegates/flinkArtifactsDelegate.ts`	Delegate for handling artifacts mode logic and error handling
`src/viewProviders/multiViewDelegates/flinkUDFsDelegate.ts`	Delegate for handling UDFs mode with placeholder implementation
`src/commands/flinkArtifacts.ts`	New command for switching to artifacts mode
`src/commands/flinkUDFs.ts`	New command for switching to UDFs mode
`src/models/flinkUDF.ts`	New model and tree item classes for Flink UDFs
`src/emitters.ts`	Added event emitter for view mode switching
`src/context/values.ts`	Added context value for tracking current view mode
`src/extension.ts`	Updated to register new commands and initialize view mode context
`package.json`	Added command definitions and menu contributions for mode switching

Copilot · 2025-08-18T13:55:53Z

src/models/flinkUDF.ts

+    this.environmentId = props.environmentId;
+    this.id = props.id;
+    this.name = props.name;
+    this.description = props.description;


The constructor assigns props.provider and props.region to the instance properties, but these assignments are missing. The constructor only assigns the first 6 properties but not provider and region.

Suggested change

this.description = props.description;

this.description = props.description;

this.provider = props.provider;

this.region = props.region;

Copilot · 2025-08-18T13:55:54Z

src/viewProviders/multiViewDelegates/flinkArtifactsDelegate.ts

+  let message = "Failed to load Flink artifacts.";
+
+  if (isResponseError(error)) {
+    const status = error.response.status;


[nitpick] The variable status is extracted but could be used inline in the switch statement to reduce unnecessary variable declaration.

Suggested change

const status = error.response.status;

Copilot · 2025-08-18T13:55:55Z

src/viewProviders/multiViewDelegates/flinkArtifactsDelegate.ts

+      .json()
+      .catch((err) => {
+        logger.debug("Failed to parse error response as JSON", err);
+      });


The error response JSON parsing is attempted but the result is not used. Consider removing this unused code or storing the result if it's needed for logging.

Suggested change

});

Cerchie · 2025-08-18T14:13:50Z

package.json

+      {
+        "command": "confluent.flink.setUDFsViewMode",
+        "title": "Switch to Flink UDFs",
+        "icon": "$(code)",


These icons make sense given what we have to work with, but I'm still not feeling that the functionality is obvious. Plus, as you mentioned, there was the idea of cycling through 3 modalities as well. I'll be interested to hear what design has to say

Cerchie · 2025-08-18T14:15:33Z

src/context/values.ts

  kafkaClusterSelected = "confluent.kafkaClusterSelected",
-  /** The user clicked a Schema Registry tree item. */
+  /** The user focused a Schema Registry for the Schemas view. */


this is for key-navigation?

No, these comment changes were just for clarity/accuracy since the event can fire if the "Select Kafka Cluster / Schema Registry" can be triggered from a quickpick as well as clicking on their respective tree items

Cerchie · 2025-08-18T14:16:59Z

src/extension.ts

@@ -407,6 +412,11 @@ async function setupContextValues() {
    SCHEMA_URI_SCHEME,
    MESSAGE_URI_SCHEME,
  ]);
+  // set the initial Flink artifacts view mode to "Artifacts" so the UDF mode toggle is visible


I wonder if there's a way to set it to something like "last resource touched". Like, if the user was looking at a UDF and re-opens the sidebar, they see UDFs. Might be impossible?

Cerchie

jlrobins · 2025-08-20T16:27:07Z

src/viewProviders/baseModels/multiViewBase.ts

+  }
+
+  getChildren(element?: T): T[] {
+    if (!this.currentDelegate) {


When could this.currentDelegate ever be undefined?

src/viewProviders/baseModels/multiViewBase.ts

jlrobins · 2025-08-20T16:32:27Z

src/viewProviders/flinkArtifacts.ts

-{
-  readonly kind = "flinkArtifacts";
-  loggerName = "viewProviders.flinkArtifacts";
+/**


Declare a specific type for FlinkArtifact | FlinkUdf ala:

export type ArtifactOrUdf = FlinkArtifact | FlinkUdf

simplifies a few lines.

jlrobins · 2025-08-20T16:38:40Z

src/viewProviders/baseModels/multiViewBase.ts

+    if (!this.parent.resource) {
+      return [];
+    }
+    return this.parent.filterChildren(element, this.children);


Revise this so that ViewProviderDelegate::getChildren() just returns this.children, period, and then have MultiModeViewProvider::getChildren() responsible for making the call to get the current delegate's children, then itself making its own call to its filter.

Less intertwining that way.

jlrobins · 2025-08-20T16:42:51Z

src/viewProviders/multiViewDelegates/flinkArtifactsDelegate.ts

+    super();
+  }
+
+  async fetchChildren(): Promise<FlinkArtifact[]> {


Perhaps if the base definition of ViewProviderDelegate.fetchChildren() took either the parent as a parameter, or perhaps even more directly whatever the parameterized type for the compute pool is, then would both simplify the constructor calls for the delegates (no longer needing to pass in parent) and also reduce cross-class data member reach-ins.

That is, allows for less tight coupling between the delegator and delegate. The fetchChildren() implementations only need the current computePool, and that can be passed on the stack in the method call, and I think we gots a generic type for it in the mix already.

Bottom line: Revising to get the the delegator/delegatees to not reach into each other's data members at all, as well as to not have circular references is Good Design.

shouples added 7 commits August 8, 2025 19:17

set up multi-view base classes and Flink Artifacts/UDFs delegates

fbf4034

add commands and context values for toggling modes

0b70dc3

add basic UDF model

b5b670f

move multi-view enum and delegates into subdir; update imports

013b312

add basic MultiModeViewProvider and ViewProviderDelegate tests

bf594cc

split up tests between multi-view provider and delegates

b0a3869

fix import

3d7e5dd

shouples mentioned this pull request Aug 9, 2025

prototype: toggle-able view provider [DO NOT MERGE] #2243

Closed

shouples linked an issue Aug 9, 2025 that may be closed by this pull request

Prototype toggling between UDF and Artifacts in a single view #2232

Open

shouples requested a review from jlrobins August 9, 2025 01:08

shouples commented Aug 9, 2025

View reviewed changes

src/viewProviders/baseModels/multiViewBase.ts Show resolved Hide resolved

shouples marked this pull request as ready for review August 18, 2025 13:42

Copilot AI review requested due to automatic review settings August 18, 2025 13:42

shouples requested a review from a team as a code owner August 18, 2025 13:42

Copilot AI reviewed Aug 18, 2025

View reviewed changes

Cerchie reviewed Aug 18, 2025

View reviewed changes

Cerchie approved these changes Aug 18, 2025

View reviewed changes

jlrobins reviewed Aug 20, 2025

View reviewed changes


		describe("FlinkArtifactsViewProvider", () => {

feat: add multi-mode view for Flink Artifacts/UDFs #2313

Are you sure you want to change the base?

feat: add multi-mode view for Flink Artifacts/UDFs #2313

Conversation

shouples commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Any additional details or context that should be provided?

Pull request checklist

Tests

Other

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqube-confluent bot commented Aug 9, 2025

Analysis Details

6 Issues

Coverage and Duplications

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cerchie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shouples commented Aug 8, 2025 •

edited

Loading