You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/10/22 18:48:36 UTC
[GitHub] [superset] john-bodley opened a new pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
john-bodley opened a new pull request #17202:
URL: https://github.com/apache/superset/pull/17202
### SUMMARY
There could be some instances where ad-hoc metrics for specific datasets should be disabled, i.e., the dataset owner specifically does not users to create ad-hoc non-sanctioned metrics. This PR adds—at the dataset level—a custom override for disabling ad-hoc metrics, i.e., removing the `SIMPLE` and `CUSTOM SQL` tabs.
Note for the native Druid connector I also removed the `CUSTOM SQL` tab (which housed a message as to why this was invalid) to ensure consistency with the ad-hoc filters.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
#### Disallowed
<img width="315" alt="Screen Shot 2021-10-22 at 11 29 21 AM" src="https://user-images.githubusercontent.com/4567245/138507423-ab3a5916-4675-4b75-b088-7b3b115e4c10.png">
#### Allowed (default)
<img width="316" alt="Screen Shot 2021-10-22 at 11 30 10 AM" src="https://user-images.githubusercontent.com/4567245/138507430-4d751d93-fe83-4a7e-8bc7-fd28715cd245.png">
### TESTING INSTRUCTIONS
Added unit tests.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-962140141
Thanks @etr2460 for the help with this PR. I _believe_ it is ready for review.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r745111750
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -337,6 +341,13 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
) &&
savedMetric?.metric_name !== propsSavedMetric?.metric_name);
+ let extra = {};
+ if (datasource?.extra) {
+ try {
+ extra = JSON.parse(datasource.extra);
+ } catch {} // eslint-disable-line no-empty
+ }
Review comment:
The point was exactly that having the API return a JSON object so this "pattern" does not have to be repeated everywhere `extra` is needed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r745111750
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -337,6 +341,13 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
) &&
savedMetric?.metric_name !== propsSavedMetric?.metric_name);
+ let extra = {};
+ if (datasource?.extra) {
+ try {
+ extra = JSON.parse(datasource.extra);
+ } catch {} // eslint-disable-line no-empty
+ }
Review comment:
The point was having the API return a JSON object so this "pattern" does not need to be repeated everywhere `extra` is needed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r741599470
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
##########
@@ -32,7 +32,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment about if datasource is always here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -397,30 +411,26 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
key={EXPRESSION_TYPES.SQL}
tab={t('Custom SQL')}
Review comment:
same comment here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
##########
@@ -34,7 +34,7 @@ const propTypes = {
savedMetrics: PropTypes.arrayOf(savedMetricType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
multi: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
should we add a tooltip on the tab with details on why it's disabled?
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
##########
@@ -33,7 +33,7 @@ export type AdhocMetricPopoverTriggerProps = {
columns: { column_name: string; type: string }[];
savedMetricsOptions: savedMetricType[];
savedMetric: savedMetricType;
- datasourceType: string;
+ datasource: Datasource;
Review comment:
same comment about if this should be `datasource: Datasource` or `datasource?: Datasource`
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
##########
@@ -48,7 +48,7 @@ const propTypes = {
isLoading: PropTypes.bool,
multi: PropTypes.bool,
clearable: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -50,7 +50,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
if datasource is marked as `isRequired` then you shouldn't need the `datasource?.type` below and can just do `datasource.type`. But if datasource isn't passed in sometimes, then you should remove the `isRequired` part
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743124163
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
@etr2460 could you be more explicit? I'm not sure how we display a tooltip on a tab which is disabled and thus not accessible.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (777936f) into [master](https://codecov.io/gh/apache/superset/commit/80a459f43bf27b29ce71f343f98e189d283cc52b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (80a459f) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
==========================================
+ Coverage 76.93% 76.95% +0.01%
==========================================
Files 1039 1038 -1
Lines 55583 55597 +14
Branches 7577 7576 -1
==========================================
+ Hits 42763 42782 +19
+ Misses 12570 12567 -3
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <100.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.87% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `80.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (ø)` | |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `85.60% <100.00%> (+2.14%)` | :arrow_up: |
| [...erset-frontend/src/components/Pagination/index.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUGFnaW5hdGlvbi9pbmRleC50c3g=) | `72.72% <0.00%> (-27.28%)` | :arrow_down: |
| [...frontend/src/dashboard/components/Header/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci9pbmRleC5qc3g=) | `68.30% <0.00%> (-0.41%)` | :arrow_down: |
| [...et-frontend/src/components/TableSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci9pbmRleC50c3g=) | `74.28% <0.00%> (-0.25%)` | :arrow_down: |
| ... and [31 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [80a459f...777936f](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743762734
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
Meaning a tooltip that is displayed when you hover over the disabled tab selector. something like this? maybe with different content depending on why it's disabled?
```tsx
tab={<Tooltip content={t('You may not define ad hoc metrics because it is disabled by the dataset owner.')}>{t('Simple')}</Tooltip>
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (777936f) into [master](https://codecov.io/gh/apache/superset/commit/37909aace0b8968475c2c731891cd9dc52f8de85?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37909aa) will **decrease** coverage by `0.07%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 777936f differs from pull request most recent head c5f1a86. Consider uploading reports for the commit c5f1a86 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
==========================================
- Coverage 77.02% 76.95% -0.08%
==========================================
Files 1037 1038 +1
Lines 55629 55597 -32
Branches 7594 7576 -18
==========================================
- Hits 42848 42782 -66
- Misses 12531 12567 +36
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <100.00%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.87% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `80.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (ø)` | |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `85.60% <100.00%> (+2.14%)` | :arrow_up: |
| [superset-frontend/src/CRUD/Field.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQudHN4) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Fieldset.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGRzZXQuanN4) | `85.71% <0.00%> (-14.29%)` | :arrow_down: |
| [superset/annotation\_layers/annotations/schemas.py](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvYW5ub3RhdGlvbl9sYXllcnMvYW5ub3RhdGlvbnMvc2NoZW1hcy5weQ==) | `86.66% <0.00%> (-13.34%)` | :arrow_down: |
| ... and [24 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ce558e0...c5f1a86](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6366b5b) into [master](https://codecov.io/gh/apache/superset/commit/fa44325a368dab45f24204c2f8241a1c7afa3d86?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fa44325) will **increase** coverage by `0.00%`.
> The diff coverage is `73.33%`.
> :exclamation: Current head 6366b5b differs from pull request most recent head a3df424. Consider uploading reports for the commit a3df424 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
=======================================
Coverage 77.09% 77.09%
=======================================
Files 1037 1037
Lines 55650 55637 -13
Branches 7603 7596 -7
=======================================
- Hits 42903 42895 -8
+ Misses 12497 12494 -3
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.26% <73.33%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `61.76% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...components/controls/FixedOrMetricControl/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaXhlZE9yTWV0cmljQ29udHJvbC9pbmRleC5qc3g=) | `83.92% <ø> (ø)` | |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `75.00% <ø> (ø)` | |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `100.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `38.18% <ø> (ø)` | |
| [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `30.55% <ø> (+0.82%)` | :arrow_up: |
| [...frontend/src/views/CRUD/alert/AlertReportModal.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvQWxlcnRSZXBvcnRNb2RhbC50c3g=) | `60.70% <20.00%> (-0.44%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fa44325...a3df424](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r734816035
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,58 +378,57 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
- <FormItem label={t('column')}>
- <Select
- options={columns.map(column => ({
- value: column.column_name,
- label: column.verbose_name || column.column_name,
- key: column.id,
- customLabel: this.renderColumnOption(column),
- }))}
- {...columnSelectProps}
- />
- </FormItem>
- <FormItem label={t('aggregate')}>
- <Select
- options={AGGREGATES_OPTIONS.map(option => ({
- value: option,
- label: option,
- key: option,
- }))}
- {...aggregateSelectProps}
- />
- </FormItem>
- </Tabs.TabPane>
- <Tabs.TabPane
- key={EXPRESSION_TYPES.SQL}
- tab={t('Custom SQL')}
- data-test="adhoc-metric-edit-tab#custom"
- >
- {this.props.datasourceType !== 'druid' ? (
- <SQLEditor
- data-test="sql-editor"
- showLoadingForImport
- ref={this.handleAceEditorRef}
- keywords={keywords}
- height={`${this.state.height - 80}px`}
- onChange={this.onSqlExpressionChange}
- width="100%"
- showGutter={false}
- value={
- adhocMetric.sqlExpression || adhocMetric.translateToSql()
- }
- editorProps={{ $blockScrolling: true }}
- enableLiveAutocompletion
- className="filter-sql-editor"
- wrapEnabled
- />
- ) : (
- <div className="custom-sql-disabled-message">
- Custom SQL Metrics are not available on druid datasources
- </div>
+ {extra.disallow_adhoc_metrics !== true && (
+ <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
Review comment:
Either way works, though I feel we need consistency with the ad-hoc filters.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (777936f) into [master](https://codecov.io/gh/apache/superset/commit/fa44325a368dab45f24204c2f8241a1c7afa3d86?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fa44325) will **decrease** coverage by `0.14%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 777936f differs from pull request most recent head 1a5be9a. Consider uploading reports for the commit 1a5be9a to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
==========================================
- Coverage 77.09% 76.95% -0.15%
==========================================
Files 1037 1038 +1
Lines 55650 55597 -53
Branches 7603 7576 -27
==========================================
- Hits 42903 42782 -121
- Misses 12497 12567 +70
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <100.00%> (-0.25%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.87% <ø> (-19.89%)` | :arrow_down: |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `80.00% <ø> (-20.00%)` | :arrow_down: |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (+3.87%)` | :arrow_up: |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `85.60% <100.00%> (+2.14%)` | :arrow_up: |
| [superset-frontend/src/CRUD/Field.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQudHN4) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| ... and [29 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fa44325...1a5be9a](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-957106627
@etr2460 this is ready for rearview. Note the state of the components within disabled tabs is somewhat atypical, i.e., non disabled, however this will only occur if a dataset is disabled after the fact. Note existing ad-hoc metrics will be grandfathered in, i.e., the chart will remain functional, which is desirable.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley merged pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #17202:
URL: https://github.com/apache/superset/pull/17202
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743929535
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -337,6 +341,13 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
) &&
savedMetric?.metric_name !== propsSavedMetric?.metric_name);
+ let extra = {};
+ if (datasource?.extra) {
+ try {
+ extra = JSON.parse(datasource.extra);
+ } catch {} // eslint-disable-line no-empty
+ }
Review comment:
Can the API return `extra` as a JSON object itself? Even if the API doesn't, this feels like something should be handled earlier than in the metric popover.
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +381,23 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={
+ extra.disallow_adhoc_metrics ? (
+ <Tooltip
+ title={t(
+ 'Simple ad-hoc metrics are not enabled for this dataset',
Review comment:
How about use the same message for both "SIMPLE" and "CUSTOM SQL":
```
Ad-hoc metrics are not supported by this dataset, use saved metrics instead.
```
Having different messages kind of means one can disallow SIMPLE and CUSTOM SQL separately.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-957106627
@etr2460 this is ready for rearview. Note the state of the components within disabled tabs is somewhat atypical, i.e., non disabled, however this will only occur if a dataset is disabled after the fact. Note existing ad-hoc metrics will be grandfathered in, i.e., the chart will remain functional, which is desirable.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743961441
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +381,23 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={
+ extra.disallow_adhoc_metrics ? (
+ <Tooltip
+ title={t(
+ 'Simple ad-hoc metrics are not enabled for this dataset',
Review comment:
How about use the same message for both "SIMPLE" and "CUSTOM SQL":
```
Ad-hoc metrics are not supported by this dataset, use saved metrics instead.
```
Having different messages kind of implies one can disallow SIMPLE and CUSTOM SQL separately.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r737017754
##########
File path: superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx
##########
@@ -173,90 +168,66 @@ export default class AdhocFilterEditPopover extends React.Component {
onResize,
datasource,
partitionColumn,
- sections = ['SIMPLE', 'CUSTOM_SQL'],
theme,
operators,
...popoverProps
} = this.props;
const { adhocFilter } = this.state;
-
- const resultSections =
- datasource?.type === 'druid'
- ? sections.filter(s => s !== 'CUSTOM_SQL')
- : sections;
-
const stateIsValid = adhocFilter.isValid();
const hasUnsavedChanges = !adhocFilter.equals(propsAdhocFilter);
- const sectionRenders = {};
-
- sectionRenders.CUSTOM_SQL = (
Review comment:
Moved to be inline.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r734812546
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,58 +378,57 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
- <FormItem label={t('column')}>
- <Select
- options={columns.map(column => ({
- value: column.column_name,
- label: column.verbose_name || column.column_name,
- key: column.id,
- customLabel: this.renderColumnOption(column),
- }))}
- {...columnSelectProps}
- />
- </FormItem>
- <FormItem label={t('aggregate')}>
- <Select
- options={AGGREGATES_OPTIONS.map(option => ({
- value: option,
- label: option,
- key: option,
- }))}
- {...aggregateSelectProps}
- />
- </FormItem>
- </Tabs.TabPane>
- <Tabs.TabPane
- key={EXPRESSION_TYPES.SQL}
- tab={t('Custom SQL')}
- data-test="adhoc-metric-edit-tab#custom"
- >
- {this.props.datasourceType !== 'druid' ? (
- <SQLEditor
- data-test="sql-editor"
- showLoadingForImport
- ref={this.handleAceEditorRef}
- keywords={keywords}
- height={`${this.state.height - 80}px`}
- onChange={this.onSqlExpressionChange}
- width="100%"
- showGutter={false}
- value={
- adhocMetric.sqlExpression || adhocMetric.translateToSql()
- }
- editorProps={{ $blockScrolling: true }}
- enableLiveAutocompletion
- className="filter-sql-editor"
- wrapEnabled
- />
- ) : (
- <div className="custom-sql-disabled-message">
- Custom SQL Metrics are not available on druid datasources
- </div>
+ {extra.disallow_adhoc_metrics !== true && (
+ <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
Review comment:
how do you feel about keeping the tabs visible, but instead setting `disabled={extra.disallow_adhoc_metrics}`?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6366b5b) into [master](https://codecov.io/gh/apache/superset/commit/fa44325a368dab45f24204c2f8241a1c7afa3d86?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fa44325) will **increase** coverage by `0.00%`.
> The diff coverage is `73.33%`.
> :exclamation: Current head 6366b5b differs from pull request most recent head da996e0. Consider uploading reports for the commit da996e0 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
=======================================
Coverage 77.09% 77.09%
=======================================
Files 1037 1037
Lines 55650 55637 -13
Branches 7603 7596 -7
=======================================
- Hits 42903 42895 -8
+ Misses 12497 12494 -3
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.26% <73.33%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `61.76% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...components/controls/FixedOrMetricControl/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaXhlZE9yTWV0cmljQ29udHJvbC9pbmRleC5qc3g=) | `83.92% <ø> (ø)` | |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `75.00% <ø> (ø)` | |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `100.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `38.18% <ø> (ø)` | |
| [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `30.55% <ø> (+0.82%)` | :arrow_up: |
| [...frontend/src/views/CRUD/alert/AlertReportModal.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvQWxlcnRSZXBvcnRNb2RhbC50c3g=) | `60.70% <20.00%> (-0.44%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fa44325...da996e0](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743124163
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
@etr2460 could you be more explicit? I'm not sure how we display a tooltip on a tab which is disabled and thus not accessible.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743762734
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
Meaning a tooltip that is displayed when you hover over the disabled tab selector. something like this? maybe with different content depending on why it's disabled?
```tsx
tab={<Tooltip content={t('You may not define ad hoc metrics because it is disabled by the dataset owner.')}>{t('Simple')}</Tooltip>
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r744933113
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +381,23 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={
+ extra.disallow_adhoc_metrics ? (
+ <Tooltip
+ title={t(
+ 'Simple ad-hoc metrics are not enabled for this dataset',
Review comment:
@ktmud the reason for the slightly different messages is the Druid NoSQL connector also disallows the `CUSTOM SQL` tab and thus I wanted to ensure consistency between those two experiences.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r734838842
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,58 +378,57 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
- <FormItem label={t('column')}>
- <Select
- options={columns.map(column => ({
- value: column.column_name,
- label: column.verbose_name || column.column_name,
- key: column.id,
- customLabel: this.renderColumnOption(column),
- }))}
- {...columnSelectProps}
- />
- </FormItem>
- <FormItem label={t('aggregate')}>
- <Select
- options={AGGREGATES_OPTIONS.map(option => ({
- value: option,
- label: option,
- key: option,
- }))}
- {...aggregateSelectProps}
- />
- </FormItem>
- </Tabs.TabPane>
- <Tabs.TabPane
- key={EXPRESSION_TYPES.SQL}
- tab={t('Custom SQL')}
- data-test="adhoc-metric-edit-tab#custom"
- >
- {this.props.datasourceType !== 'druid' ? (
- <SQLEditor
- data-test="sql-editor"
- showLoadingForImport
- ref={this.handleAceEditorRef}
- keywords={keywords}
- height={`${this.state.height - 80}px`}
- onChange={this.onSqlExpressionChange}
- width="100%"
- showGutter={false}
- value={
- adhocMetric.sqlExpression || adhocMetric.translateToSql()
- }
- editorProps={{ $blockScrolling: true }}
- enableLiveAutocompletion
- className="filter-sql-editor"
- wrapEnabled
- />
- ) : (
- <div className="custom-sql-disabled-message">
- Custom SQL Metrics are not available on druid datasources
- </div>
+ {extra.disallow_adhoc_metrics !== true && (
+ <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
Review comment:
@etr2460 yes it removed the tab, but I agree that disablement is fine for consistency. Also the Druid NoSQL connector is rarely used/supported and slated for deprecation in Superset 2.0.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r741599470
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
##########
@@ -32,7 +32,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment about if datasource is always here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -397,30 +411,26 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
key={EXPRESSION_TYPES.SQL}
tab={t('Custom SQL')}
Review comment:
same comment here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
##########
@@ -34,7 +34,7 @@ const propTypes = {
savedMetrics: PropTypes.arrayOf(savedMetricType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
multi: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
should we add a tooltip on the tab with details on why it's disabled?
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
##########
@@ -33,7 +33,7 @@ export type AdhocMetricPopoverTriggerProps = {
columns: { column_name: string; type: string }[];
savedMetricsOptions: savedMetricType[];
savedMetric: savedMetricType;
- datasourceType: string;
+ datasource: Datasource;
Review comment:
same comment about if this should be `datasource: Datasource` or `datasource?: Datasource`
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
##########
@@ -48,7 +48,7 @@ const propTypes = {
isLoading: PropTypes.bool,
multi: PropTypes.bool,
clearable: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -50,7 +50,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
if datasource is marked as `isRequired` then you shouldn't need the `datasource?.type` below and can just do `datasource.type`. But if datasource isn't passed in sometimes, then you should remove the `isRequired` part
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r741599470
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
##########
@@ -32,7 +32,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment about if datasource is always here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -397,30 +411,26 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
key={EXPRESSION_TYPES.SQL}
tab={t('Custom SQL')}
Review comment:
same comment here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
##########
@@ -34,7 +34,7 @@ const propTypes = {
savedMetrics: PropTypes.arrayOf(savedMetricType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
multi: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
should we add a tooltip on the tab with details on why it's disabled?
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
##########
@@ -33,7 +33,7 @@ export type AdhocMetricPopoverTriggerProps = {
columns: { column_name: string; type: string }[];
savedMetricsOptions: savedMetricType[];
savedMetric: savedMetricType;
- datasourceType: string;
+ datasource: Datasource;
Review comment:
same comment about if this should be `datasource: Datasource` or `datasource?: Datasource`
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
##########
@@ -48,7 +48,7 @@ const propTypes = {
isLoading: PropTypes.bool,
multi: PropTypes.bool,
clearable: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -50,7 +50,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
if datasource is marked as `isRequired` then you shouldn't need the `datasource?.type` below and can just do `datasource.type`. But if datasource isn't passed in sometimes, then you should remove the `isRequired` part
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (777936f) into [master](https://codecov.io/gh/apache/superset/commit/fa44325a368dab45f24204c2f8241a1c7afa3d86?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fa44325) will **decrease** coverage by `0.06%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 777936f differs from pull request most recent head c85f8af. Consider uploading reports for the commit c85f8af to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
==========================================
- Coverage 77.01% 76.95% -0.07%
==========================================
Files 1037 1038 +1
Lines 55650 55597 -53
Branches 7603 7576 -27
==========================================
- Hits 42859 42782 -77
- Misses 12541 12567 +26
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <100.00%> (-0.25%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.87% <ø> (-19.89%)` | :arrow_down: |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `80.00% <ø> (-20.00%)` | :arrow_down: |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (+3.87%)` | :arrow_up: |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `85.60% <100.00%> (+2.14%)` | :arrow_up: |
| [superset-frontend/src/CRUD/Field.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQudHN4) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| ... and [30 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fa44325...c85f8af](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r745111750
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -337,6 +341,13 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
) &&
savedMetric?.metric_name !== propsSavedMetric?.metric_name);
+ let extra = {};
+ if (datasource?.extra) {
+ try {
+ extra = JSON.parse(datasource.extra);
+ } catch {} // eslint-disable-line no-empty
+ }
Review comment:
The point was exactly that having the API return a JSON object so this "pattern" does not need to be repeated everywhere `extra` is needed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-962140141
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r734820663
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,58 +378,57 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
- <FormItem label={t('column')}>
- <Select
- options={columns.map(column => ({
- value: column.column_name,
- label: column.verbose_name || column.column_name,
- key: column.id,
- customLabel: this.renderColumnOption(column),
- }))}
- {...columnSelectProps}
- />
- </FormItem>
- <FormItem label={t('aggregate')}>
- <Select
- options={AGGREGATES_OPTIONS.map(option => ({
- value: option,
- label: option,
- key: option,
- }))}
- {...aggregateSelectProps}
- />
- </FormItem>
- </Tabs.TabPane>
- <Tabs.TabPane
- key={EXPRESSION_TYPES.SQL}
- tab={t('Custom SQL')}
- data-test="adhoc-metric-edit-tab#custom"
- >
- {this.props.datasourceType !== 'druid' ? (
- <SQLEditor
- data-test="sql-editor"
- showLoadingForImport
- ref={this.handleAceEditorRef}
- keywords={keywords}
- height={`${this.state.height - 80}px`}
- onChange={this.onSqlExpressionChange}
- width="100%"
- showGutter={false}
- value={
- adhocMetric.sqlExpression || adhocMetric.translateToSql()
- }
- editorProps={{ $blockScrolling: true }}
- enableLiveAutocompletion
- className="filter-sql-editor"
- wrapEnabled
- />
- ) : (
- <div className="custom-sql-disabled-message">
- Custom SQL Metrics are not available on druid datasources
- </div>
+ {extra.disallow_adhoc_metrics !== true && (
+ <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
Review comment:
what does the ad-hoc filters popover do? the approach you had here?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (777936f) into [master](https://codecov.io/gh/apache/superset/commit/b7e7ef283150837c2c1d78719c11e20424c67d87?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b7e7ef2) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 777936f differs from pull request most recent head b1f5698. Consider uploading reports for the commit b1f5698 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
==========================================
- Coverage 76.96% 76.95% -0.01%
==========================================
Files 1037 1038 +1
Lines 55608 55597 -11
Branches 7588 7576 -12
==========================================
- Hits 42796 42782 -14
- Misses 12562 12567 +5
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.87% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `80.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (ø)` | |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `85.60% <100.00%> (+2.14%)` | :arrow_up: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `78.09% <0.00%> (-3.80%)` | :arrow_down: |
| [...ponents/filterscope/renderFilterScopeTreeNodes.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2ZpbHRlcnNjb3BlL3JlbmRlckZpbHRlclNjb3BlVHJlZU5vZGVzLmpzeA==) | `86.66% <0.00%> (-0.84%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b7e7ef2...b1f5698](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-957106627
@etr2460 this is ready for rearview. Note the state of the components within disabled tabs is somewhat atypical, i.e., non disabled, however this will only occur if a dataset is disabled after the fact. Note existing ad-hoc metrics will be grandfathered in, i.e., the chart will remain functional, which is desirable.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r741599470
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
##########
@@ -32,7 +32,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment about if datasource is always here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -397,30 +411,26 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
key={EXPRESSION_TYPES.SQL}
tab={t('Custom SQL')}
Review comment:
same comment here
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
##########
@@ -34,7 +34,7 @@ const propTypes = {
savedMetrics: PropTypes.arrayOf(savedMetricType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
multi: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
should we add a tooltip on the tab with details on why it's disabled?
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
##########
@@ -33,7 +33,7 @@ export type AdhocMetricPopoverTriggerProps = {
columns: { column_name: string; type: string }[];
savedMetricsOptions: savedMetricType[];
savedMetric: savedMetricType;
- datasourceType: string;
+ datasource: Datasource;
Review comment:
same comment about if this should be `datasource: Datasource` or `datasource?: Datasource`
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
##########
@@ -48,7 +48,7 @@ const propTypes = {
isLoading: PropTypes.bool,
multi: PropTypes.bool,
clearable: PropTypes.bool,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
same comment
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -50,7 +50,7 @@ const propTypes = {
columns: PropTypes.arrayOf(columnType),
savedMetricsOptions: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
- datasourceType: PropTypes.string,
+ datasource: PropTypes.object.isRequired,
Review comment:
if datasource is marked as `isRequired` then you shouldn't need the `datasource?.type` below and can just do `datasource.type`. But if datasource isn't passed in sometimes, then you should remove the `isRequired` part
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743762734
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
Meaning a tooltip that is displayed when you hover over the disabled tab selector. something like this? maybe with different content depending on why it's disabled?
```tsx
tab={<Tooltip content={t('You may not define ad hoc metrics because it is disabled by the dataset owner.')}>{t('Simple')}</Tooltip>
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (777936f) into [master](https://codecov.io/gh/apache/superset/commit/ce558e07645027ebe9dfd36599ab23157046b263?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce558e0) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 777936f differs from pull request most recent head 8a43d2d. Consider uploading reports for the commit 8a43d2d to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
==========================================
- Coverage 76.95% 76.95% -0.01%
==========================================
Files 1037 1038 +1
Lines 55629 55597 -32
Branches 7594 7576 -18
==========================================
- Hits 42807 42782 -25
+ Misses 12572 12567 -5
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <100.00%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.87% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `80.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (ø)` | |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `85.60% <100.00%> (+2.14%)` | :arrow_up: |
| [superset-frontend/src/CRUD/Field.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQudHN4) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Fieldset.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGRzZXQuanN4) | `85.71% <0.00%> (-14.29%)` | :arrow_down: |
| [superset/annotation\_layers/annotations/schemas.py](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvYW5ub3RhdGlvbl9sYXllcnMvYW5ub3RhdGlvbnMvc2NoZW1hcy5weQ==) | `86.66% <0.00%> (-13.34%)` | :arrow_down: |
| ... and [25 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ce558e0...8a43d2d](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (777936f) into [master](https://codecov.io/gh/apache/superset/commit/ce558e07645027ebe9dfd36599ab23157046b263?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce558e0) will **decrease** coverage by `0.07%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 777936f differs from pull request most recent head 08258af. Consider uploading reports for the commit 08258af to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
==========================================
- Coverage 77.02% 76.95% -0.08%
==========================================
Files 1037 1038 +1
Lines 55629 55597 -32
Branches 7594 7576 -18
==========================================
- Hits 42851 42782 -69
- Misses 12528 12567 +39
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <100.00%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.87% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `80.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (ø)` | |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `85.60% <100.00%> (+2.14%)` | :arrow_up: |
| [superset-frontend/src/CRUD/Field.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQudHN4) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Fieldset.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGRzZXQuanN4) | `85.71% <0.00%> (-14.29%)` | :arrow_down: |
| [superset/annotation\_layers/annotations/schemas.py](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvYW5ub3RhdGlvbl9sYXllcnMvYW5ub3RhdGlvbnMvc2NoZW1hcy5weQ==) | `86.66% <0.00%> (-13.34%)` | :arrow_down: |
| ... and [24 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ce558e0...08258af](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17202:
URL: https://github.com/apache/superset/pull/17202#issuecomment-951145991
# [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17202](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6366b5b) into [master](https://codecov.io/gh/apache/superset/commit/fa44325a368dab45f24204c2f8241a1c7afa3d86?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fa44325) will **increase** coverage by `0.00%`.
> The diff coverage is `73.33%`.
> :exclamation: Current head 6366b5b differs from pull request most recent head 1a5be9a. Consider uploading reports for the commit 1a5be9a to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17202 +/- ##
=======================================
Coverage 77.09% 77.09%
=======================================
Files 1037 1037
Lines 55650 55637 -13
Branches 7603 7596 -7
=======================================
- Hits 42903 42895 -8
+ Misses 12497 12494 -3
+ Partials 250 248 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.26% <73.33%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/explore/components/ExploreContentPopover.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ29udGVudFBvcG92ZXIudHN4) | `100.00% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `61.76% <ø> (ø)` | |
| [...ols/FilterControl/AdhocFilterEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXIvaW5kZXguanN4) | `67.85% <ø> (-1.59%)` | :arrow_down: |
| [...components/controls/FixedOrMetricControl/index.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaXhlZE9yTWV0cmljQ29udHJvbC9pbmRleC5qc3g=) | `83.92% <ø> (ø)` | |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `75.00% <ø> (ø)` | |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `100.00% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `38.18% <ø> (ø)` | |
| [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `30.55% <ø> (+0.82%)` | :arrow_up: |
| [...frontend/src/views/CRUD/alert/AlertReportModal.tsx](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvQWxlcnRSZXBvcnRNb2RhbC50c3g=) | `60.70% <20.00%> (-0.44%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/17202/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fa44325...1a5be9a](https://codecov.io/gh/apache/superset/pull/17202?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r744933802
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -337,6 +341,13 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
) &&
savedMetric?.metric_name !== propsSavedMetric?.metric_name);
+ let extra = {};
+ if (datasource?.extra) {
+ try {
+ extra = JSON.parse(datasource.extra);
+ } catch {} // eslint-disable-line no-empty
+ }
Review comment:
I'm unsure, though it is a pattern used [elsewhere](https://github.com/apache/superset/blob/e32acd22fd204ed47d09dd9d445218f50c309011/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx#L205-L210).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r745114063
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +381,23 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={
+ extra.disallow_adhoc_metrics ? (
+ <Tooltip
+ title={t(
+ 'Simple ad-hoc metrics are not enabled for this dataset',
Review comment:
I think these are two different scenarios. You can still use a different message for native Druid. IMO clarity is more important than consistency. If we have only one config value `disallow_ahoc_metrics`, then the message should corresponds to what this config value does----which isn't disabling SIMPLE and CUSTOM SQL separately.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a change in pull request #17202: feat(metrics): Provide override for disabling ad-hoc metrics
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17202:
URL: https://github.com/apache/superset/pull/17202#discussion_r743124163
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx
##########
@@ -370,7 +380,11 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
/>
</FormItem>
</Tabs.TabPane>
- <Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
+ <Tabs.TabPane
+ key={EXPRESSION_TYPES.SIMPLE}
+ tab={t('Simple')}
Review comment:
@etr2460 could you be more explicit? I'm not sure how we display a tooltip on a tab which is disabled and thus not accessible.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org