Skip to content

feat(knowledge): Adding create new hub and artifacts experience in knowledge wizard#8937

Merged
preetriti1 merged 5 commits intomainfrom
priti/addfiles
Mar 20, 2026
Merged

feat(knowledge): Adding create new hub and artifacts experience in knowledge wizard#8937
preetriti1 merged 5 commits intomainfrom
priti/addfiles

Conversation

@preetriti1
Copy link
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

Added upload files component in designer-ui package.
Consumed new upload files component in AddFiles panel to be able to create artifacts from local files.
Passed in upload file function callback (multi part upload) from host portal. This will be implemented in host rather the package.
Updated all other components to enable create group and create artifacts in the wizard. Also added validation methods.

This is a new feature, so no breaking changes. Since development in backend and UI is in progress it looks like major API/versions are being changed but this is expected before a release.

Impact of Change

  • Users: Users will be able to create new hubs and artifacts from portal.
  • Developers: Common components like upload file created in this PR, can be leveraged in other UIs.
  • System: Validation methods in all create experience will help in successful creation. No breaking change

Test Plan

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

Contributors

@bonicaayala @ecfan @divyaswarnkar @bjbennet

Screenshots/Videos

create

@preetriti1 preetriti1 added the risk:medium Medium risk change with potential impact label Mar 19, 2026
Copilot AI review requested due to automatic review settings March 19, 2026 07:35
Copy link
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 introduces a new “create hub / upload local artifacts” experience to the Knowledge wizard by adding a reusable file drop zone component and wiring upload + validation flows through the Knowledge Hub wizard and panels.

Changes:

  • Added shared UploadFile / UploadFileHandler types and implemented UI flows to upload a local file as a knowledge artifact.
  • Added a new FileDropZone component in designer-ui and integrated it into the Knowledge “Add files” panel (including file list + name validation).
  • Updated “Create group” to validate hub names and show loading/creating states, plus added/updated unit tests and localization strings.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
libs/logic-apps-shared/src/utils/src/lib/models/knowledge.ts Adds upload-related shared types (UploadFile, UploadFileHandler).
libs/designer/src/lib/ui/knowledge/wizard/knowledgehub.tsx Wires upload handler into the wizard and passes selected hub into the panel.
libs/designer/src/lib/ui/knowledge/panel/styles.ts Adds styles specific to the Add Files panel layout/table.
libs/designer/src/lib/ui/knowledge/panel/panelroot.tsx Passes selected hub + upload handler into AddFilePanel.
libs/designer/src/lib/ui/knowledge/panel/addfile.tsx Implements Add Files UX: drop zone, file table, validation, upload + error UI.
libs/designer/src/lib/ui/knowledge/panel/test/panelroot.spec.tsx Expands panel routing tests and verifies props passed to child panels.
libs/designer/src/lib/ui/knowledge/panel/test/addfile.spec.tsx Adds tests around new Add Files panel behaviors (drop zone, validation, modal).
libs/designer/src/lib/ui/knowledge/modals/styles.ts Adds styling for updated Create Group modal layout.
libs/designer/src/lib/ui/knowledge/modals/creategroup.tsx Adds hub-name validation and loading/creating UI to Create Group modal.
libs/designer/src/lib/ui/knowledge/modals/test/creategroup.spec.tsx Adds tests for Create Group validation and loading/creating states.
libs/designer/src/lib/core/knowledge/utils/helper.ts Adds upload helper + name validation helpers; adjusts create hub behavior.
libs/designer-ui/src/lib/index.ts Exports the new filedropzone module.
libs/designer-ui/src/lib/filedropzone/styles.ts Styling for the new FileDropZone component.
libs/designer-ui/src/lib/filedropzone/index.tsx New drag/drop + browse file input component.
libs/designer-ui/src/lib/filedropzone/test/filedropzone.spec.tsx Unit tests for FileDropZone interaction and filtering behavior.
apps/Standalone/src/knowledge/app/KnowledgeHub.tsx Plumbs an upload implementation into KnowledgeHubWizard for Standalone.
Localize/lang/strings.json Adds localized strings for new UI labels/messages.

@github-actions
Copy link

github-actions bot commented Mar 19, 2026

📊 Coverage check completed. See workflow run for details.

@ecfan ecfan self-assigned this Mar 19, 2026
@preetriti1 preetriti1 requested a review from ecfan March 20, 2026 01:27
@github-actions
Copy link

github-actions bot commented Mar 20, 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: feat(knowledge): Adding create new hub and artifacts experience in knowledge wizard
  • Issue: Title is generally clear and follows conventional commit style. It could be slightly more specific about the upload capability (artifact upload) that was added, but it's acceptable as-is.
  • Recommendation: Optionally make the scope a little more explicit: feat(knowledge): add create-hub + artifact upload experience in knowledge wizard.

Commit Type

  • Properly selected (feature)
  • Note: Exactly one commit type is selected in the PR body which is correct.

