Fix search and filter bugs against the Ballerina Central API - #50
Conversation
Fixes wso2/product-integrator#1853. - Multi-word text search returned no results: escapeSolrQuery escaped the space character, turning "SAP Business" into the wildcard term *sap\ business*, which never matches since the API's wildcard is a literal (non-analyzed) pattern. Space is no longer escaped. - Area filters combining with another AND clause could silently return 0 results when the Area value contains "&" (e.g. "Finance & Accounting" + a Vendor filter) — verified live that the same clauses in the opposite order parse correctly. Area filters are now built last in the query. - The search API's keyword: field isn't an exact match on one tag — it matches loosely against a package's whole keyword list (e.g. keyword:Vendor/OpenAI also matches azure.openai.text, whose real vendor is Microsoft, because of an unrelated bare keyword "Azure OpenAI"). This silently mixed wrong connectors into filtered results. Added filterByExactKeywords() to re-check the exact Area/Vendor/Type tag client-side, mirroring the existing excludeHidden/filterByRelevance post-processing. - The hardcoded "HRMS" sidebar area matched zero live connectors; the actual tag is "Human Resources". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughSummary
WalkthroughThe search client preserves spaces in Solr terms and splits non-wildcard multi-word searches into separate 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/rest-client/rest-client.test.ts`:
- Around line 173-184: Add a Connector entry to the types filter in the
searchPackages call, then capture the URL index for keyword:Type/Connector and
assert it appears before the existing area clause, while preserving the
vendor-before-area ordering assertions.
In `@src/lib/rest-client/rest-client.ts`:
- Around line 526-541: Apply pagination only after exact-keyword filtering:
update the single-combination path around executeSingleSearch to retrieve and
filter the complete logical result set, then calculate the exact count and slice
the requested page. Apply the same ordering to each multi-combination result
path at src/lib/rest-client/rest-client.ts lines 574-575 before merged
pagination; both sites require changes, and remove the
fixed-buffer/proportional-count approach.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b5e89c4e-0465-4039-849c-9b0b49e95eeb
📒 Files selected for processing (3)
src/lib/rest-client/rest-client.test.tssrc/lib/rest-client/rest-client.tssrc/pages/HomePage.tsx
… search bug - npm run lint runs prettier --check, which failed on the previous commit since it was never run through `npm run format`. Fixed formatting. - CodeRabbit review: the vendor/type-before-area regression test only asserted vendor ordering, not type. Added a Type filter and assert it too. - CodeRabbit review (major): the server-side-pagination fast path used a fixed HIDDEN_PACKAGES-sized overfetch buffer to compensate for filterByExactKeywords removing false positives, but that buffer isn't sized for an open-ended problem (unlike the small, static hidden-package list) — a page could come up short, or the reported count could be a rough proportional estimate rather than exact. Restructured searchPackages so any Area/Vendor/Type filter now always fetches the complete result set, filters exactly, and paginates client-side — the same pattern already used and trusted for text search and name sorts. The buffer/estimate approach is now used only for the filter-free browse case, where HIDDEN_PACKAGES is the only possible exclusion and its bounded size actually justifies the buffer. - Found while re-verifying the PR live: searching "dynamics 365" returned nothing. The API's `*dynamics 365*` literal wildcard term returns 0 results (verified live) even though `*dynamics*` and `*365*` each match, and the connector's own keywords contain the phrase. Multi-word search queries are now split into separate ANDed wildcard terms (`*dynamics* AND *365*`) instead of one literal phrase term — verified this doesn't regress the original "SAP Business" fix. All typecheck/lint/format/test checks pass locally (89 tests), and all of the above were re-verified against the running app and the live Central API, not just mocks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/rest-client/rest-client.test.ts (1)
285-319: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake complete-fetch regression mocks request-sensitive.
The mocks return all packages regardless of request parameters. These tests can pass if batch requests use a page-sized limit.
src/lib/rest-client/rest-client.test.ts#L285-L319: make the mock apply the requestedlimitandoffset, so partial fetches cannot produce the expected pages.src/lib/rest-client/rest-client.test.ts#L321-L340: use a count above the normal page size and assert each batch request has a limit sufficient for the complete result set.As per path instructions, provide concise, actionable feedback focused on correctness and best practices.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/rest-client/rest-client.test.ts` around lines 285 - 319, Update the regression tests around searchPackages in src/lib/rest-client/rest-client.test.ts:285-319 and 321-340. Make the mockFetch response honor each request’s limit and offset so page-sized partial fetches cannot satisfy the expected results; in the second test, use a dataset larger than the normal page size and assert every batch request requests enough results to cover the complete set.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/rest-client/rest-client.test.ts`:
- Around line 285-319: Update the regression tests around searchPackages in
src/lib/rest-client/rest-client.test.ts:285-319 and 321-340. Make the mockFetch
response honor each request’s limit and offset so page-sized partial fetches
cannot satisfy the expected results; in the second test, use a dataset larger
than the normal page size and assert every batch request requests enough results
to cover the complete set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f8de1fcd-a856-4224-a1f3-f61d5f2a9888
📒 Files selected for processing (2)
src/lib/rest-client/rest-client.test.tssrc/lib/rest-client/rest-client.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/rest-client/rest-client.ts
RDPerera
left a comment
There was a problem hiding this comment.
PR body's fourth bullet says the sidebar entry was renamed HRMS → Human Resources. Seems that is missing or re-aligned for ealier state for some reason ?
I reverted the change, as we can't change the category now |
Summary
Fixes #1853 and two related filtering bugs found while auditing search/filter behavior against the live Ballerina Central search API.
escapeSolrQueryescaped the space character, turning a query like "SAP Business" into the wildcard term*sap\ business*. The API's*...*wildcard is a literal, non-analyzed pattern, so the escaped backslash-space never matches. Space is no longer escaped.&(e.g. "Finance & Accounting") make the API's query parser return 0 results when followed by anotherAND keyword:...clause. Verified live that the identical clauses in the opposite order parse correctly, so Area filters are now always added last in the built query.keyword:field isn't an exact match — it can silently mix in wrong connectors. Verified live thatkeyword:Vendor/OpenAIalso matchesazure.openai.text(real vendor: Microsoft), purely because it carries an unrelated bare keyword"Azure OpenAI". The same effect made a "Finance & Accounting" + "Microsoft" filter return 30 connectors instead of the correct 11, including several from unrelated areas (Accounts Payable, Tax, HR, etc). AddedfilterByExactKeywords(), a client-side post-filter that re-checks the exactArea//Vendor//Type/tag, mirroring the existingexcludeHidden/filterByRelevancepost-processing pattern.Area/Human Resources(0 packages haveArea/HRMS, 2 haveArea/Human Resources). Renamed the hardcoded sidebar entry.All of the above were verified directly against the live
api.central.ballerina.iosearch endpoint before and after the fix, not just against mocked tests.Test plan
npm run typecheckpassesnpm run lintpassesnpm test— 87 tests pass, including 3 new regression tests:azure.openai.textunder aVendor/OpenAIfilter) is dropped byfilterByExactKeywords🤖 Generated with Claude Code