feat: metric event name combobox fed by warehouse events - #8234
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughThe change adds request and response contracts for warehouse connection events and exposes a query hook. It adds a creatable event selector that loads events, builds options, and warns when an event is not known by a supported warehouse. The metric creation form now uses this selector instead of a free-text event input. Unit tests cover event option creation and unknown-event detection. Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
github-actions
Bot
added
front-end
Docker builds report
|
✅ private-cloud · depot-ubuntu-latest-16 — run #19080 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19080 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19080 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19080 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19078 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19078 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19078 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19078 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19061 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19061 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19061 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19061 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
ed02717 to
cc6c91d
Compare
github-actions
Bot
added
feature
cc6c91d to
2d7400f
Compare
github-actions
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4c4f1803-d41f-49fd-86be-0edc0352005e
📒 Files selected for processing (9)
frontend/common/services/useWarehouseConnection.tsfrontend/common/types/requests.tsfrontend/common/types/responses.tsfrontend/web/components/experiments/CreateMetricForm/CreateMetricForm.tsxfrontend/web/components/experiments/EventNameSelect/EventNameSelect.scssfrontend/web/components/experiments/EventNameSelect/EventNameSelect.tsxfrontend/web/components/experiments/EventNameSelect/__tests__/utils.test.tsfrontend/web/components/experiments/EventNameSelect/index.tsfrontend/web/components/experiments/EventNameSelect/utils.ts
| getWarehouseConnectionEvents: builder.query< | ||
| Res['warehouseConnectionEvents'], | ||
| Req['getWarehouseConnectionEvents'] | ||
| >({ | ||
| providesTags: [{ id: 'EVENTS', type: 'WarehouseConnection' }], | ||
| query: ({ environmentId, id }) => ({ | ||
| url: `environments/${environmentId}/warehouse-connections/${id}/events/`, | ||
| }), | ||
| }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Invalidate the event-list cache after a connection update.
getWarehouseConnectionEvents provides WarehouseConnection:EVENTS, but updateWarehouseConnection invalidates only WarehouseConnection:LIST. If an administrator changes the connection configuration or credentials, an open metric form can continue to show event names from the previous source.
Add the EVENTS tag to updateWarehouseConnection.invalidatesTags.
Proposed fix
- invalidatesTags: [{ id: 'LIST', type: 'WarehouseConnection' }],
+ invalidatesTags: [
+ { id: 'LIST', type: 'WarehouseConnection' },
+ { id: 'EVENTS', type: 'WarehouseConnection' },
+ ],| const options = useMemo(() => buildEventOptions(data?.events), [data?.events]) | ||
| const showWarning = isSuccess && isUnknownEvent(value, data?.events) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not show an unknown-event warning for a truncated result.
When data.is_truncated is true, an event absent from data.events can still exist in the warehouse. The current condition incorrectly tells users that the warehouse has not received that event.
Only show this warning when the response is complete. Add a component test for a truncated response.
Proposed fix
- const showWarning = isSuccess && isUnknownEvent(value, data?.events)
+ const showWarning =
+ isSuccess &&
+ !data?.is_truncated &&
+ isUnknownEvent(value, data?.events)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const options = useMemo(() => buildEventOptions(data?.events), [data?.events]) | |
| const showWarning = isSuccess && isUnknownEvent(value, data?.events) | |
| const options = useMemo(() => buildEventOptions(data?.events), [data?.events]) | |
| const showWarning = | |
| isSuccess && | |
| !data?.is_truncated && | |
| isUnknownEvent(value, data?.events) |
1 participant
docs/if required so people know about the feature.Changes
Stacked on #8231.
Turns the Event name field in metric creation into a combo box listing the distinct event names from the environment's connected warehouse, most recently seen first.
EventNameSelectcomponent wrapping react-select'sCreatableSelect: type-to-filter, free text kept via aUse "xyz"option, and an inline warning when the chosen event hasn't been received by the warehouse yet.getWarehouseConnectionEventsRTK Query endpoint and response types; the edit-metric flow inherits the combo box.is_truncatedflag is the hook for server-side narrowing later.How did you test this code?