Skip to content

fix(DesignerV2): Fixed nested workflow clickthrough button#9018

Merged
rllyy97 merged 7 commits intomainfrom
riley/child-run-std-fix
Apr 7, 2026
Merged

fix(DesignerV2): Fixed nested workflow clickthrough button#9018
rllyy97 merged 7 commits intomainfrom
riley/child-run-std-fix

Conversation

@rllyy97
Copy link
Copy Markdown
Contributor

@rllyy97 rllyy97 commented Apr 6, 2026

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

Fixed visibility for the nested workflow clickthrough button
Fixed some small styling issues related to the action buttons
Added workflow clickthrough to the node context menu
Fixes #6974

Impact of Change

  • Users: Users will now be able to access nested workflow runs in standard
  • Developers: N/A
  • System: N/A

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Contributors

@rllyy97

Screenshots/Videos

image image

@rllyy97 rllyy97 added the risk:low Low risk change with minimal impact label Apr 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 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: fix(DesignerV2): Fixed nested workflow clickthrough button
  • Issue: Title is acceptable and follows conventional commit scope/type, but uses past tense (“Fixed”) and is a bit narrow given the changes (adds context-menu item, localization, new hook, tests and snapshot updates). A more precise, imperative title would be clearer.
  • Recommendation: Use imperative tense and broaden to reflect additional work. Example: fix(DesignerV2): add nested workflow clickthrough (panel + context menu) and UI fixes

Commit Type

  • Properly selected (fix).
  • Only one commit type selected which is correct.

⚠️ Risk Level

  • Assessment: PR body selects Low and repository labels include risk:low. Based on the code diff this touches multiple areas across designer-ui and designer-v2 (panel header UI, context menu, monitoring tab, new React Query hook, calls to RunService.getActionLinks, HostService.openMonitorView, and added tests/snapshots). This is cross-cutting and introduces new data flows and a shared cache key — I recommend Medium risk.
  • Recommendation: If you agree with this assessment, please update the risk label to risk:medium and update the PR Risk Level selection in the body to Medium. If you believe the changes are still Low risk, add a short justification to the PR body explaining why this change is limited in scope (for example: "all new code is behind feature gates / checks and no behavioral changes for existing flows unless monitored data is present").

What & Why

  • Current: "Fixed visibility for the nested workflow clickthrough button\nFixed some small styling issues related to the action buttons\nAdded workflow clickthrough to the node context menu\nFixes Request for a feature implementation in Standard Logic App's Workflow Operations action #6974"
  • Issue: None critical — this field is present and concise.
  • Recommendation: You could add a short sentence calling out the new shared React Query cache (useRawInputsOutputs) and the HostService dependency so reviewers know there are monitoring/host integration implications.

⚠️ Impact of Change

  • Issue: The PR lists Users: describes behavior change; Developers and System are marked N/A. Given the diff, there is developer-visible impact (new API/hook, new helper functions, and a shared query key) and system considerations (additional API calls when fetching raw inputs/outputs). These should be documented.
  • Recommendation: Update the Impact section with concrete notes:
    • Users: Users can open nested workflow runs via the panel header button and context menu (already described).
    • Developers: New helper functions and hook added (childWorkflowHelpers.ts, useRawInputsOutputs.ts) + changes to PanelHeader and context menu code. There is a new shared React Query cache key ['actionInputsOutputs', { nodeId, ... }] — other code should be aware when reading/fetching that key.
    • System: The Monitoring/RunService is now used by multiple components to fetch raw inputs/outputs (RunService.getActionLinks). This may increase monitoring API calls; ensure caching behavior and rate considerations are acceptable.

Test Plan

  • Assessment: Unit tests were added/updated (new childWorkflowHelpers.spec.ts and snapshot updates) and snapshots were updated to reflect UI changes. Manual testing is marked completed.
  • Note: E2E tests are not included — that's OK if manual testing and unit coverage are sufficient, but consider adding an integration or E2E test that covers the HostService.openMonitorView path if feasible (or document why not).

Contributors

  • Contributor is listed (@rllyy97). Good credit is present.

Screenshots/Videos

  • Screenshots are included and helpful for a UI change.

Summary Table

Section Status Recommendation
Title ⚠️ Use imperative tense; broaden to reflect context menu + hook/snapshots changes
Commit Type No change needed
Risk Level ⚠️ Recommend raising to Medium or justify why Low is still accurate
What & Why Consider mentioning the new shared React Query cache and HostService usage
Impact of Change ⚠️ Fill Developer/System impact items (new hooks/API calls/cache)
Test Plan Unit tests present; consider E2E/integration for host openMonitorView path
Contributors No change needed
Screenshots/Videos No change needed

Final Message
This PR mostly follows the template and includes unit tests and screenshots, so it passes the PR title/body checklist. My advised risk level is Medium, higher than the author’s Low selection, because the change touches multiple UI surfaces, adds a shared data-fetch hook and cache key, and introduces additional calls to RunService + HostService integration points. Please either update the PR risk selection/label to risk:medium or add a short justification in the PR body explaining why this is still Low risk.

