ayush-sharaf opened a new pull request, #42563:
URL: https://github.com/apache/superset/pull/42563
### SUMMARY
Drill By is instrumented with four log events (added in #23854):
`drill_by_modal_opened`, `further_drill_by`, `drill_by_edit_chart` and
`drill_by_breadcrumb_clicked`. Its sibling feature **Drill to Detail** has
none, so there is no way to tell from the `logs` table that a user opened Drill
to Detail, or on which chart.
It cannot be reconstructed server-side either. Opening the modal issues
exactly one request, `POST /datasource/samples`, whose body is built by
`getDrillPayload()` as `{granularity, time_range, filters, extras}` and whose
query args are `datasource_type`, `datasource_id`, `dashboard_id`, `force`,
`page`, `per_page` — none of which reference a chart. Because one dataset
commonly backs many charts on the same dashboard, `datasource_id +
dashboard_id` is not enough to identify the chart.
`DatasetRestApi.get_drill_info` does write a log row, but it is not a drill
signal: `useDatasetDrillInfo` prefetches it when the chart *renders* (gated
only on `canDrillToDetail`), so it emits one row per dataset per dashboard load
even when the user never drills, and its payload carries only `pk` (the dataset
id) and `rison.dashboard_id`.
This PR adds `LOG_ACTIONS_DRILL_TO_DETAIL_MODAL_OPENED`
(`drill_to_detail_modal_opened`), dispatched from `DrillDetailModal` with
`slice_id`, mirroring `DrillByModal`. Because both entry points — the chart
header ⋮ menu and the right-click context menu — render this same modal, one
dispatch covers both.
**Implementation note.** Unlike `DrillByModal`, which mounts when it is
opened, `DrillDetailModal` stays mounted for the lifetime of the chart and is
toggled via its `showModal` prop. The effect is therefore gated on `showModal`
rather than firing on mount, so no event is emitted while the modal is closed.
Reopening the modal emits a new event, which is the intended behaviour.
`DrillDetailModal` already receives `chartId` as a prop and already resolves
the chart name from `sliceEntities` to render its own title, so no new data
plumbing was needed.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — no UI change.
### TESTING INSTRUCTIONS
Two unit tests are added to `DrillDetailModal.test.tsx`:
- `should log an event when the modal is opened` — asserts `logEvent` is
called with `drill_to_detail_modal_opened` and `{ slice_id }`.
- `should not log an event while the modal is closed` — renders with
`showModal={false}` and asserts nothing is logged.
```
npm run test -- src/components/Chart/DrillDetail/DrillDetailModal.test.tsx
```
Manually:
1. Open a dashboard containing a chart built on a drillable dataset.
2. Open the chart's ⋮ menu and click **Drill to detail** (and separately,
right-click a data point and use **Drill to detail by**).
3. Confirm a `drill_to_detail_modal_opened` event is recorded with the
correct `slice_id` — visible in the `/superset/log/` request payload, or in the
`logs` table with `EVENT_LOGGER` configured.
4. Load the dashboard without drilling and confirm no such event is emitted.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #42562
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
