Skip to content

fix(knowledge-designer): Hiding the knowledge hub parameter from new designer#9082

Merged
preetriti1 merged 3 commits intomainfrom
priti/retake
Apr 21, 2026
Merged

fix(knowledge-designer): Hiding the knowledge hub parameter from new designer#9082
preetriti1 merged 3 commits intomainfrom
priti/retake

Conversation

@preetriti1
Copy link
Copy Markdown
Contributor

@preetriti1 preetriti1 commented Apr 20, 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

Hiding the newly added knowledge hub parameter from agent connector in new designer. This should not impact anything in current GA designer for this new feature to be released.
Also updated the learn more links from 3 places in knowledge hub UI

Impact of Change

  • Users: Users would not be able to configure knowledge hub from preview designer
  • Developers: Every feature released should be tested from both preview and GA designer for the time being
  • System: NA

Test Plan

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

Contributors

@divyaswarnkar

Screenshots/Videos

in V2
Before:
image

After:
image

in V1.
Feature flag on -
image

Feature flag off -
image

CSS fix for long name
image

Copilot AI review requested due to automatic review settings April 20, 2026 22:05
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 updates the Logic Apps designer UX to hide the Knowledge Hub/Knowledge Base parameter in the new (v2) designer, while also updating “Learn more” links in Knowledge Hub UI surfaces.

Changes:

  • Hide parameters with editor type knowledgebase from the v2 designer parameter list (toParameterInfoMap).
  • Update Knowledge Hub “Learn more” links to a single fwlink URL in multiple UI locations.
  • Add unit tests validating that knowledgebase editor parameters are excluded from toParameterInfoMap.

Reviewed changes

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

File Description
libs/designer/src/lib/ui/knowledge/panel/files/uploadfile.tsx Updates “Learn more” links in the Knowledge Hub file upload panel sections.
libs/designer/src/lib/ui/knowledge/editor/index.tsx Updates the Knowledge Hub editor “Learn more” link URL.
libs/designer-v2/src/lib/core/utils/parameters/helper.ts Filters out knowledgebase editor parameters when generating v2 parameter infos.
libs/designer-v2/src/lib/core/utils/parameters/test/helper-agentParams.spec.ts Adds tests ensuring knowledgebase editor parameters are excluded.

Comment thread libs/designer-v2/src/lib/core/utils/parameters/helper.ts
Comment thread libs/designer/src/lib/ui/knowledge/editor/index.tsx Outdated
Comment thread libs/designer/src/lib/ui/knowledge/panel/files/uploadfile.tsx
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2026

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer-v2/src/lib/core/utils/parameters/helper.ts - 42% covered (needs improvement)
⚠️ libs/designer/src/lib/core/utils/parameters/helper.ts - 40% covered (needs improvement)

Please add tests for the uncovered files before merging.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 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(knowledge-designer): Hiding the knowledge hub parameter from new designer
  • Issue: Minor style/formatting. The title uses the gerund "Hiding" instead of the conventional imperative verb (e.g., "Hide"). Commit message/PR title convention in this repo generally prefers imperative present tense.
  • Recommendation: Use an imperative form and be concise about scope. Example: fix(knowledge-designer): Hide knowledge hub parameter in new designer

Commit Type

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

Risk Level

  • The PR body selects Medium and the PR has the label risk:medium. From the code changes (modifying parameter visibility logic across designer libs and UI links/styling), the advised risk is also Medium, so the label and body match and are appropriate.

What & Why

  • Current: "Hiding the newly added knowledge hub parameter from agent connector in new designer... Also updated the learn more links from 3 places in knowledge hub UI"
  • Issue: The description is brief but acceptable. One important detail to clarify: the change behaves differently between the new designer (libs/designer-v2) and the GA/old designer (libs/designer). In designer-v2 you unconditionally exclude parameters whose editor equals 'knowledgebase', whereas in the GA designer you added feature-flag-aware logic (WorkflowService().isKnowledgeHubEnabled). This divergence should be explicitly documented in "What & Why" so reviewers/maintainers understand intent.
  • Recommendation: Expand to a one-line clarification such as: "In the new designer (designer-v2) knowledgebase editor parameters are hidden unconditionally; in the GA designer (designer) the visibility is controlled by WorkflowService().isKnowledgeHubEnabled so GA remains unaffected unless the flag is off."

Impact of Change

  • The Impact section is present but please make the scope explicit (new vs GA designer) as called out above.
  • Recommendation:
    • Users: Preview/new-designer users will not see or be able to configure knowledge-hub/knowledgebase parameters; GA (existing) designer behavior remains controlled by feature flag.
    • Developers: Note the two different code paths (designer-v2 unconditional hide vs designer feature-flag check). Recommend adding a comment in code or the PR to justify this divergence and where to revert/align in future.
    • System: No perf/infra changes; primarily UI/behavioral.

Test Plan

  • Unit tests added/updated: Yes — I confirmed new/updated unit test files in both libs/designer-v2 and libs/designer covering knowledgebase hiding and KnowledgeHub enabled/disabled scenarios.
  • E2E tests: None added. This is OK for this change, but consider an E2E/UI test to validate the visible/hidden behavior for the UI path if you have E2E coverage for designer flows.
  • Manual testing: Marked as completed and screenshots provided for visual changes.

Contributors

  • @divyaswarnkar is noted. Good to credit others who contributed. If PMs/designers helped with screenshots or acceptance criteria, consider calling them out here (optional).

Screenshots/Videos

  • Screenshots provided for V1 and V2 and CSS fix. Good — they help validate visual/UI changes.

Summary Table

Section Status Recommendation
Title ⚠️ Use imperative verb: fix(knowledge-designer): Hide knowledge hub parameter in new designer
Commit Type No change needed
Risk Level risk:medium is appropriate
What & Why Clarify difference between new designer (v2) and GA designer behavior in the description
Impact of Change Expand on user/dev impact and mention feature-flag behavior explicitly
Test Plan Unit tests present. Consider adding E2E UI coverage for visibility behavior (optional)
Contributors Credit other contributors if applicable (optional)
Screenshots/Videos Good coverage provided

Final Notes and Action Items

  • Please update the PR title to use the imperative form (suggestion above).
  • In the PR body, explicitly call out that designer-v2 hides knowledgebase parameters unconditionally while the GA designer uses WorkflowService().isKnowledgeHubEnabled (default true) so GA behavior is unaffected — this prevents confusion for reviewers and future maintainers.
  • Consider aligning the checks between designer-v2 and designer (or add a comment explaining why they differ). Also consider using the same constant/enum for the editor value in both places instead of a hard-coded string ('knowledgebase') to avoid typos and maintain consistency.
  • Optional: add or note an E2E/UI test for the visibility change if you have E2E testing in this area.

Please update the PR title/body with the suggested clarifications and then this can be merged. Thank you for the thorough test coverage and screenshots — very helpful!


Last updated: Tue, 21 Apr 2026 07:17:08 GMT

@preetriti1 preetriti1 added the risk:medium Medium risk change with potential impact label Apr 21, 2026
@preetriti1 preetriti1 merged commit 9e9e22c into main Apr 21, 2026
21 of 22 checks passed
@preetriti1 preetriti1 deleted the priti/retake branch April 21, 2026 08:47
preetriti1 added a commit that referenced this pull request Apr 21, 2026
…designer (#9082) (#9087)

* fix(knowledge-designer): Hiding the knowledge hub parameter from new designer

* Updating with correct forward links and adding designer feature flag
along with some css updates

* Fixing tests

---------

Co-authored-by: Priti Sambandam <psamband@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants