Added lazy loading for enum select field in the advanced search#29736
Conversation
Rohit0301
added
the
safe to test
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await lazyLoadTable.delete(apiContext); | ||
| } finally { | ||
| await afterAction(); | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| test( | ||
| 'should lazy-load enum options on scroll in the advanced filter value dropdown', | ||
| async ({ page }) => { | ||
| test.slow(); | ||
|
|
||
| await redirectToHomePage(page); | ||
| await sidebarClick(page, SidebarItem.EXPLORE); | ||
| await showAdvancedSearchDialog(page); | ||
|
|
||
| const ruleLocator = page.locator('.rule').nth(0); |
There was a problem hiding this comment.
afterAll cleanup
The afterAll hook deletes lazyLoadTable but never removes the enumCPName custom property that was registered on the table metadata type in beforeAll. Every CI run will therefore accumulate orphaned enum-lazy-{uuid} custom properties on the system-wide table type, which could affect other tests that enumerate custom properties or render the advanced-search field list.
🟡 Playwright Results — all passed (25 flaky)✅ 4512 passed · ❌ 0 failed · 🟡 25 flaky · ⏭️ 55 skipped
🟡 25 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
…lter Replaces the static listValues render with asyncFetch + useLoadMore for enum custom properties. Opens the dropdown instantly with the first 100 options, adds a "Load more..." button for pagination, and supports searching across all values without clicking Load more. Adds Playwright tests covering the Load more button flow (click → scroll → page-2 item visible) and the search-across-pages flow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
Jest test CoverageUI tests summary
|
|
Code Review ✅ Approved 5 resolved / 5 findingsIntroduces lazy-loaded, paginated enum select fields for advanced search, but fails to clean up custom properties in the E2E test suite and lacks proper debounce cleanup in the new widget. ✅ 5 resolved✅ Quality: New factory uses
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
* Added lazy loading for enum select field in the advanced search * fix(ui): lazy load enum custom property options in advanced search filter Replaces the static listValues render with asyncFetch + useLoadMore for enum custom properties. Opens the dropdown instantly with the first 100 options, adds a "Load more..." button for pagination, and supports searching across all values without clicking Load more. Adds Playwright tests covering the Load more button flow (click → scroll → page-2 item visible) and the search-across-pages flow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * lint fix * fixed unit test * fix test * lint fix --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
3 participants
Describe your changes:
Screen.Recording.2026-07-06.at.12.57.53.PM.mov
Fixes 4818
I worked on ... because ...
Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Summary by Gitar
EnumAsyncSelectWidgetto support lazy-loading and pagination for enum fields in advanced search.AdvancedSearchClassBaseto useEnumAsyncSelectWidgetfor enum custom properties, replacing staticlistValueswith an async fetch implementation.buildEnumAsyncFetchand updated existing search configuration tests.This will update automatically on new commits.
Greptile Summary
This PR adds lazy-loading pagination for enum custom property fields in the advanced search filter, replacing the static
listValuesapproach with anasyncFetchfunction that returns 100 values per page with optional text-based filtering.buildEnumAsyncFetchis introduced onAdvancedSearchClassBaseas a closure over the enum values array; it filters by search query and slices by offset, returning ahasMoreflag to drive the "Load more" UI affordance.case 'enum'branch ingetCustomPropertiesSubFieldsnow setsasyncFetch,useAsyncSearch: true, anduseLoadMore: trueinstead of callinggetCustomPropertyAdvanceSearchEnumOptions.buildEnumAsyncFetchcover pagination, filtering, and edge cases; a new Playwright suite verifies the "Load more" button and search-through-all-values behaviour end-to-end.Confidence Score: 4/5
Mostly safe to merge; the unsafe property access on
config.valueswhencustomPropertyConfigis absent can crash the advanced-search panel for any enum field missing its config object.The core lazy-loading logic in
buildEnumAsyncFetchis clean and well-tested. The one defect flagged in a previous review thread remains unaddressed:(field.customPropertyConfig?.config as CustomPropertyEnumConfig).valuesthrows a TypeError before the?? []fallback can fire whencustomPropertyConfigis absent, which would crash the advanced-search panel for any enum property whose config has not been populated.AdvancedSearchClassBase.ts — specifically the
case 'enum'block at theenumValuesassignment.Important Files Changed
buildEnumAsyncFetchand switches theenumcustom property case to asyncFetch/useLoadMore; contains an unsafe property access onconfig.valuesthat will throw whencustomPropertyConfigis absent.buildEnumAsyncFetchunit tests covering pagination, filtering, and edge cases.enumCPIdis captured but never referenced again, and the test block has noafterAllto clean up either the created table or the enum custom property.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[getCustomPropertiesSubFields\ncalled for enum field] --> B{customPropertyConfig\nexists?} B -- yes --> C[Access .config.values] B -- no --> X[TypeError thrown\nfallback unreachable] C --> D[buildEnumAsyncFetch\nwraps values in closure] D --> E[Field registered with\nasyncFetch and useLoadMore] E --> G[User opens dropdown] G --> H[asyncFetch called\nsearch empty, offset 0] H --> I{Search query\npresent?} I -- yes --> J[Filter by\ncase-insensitive substring] I -- no --> K[Use full values array] J --> L[Slice offset to offset+100] K --> L L --> M[Return page and hasMore] M --> N{hasMore true?} N -- yes --> O[Show Load more button] N -- no --> P[End of list] O --> Q[User clicks Load more\noffset increments by 100] Q --> H%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[getCustomPropertiesSubFields\ncalled for enum field] --> B{customPropertyConfig\nexists?} B -- yes --> C[Access .config.values] B -- no --> X[TypeError thrown\nfallback unreachable] C --> D[buildEnumAsyncFetch\nwraps values in closure] D --> E[Field registered with\nasyncFetch and useLoadMore] E --> G[User opens dropdown] G --> H[asyncFetch called\nsearch empty, offset 0] H --> I{Search query\npresent?} I -- yes --> J[Filter by\ncase-insensitive substring] I -- no --> K[Use full values array] J --> L[Slice offset to offset+100] K --> L L --> M[Return page and hasMore] M --> N{hasMore true?} N -- yes --> O[Show Load more button] N -- no --> P[End of list] O --> Q[User clicks Load more\noffset increments by 100] Q --> HReviews (7): Last reviewed commit: "Merge branch 'main' into enum-lazyload" | Re-trigger Greptile