Skip to content

refactor(knowledge): add toaster notifications, centralized error handling, and file upload hooks#9024

Merged
preetriti1 merged 7 commits intomainfrom
priti/know
Apr 14, 2026
Merged

refactor(knowledge): add toaster notifications, centralized error handling, and file upload hooks#9024
preetriti1 merged 7 commits intomainfrom
priti/know

Conversation

@preetriti1
Copy link
Copy Markdown
Contributor

@preetriti1 preetriti1 commented Apr 7, 2026

…rd and designer

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 error handling and success / failure notification toasters during create calls.
Refactoring to reuse the code between add file from dialog and drawer

Impact of Change

  • Users: New toaster notifications will show success/failure for connection creation/updating and file uploads; message wording changes for empty-state/placeholders.
  • Developers: New hook (useFileHooks), new ToasterNotification component, changes to createOrUpdateConnection signature (callers must pass isCreate for updates), and new optionsSlice actions (setNotification/clearNotification). Ensure dependent modules update usage where applicable.
  • System:Minor — additional logging and toast dispatching; no new backend/API changes. However, the createOrUpdateConnection function now logs errors and accepts an isCreate flag; callers that relied on the previous signature must be updated.

Test Plan

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

Contributors

@divyaswarnkar

Screenshots/Videos

Copilot AI review requested due to automatic review settings April 7, 2026 17:32
@preetriti1 preetriti1 added the risk:medium Medium risk change with potential impact label Apr 7, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 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: refactor(knowledge): add toaster notifications, centralized error handling, and file upload hooks
  • Issue: None — title is clear, follows conventional commit style, and accurately summarizes the main changes.
  • Recommendation: Keep as-is.

Commit Type

  • Properly selected (refactor).
  • Note: Only one commit type is selected which is correct.

Risk Level

  • Assessment: The PR modifies many UI flows, introduces a new notification system, changes createOrUpdateConnection signature, and updates many callers/tests. This is a moderate-scope change.
  • The PR label risk:medium is present and matches the PR body selection.

What & Why

  • Current: "Added error handling and success / failure notification toasters during create calls.\nRefactoring to reuse the code between add file from dialog and drawer"
  • Issue: None — succinct and appropriate.
  • Recommendation: (Optional) You can add a one-line note calling out the breaking-change risk (createOrUpdateConnection signature change) so reviewers focus on caller updates.

Impact of Change

  • The PR body describes Users, Developers, and System impacts. This matches the code diff which adds toaster UI components, a new Redux notification field/actions, changes to createOrUpdateConnection, new hooks (useFileHooks), and many UI updates.
  • Recommendation: Consider adding a short "Compatibility / Migration" bullet reminding integrators to update any external consumers of createOrUpdateConnection if they call it from external packages or plugins.
    • Users: Toaster notifications and updated wording (as described in the PR). Good.
    • Developers: Correctly lists signature change and new hooks/components — ensure any internal or external callers are updated. Add migration notes if this function is exported for other packages.
    • System: Minor runtime overhead for toasts/logging — documented in the PR body.

Test Plan

  • Assessment: The PR body claims unit tests added and manual testing completed. The code diff contains multiple updated and new unit tests across the knowledge area (many test files added/updated), so the claim is validated.
  • Recommendation: Good. (Optional) Consider adding an integration or E2E test validating the toast behavior and the create/update failure path so UI flow is covered end-to-end.

Contributors

  • Assessment: Contributors section is filled with @divyaswarnkar. That's fine. If others (PM, designer) contributed, consider listing them — optional.

⚠️ Screenshots/Videos

  • Assessment: Not provided. This is acceptable for non-visual logic changes, but since you add toasters and UI behavior it's optional but recommended to include a short gif or screenshot of the toaster in the future to help reviewers/QA.

Summary Table

Section Status Recommendation
Title Keep as-is.
Commit Type Keep as-is.
Risk Level Matches label; keep risk:medium.
What & Why Consider adding a one-line migration note about changed signature.
Impact of Change Add compatibility/migration note for exported APIs.
Test Plan Unit tests are included. Add E2E/integration tests for notification flows if possible.
Contributors OK. Optionally list PM/designers.
Screenshots/Videos ⚠️ Consider adding a short screenshot/gif of toasters.

