Skip to content

feat(designer) - Show operation default setting values and host settings in the designer#9253

Open
JoaquimMalcampo wants to merge 13 commits into
mainfrom
jmalcampo/nodeSettingDetailsEnhancements
Open

feat(designer) - Show operation default setting values and host settings in the designer#9253
JoaquimMalcampo wants to merge 13 commits into
mainfrom
jmalcampo/nodeSettingDetailsEnhancements

Conversation

@JoaquimMalcampo

@JoaquimMalcampo JoaquimMalcampo commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Adds UI visibility for operation setting defaults returned by the settingDefaults API. Previously, settings like retry policy showed "Default" with no detail fields when no explicit value was set in the workflow definition. Now the designer fetches defaults from the host and displays actual values (e.g., "Exponential interval", count, intervals) while preserving the existing serialization behavior — no changes are written to the workflow JSON.

Additionally, unknown host-level settings returned by the API (e.g., maxMessageBatchSize) are now surfaced in a new read-only "Host settings" section in the settings panel.

Changes

  • settingDefaults.ts - React Query cached fetch for the settingDefaults API endpoint. Applies merge logic for returned setting values and routes additional settings as hostSettings.
  • added defaultHint to SettingData and hostSettings map to Settings interface
  • serializer.ts - if defaultHint is present, does not serialize on retry
  • hostsettings.tsx - dynamic read only section that renders host settings as text fields and is included with the rest of the settings sections in index.tsx
  • operationmanifest.ts (standard/consumption/base) - add getSettingDefaults to operation manifest service interface and implementations.

Impact of Change

  • Users:
    Settings panel now shows actual default values for retry policy and other settings instead of an opaque "Default" label. Host-level read-only settings (e.g., maxMessageBatchSize) are visible in a new collapsible section. No changes to saved workflow definitions.
  • Developers:
    New defaultHint field on SettingData — indicates the value came from API defaults and should not be serialized. hostSettings on Settings is a dynamic Record<string, SettingData> for arbitrary host-returned keys. KNOWN_SETTING_KEYS set in settingDefaults.ts controls which API keys are treated as known vs host-level.
  • System:
    One additional HTTP call per operation during deserialization (cached by React Query). No changes to workflow JSON serialization output.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: VS Code Extension Development Host with Standard Logic App (Service Bus completeQueueMessageV2 operation)

Contributors

Screenshots/Videos

setting_enhancements_part2

Copilot AI review requested due to automatic review settings June 5, 2026 17:09
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: feat(designer) - Show operation default setting values and host settings in the designer
  • Issue: None blocking. The title is descriptive and clearly indicates the designer-facing scope.
  • Recommendation: No change required.

Commit Type

  • Properly selected (feature).
  • Only one commit type is checked, which matches the template requirement.

⚠️ Risk Level

  • The selected risk level is Medium, and that is reasonable for the diff size and the user impact.
  • Advised risk level: Medium (matches the submitter’s estimate).

What & Why

  • Current: Clear and specific description of the new default-setting visibility behavior and host settings exposure.
  • Issue: None blocking.
  • Recommendation: Optional only: consider shortening the paragraph slightly for readability, but the content is good as-is.

Impact of Change

  • The section is sufficiently detailed and maps well to the actual code changes.
  • Recommendation:
    • Users: Good as written.
    • Developers: Good as written.
    • System: Good as written.

Test Plan

  • The PR includes unit tests in the diff (retryPolicy.spec.ts and settingDefaults.spec.ts), which satisfies the test-plan requirement.
  • Manual testing is also documented.
  • No additional E2E test is required for this review.

Contributors

  • Blank, but this is allowed by the template.
  • Recommendation: Optional — add contributors if PM/design/engineering collaborators contributed, but this is not required.

Screenshots/Videos

  • Included, and appropriate given the UI changes.
  • Recommendation: No change required.

Summary Table

Section Status Recommendation
Title No change required.
Commit Type Only one type selected (feature).
Risk Level Medium is appropriate for this change.
What & Why Clear and sufficiently detailed.
Impact of Change Well aligned with the implementation.
Test Plan Unit tests are present in the diff.
Contributors Blank is acceptable.
Screenshots/Videos Screenshot provided and relevant.

Overall, this PR passes review for title/body compliance. The advised risk level matches the submitter’s Medium selection.


Last updated: Wed, 17 Jun 2026 19:39:04 GMT

Copilot AI left a comment

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.

Pull request overview

This PR enhances the Designer settings panel to display concrete default operation setting values returned by the backend settingDefaults API (instead of an opaque “Default”), and surfaces unknown host-level settings in a new read-only “Host settings” section—while keeping workflow JSON serialization behavior unchanged.

Changes:

  • Add a React Query–cached settingDefaults fetch + merge path during operation initialization/deserialization.
  • Extend settings types (SettingData, Settings, SettingSectionName) to support read-only/default-hint semantics and host-level settings display.
  • Add a new Settings panel section to render host-level settings as read-only text fields.

Reviewed changes

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

