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

[preetriti1]

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

Screenshots/Videos

[github-actions]

🤖 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): add toaster notifications, centralized error handling, and file upload hooks (#9024)
• Issue: Title is generally good (conventional style, scope provided). The only minor note: it includes a parenthetical reference (#9024) which is often unnecessary in the PR title (the PR description already references issues/PRs). Not a blocker — optional clean-up.
• Recommendation: Keep as-is or remove the trailing (#9024) for brevity and consistency with repository title conventions. Example: fix(knowledge): add toaster notifications, centralized error handling, and file upload hooks.

---

✅ Commit Type

• Properly selected (fix). Only one option checked, which is correct for this change.
• Note: If the PR also contains refactors (lots of test + structural changes), you can mention that in the body as secondary context (but do NOT select multiple checkboxes in the template).

---

✅ Risk Level

• The PR selected Medium and the repository has the risk:medium label applied — this matches the PR body label selection.
• Advisement: Based on the code diff, I recommend a higher risk classification (risk:high). See details below — I added this advice because the changes include breaking semantics in core utilities (error propagation now throws instead of returning undefined), a signature change to createOrUpdateConnection, and broad UI + state changes that touch many modules. Please see the Summary and Required Actions section.

---

✅ What & Why

• Current: "Added error handling and success / failure notification toasters during create calls. Refactoring to reuse the code between add file from dialog and drawer"
• Issue: The description is concise and correct, but does not call out the breaking/behavioral changes explicitly (e.g., changed function signatures, thrown errors where previously handled silently).
• Recommendation: Add a short migration/impact note here: e.g., "Breaking changes: createOrUpdateConnection now takes an isCreate boolean (callers must be updated) and createKnowledgeHub now throws errors instead of returning undefined on failure; callers must be updated to handle rejected Promises." This helps reviewers and integrators quickly identify required changes.

---

✅ Impact of Change

• The PR body includes Users, Developers, System sections explaining the main impacts. Good coverage.
• Issue: Please explicitly list the key public APIs/entry points that changed (for example: libs/designer/src/lib/core/knowledge/utils/connection.ts -> createOrUpdateConnection signature change; libs/designer/src/lib/core/knowledge/utils/helper.ts -> createKnowledgeHub now throws). This helps release notes and downstream teams.
• Recommendation: Add a short bullet in Developers noting exact files / exported functions that changed and recommended migration steps (e.g., update callers to pass isCreate and add try/catch for thrown errors). Also note if any consumers outside this repo will be affected.
  • Users: Toaster notifications will show success/failure for connection creation/updating and file uploads. Wording changes to placeholders/empty state.
  • Developers: Call out the changed signatures and thrown errors: update callers of createOrUpdateConnection and createKnowledgeHub. Update any integration code expecting prior behaviour.
  • System: Additional client-side logging and toast dispatching; no backend/API changes declared, but run a full regression on knowledge hub flows.

---

✅ Test Plan

• Assessment: The PR claims Unit tests added/updated and manual testing completed. The code diff includes a substantial set of updated/added unit tests covering notification behavior, connection handling, file upload hooks, and UI changes — this validates the claim.
• Recommendation: Consider adding or running integration/E2E tests for the following flows (not required for PR to pass but recommended given breadth):
  • Create connection (happy + error path)
  • Update connection (happy + error path)
  • Create group and file upload flows end-to-end to verify toast notifications and state updates in real app flows
• If E2E couldn't be added, add a short note in the Test Plan explaining why E2E was omitted and which manual test scenarios were exercised.

---

⚠️ Contributors

• Assessment: Contributors section is blank.
• Recommendation: Add contributors (PMs, designers, reviewers) if applicable. If none, add a short note or leave blank intentionally — but it's helpful to credit folks.

---

⚠️ Screenshots/Videos

• Assessment: None provided. This PR introduces visible UI changes (toaster notifications, message bars). Visual proof is recommended.
• Recommendation: Add a couple of screenshots or a short screen recording showing the toaster notifications (success + failure) and the message bars where applicable (create/update connection errors). This makes it easier for reviewers and designers to verify copy and styling.

---

Summary Table

Section	Status	Recommendation
Title	✅	Optional: remove trailing (#9024) for cleanliness.
Commit Type	✅	No change needed.
Risk Level	✅ (body/label matched)	Advised risk is HIGH — see below and update label if you accept that assessment.
What & Why	✅	Add explicit breaking/migration note for changed function semantics.
Impact of Change	✅	Add explicit list of changed exported functions and migration steps.
Test Plan	✅	Good unit tests present. Consider adding E2E or document manual scenarios.
Contributors	⚠️	Add contributors or note none.
Screenshots/Videos	⚠️	Add screenshots/videos for the new toasts and message bars.

---

Final notes and required actions

• PR body mostly follows the template and is well filled out — good job.
• Risk re-evaluation: I recommend bumping to risk:high. Rationale:
  • createOrUpdateConnection signature changed (added isCreate flag) and multiple code paths now call it with that flag; this is a breaking API for any external consumers or internal callers not updated.
  • createKnowledgeHub now throws errors instead of returning undefined on failures — that is a semantic change and callers must handle rejected Promises.
  • Many UI/state files changed (optionsSlice notification state, dispatching notifications across multiple panels, new hooks) which increases surface area for regressions, especially in flows that cross module boundaries.
  • The change set is large (33 files changed, ~1700 additions, ~1000 deletions) which implies non-trivial regression risk in runtime behavior.
• Recommendations before merging (minimum):
  1. Add an explicit Migration / Breaking Changes section in the PR body listing the changed exported functions (createOrUpdateConnection, createKnowledgeHub) and exactly what callers must change.
  2. Add screenshots / short video demonstrating the new toaster notification (success and failure) and message bars so reviewers and designers can confirm copy and placement.
  3. If you accept the higher risk assessment, either change the PR label to risk:high or add a justification in the PR body that explains why it should remain risk:medium. Ensure release managers are warned.
  4. Add a short note in Test Plan describing manual test scenarios exercised and whether any integration/E2E tests are planned (and why not included if omitted).
  5. Remove the needs-pr-update label once you update the PR body with the migration notes and screenshots.

Please update the PR title/body with the migration notes and (optionally) screenshots. If you want, I can produce a concise migration bullet list based on the diff to paste into the PR description.

Summary decision: PR body/template PASS — but advised risk is higher than the submitter's selection (recommend risk:high).

Thank you for the thorough tests and coverage. Please make the small additions above (migration notes, screenshots, and a short justification if you keep risk:medium) before merging.

---

Last updated: Tue, 14 Apr 2026 15:20:27 GMT

[github-actions]

📊 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.