Final Notes & Actionable Recommendations

  • Overall: PASS — the PR title and body follow the template and are consistent with the code changes. The risk level label (risk:medium) matches the changes in the diff and is appropriate for this scope.
  • Key suggestions to include before merging (small checklist you can add to the PR):
    1. Add a short "Compatibility / Migration" note in the body calling out the createOrUpdateConnection signature change (new isCreate flag) and confirming all internal callers were updated (they appear updated in this diff — mention it explicitly). If this function is part of a public API used by external repos, call out the breaking change and proposed migration steps.
    2. Add a short changelog entry or release note summarizing the new notifications and hook additions so consumers know about UI/behavior changes.
    3. Consider adding a brief E2E or integration test for the toast lifecycle (success and failure flows) if the test matrix allows it. Unit tests exist and were updated — good.
    4. (Optional) Add a screenshot or short gif showing the toaster UI to help QA and designers verify copy and layout.
    5. Make sure localization pipeline is run for the many added strings (you added many intl keys — ensure they are extracted and staged for translation as appropriate).

Please update the PR description with the small migration / compatibility note and consider the optional additions above. Thanks — this is a well-structured PR with tests and appropriate risk labeling.

Please update the PR title and body as recommended above, then re-submit if you make changes. Thank you!


Last updated: Tue, 14 Apr 2026 08:12:29 GMT

@preetriti1 preetriti1 changed the title fix(knowledge): Error handling and content changes for knowledge wiza… refactor(knowledge): add toaster notifications, centralized error handling, and file upload hooks Apr 7, 2026
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 improves the Knowledge Hub UX in the designer by adding explicit success/failure notifications (toasts and inline error bars) for connection create/update and file upload flows, and refactors duplicated “add file” logic into a shared hook used by both the drawer panel and modal.

Changes:

  • Added notification plumbing via Redux (optionsSlice.notification) and a reusable ToasterNotification component.
  • Added error handling + user-visible error UI for connection create/update, plus success notifications for create/update/upload/group creation.
  • Refactored add-file flows to reuse shared logic (useFileHooks) and updated/added unit tests accordingly.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Localize/lang/strings.json Adds/removes localized strings for new success/failure messages and updated empty-state copy.
libs/designer/src/lib/ui/knowledge/wizard/knowledgelist.tsx Memoizes intl text/columns and dispatches notifications on delete completion.
libs/designer/src/lib/ui/knowledge/wizard/knowledgehub.tsx Shows toaster notifications, wires notification state, and adds toast on group creation/deletion.
libs/designer/src/lib/ui/knowledge/wizard/test/knowledgelist.spec.tsx Updates tests to include Redux provider for new dispatch usage.
libs/designer/src/lib/ui/knowledge/panel/files/useFileHooks.tsx New shared hook for upload footer state, validation, upload handling, and notifications.
libs/designer/src/lib/ui/knowledge/panel/files/uploadfile.tsx Adds success notification on group creation; memoizes intl text.
libs/designer/src/lib/ui/knowledge/panel/files/filelist.tsx Memoizes intl text and column definitions to reduce rerenders.
libs/designer/src/lib/ui/knowledge/panel/files/addfile.tsx Refactors drawer panel to use useFileHooks and dispatch toast notifications.
libs/designer/src/lib/ui/knowledge/panel/connection/usepaneltabs.tsx Adds onCreate/onError callbacks and reports create failures to UI.
libs/designer/src/lib/ui/knowledge/panel/connection/tabs/model.tsx Memoizes intl text.
libs/designer/src/lib/ui/knowledge/panel/connection/tabs/basics.tsx Memoizes intl text.
libs/designer/src/lib/ui/knowledge/panel/connection/edit.tsx Adds inline error MessageBar + success toast; logs via updated core util signature.
libs/designer/src/lib/ui/knowledge/panel/connection/create.tsx Adds inline error MessageBar + success toast via callbacks.
libs/designer/src/lib/ui/knowledge/panel/connection/test/usepaneltabs.spec.tsx Updates hook tests to assert onError behavior instead of console.error.
libs/designer/src/lib/ui/knowledge/panel/connection/test/edit.spec.tsx Updates tests to validate inline error MessageBar rendering.
libs/designer/src/lib/ui/knowledge/panel/connection/test/create.spec.tsx Adds tests for error MessageBar rendering via captured onError.
libs/designer/src/lib/ui/knowledge/notification.tsx New reusable Fluent UI toaster wrapper component with auto-clear behavior.
libs/designer/src/lib/ui/knowledge/modals/delete.tsx Memoizes derived values and intl text; keeps existing delete behavior + notification payload.
libs/designer/src/lib/ui/knowledge/modals/creategroup.tsx Memoizes intl text.
libs/designer/src/lib/ui/knowledge/editor/index.tsx Updates empty-state copy to new localized strings; memoizes intl text.
libs/designer/src/lib/ui/knowledge/editor/files.tsx Refactors modal upload flow to reuse useFileHooks and designer-ui footer.
libs/designer/src/lib/ui/knowledge/editor/connection.tsx Adds inline error MessageBar and wires onError into create-connection tabs.
libs/designer/src/lib/ui/knowledge/editor/test/index.spec.tsx Updates assertions for changed empty-state strings.
libs/designer/src/lib/ui/knowledge/editor/test/files.spec.tsx Updates cancel-button behavior expectation during upload.
libs/designer/src/lib/ui/knowledge/editor/test/connection.spec.tsx Adds tests for error MessageBar rendering via captured onError.
libs/designer/src/lib/ui/knowledge/test/notification.spec.tsx New unit tests for ToasterNotification dispatch behavior and onClear.
libs/designer/src/lib/core/state/knowledge/optionsSlice.ts Adds notification field and actions to set/clear notifications.
libs/designer/src/lib/core/knowledge/utils/connection.ts Adds logging + isCreate flag to distinguish create vs update failures.
libs/designer/src/lib/core/knowledge/utils/test/connection.spec.ts Updates mocks for newly used LoggerService/LogEntryLevel and typing in intl mock.