Show a summary per file
File Description
Localize/lang/strings.json Adds localization entry for the new “Host settings” section title.
libs/logic-apps-shared/src/designer-client-services/lib/operationmanifest.ts Extends the operation manifest service interface with optional getSettingDefaults.
libs/logic-apps-shared/src/designer-client-services/lib/standard/operationmanifest.ts Implements getSettingDefaults for Standard (including hybrid proxy path).
libs/logic-apps-shared/src/designer-client-services/lib/consumption/operationmanifest.ts Implements getSettingDefaults for Consumption using the existing baseUrl/connectorId URL pattern.
libs/designer/src/lib/ui/settings/sections/hostsettings.tsx New UI section that renders host-level settings as read-only fields.
libs/designer/src/lib/ui/settings/index.tsx Wires the new Host Settings section into the Settings panel and centralizes SettingSectionName.
libs/designer/src/lib/core/state/setting/settingInterface.ts Adds HOSTSETTINGS to the settings section name map.
libs/designer/src/lib/core/queries/settingDefaults.ts Adds supported-settings key extraction, React Query fetch, and merge logic (including hostSettings routing).
libs/designer/src/lib/core/actions/bjsworkflow/settings.ts Extends settings types to support readOnly, defaultHint, and hostSettings.
libs/designer/src/lib/core/actions/bjsworkflow/serializer.ts Avoids serializing retry policy when it was populated from API defaults (via defaultHint).
libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts Fetches and merges setting defaults during manifest-based operation initialization.
libs/designer/src/lib/core/actions/bjsworkflow/add.ts Fetches and merges setting defaults during operation creation/initialization flows.
libs/designer-v2/src/lib/core/actions/bjsworkflow/settings.ts Adds readOnly to v2 SettingData to keep parity for read-only settings support.

Comment thread libs/designer/src/lib/ui/settings/sections/hostsettings.tsx
Comment thread libs/designer/src/lib/core/queries/settingDefaults.ts
Comment thread libs/designer/src/lib/ui/settings/sections/hostsettings.tsx Outdated
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer/src/lib/core/queries/settingDefaults.ts - 14% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/ui/settings/sections/hostsettings.tsx - 17% covered (needs improvement)
⚠️ libs/designer/src/lib/ui/settings/sections/hostsettings.tsx - 17% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts - 17% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/core/actions/bjsworkflow/serializer.ts - 37% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/core/actions/bjsworkflow/settings.ts - 17% covered (needs improvement)
⚠️ libs/designer/src/lib/core/actions/bjsworkflow/add.ts - 5% covered (needs improvement)
⚠️ libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts - 4% covered (needs improvement)
⚠️ libs/designer/src/lib/core/actions/bjsworkflow/serializer.ts - 16% covered (needs improvement)
⚠️ libs/designer/src/lib/core/actions/bjsworkflow/settings.ts - 17% covered (needs improvement)
⚠️ libs/logic-apps-shared/src/designer-client-services/lib/consumption/operationmanifest.ts - 75% covered (needs improvement)
⚠️ libs/logic-apps-shared/src/designer-client-services/lib/standard/operationmanifest.ts - 14% covered (needs improvement)

Please add tests for the uncovered files before merging.

@JoaquimMalcampo JoaquimMalcampo changed the title Jmalcampo/node setting details enhancements Show operation default setting values and host settings in the designer Jun 5, 2026
@JoaquimMalcampo JoaquimMalcampo changed the title Show operation default setting values and host settings in the designer feat(designer): Show operation default setting values and host settings in the designer Jun 15, 2026
@JoaquimMalcampo JoaquimMalcampo changed the title feat(designer): Show operation default setting values and host settings in the designer feat(designer) - Show operation default setting values and host settings in the designer Jun 15, 2026
);

settings = addDefaultSecureSettings(settings, connector?.properties?.isSecureByDefault ?? false);
const swaggerDefaults = await fetchSettingDefaults(

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.

mcpDefaults, manifestDefaults, and swaggerDefaults are fetched the same. Can these be combined earlier in the method?

import { getReactQueryClient } from '../ReactQueryProvider';
import Constants from '../../common/constants';

const KNOWN_SETTING_KEYS = new Set<string>([

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.

how is the settings view usually updated with new settings? Is it possible that this list will fall out of sync if another setting is added elsewhere? If possible, we should be pulling these values from some source of truth rather than hardcoded here

supportedSettings: string[],
workflowKind?: string
): Promise<Record<string, any> | undefined> => {
const service = OperationManifestService();

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.

nit: 'service' is vague here

queryParameters: { 'api-version': apiVersion },
content: { settings: supportedSettings, workflowKind },
});
} catch {

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.

are there any exceptions which should be surfaced here rather than silently failing

if (isReadOnly) {
(merged as Record<string, unknown>)[settingKey] = { ...existing, value: defaultValue, readOnly: true };
} else if (settingKey === 'retryPolicy' && isDefaultRetryPolicy(existing.value)) {
(merged as Record<string, unknown>)[settingKey] = { ...existing, value: defaultValue, defaultHint: defaultValue };

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.

Is retryPolicy the only setting that uses defaultHint? I see it's included on the SettingData interface but only see usages for this setting

@andrew-eldridge

Copy link
Copy Markdown
Contributor

some of the copilot comments seem valid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants