Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

fix(plugin-chart-table): ignore duplicate percent metrics #994

Merged
merged 2 commits into from
Mar 8, 2021
Merged

fix(plugin-chart-table): ignore duplicate percent metrics #994

merged 2 commits into from
Mar 8, 2021

Conversation

ktmud
Copy link
Contributor

@ktmud ktmud commented Mar 8, 2021

🐛 Bug Fix

Silently ignore duplicate percent metrics.

Currently we do this for metrics, but adding percent metrics with the same name may still cause an error. This fixes it.

Before

duplicate-percent-metric-error

After

duplicate-percent-metrics

@ktmud ktmud requested a review from a team as a code owner March 8, 2021 07:25
@vercel
Copy link

vercel bot commented Mar 8, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/superset/superset-ui/E23AKHHBypVdDBLHSswbF4uR7tak
✅ Preview: https://superset-ui-git-aggregate-is-groupby-superset.vercel.app

@ktmud
Copy link
Contributor Author

ktmud commented Mar 8, 2021

Please ignore the bad branch name.

@codecov
Copy link

codecov bot commented Mar 8, 2021 •
edited
Loading

Codecov Report

Merging #994 (3ac5dab) into master (deeee7c) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #994   +/-   ##
=======================================
  Coverage   27.94%   27.94%           
=======================================
  Files         413      413           
  Lines        8523     8523           
  Branches     1204     1204           
=======================================
  Hits         2382     2382           
  Misses       5987     5987           
  Partials      154      154
Impacted Files Coverage Δ
plugins/plugin-chart-table/src/index.ts 0.00% <ø> (ø)
plugins/plugin-chart-table/src/buildQuery.ts 63.04% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update deeee7c...3ac5dab. Read the comment docs.

@ktmud ktmud force-pushed the aggregate-is-groupby branch from 4d774bc to 3ac5dab Compare March 8, 2021 17:59
ktmud
ktmud commented Mar 8, 2021
View reviewed changes
plugins/plugin-chart-table/src/buildQuery.ts
@@ -146,4 +146,4 @@ const cachedBuildQuery = (): BuildQuery<TableChartFormData> => {
);
};

export default cachedBuildQuery;
export default cachedBuildQuery();
Copy link
Contributor Author

@ktmud ktmud Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also changed how the default buildQuery function is exported.

cc @simcha90

@ktmud ktmud merged commit c14bf82 into master Mar 8, 2021
@delete-merged-branch delete-merged-branch bot deleted the aggregate-is-groupby branch March 8, 2021 19:23
@villebro villebro mentioned this pull request Mar 9, 2021
Merged
6 tasks
NejcZdovc pushed a commit to blotoutio/superset-ui that referenced this pull request Apr 20, 2021
@ktmud @NejcZdovc
fix(plugin-chart-table): ignore duplicate percent metrics (apache-sup…
eaba19e 
…erset#994)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@villebro villebro villebro approved these changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ktmud @villebro