feat(ourlogs): add 'all columns' option to export modal#116192
feat(ourlogs): add 'all columns' option to export modal#116192JoshuaKGoldberg wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
Scope: Frontend
📊 Type Coverage Diff
🔍 1 new type safety issue introducedType assertions (
This is informational only and does not block the PR. |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0c8ea05. Configure here.
| formatNumber(ROW_COUNT_VALUE_SYNC_LIMIT) | ||
| )} | ||
| {columnsValue === 'all' | ||
| ? t('Your file will be sent to your email address.') |
There was a problem hiding this comment.
All columns promises email incorrectly
Medium Severity
The modal incorrectly states that files will be emailed when 'All columns' is selected. For logs, the trace_item_full_export often completes synchronously, resulting in an immediate browser download instead of an email notification.
Reviewed by Cursor Bugbot for commit 0c8ea05. Configure here.
| if (passedSyncLimit) { | ||
| if (isAllColumns || passedSyncLimit) { | ||
| await handleDataExport({ | ||
| format: value.format, |
There was a problem hiding this comment.
Analytics misreports exported columns
Low Severity
When exporting "All columns", the explore.table_exported analytics event still includes the original queryInfo.field (selected columns). This means analytics incorrectly reports the table's selected columns instead of reflecting a full-column export, despite the actual export request being configured for all columns.
Reviewed by Cursor Bugbot for commit 0c8ea05. Configure here.
| queryInfo: isAllColumns ? {...payload.queryInfo, field: []} : payload.queryInfo, | ||
| queryType: isAllColumns | ||
| ? ExportQueryType.TRACE_ITEM_FULL_EXPORT | ||
| : ExportQueryType.EXPLORE, |
There was a problem hiding this comment.
FYI @manessaraj - I just want to make sure I haven't messed up this data format again?
There was a problem hiding this comment.
Todo: we actually do have designs for this, I'll tweak this UI to match the Figma (https://www.figma.com/design/zwH7VEk2Ff18WfFoT9J66s/%F0%9F%94%8D-Explore?node-id=2458-23335&t=JUopNRF0c8TFt6Q9-0).
1 participant
Closes LOGS-711.