Comment thread libs/designer/src/lib/core/state/knowledge/optionsSlice.ts
Comment thread libs/designer/src/lib/ui/knowledge/panel/files/useFileHooks.tsx
Comment thread libs/designer/src/lib/ui/knowledge/panel/files/uploadfile.tsx
Comment thread libs/designer/src/lib/ui/knowledge/modals/delete.tsx
Comment thread libs/designer/src/lib/ui/knowledge/editor/index.tsx
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer/src/lib/core/state/knowledge/optionsSlice.ts - 50% covered (needs improvement)
⚠️ libs/designer/src/lib/ui/knowledge/panel/files/addfile.tsx - 75% covered (needs improvement)

Please add tests for the uncovered files before merging.

dependabot bot and others added 6 commits April 13, 2026 15:45
Bumps [lodash](https://github.com/lodash/lodash) from 4.17.21 to 4.18.1.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.21...4.18.1)

---
updated-dependencies:
- dependency-name: lodash
  dependency-version: 4.18.1
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Carlos Castro <ccastrotrejo@microsoft.com>
…oundry (#9012)

fixed deploymentModelProperties not updating properly issue
* Fixed child run issue in standard logic apps

* Fixed duplicate IO requests

* Fixed data import issue with io

* Added clickthrough to context menu
@preetriti1 preetriti1 enabled auto-merge (squash) April 14, 2026 08:28
@preetriti1 preetriti1 merged commit b57cf20 into main Apr 14, 2026
14 checks passed
@preetriti1 preetriti1 deleted the priti/know branch April 14, 2026 08:38
preetriti1 added a commit that referenced this pull request Apr 14, 2026
…dling, and file upload hooks (#9024)

* fix(knowledge): Error handling and content changes for knowledge wizard and designer

* chore(deps): bump lodash from 4.17.21 to 4.18.1 (#8996)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.21 to 4.18.1.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.21...4.18.1)

---
updated-dependencies:
- dependency-name: lodash
  dependency-version: 4.18.1
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Carlos Castro <ccastrotrejo@microsoft.com>

* fix(designer): Only populate deploymentModelProperties for MicrosoftFoundry (#9012)

fixed deploymentModelProperties not updating properly issue

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

* Fixed child run issue in standard logic apps

* Fixed duplicate IO requests

* Fixed data import issue with io

* Added clickthrough to context menu

* Fixing issues found in knowledge hub creation

* Fixing tests

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Priti Sambandam <psamband@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Carlos Castro <ccastrotrejo@microsoft.com>
Co-authored-by: Elaina Lee <144840522+Elaina-Lee@users.noreply.github.com>
Co-authored-by: Riley Evans <rllyy97@gmail.com>
preetriti1 added a commit that referenced this pull request Apr 14, 2026
…, and file upload hooks (#9024) (#9058)

refactor(knowledge): add toaster notifications, centralized error handling, and file upload hooks (#9024)

* fix(knowledge): Error handling and content changes for knowledge wizard and designer

* chore(deps): bump lodash from 4.17.21 to 4.18.1 (#8996)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.21 to 4.18.1.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.21...4.18.1)

---
updated-dependencies:
- dependency-name: lodash
  dependency-version: 4.18.1
  dependency-type: direct:production
...





* fix(designer): Only populate deploymentModelProperties for MicrosoftFoundry (#9012)

fixed deploymentModelProperties not updating properly issue

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

* Fixed child run issue in standard logic apps

* Fixed duplicate IO requests

* Fixed data import issue with io

* Added clickthrough to context menu

* Fixing issues found in knowledge hub creation

* Fixing tests

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Priti Sambandam <psamband@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Carlos Castro <ccastrotrejo@microsoft.com>
Co-authored-by: Elaina Lee <144840522+Elaina-Lee@users.noreply.github.com>
Co-authored-by: Riley Evans <rllyy97@gmail.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.

4 participants