⚠️ Risk Level

  • Assessment: PR description and labels indicate Medium risk and the repo has a risk:medium label present.
  • Advised: HIGH (see reasoning below)
  • Note: I recommend upgrading the PR risk to High. This change introduces client-side upload behavior to a management endpoint, touches authentication/authorization handling (Authorization header usage), adds new network requests and file handling across multiple packages, and modifies many UI and core helpers. Those factors increase blast radius and warrant higher scrutiny (security review, integration/E2E testing, and possible sign-off from platform/infra). Please add or update the risk label to risk:high if you agree.

What & Why

  • Current: "Added upload files component in designer-ui package..." (present and reasonably descriptive)
  • Issue: The description is present and adequate, but the body should call out key operational/architectural changes (server endpoint used, auth token usage, any environment variables required at runtime) explicitly so reviewers and platform owners can assess impact.
  • Recommendation: Add one short bullet stating: "This introduces a client-side PUT to management API for artifact uploads using environment.armToken; the host portal provides the token. Ensure token handling and server-side validation are in place." This helps security reviewers and release owners.

Impact of Change

  • Issue: Impact section is present and correct at a high level.
  • Recommendation: Expand to explicitly list:
    • Users: new ability to upload artifacts and create hubs from the portal.
    • Developers: new public component (FileDropZone) and UploadFile types + UploadFileHandler; callers must implement onUploadArtifact. Add note about changed helper APIs (validateHubNameAvailability/validateArtifactNameAvailability) if they are exported.
    • System: server receives new PUT uploads, must validate content and size limits; potential increases in storage/ingress.

Test Plan

  • Assessment: Unit tests added/updated — confirmed in diff (filedropzone tests, helper tests, many component tests). Manual testing completed — indicated in PR body. E2E tests not present.
  • Issue / Recommendation: Because this change adds file upload flows and server interactions, consider adding at least one integration/E2E test (happy path + failure path) to exercise the upload end-to-end (including auth). If E2E is not possible in this branch, add a clear note in the Test Plan explaining why and what will cover E2E (e.g., separate pipeline or follow-up PR). Also ensure unit tests cover key edge cases (large file rejection, progress updates, authorization errors).

⚠️ Contributors


⚠️ Screenshots/Videos

  • Assessment: Screenshot present in body.
  • Recommendation: If UX changes affect many screens, consider a small video/gif for the upload flow (drag/drop, progress, error) in the PR description to help reviewers.

Summary Table

Section Status Recommendation
Title Optionally mention "artifact upload" if you want more specificity.
Commit Type No change needed.
Risk Level ⚠️ Change label to risk:high. Add a short security note in PR body.
What & Why Add a sentence about client-side uploads, token usage and backend expectations.
Impact of Change Expand to call-out system/backend impact and required platform validations.
Test Plan Add E2E/integration tests or explain why they are deferred.
Contributors ⚠️ Good list. Consider adding backend/infra owners if applicable.
Screenshots/Videos ⚠️ Consider adding a short recording of the upload flow (optional but useful).

Actionable items I recommend before merging

  1. Update PR risk label (and PR text Risk Level) to High and add a short justification: new client-side upload flow + auth header usage + server-side surface change.
  2. Add an explicit security note in the PR body addressing how environment.armToken is supplied/rotated and why it’s safe to put it in client code (or, preferably, move upload auth to host-managed server-side flow if that reduces token exposure). Call out if the portal runtime ensures token scope and short lifetime.
  3. Add/confirm E2E or integration tests for the upload flow (successful upload, auth failure, and file-too-large rejection). If these will be added in a follow-up PR, state that clearly in Test Plan with a link to the plan/issue.
  4. Add an explicit note of the API contract (management endpoint path and expected payloads) and required backend validations (size limits, content-type verification, auth scope checks). This helps reviewers and platform owners understand the backend surface change.
  5. Double-check error handling and UX for upload failures (the code uses xhr and sets user-facing message — make sure all error codes and message strings are handled and localized). Add a unit test that simulates upload failure to assert the formatted error message is shown.
  6. Confirm there are no secrets accidentally committed (I see usage of environment.armToken — ensure that environment file does not contain secrets in repo). If environment variables are required, document them in PR.
  7. If possible, add a short note to the PR about performance implications (e.g., large uploads, throttling), or limits and how the UI handles them.

If you want, I can generate a suggested one-paragraph update to the PR body (risk justification + security note + test plan addition) that you can paste into the PR description.


Final verdict: This PR body largely follows the required template and contains tests. However, because of the client-side upload behavior and cross-cutting changes, I advise increasing the risk level to HIGH and adding the security/integration test clarifications above before merging.


Last updated: Fri, 20 Mar 2026 02:07:25 GMT

@preetriti1 preetriti1 enabled auto-merge (squash) March 20, 2026 03:45
@preetriti1 preetriti1 merged commit 75999c1 into main Mar 20, 2026
14 of 15 checks passed
@preetriti1 preetriti1 deleted the priti/addfiles branch March 20, 2026 03:55
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.

5 participants