-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(explore): Allow only saved metrics and columns #27539
fix(explore): Allow only saved metrics and columns #27539
Conversation
col => col.column_name === (item.value as ColumnMeta).column_name, | ||
); | ||
const columnName = (item.value as ColumnMeta).column_name; | ||
return availableColumnSet.has(columnName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thank you!
}, | ||
); | ||
expect( | ||
await screen.findByText('Drop columns/metrics here or click'), | ||
).toBeInTheDocument(); | ||
}); | ||
|
||
test('cannot drop a column that is not part of the simple column selection', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #27539 +/- ##
==========================================
+ Coverage 69.73% 69.79% +0.06%
==========================================
Files 1909 1909
Lines 74692 74722 +30
Branches 8325 8334 +9
==========================================
+ Hits 52086 52152 +66
+ Misses 20556 20517 -39
- Partials 2050 2053 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
SUMMARY
Airbnb had previously overridden an option to disable ad-hoc metrics with commit #17202.
However, with the D&D option now enabled, users can accidentally drop an ad-hoc metric, thus gaining access to the previously disabled (tab) option.
To address this, the commit includes a condition to check whether the item being dropped is among the saved metrics, preventing such cases.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
before--adhoc-metric-drop.mov
After:
after--adhoc-metric-drop.mov
TESTING INSTRUCTIONS
Set a dataset extra with `{ "disallow_adhoc_metrics": true }'
Create a chart with the dataset and then try to drop a column to "Metric" which should be blocked
ADDITIONAL INFORMATION