Skip to content

UIEXT-3490 WebUI Migration Model to Binary Object#45

Open
tcrundall-tng wants to merge 2 commits intomasterfrom
enh/UIEXT-3490-webui-migration-model-to-binary-object
Open

UIEXT-3490 WebUI Migration Model to Binary Object#45
tcrundall-tng wants to merge 2 commits intomasterfrom
enh/UIEXT-3490-webui-migration-model-to-binary-object

Conversation

@tcrundall-tng
Copy link

No description provided.

…oicesProvider

AP-23709 (Support username and password fields instead of flow variables in OAuth2 Authenticator nodes)
@tcrundall-tng tcrundall-tng requested a review from a team as a code owner March 25, 2026 12:16
@tcrundall-tng tcrundall-tng requested review from Copilot and knime-ghub-bot and removed request for a team March 25, 2026 12:16
@tcrundall-tng
Copy link
Author

⚠️ Should only be merged once #43 is merged ⚠️

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

Migrates legacy node/dialog settings toward the WebUI/default-dialog infrastructure, including moving the “Model to Binary Object” node to NodeParameters and updating Box Authenticator app credentials handling to a structured credentials object.

Changes:

  • Migrated “Model to Binary Object” dialog from legacy DefaultNodeSettingsPane to WebUI NodeParameters + DefaultNodeDialog, and replaced the XML node description with a programmatic NodeDescription.
  • Updated Box Authenticator’s app configuration from a credentials flow-variable string to a LegacyCredentials object with migration + validation wiring.
  • Bumped bundle/test versions and updated snapshot tests to match the new WebUI schemas.

Reviewed changes

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

Show a summary per file
File Description
org.knime.ext.box.authenticator/src-deprecated/org/knime/ext/box/authenticator/node/BoxAuthenticatorSettings.java Adds @ValueReference needed for Box app settings validation.
org.knime.ext.box.authenticator/src-deprecated/org/knime/ext/box/authenticator/node/BoxAppSettings.java Replaces flow-variable string with LegacyCredentials + migration/validation hooks.
org.knime.ext.box.authenticator/META-INF/MANIFEST.MF Version bump to 5.12.0.
org.knime.ext.box.authenticator.tests/pom.xml Test artifact revision bump to 5.12.0.
org.knime.ext.box.authenticator.tests/META-INF/MANIFEST.MF Version bump + Fragment-Host range adjusted.
org.knime.ext.box.authenticator.tests/files/test_snapshots/...BoxAuthenticatorSettingsTest.snap Snapshot updated for new credential object schema and update triggers.
org.knime.base.filehandling/src/.../ModelToBinaryObjectNodeParameters.java Introduces WebUI parameters for the node.
org.knime.base.filehandling/src/.../ModelToBinaryObjectNodeFactory.java Implements WebUI dialog/description/KAI interfaces and constructs a NodeDescription programmatically.
org.knime.base.filehandling/src/.../ModelToBinaryObjectNodeFactory.xml Removes legacy XML node description.
org.knime.base.filehandling.tests/src/.../ModelToBinaryObjectNodeParametersTest.java Adds snapshot test for the new parameters.
org.knime.base.filehandling.tests/files/test_snapshots/...ModelToBinaryObjectNodeParametersTest*.snap New snapshots for JSON forms + settings structure.
org.knime.base.filehandling.tests/files/node_settings/ModelToBinaryObjectNodeParameters.xml Adds test settings fixture for snapshot loading.
Comments suppressed due to low confidence (1)

org.knime.base.filehandling/src/org/knime/base/filehandling/binaryobjects/model/to/port/ModelToBinaryObjectNodeParameters.java:65

  • m_columnName now defaults to an empty string, but the legacy SettingsFactory.createColumnNameSettings() default is "model" and ModelToBinaryObjectNodeModel.checkSettings() rejects an empty column name. This changes the out-of-the-box behavior and can make a newly added node invalid until manually edited. Align the default with the legacy settings (e.g., "model") and/or add a required/non-empty validation consistent with the model's constraint.

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

@tcrundall-tng tcrundall-tng force-pushed the enh/UIEXT-3490-webui-migration-model-to-binary-object branch from c99fbbc to d5102a2 Compare March 25, 2026 13:51
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
7.6% Duplication on New Code (required ≤ 1%)

See analysis details on SonarQube Cloud

@reissinj reissinj self-requested a review March 25, 2026 15:07
import org.knime.node.impl.description.PortDescription;
import java.util.List;
import static org.knime.node.impl.description.PortDescription.fixedPort;

Copy link
Contributor

Choose a reason for hiding this comment

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

Small thing: put the imports in the right order

Copy link
Author

Choose a reason for hiding this comment

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

done

UIEXT-3490 (WebUI-Migration Model to Binary Object)
@tcrundall-tng tcrundall-tng force-pushed the enh/UIEXT-3490-webui-migration-model-to-binary-object branch from d5102a2 to 08f5a5e Compare March 25, 2026 16:03
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