You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "justinpark (via GitHub)" <gi...@apache.org> on 2023/04/07 22:13:33 UTC
[GitHub] [superset] justinpark opened a new pull request, #23627: fix(chart): chart updates are not retained
justinpark opened a new pull request, #23627:
URL: https://github.com/apache/superset/pull/23627
### SUMMARY
When a table chart saved on a dashboard and edited from the dashboard, the table reloads in the edit view and the updated option returns to the previous setting.
This bug regression of [#20498 ](https://github.com/apache/superset/pull/20498/files#diff-d682f497999ba5051d4e8efe3a9a7c06d9cc97026e134d5f2eddcffe1e902f78R51) which reload the `fetchExploreData` but it always overrides with the dashboardContext (which retain the origin settings).
This hotfix adds the condition of overwrite saving to retrieve the changes from updated exploreFormData.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
- After:
https://user-images.githubusercontent.com/1392866/230680771-e68eae52-5710-42fd-a044-d9d57eae254a.mov
- Before:
https://user-images.githubusercontent.com/1392866/230680765-2369dc46-97ce-4c25-bd3a-c103f35acad8.mov
### TESTING INSTRUCTIONS
1. Create a new Table chart
2. Save and publish the chart to a dashboard
3. Open the dashboard, and click "Edit" on the chart from the dashboard
4. Go to "Customize" and uncheck "Cell Bars"
5. Click save. The Cell Bars option returns to checked after saving.
6. Exit and go to the "Charts" list in Superset
7. Open your saved chart from the Charts list directly
8. Go to "Customize" and uncheck "Cell Bars"
9. Click save. The Cell Bars option retains the unchecked state
### ADDITIONAL INFORMATION
- [ ] 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
cc: @michael-s-molina @ktmud cc: @john-bodley
--
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] justinpark commented on a diff in pull request #23627: fix(chart): chart updates are not retained
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #23627:
URL: https://github.com/apache/superset/pull/23627#discussion_r1163078027
##########
superset-frontend/spec/fixtures/mockExploreFormData.ts:
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+
+import { JsonObject } from '@superset-ui/core';
+
+export const getExploreFormData = (overrides: JsonObject = {}) => ({
+ adhoc_filters: [
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SIMPLE' as const,
+ operator: 'IN' as const,
+ subject: 'gender',
+ comparator: ['boys'],
+ filterOptionName: '123',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "name = 'John'",
+ filterOptionName: '456',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "city = 'Warsaw'",
+ filterOptionName: '567',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SIMPLE' as const,
+ operator: 'TEMPORAL_RANGE' as const,
+ subject: 'ds',
+ comparator: 'No filter',
+ filterOptionName: '678',
+ },
+ ],
+ adhoc_filters_b: [
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "country = 'Poland'",
+ filterOptionName: '789',
+ },
+ ],
+ applied_time_extras: {},
+ color_scheme: 'supersetColors',
+ datasource: '2__table',
+ granularity_sqla: 'ds',
+ groupby: ['gender'],
+ metric: {
+ aggregate: 'SUM',
+ column: {
+ column_name: 'num',
+ type: 'BIGINT',
+ },
+ expressionType: 'SIMPLE',
+ label: 'Births',
+ },
+ slice_id: 46,
+ time_range: '100 years ago : now',
+ viz_type: 'pie',
+ ...overrides,
+});
+
+export const getDashboardFormData = (overrides: JsonObject = {}) => ({
Review Comment:
Good point. I will change this mock function location
--
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] michael-s-molina commented on a diff in pull request #23627: fix(chart): chart updates are not retained
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #23627:
URL: https://github.com/apache/superset/pull/23627#discussion_r1162929190
##########
superset-frontend/src/pages/Chart/Chart.test.tsx:
##########
@@ -0,0 +1,217 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import fetchMock from 'fetch-mock';
+import configureStore from 'redux-mock-store';
+import { Link } from 'react-router-dom';
+import {
+ render,
+ waitFor,
+ screen,
+ fireEvent,
+} from 'spec/helpers/testing-library';
+import {
+ getDashboardFormData,
+ getExploreFormData,
+} from 'spec/fixtures/mockExploreFormData';
+import { LocalStorageKeys } from 'src/utils/localStorageHelpers';
+import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters';
+import { URL_PARAMS } from 'src/constants';
+
+import ChartPage from '.';
+
+jest.mock('src/explore/components/ExploreViewContainer', () => () => (
+ <div data-test="mock-explore-view-container" />
+));
+jest.mock('src/dashboard/util/charts/getFormDataWithExtraFilters');
+
+const mockStore = configureStore([thunk]);
+
+describe('ChartPage', () => {
+ afterEach(() => {
+ fetchMock.reset();
+ });
+
+ test('fetches metadata on mount', async () => {
+ const store = mockStore({
+ explore: {},
+ });
+ const exploreApiRoute = 'glob:*/api/v1/explore/*';
+ const exploreFormData = getExploreFormData({
+ viz_type: 'table',
+ show_cell_bars: true,
+ });
+ fetchMock.get(exploreApiRoute, {
+ result: { dataset: { id: 1 }, form_data: exploreFormData },
+ });
+ const { getByTestId } = render(<ChartPage />, {
+ useRouter: true,
+ useRedux: true,
+ store,
+ });
+ await waitFor(() =>
+ expect(fetchMock.calls(exploreApiRoute).length).toBe(1),
+ );
+ expect(getByTestId('mock-explore-view-container')).toBeInTheDocument();
+ expect(store.getActions()).toContainEqual({
Review Comment:
@justinpark React Testing Library has a different view on how to write tests. One of its main concerns is accessibility. It differs from Enzyme in the sense that it's more focused on how the user interacts with the app rather then implementation details. Check this quote from their [site](https://testing-library.com/docs/#the-problem):
> You want to write maintainable tests that give you high confidence that your components are working for your users. As a part of this goal, you want your tests to avoid including implementation details so refactors of your components (changes to implementation but not functionality) don't break your tests and slow you and your team down.
In your case, you could write tests that check if the "Show cell bars" is checked rather then test if an specific Redux action is fired. This will make your tests more durable in a scenario where the implementation details change, such as the redux action name, or the action structure, but the interface, and the way users interact with it, remain the same.
--
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 #23627: fix(chart): chart updates are not retained
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23627:
URL: https://github.com/apache/superset/pull/23627#issuecomment-1500713758
## [Codecov](https://codecov.io/gh/apache/superset/pull/23627?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 [#23627](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8a9d7fe) into [master](https://codecov.io/gh/apache/superset/commit/3cff2b0a58cc935e0305e4a7b56b86dcd7db3e63?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cff2b0) will **increase** coverage by `0.05%`.
> The diff coverage is `64.78%`.
> :exclamation: Current head 8a9d7fe differs from pull request most recent head 3db7ce8. Consider uploading reports for the commit 3db7ce8 to get more accurate results
```diff
@@ Coverage Diff @@
## master #23627 +/- ##
==========================================
+ Coverage 67.72% 67.77% +0.05%
==========================================
Files 1916 1918 +2
Lines 74033 74157 +124
Branches 8040 8053 +13
==========================================
+ Hits 50137 50259 +122
+ Misses 21848 21845 -3
- Partials 2048 2053 +5
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `54.00% <48.87%> (+0.04%)` | :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/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../plugin-chart-echarts/src/Funnel/transformProps.ts](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvRnVubmVsL3RyYW5zZm9ybVByb3BzLnRz) | `72.34% <ø> (ø)` | |
| [...s/plugin-chart-echarts/src/Graph/transformProps.ts](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR3JhcGgvdHJhbnNmb3JtUHJvcHMudHM=) | `72.36% <ø> (ø)` | |
| [...hart-echarts/src/MixedTimeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL3RyYW5zZm9ybVByb3BzLnRz) | `0.00% <ø> (ø)` | |
| [...ins/plugin-chart-echarts/src/Pie/transformProps.ts](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvUGllL3RyYW5zZm9ybVByb3BzLnRz) | `55.71% <ø> (ø)` | |
| [...s/plugin-chart-echarts/src/Radar/transformProps.ts](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvUmFkYXIvdHJhbnNmb3JtUHJvcHMudHM=) | `0.00% <ø> (ø)` | |
| [...gin-chart-echarts/src/Timeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1Qcm9wcy50cw==) | `58.33% <ø> (ø)` | |
| [...d/plugins/plugin-chart-echarts/src/utils/series.ts](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvdXRpbHMvc2VyaWVzLnRz) | `88.48% <ø> (ø)` | |
| [...rc/components/Datasource/ChangeDatasourceModal.tsx](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YXNvdXJjZS9DaGFuZ2VEYXRhc291cmNlTW9kYWwudHN4) | `78.68% <ø> (ø)` | |
| [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `93.75% <ø> (ø)` | |
| [...src/dashboard/components/PropertiesModal/index.tsx](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC9pbmRleC50c3g=) | `64.03% <ø> (ø)` | |
| ... and [119 more](https://codecov.io/gh/apache/superset/pull/23627?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
... and [4 files with indirect coverage changes](https://codecov.io/gh/apache/superset/pull/23627/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] justinpark commented on a diff in pull request #23627: fix(chart): chart updates are not retained
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #23627:
URL: https://github.com/apache/superset/pull/23627#discussion_r1167237369
##########
superset-frontend/spec/fixtures/mockExploreFormData.ts:
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+
+import { JsonObject } from '@superset-ui/core';
+
+export const getExploreFormData = (overrides: JsonObject = {}) => ({
+ adhoc_filters: [
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SIMPLE' as const,
+ operator: 'IN' as const,
+ subject: 'gender',
+ comparator: ['boys'],
+ filterOptionName: '123',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "name = 'John'",
+ filterOptionName: '456',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "city = 'Warsaw'",
+ filterOptionName: '567',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SIMPLE' as const,
+ operator: 'TEMPORAL_RANGE' as const,
+ subject: 'ds',
+ comparator: 'No filter',
+ filterOptionName: '678',
+ },
+ ],
+ adhoc_filters_b: [
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "country = 'Poland'",
+ filterOptionName: '789',
+ },
+ ],
+ applied_time_extras: {},
+ color_scheme: 'supersetColors',
+ datasource: '2__table',
+ granularity_sqla: 'ds',
+ groupby: ['gender'],
+ metric: {
+ aggregate: 'SUM',
+ column: {
+ column_name: 'num',
+ type: 'BIGINT',
+ },
+ expressionType: 'SIMPLE',
+ label: 'Births',
+ },
+ slice_id: 46,
+ time_range: '100 years ago : now',
+ viz_type: 'pie',
+ ...overrides,
+});
+
+export const getDashboardFormData = (overrides: JsonObject = {}) => ({
Review Comment:
@michael-s-molina could you review the PR again?
--
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] justinpark commented on pull request #23627: fix(chart): chart updates are not retained
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on PR #23627:
URL: https://github.com/apache/superset/pull/23627#issuecomment-1526571393
@rusackas could you help to stamp this PR to release the blocker?
--
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 #23627: fix(chart): chart updates are not retained
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley merged PR #23627:
URL: https://github.com/apache/superset/pull/23627
--
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] justinpark commented on a diff in pull request #23627: fix(chart): chart updates are not retained
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #23627:
URL: https://github.com/apache/superset/pull/23627#discussion_r1174092052
##########
superset-frontend/src/pages/Chart/Chart.test.tsx:
##########
@@ -0,0 +1,217 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import fetchMock from 'fetch-mock';
+import configureStore from 'redux-mock-store';
+import { Link } from 'react-router-dom';
+import {
+ render,
+ waitFor,
+ screen,
+ fireEvent,
+} from 'spec/helpers/testing-library';
+import {
+ getDashboardFormData,
+ getExploreFormData,
+} from 'spec/fixtures/mockExploreFormData';
+import { LocalStorageKeys } from 'src/utils/localStorageHelpers';
+import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters';
+import { URL_PARAMS } from 'src/constants';
+
+import ChartPage from '.';
+
+jest.mock('src/explore/components/ExploreViewContainer', () => () => (
+ <div data-test="mock-explore-view-container" />
+));
+jest.mock('src/dashboard/util/charts/getFormDataWithExtraFilters');
+
+const mockStore = configureStore([thunk]);
+
+describe('ChartPage', () => {
+ afterEach(() => {
+ fetchMock.reset();
+ });
+
+ test('fetches metadata on mount', async () => {
+ const store = mockStore({
+ explore: {},
+ });
+ const exploreApiRoute = 'glob:*/api/v1/explore/*';
+ const exploreFormData = getExploreFormData({
+ viz_type: 'table',
+ show_cell_bars: true,
+ });
+ fetchMock.get(exploreApiRoute, {
+ result: { dataset: { id: 1 }, form_data: exploreFormData },
+ });
+ const { getByTestId } = render(<ChartPage />, {
+ useRouter: true,
+ useRedux: true,
+ store,
+ });
+ await waitFor(() =>
+ expect(fetchMock.calls(exploreApiRoute).length).toBe(1),
+ );
+ expect(getByTestId('mock-explore-view-container')).toBeInTheDocument();
+ expect(store.getActions()).toContainEqual({
Review Comment:
@michael-s-molina I updated the test code as you suggested. (but using partial rendering since full ChartContainer requires a lot of dependancies)
--
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] michael-s-molina commented on a diff in pull request #23627: fix(chart): chart updates are not retained
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #23627:
URL: https://github.com/apache/superset/pull/23627#discussion_r1162933102
##########
superset-frontend/spec/fixtures/mockExploreFormData.ts:
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+
+import { JsonObject } from '@superset-ui/core';
+
+export const getExploreFormData = (overrides: JsonObject = {}) => ({
+ adhoc_filters: [
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SIMPLE' as const,
+ operator: 'IN' as const,
+ subject: 'gender',
+ comparator: ['boys'],
+ filterOptionName: '123',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "name = 'John'",
+ filterOptionName: '456',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "city = 'Warsaw'",
+ filterOptionName: '567',
+ },
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SIMPLE' as const,
+ operator: 'TEMPORAL_RANGE' as const,
+ subject: 'ds',
+ comparator: 'No filter',
+ filterOptionName: '678',
+ },
+ ],
+ adhoc_filters_b: [
+ {
+ clause: 'WHERE' as const,
+ expressionType: 'SQL' as const,
+ operator: null,
+ subject: null,
+ comparator: null,
+ sqlExpression: "country = 'Poland'",
+ filterOptionName: '789',
+ },
+ ],
+ applied_time_extras: {},
+ color_scheme: 'supersetColors',
+ datasource: '2__table',
+ granularity_sqla: 'ds',
+ groupby: ['gender'],
+ metric: {
+ aggregate: 'SUM',
+ column: {
+ column_name: 'num',
+ type: 'BIGINT',
+ },
+ expressionType: 'SIMPLE',
+ label: 'Births',
+ },
+ slice_id: 46,
+ time_range: '100 years ago : now',
+ viz_type: 'pie',
+ ...overrides,
+});
+
+export const getDashboardFormData = (overrides: JsonObject = {}) => ({
Review Comment:
Should this be in its own file? Could this be used by another test that is not inside Explore?
--
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] michael-s-molina commented on a diff in pull request #23627: fix(chart): chart updates are not retained
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #23627:
URL: https://github.com/apache/superset/pull/23627#discussion_r1162931517
##########
superset-frontend/spec/fixtures/mockExploreFormData.ts:
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+
+import { JsonObject } from '@superset-ui/core';
+
+export const getExploreFormData = (overrides: JsonObject = {}) => ({
Review Comment:
Really nice improvement to extract the mock.
--
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