Recommended next steps (clear, actionable):

  • Update the PR title to imperative tense and optionally expand scope to reflect context menu and raw inputs/outputs hook (example: fix(DesignerV2): add nested workflow clickthrough (panel + context menu) and UI fixes).
  • Update the Risk Level checkbox and repository label to Medium — or add a justification in the PR body if you want to keep Low.
  • Expand "Impact of Change" to include:
    • Developers: mention new files (childWorkflowHelpers.ts, useRawInputsOutputs.ts) and the new shared React Query cache key ['actionInputsOutputs', { nodeId, actionTrackingId, startTime, endTime }].
    • System: note the additional RunService.getActionLinks calls and potential monitoring API load; mention caching/placeholderData behavior.
  • If possible, add (or explain omission of) an integration/E2E test for the HostService.openMonitorView flow or document why it's covered manually.

Once you make the above small updates (title and risk/impact details), re-run CI and we can proceed to final review. Thanks for the thorough PR — the added tests and localization strings are great to see!


Last updated: Tue, 07 Apr 2026 16:00:00 GMT

@rllyy97 rllyy97 marked this pull request as ready for review April 6, 2026 21:54
Copilot AI review requested due to automatic review settings April 6, 2026 21:54
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 503

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

📊 Coverage Check

The following changed files need attention:

libs/designer-v2/src/lib/ui/menuItems/resubmitMenuItem.tsx - 0% covered
libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx - 0% covered

⚠️ libs/designer-v2/src/lib/ui/menuItems/showLogicAppRunMenuItem.tsx - 33% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/ui/common/DesignerContextualMenu/DesignerContextualMenu.tsx - 67% covered (needs improvement)

Please add tests for the uncovered files before merging.

Copy link
Copy Markdown
Contributor

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 fixes the nested workflow “Show Logic App run details” clickthrough in DesignerV2 by ensuring the panel header has access to the raw run inputs/outputs (before schema binding can drop important fields), and updates related header button styling.

Changes:

  • Update monitoring inputs/outputs React Query caching to refresh based on stable run identifiers.
  • Add shared helpers + unit tests to reliably extract child workflow ID and child run ID across raw/bound formats.
  • Update panel header buttons (icons + layout) and wire the clickthrough to use raw (pre-binding) inputs/outputs.

Reviewed changes

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

Show a summary per file
File Description
libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/monitoringTab/monitoringTab.tsx Adjusts the React Query key and switches to placeholderData to better track run changes without manual refetch.
libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx Reads raw inputs/outputs via React Query cache and uses them to enable/open nested workflow run details.
libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/childWorkflowHelpers.ts Adds reusable parsing helpers for child workflow run/workflow IDs across formats.
libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/test/childWorkflowHelpers.spec.ts Adds unit coverage for helper parsing behavior and edge cases.
libs/designer-ui/src/lib/panel/panelheader/panelheader.tsx Updates header button icons to Fluent v9 icon components and tweaks button rendering.
libs/designer-ui/src/lib/panel/panel.less Updates header button container styling (spacing/layout).
Comments suppressed due to low confidence (3)

libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/monitoringTab/monitoringTab.tsx:49

  • placeholderData makes TanStack Query return data immediately while the request is still fetching (with isLoading false). This effect will dispatch initializeInputsOutputsBinding with the placeholder { inputs: {}, outputs: {} } and then dispatch again when real data arrives, which can transiently clear/overwrite bound inputs/outputs and cause flicker. Consider gating the dispatch on !isFetching or !isPlaceholderData (or switch back to initialData if you intentionally want the initial dispatch).
      <InputsPanel
        runMetaData={runMetaData}
        brandColor={brandColor}
        isLoading={isFetching || isLoading}
        isError={isError}
        nodeId={selectedNodeId}
      />
      <OutputsPanel
        runMetaData={runMetaData}
        brandColor={brandColor}

libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx:218

  • This useQuery runs for every node selection, even when the node isn't a Workflow-type action (the only case where the "Show Logic App run details" header button can appear). Since getActionLinks can trigger extra network requests, consider adding an enabled condition (e.g., only when nodeType is Workflow and the panel is in run mode) to avoid unnecessary fetches and reduce overhead.

  const showLogicAppRunClick = useCallback(() => {
    const workflowId = getChildWorkflowIdFromInputs(rawInputsOutputs?.inputs);
    if (workflowId && runName && !!HostService()) {
      HostService().openMonitorView?.(workflowId, runName);
    }
  }, [rawInputsOutputs?.inputs, runName]);

  // Re-render panel when undo/redo is performed to update panel parameter values, title etc.
  const undoRedoClickToggle = useUndoRedoClickToggle();

  return (
    <PanelContainer

libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx:290

  • handleTrackEvent is an intentionally empty function, but the previous eslint disable comment for @typescript-eslint/no-empty-function was removed. If that rule is enabled in this package (it appears to be used elsewhere), this will fail lint/CI. Either restore the disable for this line, implement tracking, or remove the prop wiring if a no-op is acceptable.

@rllyy97 rllyy97 enabled auto-merge (squash) April 6, 2026 22:00
@rllyy97 rllyy97 force-pushed the riley/child-run-std-fix branch from 4bac878 to c63c9f9 Compare April 7, 2026 15:34
@rllyy97 rllyy97 force-pushed the riley/child-run-std-fix branch from c63c9f9 to e45b1a5 Compare April 7, 2026 15:58
@rllyy97 rllyy97 merged commit 5f75e7b into main Apr 7, 2026
12 of 14 checks passed
@rllyy97 rllyy97 deleted the riley/child-run-std-fix branch April 7, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Request for a feature implementation in Standard Logic App's Workflow Operations action

4 participants