fix(Designer): Add null safety to localeCompare in connector sort

[rllyy97]

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

The defaultSortConnectors comparator in browseView.tsx was not handling connectors with undefined displayName, causing a TypeError when Array.sort called localeCompare on undefined. This adds nullish coalescing to both sides of the comparison, matching the pattern already used in designer-v2's connectorBrowse.tsx.

Impact of Change

• Users: Fixes designer crash in client code
• Developers: N/A
• System: N/A

Test Plan

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

Confirmed connector sort is working without crashing

Contributors

@rllyy97

Screenshots/Videos

N/A

[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(Designer): Add null safety to localeCompare in connector sort
• Issue: None — title is clear, scoped, and follows conventional commit style.
• Recommendation: No change required.

---

✅ Commit Type

• Properly selected (fix).
• Commit type section shows only one selection (fix) which is correct for this change.

---

✅ Risk Level

• The PR is labeled risk:low and the PR body selects Low. This matches the code changes (small, narrowly scoped fix + tests).

---

✅ What & Why

• Current: The PR body includes a concise explanation: the comparator called localeCompare on possibly undefined displayName, causing a TypeError; this change uses nullish coalescing to avoid that.
• Issue: None — explanation is clear and matches the code change.
• Recommendation: You can leave as-is.

---

❌ Impact of Change

• The Impact section is present but minimal (Users: Fixes designer crash; Developers/System: N/A).
• Recommendation: Expand slightly to note that unit tests were added and where to find them. Example:
  • Users: Fixes a client crash when connector displayName is undefined.
  • Developers: Adds unit tests covering BrowseView sorting/filtering and null displayName handling (see libs/designer/src/lib/ui/panel/recommendation/test/browseView.spec.tsx).
  • System: No runtime or perf impact expected.

---

❌ Test Plan

• Assessment: The code diff includes a new unit test file (libs/designer/src/lib/ui/panel/recommendation/test/browseView.spec.tsx) with many tests, but the PR body did NOT check the "Unit tests added/updated" box and only checked "Manual testing completed".

• Recommendation: Update the Test Plan section to mark the Unit tests box. Also either:

  • keep Manual testing and add a short explanation of what manual steps were taken (if applicable), or
  • uncheck Manual testing if unit tests cover the change and no separate manual verification was needed.

  Example update to Test Plan:

  • Unit tests added/updated — added browseView.spec.tsx covering sorting, filters, and null displayName
  • E2E tests added/updated
  • Manual testing completed — verified connector sort no longer throws when displayName is undefined

  Also please include how to run the tests locally (e.g. yarn test libs/designer --watch or repo-specific command) so reviewers can reproduce.

---

✅ Contributors

• Assessment: Contributors section lists @rllyy97. Good — credit given.
• Recommendation: (Optional) If others reviewed or assisted, add them here.

---

✅ Screenshots/Videos

• Assessment: N/A — this is a non-visual bugfix. Correct to leave blank.

---

Summary Table

Section	Status	Recommendation
Title	✅	No change needed.
Commit Type	✅	No change needed.
Risk Level	✅	No change needed.
What & Why	✅	No change needed.
Impact of Change	❌	Expand to mention added unit tests and brief developer/system impact.
Test Plan	❌	Tick the Unit tests added/updated box and include how to run tests; explain manual testing if kept.
Contributors	✅	No change needed (optional: add additional contributors if any).
Screenshots/Videos	✅	No change needed.

---

Summary

This PR is small, safe, and appropriate to be labeled risk:low. The title and high-level body are good. The main inconsistency is the Test Plan: unit tests were added but the box was not checked. Please update the PR body to reflect the added unit tests and optionally expand the Impact section to mention the tests and how to run them. Once you update the Test Plan checkbox and the small Impact note, this PR should be ready to merge.

Suggested concrete edits to your PR body:

• In Test Plan, mark the Unit tests added/updated checkbox and add one line pointing to the new test file:
  • Unit tests added/updated — see libs/designer/src/lib/ui/panel/recommendation/test/browseView.spec.tsx
• Optionally keep Manual testing but add a short line describing what was manually verified.
• In Impact of Change, mention the test addition and that no system/perf changes are expected.

Thank you — please update the Test Plan and Impact sections and re-submit. If you'd like, I can re-check after you update the PR body.

---

Last updated: Fri, 20 Mar 2026 19:40:39 GMT

[Copilot]

Pull request overview

Fixes a crash in the classic Designer connector browse panel by making the connector sort comparator resilient to missing displayName values (aligning behavior with existing patterns elsewhere in the repo).

Changes:

• Updated the connector sort comparator to coalesce displayName values to empty strings before calling localeCompare.

[github-actions]

📊 Coverage Check

🎉 All changed files have adequate test coverage!