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 2020/10/16 00:44:59 UTC
[GitHub] [incubator-superset] eschutho opened a new pull request #10834: fix: dashboard edit/save errors
eschutho opened a new pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834
### SUMMARY
Some functionality on dashboard edit isn't being properly updated in the UI after save, although it is being saved to the DB and shows up on a refresh.
Some items that this fixes:
Dashboard Title when edited in the modal does not refresh on save (done)
Empty json metadata errors silently (done)
If colorScheme is edited in json metadata, it should also update the dropdown in the edit modal and take precedence(done)
label colors in json metadata are not saving properly (done)
Also added wrapping to the json metadata window.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Save title and update color scheme
changing color scheme in metadata updates the dropdown
changing label color in metadata persists after save. You still need to refresh in order to see it updated in the chart, but we can fix that separately.
invalid color scheme string in json metadata shows an error
### TEST PLAN
Cypress tests
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: https://github.com/apache/incubator-superset/issues/10655
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] 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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/0e97c4f66c5449bfbf7b195073b02550a071de0d?el=desc) will **increase** coverage by `0.08%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
+ Coverage 61.56% 61.65% +0.08%
==========================================
Files 835 835
Lines 39653 39679 +26
Branches 3604 3619 +15
==========================================
+ Hits 24414 24464 +50
+ Misses 15058 15035 -23
+ Partials 181 180 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.90% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.91% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-0.82%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (+31.03%)` | :arrow_up: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `70.00% <55.17%> (+24.02%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (ø)` | |
| [...end/src/components/Select/SupersetStyledSelect.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1N1cGVyc2V0U3R5bGVkU2VsZWN0LnRzeA==) | `74.44% <0.00%> (+1.11%)` | :arrow_up: |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `61.11% <0.00%> (+5.55%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [0e97c4f...657d4c5](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r499057404
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js
##########
@@ -16,55 +16,118 @@
* specific language governing permissions and limitations
* under the License.
*/
+
+import shortid from 'shortid';
import readResponseBlob from '../../utils/readResponseBlob';
import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
describe('Dashboard save action', () => {
- let dashboardId;
-
beforeEach(() => {
cy.server();
cy.login();
cy.visit(WORLD_HEALTH_DASHBOARD);
+ });
- cy.get('#app').then(data => {
- const bootstrapData = JSON.parse(data[0].dataset.bootstrap);
- const dashboard = bootstrapData.dashboard_data;
- dashboardId = dashboard.id;
- cy.route('POST', `/superset/copy_dash/${dashboardId}/`).as('copyRequest');
- });
+ // it('should save as new dashboard', () => {
Review comment:
that was just for temporary testing purposes while this was a draft.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9aef9c5764ebf995c9ab8955684b408397037e1e?el=desc) will **decrease** coverage by `1.45%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.50% 60.04% -1.46%
==========================================
Files 834 393 -441
Lines 39557 24836 -14721
Branches 3610 0 -3610
==========================================
- Hits 24329 14913 -9416
+ Misses 15047 9923 -5124
+ Partials 181 0 -181
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `60.04% <ø> (-0.74%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.43% <0.00%> (-1.68%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `73.98% <0.00%> (-0.49%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.47% <0.00%> (-0.27%)` | :arrow_down: |
| ... and [437 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [9aef9c5...b366ff7](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r499779700
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts
##########
@@ -0,0 +1,189 @@
+/**
+ * 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-next-line import/no-extraneous-dependencies
+import * as ace from 'brace';
+import shortid from 'shortid';
+import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
+
+function selectColorScheme(color: string) {
+ // open color scheme dropdown
+ cy.get('.modal-body')
+ .contains('Color Scheme')
+ .parents('.ControlHeader')
+ .next('.Select')
+ .click()
+ .then($colorSelect => {
+ // select a new color scheme
+ cy.wrap($colorSelect).find(`[data-test="${color}"]`).click();
+ });
+}
+
+function assertMetadata(text: string) {
+ const regex = new RegExp(text);
+ cy.get('.modal-body')
+ .find('#json_metadata')
+ .should('be.visible')
+ .then(() => {
+ const metadata = cy.$$('#json_metadata')[0];
+
+ // cypress can read this locally, but not in ci
+ // so we have to use the ace module directly to fetch the value
+ expect(ace.edit(metadata).getValue()).to.match(regex);
+ });
+}
+
+function typeMetadata(text: string) {
+ cy.get('.modal-body').find('#json_metadata').should('be.visible').type(text);
+}
+
+function openAdvancedProperties() {
+ return cy
+ .get('.modal-body')
+ .contains('Advanced')
+ .should('be.visible')
+ .click();
+}
+
+function openDashboardEditProperties() {
+ // open dashboard properties edit modal
+ cy.get('#save-dash-split-button').trigger('click', { force: true });
+ cy.get('.dropdown-menu').contains('Edit dashboard properties').click();
+}
+
+describe('Dashboard edit action', () => {
+ beforeEach(() => {
+ cy.server();
+ cy.login();
+ cy.visit(WORLD_HEALTH_DASHBOARD);
+ cy.route(`/api/v1/dashboard/1`).as('dashboardGet');
+ cy.get('.dashboard-grid', { timeout: 50000 })
+ .should('be.visible') // wait for 50 secs to load dashboard
+ .then(() => {
+ cy.get('.dashboard-header [data-test=pencil]')
+ .should('be.visible')
+ .click();
+ openDashboardEditProperties();
+ });
+ });
+
+ xit('should update the title', () => {
Review comment:
reran an confirmed which ones aren't passing on CI still. I also added comments to show why they are "exited".
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts
##########
@@ -0,0 +1,189 @@
+/**
+ * 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-next-line import/no-extraneous-dependencies
+import * as ace from 'brace';
+import shortid from 'shortid';
+import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
+
+function selectColorScheme(color: string) {
+ // open color scheme dropdown
+ cy.get('.modal-body')
+ .contains('Color Scheme')
+ .parents('.ControlHeader')
+ .next('.Select')
+ .click()
+ .then($colorSelect => {
+ // select a new color scheme
+ cy.wrap($colorSelect).find(`[data-test="${color}"]`).click();
+ });
+}
+
+function assertMetadata(text: string) {
+ const regex = new RegExp(text);
+ cy.get('.modal-body')
+ .find('#json_metadata')
+ .should('be.visible')
+ .then(() => {
+ const metadata = cy.$$('#json_metadata')[0];
+
+ // cypress can read this locally, but not in ci
+ // so we have to use the ace module directly to fetch the value
+ expect(ace.edit(metadata).getValue()).to.match(regex);
+ });
+}
+
+function typeMetadata(text: string) {
+ cy.get('.modal-body').find('#json_metadata').should('be.visible').type(text);
+}
+
+function openAdvancedProperties() {
+ return cy
+ .get('.modal-body')
+ .contains('Advanced')
+ .should('be.visible')
+ .click();
+}
+
+function openDashboardEditProperties() {
+ // open dashboard properties edit modal
+ cy.get('#save-dash-split-button').trigger('click', { force: true });
+ cy.get('.dropdown-menu').contains('Edit dashboard properties').click();
+}
+
+describe('Dashboard edit action', () => {
+ beforeEach(() => {
+ cy.server();
+ cy.login();
+ cy.visit(WORLD_HEALTH_DASHBOARD);
+ cy.route(`/api/v1/dashboard/1`).as('dashboardGet');
+ cy.get('.dashboard-grid', { timeout: 50000 })
+ .should('be.visible') // wait for 50 secs to load dashboard
+ .then(() => {
+ cy.get('.dashboard-header [data-test=pencil]')
+ .should('be.visible')
+ .click();
+ openDashboardEditProperties();
+ });
+ });
+
+ xit('should update the title', () => {
Review comment:
reran and confirmed which ones aren't passing on CI still. I also added comments to show why they are "exited".
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d95b7c2a73ecd0bf0f44434d80ab8496d9cb6311?el=desc) will **decrease** coverage by `4.54%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.88% 61.34% -4.55%
==========================================
Files 827 827
Lines 39070 39087 +17
Branches 3676 3691 +15
==========================================
- Hits 25741 23977 -1764
- Misses 13222 14930 +1708
- Partials 107 180 +73
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.46% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.67% <ø> (-0.69%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-16.32%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (-10.35%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `71.02% <55.17%> (+10.31%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [179 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [d95b7c2...be343c2](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho closed pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho closed pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter commented on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/eded51b2f86fa2487130b972803bca780ad30805?el=desc) will **decrease** coverage by `4.93%`.
> The diff coverage is `3.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.76% 60.83% -4.94%
==========================================
Files 816 816
Lines 38396 38413 +17
Branches 3606 3617 +11
==========================================
- Hits 25253 23368 -1885
- Misses 13035 14859 +1824
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.63% <3.33%> (-0.05%)` | :arrow_down: |
| #python | `60.35% <ø> (-1.05%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.35% <0.00%> (-18.00%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [208 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [eded51b...7f2dead](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709590523
Thanks for the feedback, too, @graceguo-supercat. I think @junlincc said that she touched base with you about improving those usability issues in a larger feature-improvement effort in the next round. Let me know if we want to move forward with any of these fixes in the near term.
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-702480353
please excuse the rebase/force push after reviews have started, but I had to revert a binary file from the original commit, and it just seemed the most straightforward way.
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-713095207
> Sounds great. Should we add your suggestion regarding timestamps in this PR or a future one?
should be in this PR. Otherwise after user save changes in Dashboard Properties modal, they will see error message (but actually changes are saved).
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-714733839
Thanks for the work! this PR is good now.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9aef9c5764ebf995c9ab8955684b408397037e1e?el=desc) will **decrease** coverage by `1.39%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.50% 60.11% -1.40%
==========================================
Files 834 393 -441
Lines 39557 24836 -14721
Branches 3610 0 -3610
==========================================
- Hits 24329 14929 -9400
+ Misses 15047 9907 -5140
+ Partials 181 0 -181
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `60.11% <ø> (-0.68%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.26% <0.00%> (-0.84%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.60% <0.00%> (-0.14%)` | :arrow_down: |
| [...c/components/ListViewCard/ListViewCard.stories.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL0xpc3RWaWV3Q2FyZC5zdG9yaWVzLnRzeA==) | | |
| [...rontend/src/SqlLab/components/ShareSqlLabQuery.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NoYXJlU3FsTGFiUXVlcnkuanN4) | | |
| ... and [435 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [9aef9c5...b366ff7](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r498956217
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts
##########
@@ -0,0 +1,189 @@
+/**
+ * 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-next-line import/no-extraneous-dependencies
+import * as ace from 'brace';
+import shortid from 'shortid';
+import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
+
+function selectColorScheme(color: string) {
+ // open color scheme dropdown
+ cy.get('.modal-body')
+ .contains('Color Scheme')
+ .parents('.ControlHeader')
+ .next('.Select')
+ .click()
+ .then($colorSelect => {
+ // select a new color scheme
+ cy.wrap($colorSelect).find(`[data-test="${color}"]`).click();
+ });
+}
+
+function assertMetadata(text: string) {
+ const regex = new RegExp(text);
+ cy.get('.modal-body')
+ .find('#json_metadata')
+ .should('be.visible')
+ .then(() => {
+ const metadata = cy.$$('#json_metadata')[0];
+
+ // cypress can read this locally, but not in ci
+ // so we have to use the ace module directly to fetch the value
+ expect(ace.edit(metadata).getValue()).to.match(regex);
+ });
+}
+
+function typeMetadata(text: string) {
+ cy.get('.modal-body').find('#json_metadata').should('be.visible').type(text);
+}
+
+function openAdvancedProperties() {
+ return cy
+ .get('.modal-body')
+ .contains('Advanced')
+ .should('be.visible')
+ .click();
+}
+
+function openDashboardEditProperties() {
+ // open dashboard properties edit modal
+ cy.get('#save-dash-split-button').trigger('click', { force: true });
+ cy.get('.dropdown-menu').contains('Edit dashboard properties').click();
+}
+
+describe('Dashboard edit action', () => {
+ beforeEach(() => {
+ cy.server();
+ cy.login();
+ cy.visit(WORLD_HEALTH_DASHBOARD);
+ cy.route(`/api/v1/dashboard/1`).as('dashboardGet');
+ cy.get('.dashboard-grid', { timeout: 50000 })
+ .should('be.visible') // wait for 50 secs to load dashboard
+ .then(() => {
+ cy.get('.dashboard-header [data-test=pencil]')
+ .should('be.visible')
+ .click();
+ openDashboardEditProperties();
+ });
+ });
+
+ xit('should update the title', () => {
Review comment:
Yes, I added a lot of new cypress tests that were passing locally but not on CI, plus a few that were flaky (edit_mode) for example. I know that Polidea has been working to improve these. For now, I opted to xit out of the new tests because I think they can add value if we can get them working. Or I can remove them.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/0e97c4f66c5449bfbf7b195073b02550a071de0d?el=desc) will **decrease** coverage by `1.38%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.56% 60.18% -1.39%
==========================================
Files 835 394 -441
Lines 39653 24935 -14718
Branches 3604 0 -3604
==========================================
- Hits 24414 15006 -9408
+ Misses 15058 9929 -5129
+ Partials 181 0 -181
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `60.18% <ø> (-0.74%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.43% <0.00%> (-1.68%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `73.98% <0.00%> (-0.49%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.47% <0.00%> (-0.27%)` | :arrow_down: |
| ... and [437 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [0e97c4f...657d4c5](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/3a08fd04f3f9ebaaa48fa554028c26062de42f02?el=desc) will **decrease** coverage by `0.52%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.70% 61.17% -0.53%
==========================================
Files 816 391 -425
Lines 38586 24485 -14101
Branches 3650 0 -3650
==========================================
- Hits 23808 14979 -8829
+ Misses 14598 9506 -5092
+ Partials 180 0 -180
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `61.17% <ø> (-0.24%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.65% <0.00%> (-8.31%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `50.76% <0.00%> (-4.24%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `82.14% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `90.69% <0.00%> (-0.65%)` | :arrow_down: |
| [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `76.47% <0.00%> (-0.50%)` | :arrow_down: |
| [superset/views/base\_api.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.90% <0.00%> (-0.36%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.12% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.70% <0.00%> (-0.14%)` | :arrow_down: |
| ... and [455 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [3a08fd0...f3a7907](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho closed pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho closed pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/0e97c4f66c5449bfbf7b195073b02550a071de0d?el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.56% 61.54% -0.03%
==========================================
Files 835 835
Lines 39653 39679 +26
Branches 3604 3619 +15
==========================================
+ Hits 24414 24419 +5
- Misses 15058 15080 +22
+ Partials 181 180 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.90% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.73% <ø> (-0.19%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-0.82%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (+31.03%)` | :arrow_up: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `70.00% <55.17%> (+24.02%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (ø)` | |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.65% <0.00%> (-8.43%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.82% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.60% <0.00%> (-0.14%)` | :arrow_down: |
| [...end/src/components/Select/SupersetStyledSelect.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1N1cGVyc2V0U3R5bGVkU2VsZWN0LnRzeA==) | `74.44% <0.00%> (+1.11%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [0e97c4f...657d4c5](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/0e97c4f66c5449bfbf7b195073b02550a071de0d?el=desc) will **decrease** coverage by `0.37%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.56% 61.19% -0.38%
==========================================
Files 835 835
Lines 39653 39679 +26
Branches 3604 3619 +15
==========================================
- Hits 24414 24281 -133
- Misses 15058 15218 +160
+ Partials 181 180 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.90% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.18% <ø> (-0.74%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-0.82%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (+31.03%)` | :arrow_up: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `70.00% <55.17%> (+24.02%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| ... and [8 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [0e97c4f...657d4c5](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho closed pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho closed pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/eded51b2f86fa2487130b972803bca780ad30805?el=desc) will **decrease** coverage by `4.69%`.
> The diff coverage is `3.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.76% 61.07% -4.70%
==========================================
Files 816 816
Lines 38396 38413 +17
Branches 3606 3617 +11
==========================================
- Hits 25253 23462 -1791
- Misses 13035 14765 +1730
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.63% <3.33%> (-0.05%)` | :arrow_down: |
| #python | `60.74% <ø> (-0.66%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.35% <0.00%> (-18.00%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [196 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [eded51b...7f2dead](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r499098099
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts
##########
@@ -0,0 +1,189 @@
+/**
+ * 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-next-line import/no-extraneous-dependencies
+import * as ace from 'brace';
+import shortid from 'shortid';
+import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
+
+function selectColorScheme(color: string) {
+ // open color scheme dropdown
+ cy.get('.modal-body')
+ .contains('Color Scheme')
+ .parents('.ControlHeader')
+ .next('.Select')
+ .click()
+ .then($colorSelect => {
+ // select a new color scheme
+ cy.wrap($colorSelect).find(`[data-test="${color}"]`).click();
+ });
+}
+
+function assertMetadata(text: string) {
+ const regex = new RegExp(text);
+ cy.get('.modal-body')
+ .find('#json_metadata')
+ .should('be.visible')
+ .then(() => {
+ const metadata = cy.$$('#json_metadata')[0];
+
+ // cypress can read this locally, but not in ci
+ // so we have to use the ace module directly to fetch the value
+ expect(ace.edit(metadata).getValue()).to.match(regex);
+ });
+}
+
+function typeMetadata(text: string) {
+ cy.get('.modal-body').find('#json_metadata').should('be.visible').type(text);
+}
+
+function openAdvancedProperties() {
+ return cy
+ .get('.modal-body')
+ .contains('Advanced')
+ .should('be.visible')
+ .click();
+}
+
+function openDashboardEditProperties() {
+ // open dashboard properties edit modal
+ cy.get('#save-dash-split-button').trigger('click', { force: true });
+ cy.get('.dropdown-menu').contains('Edit dashboard properties').click();
+}
+
+describe('Dashboard edit action', () => {
+ beforeEach(() => {
+ cy.server();
+ cy.login();
+ cy.visit(WORLD_HEALTH_DASHBOARD);
+ cy.route(`/api/v1/dashboard/1`).as('dashboardGet');
+ cy.get('.dashboard-grid', { timeout: 50000 })
+ .should('be.visible') // wait for 50 secs to load dashboard
+ .then(() => {
+ cy.get('.dashboard-header [data-test=pencil]')
+ .should('be.visible')
+ .click();
+ openDashboardEditProperties();
+ });
+ });
+
+ xit('should update the title', () => {
Review comment:
I'm rechecking now to see what tests will pass on CI.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ea105bc23b355530a2abad51eb9a35bcecbf08b5?el=desc) will **decrease** coverage by `4.40%`.
> The diff coverage is `3.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.77% 61.37% -4.41%
==========================================
Files 816 816
Lines 38406 38424 +18
Branches 3607 3617 +10
==========================================
- Hits 25263 23583 -1680
- Misses 13035 14655 +1620
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.63% <3.33%> (-0.10%)` | :arrow_down: |
| #python | `61.22% <ø> (-0.19%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.35% <0.00%> (-18.00%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [173 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [ea105bc...2041a30](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712514309
Hi @eschutho Thanks for the work!
Yes #11220 and #11305 caused conflicts for this PR, and we can make this PR work with it. The idea of preventing co-edit is, when dashboard is loaded, it carries the last_modified_time which was dashboard's changedon property stored in the database. When user edit dashboard and submit new updates to save, we send this last_modified_time with all the updates. `/save_dash/` endpoint compares the last_modified_time from incoming request with current database: if incoming last_modified_time < current data, this means the dashboard was changed **_since_** user open dashboard, so the save request will be rejected.
So for the Dashboard Properties modal, right now the problem is it didn't send last_modified_time information when it calling `/save_dash/` request, which happens after the first save is done. For other dashboard metadata change flow, we manage last_modified_time in redux store.
I just found dashboardInfo is already in redux store. so you can add a `lastModifiedTime` in dashboardInfo state, and update `lastModifiedTime` when data is updated from server-side. Similar like this:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/reducers/dashboardState.js#L110
At the same time, please update DashboardHeader container component. Instead of only getting lastModifiedTime from dashboardState:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/containers/DashboardHeader.jsx#L86
it should be:
`lastModifiedTime: Max(dashboardState.lastModifiedTime, dashboardInfo.lastModifiedTime)`
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r497845337
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts
##########
@@ -0,0 +1,190 @@
+/**
+ * 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-next-line import/no-extraneous-dependencies
+import * as ace from 'brace';
+import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
+
+const shortid = require('shortid');
Review comment:
👍 The library exports with CommonJS syntax, so ts was unhappy with that. Instead I just updated our tsconfig.
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712630022
I discussed this PR with PM in airbnb, we think should just fix the save dashboard metadata issue in this PR. For 2 `save dashboard` calls, it needs a bigger code refactor, we should address it later when we have a chance. Thanks!
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-702480353
please excuse the rebase/force push after reviews have started, but I had to revert a binary file from the original commit, and it just seemed the most straightforward way to make those changes.
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r498584476
##########
File path: superset/translations/en/LC_MESSAGES/messages.po
##########
@@ -3213,6 +3213,9 @@ msgstr ""
#~ msgid "One ore more annotation layers failed loading."
#~ msgstr ""
+#~ msgid "A valid color scheme is required"
Review comment:
kk, removed. thanks for the heads up.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9aef9c5764ebf995c9ab8955684b408397037e1e?el=desc) will **increase** coverage by `0.08%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
+ Coverage 61.50% 61.58% +0.08%
==========================================
Files 834 834
Lines 39557 39583 +26
Branches 3610 3625 +15
==========================================
+ Hits 24329 24379 +50
+ Misses 15047 15024 -23
+ Partials 181 180 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.94% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.78% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-0.82%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (+31.03%)` | :arrow_up: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `70.00% <55.17%> (+24.02%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (ø)` | |
| [...end/src/components/Select/SupersetStyledSelect.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1N1cGVyc2V0U3R5bGVkU2VsZWN0LnRzeA==) | `74.44% <0.00%> (+1.11%)` | :arrow_up: |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `61.11% <0.00%> (+5.55%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [9aef9c5...b366ff7](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho closed pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho closed pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/eded51b2f86fa2487130b972803bca780ad30805?el=desc) will **decrease** coverage by `4.54%`.
> The diff coverage is `3.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.76% 61.22% -4.55%
==========================================
Files 816 816
Lines 38396 38424 +28
Branches 3606 3617 +11
==========================================
- Hits 25253 23524 -1729
- Misses 13035 14714 +1679
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.63% <3.33%> (-0.05%)` | :arrow_down: |
| #python | `60.97% <ø> (-0.44%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.35% <0.00%> (-18.00%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [187 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [eded51b...7f2dead](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] willbarrett commented on pull request #10834: fix dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
willbarrett commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-692796601
@eschutho I think @nytai is working on a larger refactor of this functionality. Have you two talked on this?
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-693004442
yeah, we sorted it yesterday. I'm going to pick up the features that they wanted to add to this branch as well.
----------------------------------------------------------------
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.
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] [incubator-superset] mistercrunch commented on a change in pull request #10834: use conditional operator for lookup
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r487581662
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js
##########
@@ -16,55 +16,118 @@
* specific language governing permissions and limitations
* under the License.
*/
+
+import shortid from 'shortid';
import readResponseBlob from '../../utils/readResponseBlob';
import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
describe('Dashboard save action', () => {
- let dashboardId;
-
beforeEach(() => {
cy.server();
cy.login();
cy.visit(WORLD_HEALTH_DASHBOARD);
+ });
- cy.get('#app').then(data => {
- const bootstrapData = JSON.parse(data[0].dataset.bootstrap);
- const dashboard = bootstrapData.dashboard_data;
- dashboardId = dashboard.id;
- cy.route('POST', `/superset/copy_dash/${dashboardId}/`).as('copyRequest');
- });
+ // it('should save as new dashboard', () => {
Review comment:
protip: use `xit` in place of `it` to disable a test
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712514309
Hi @eschutho Thanks for the work!
Yes #11220 and #11305 caused conflicts for this PR, and we can make this PR work with it. The idea of preventing co-edit is, when dashboard is loaded, it carries the last_modified_time which was stored in the database. When user edit dashboard, we send out this last_modified_time with all the updates. `/save_dash/` endpoint compares the last_modified_time from incoming request with current database: if incoming last_modified_time < current data, the dashboard is changed since user open dashboard, so the save request will be rejected.
So for the Dashboard Properties modal, right now the problem is it didn't send last_modified_time information when it calling `/save_dash/` request, which happens after the first save is done. Dashboard Properties modal didn't use redux store, but you can manually add this parameter like this:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/reducers/dashboardState.js#L110
this can make sure the incoming request always newer than database record.
----------------------------------------------------------------
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.
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] [incubator-superset] mistercrunch commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r498615467
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts
##########
@@ -0,0 +1,189 @@
+/**
+ * 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-next-line import/no-extraneous-dependencies
+import * as ace from 'brace';
+import shortid from 'shortid';
+import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
+
+function selectColorScheme(color: string) {
+ // open color scheme dropdown
+ cy.get('.modal-body')
+ .contains('Color Scheme')
+ .parents('.ControlHeader')
+ .next('.Select')
+ .click()
+ .then($colorSelect => {
+ // select a new color scheme
+ cy.wrap($colorSelect).find(`[data-test="${color}"]`).click();
+ });
+}
+
+function assertMetadata(text: string) {
+ const regex = new RegExp(text);
+ cy.get('.modal-body')
+ .find('#json_metadata')
+ .should('be.visible')
+ .then(() => {
+ const metadata = cy.$$('#json_metadata')[0];
+
+ // cypress can read this locally, but not in ci
+ // so we have to use the ace module directly to fetch the value
+ expect(ace.edit(metadata).getValue()).to.match(regex);
+ });
+}
+
+function typeMetadata(text: string) {
+ cy.get('.modal-body').find('#json_metadata').should('be.visible').type(text);
+}
+
+function openAdvancedProperties() {
+ return cy
+ .get('.modal-body')
+ .contains('Advanced')
+ .should('be.visible')
+ .click();
+}
+
+function openDashboardEditProperties() {
+ // open dashboard properties edit modal
+ cy.get('#save-dash-split-button').trigger('click', { force: true });
+ cy.get('.dropdown-menu').contains('Edit dashboard properties').click();
+}
+
+describe('Dashboard edit action', () => {
+ beforeEach(() => {
+ cy.server();
+ cy.login();
+ cy.visit(WORLD_HEALTH_DASHBOARD);
+ cy.route(`/api/v1/dashboard/1`).as('dashboardGet');
+ cy.get('.dashboard-grid', { timeout: 50000 })
+ .should('be.visible') // wait for 50 secs to load dashboard
+ .then(() => {
+ cy.get('.dashboard-header [data-test=pencil]')
+ .should('be.visible')
+ .click();
+ openDashboardEditProperties();
+ });
+ });
+
+ xit('should update the title', () => {
Review comment:
there's a lot of `xit` everywhere, intended?
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter commented on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/eded51b2f86fa2487130b972803bca780ad30805?el=desc) will **decrease** coverage by `4.93%`.
> The diff coverage is `3.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.76% 60.83% -4.94%
==========================================
Files 816 816
Lines 38396 38413 +17
Branches 3606 3617 +11
==========================================
- Hits 25253 23368 -1885
- Misses 13035 14859 +1824
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.63% <3.33%> (-0.05%)` | :arrow_down: |
| #python | `60.35% <ø> (-1.05%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.35% <0.00%> (-18.00%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [208 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [eded51b...7f2dead](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9aef9c5764ebf995c9ab8955684b408397037e1e?el=desc) will **increase** coverage by `0.08%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
+ Coverage 61.50% 61.58% +0.08%
==========================================
Files 834 834
Lines 39557 39583 +26
Branches 3610 3625 +15
==========================================
+ Hits 24329 24379 +50
+ Misses 15047 15024 -23
+ Partials 181 180 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.94% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.78% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-0.82%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (+31.03%)` | :arrow_up: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `70.00% <55.17%> (+24.02%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (ø)` | |
| [...end/src/components/Select/SupersetStyledSelect.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1N1cGVyc2V0U3R5bGVkU2VsZWN0LnRzeA==) | `74.44% <0.00%> (+1.11%)` | :arrow_up: |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `61.11% <0.00%> (+5.55%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [9aef9c5...b366ff7](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ea105bc23b355530a2abad51eb9a35bcecbf08b5?el=desc) will **decrease** coverage by `4.30%`.
> The diff coverage is `5.55%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.77% 61.46% -4.31%
==========================================
Files 816 816
Lines 38406 38533 +127
Branches 3607 3631 +24
==========================================
- Hits 25263 23686 -1577
- Misses 13035 14661 +1626
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.89% <5.55%> (+0.16%)` | :arrow_up: |
| #python | `61.21% <ø> (-0.20%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-17.50%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `50.84% <16.66%> (-47.31%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [179 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [ea105bc...c0e053a](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712518343
Sounds great, @graceguo-supercat. The fixes here are mostly around the modal save, although there are a few changes to the label colors on the second save that may not have an effect on the overall scope if the user doesn't save afterward. The edit property modal changes still work, though. I know that there are larger usability issues here to address around the double save. I wasn't necessarily trying to address any of the dashboard save issues in this PR, but if you think it would be beneficial to address that issue here as well, I definitely can. Would that be your preference?
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-707986632
There are some discussions around improving the UI here as part of a larger effort. @junlincc, let's sync on what we can tackle now vs later. Is anything here valuable to roll out in the short term?
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-707508315
@eschutho and @mistercrunch Thanks for the work, but this save flow really confused me. See how `Set auto-refresh frequency`, `Set filter mapping` etc works,
- when user clicks `Save` button in the modal, it is only close Modal, but changes are not sent to server-side.
- when user clicks `Save` button on dashboard header, the changes are sent to server-side and saved into dashboards table.
But this `Edit dashboard properties` flow, it saved changes twice:
- when user clicks `Save` button in the modal, it saved changes by calling `/api/v1`
- when user clicks `Save` button on dashboard header, it saved changes by calling `/save_dashboard/`
Why does it have to save twice and calls different API?
I think we should have single API do `save dashboard metadata` job, and all the items in the edit dropdown list should behave 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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/0e97c4f66c5449bfbf7b195073b02550a071de0d?el=desc) will **decrease** coverage by `1.71%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.56% 59.85% -1.72%
==========================================
Files 835 394 -441
Lines 39653 24924 -14729
Branches 3604 0 -3604
==========================================
- Hits 24414 14919 -9495
+ Misses 15058 10005 -5053
+ Partials 181 0 -181
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `59.85% <ø> (-1.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `83.49% <0.00%> (-7.08%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
| ... and [454 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [0e97c4f...657d4c5](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709663681
retry tests
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712514309
Hi @eschutho Thanks for the work!
Yes #11220 and #11305 caused conflicts for this PR, and we can make this PR work with it. The idea of preventing co-edit is, when dashboard is loaded, it carries the last_modified_time which was stored in the database. When user edit dashboard and submit new updates to save, we send this last_modified_time with all the updates. `/save_dash/` endpoint compares the last_modified_time from incoming request with current database: if incoming last_modified_time < current data, this means the dashboard was changed **_since_** user open dashboard, so the save request will be rejected.
So for the Dashboard Properties modal, right now the problem is it didn't send last_modified_time information when it calling `/save_dash/` request, which happens after the first save is done. For other dashboard metadata change flow, we manage last_modified_time in redux store.
I just found dashboardInfo is already in redux store. so you can add a `lastModifiedTime` in dashboardInfo state, and update `lastModifiedTime` when data is updated from server-side. Similar like this:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/reducers/dashboardState.js#L110
At the same time, please update DashboardHeader container component. Instead of only getting lastModifiedTime from dashboardState:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/containers/DashboardHeader.jsx#L86
it should be:
`lastModifiedTime: Max(dashboardState.lastModifiedTime, dashboardInfo.lastModifiedTime)`
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-713012472
Sounds great. Should we add your suggestion regarding timestamps in this PR or a future one?
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-714761635
Great, thanks for your help @graceguo-supercat. Do you mind merging when ready? 🙏
----------------------------------------------------------------
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.
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] [incubator-superset] mistercrunch commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r497772635
##########
File path: superset/translations/en/LC_MESSAGES/messages.po
##########
@@ -3213,6 +3213,9 @@ msgstr ""
#~ msgid "One ore more annotation layers failed loading."
#~ msgstr ""
+#~ msgid "A valid color scheme is required"
Review comment:
Typically we don't `i18n` as part of daily PRs, we tend to do it in batches.
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712514309
Hi @eschutho Thanks for the work!
Yes #11220 and #11305 caused conflicts for this PR, and we can make this PR work with it. The idea of preventing co-edit is, when dashboard is loaded, it carries the last_modified_time which was stored in the database. When user edit dashboard and submit new updates to save, we send this last_modified_time with all the updates. `/save_dash/` endpoint compares the last_modified_time from incoming request with current database: if incoming last_modified_time < current data, this means the dashboard was changed **_since_** user open dashboard, so the save request will be rejected.
So for the Dashboard Properties modal, right now the problem is it didn't send last_modified_time information when it calling `/save_dash/` request, which happens after the first save is done. For other dashboard metadata change flow, we manage last_modified_time in redux store.
I just found dashboardInfo is already in redux store. so you can add a `lastModifiedTime` in dashboardInfo state, and update `lastModifiedTime` when data is updated from server-side. Similar like this:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/reducers/dashboardState.js#L110
At the same time, please update DashboardHeader container component. Instead of only getting lastModifiedTime from dashboardState:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/containers/DashboardHeader.jsx#L86
it should be:
`Max(dashboardState.lastModifiedTime, dashboardInfo.lastModifiedTime)`
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/3a08fd04f3f9ebaaa48fa554028c26062de42f02?el=desc) will **decrease** coverage by `0.52%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.70% 61.18% -0.53%
==========================================
Files 816 391 -425
Lines 38586 24485 -14101
Branches 3650 0 -3650
==========================================
- Hits 23808 14980 -8828
+ Misses 14598 9505 -5093
+ Partials 180 0 -180
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `61.18% <ø> (-0.24%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.65% <0.00%> (-8.31%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `50.76% <0.00%> (-4.24%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `90.69% <0.00%> (-0.65%)` | :arrow_down: |
| [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `76.47% <0.00%> (-0.50%)` | :arrow_down: |
| [superset/views/base\_api.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.90% <0.00%> (-0.36%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.12% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.70% <0.00%> (-0.14%)` | :arrow_down: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `84.21% <0.00%> (-0.10%)` | :arrow_down: |
| ... and [454 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [3a08fd0...f3a7907](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ea105bc23b355530a2abad51eb9a35bcecbf08b5?el=desc) will **decrease** coverage by `4.30%`.
> The diff coverage is `5.55%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.77% 61.47% -4.31%
==========================================
Files 816 816
Lines 38406 38533 +127
Branches 3607 3631 +24
==========================================
- Hits 25263 23687 -1576
- Misses 13035 14660 +1625
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.89% <5.55%> (+0.16%)` | :arrow_up: |
| #python | `61.22% <ø> (-0.19%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-17.50%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `50.84% <16.66%> (-47.31%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [178 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [ea105bc...c0e053a](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-707508315
@eschutho and @mistercrunch Thanks for the work, but this save flow really confused me. See how `Set auto-refresh frequency`, `Set filter mapping` etc works,
- when user clicks `Save` button in the modal, it is only close Modal, but changes are not sent to server-side.
- when user clicks `Save` button on dashboard header, the changes are sent to server-side and saved into dashboards table.
But this `Edit dashboard properties` flow, it saved changes twice:
- when user clicks `Save` button in the modal, it saved changes by calling `/api/v1`
- when user clicks `Save` button on dashboard header, it saved changes again, but calling `/save_dashboard/`
Why does it have to save twice and calls different API?
I think we should have single API do `save dashboard metadata` job, and all the items in the edit dropdown list should behave 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.
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] [incubator-superset] eschutho edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-693004442
yeah, we sorted it yesterday. I'm going to pick up the features that they wanted to add to this branch as well. **EDIT- the new features/refactor are going into a separate PR. This just fixes the existing bugs.
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712510112
I rebased so that we can bring in the changes from https://github.com/apache/incubator-superset/pull/11220 into this PR. Everything here still saves when you close the modal, even after refreshing the dashboard without a save.
----------------------------------------------------------------
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.
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] [incubator-superset] junlincc commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-707970395
@graceguo-supercat thanks for the additional feedback and they make sense to me. besides only having single API that does the job, we should also change `Save` label in the modal to `Finish` or `Done` to have more distinction from `Save` button on dashboard header.
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat commented on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-707508315
@eschutho and @mistercrunch Thanks for the work, but this save flow really confused me. See how `Set auto-refresh frequency`, `Set filter mapping` etc works,
- when user clicks `Save` button in the modal, it is only close Modal but not changes are not sent to server-side db
- when user clicks `Save` button on dashboard header, the changes are sent to server-side and saved into dashboards table.
But this `Edit dashboard properties` flow, saved changes twice:
- when user clicks `Save` button in the modal, it saved changes by calling `/api/v1`
- when user clicks `Save` button on dashboard header, it saved changes by calling `/save_dashboard/`
Why does it have to save twice and calls different API?
I think we should have single API do `save dashboard metadata` job, and all the items in the edit dropdown list should behave 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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712630022
I discussed this PR with PM in airbnb, we think should just fix the save dashboard metadata issue in this PR. For 2 `save dashboard` calls, it needs a bigger code refactor, we should address it later when we have a chance.
I updated my previous comment on how to fix timestamp issue, please take a look.
Thanks!
----------------------------------------------------------------
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.
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] [incubator-superset] mistercrunch commented on a change in pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r497771913
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts
##########
@@ -0,0 +1,190 @@
+/**
+ * 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-next-line import/no-extraneous-dependencies
+import * as ace from 'brace';
+import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
+
+const shortid = require('shortid');
Review comment:
`require` is out of fashion, let's us `import`!
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ea105bc23b355530a2abad51eb9a35bcecbf08b5?el=desc) will **decrease** coverage by `4.18%`.
> The diff coverage is `3.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.77% 61.59% -4.19%
==========================================
Files 816 816
Lines 38406 38528 +122
Branches 3607 3631 +24
==========================================
- Hits 25263 23732 -1531
- Misses 13035 14610 +1575
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.91% <3.33%> (+0.18%)` | :arrow_up: |
| #python | `61.40% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-17.50%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [172 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [ea105bc...57e0c9f](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/3a08fd04f3f9ebaaa48fa554028c26062de42f02?el=desc) will **decrease** coverage by `0.33%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.70% 61.36% -0.34%
==========================================
Files 816 391 -425
Lines 38586 24485 -14101
Branches 3650 0 -3650
==========================================
- Hits 23808 15025 -8783
+ Misses 14598 9460 -5138
+ Partials 180 0 -180
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `61.36% <ø> (-0.05%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/utils/decorators.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `50.76% <0.00%> (-4.24%)` | :arrow_down: |
| [superset/models/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `90.69% <0.00%> (-0.65%)` | :arrow_down: |
| [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `76.47% <0.00%> (-0.50%)` | :arrow_down: |
| [superset/views/base\_api.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.90% <0.00%> (-0.36%)` | :arrow_down: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `84.21% <0.00%> (-0.10%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.07% <0.00%> (-0.04%)` | :arrow_down: |
| [superset/errors.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXJyb3JzLnB5) | `100.00% <0.00%> (ø)` | |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `74.56% <0.00%> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `88.29% <0.00%> (ø)` | |
| [superset/charts/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL3NjaGVtYXMucHk=) | `100.00% <0.00%> (ø)` | |
| ... and [450 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [3a08fd0...f3a7907](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: use conditional operator for lookup
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-690817969
when running cypress locally/dev/docker we still need to change the baseUrl in cypress.json to hit port 8088. Will look into ways to make this configurable.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9aef9c5764ebf995c9ab8955684b408397037e1e?el=desc) will **decrease** coverage by `0.90%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.50% 60.60% -0.91%
==========================================
Files 834 393 -441
Lines 39557 24836 -14721
Branches 3610 0 -3610
==========================================
- Hits 24329 15051 -9278
+ Misses 15047 9785 -5262
+ Partials 181 0 -181
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `60.60% <ø> (-0.19%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.65% <0.00%> (-8.43%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.82% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.60% <0.00%> (-0.14%)` | :arrow_down: |
| [...et-frontend/src/messageToasts/components/Toast.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL21lc3NhZ2VUb2FzdHMvY29tcG9uZW50cy9Ub2FzdC50c3g=) | | |
| [...d/src/dashboard/util/getLeafComponentIdFromPath.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldExlYWZDb21wb25lbnRJZEZyb21QYXRoLmpz) | | |
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | | |
| [...-frontend/src/dashboard/components/SliceHeader.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyLmpzeA==) | | |
| [...src/SqlLab/utils/reduxStateToLocalStorageHelper.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi91dGlscy9yZWR1eFN0YXRlVG9Mb2NhbFN0b3JhZ2VIZWxwZXIuanM=) | | |
| ... and [432 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [9aef9c5...b366ff7](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9aef9c5764ebf995c9ab8955684b408397037e1e?el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.50% 61.47% -0.03%
==========================================
Files 834 834
Lines 39557 39583 +26
Branches 3610 3625 +15
==========================================
+ Hits 24329 24334 +5
- Misses 15047 15069 +22
+ Partials 181 180 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.94% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.60% <ø> (-0.19%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-0.82%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (+31.03%)` | :arrow_up: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `70.00% <55.17%> (+24.02%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (ø)` | |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.65% <0.00%> (-8.43%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.82% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.60% <0.00%> (-0.14%)` | :arrow_down: |
| [...end/src/components/Select/SupersetStyledSelect.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1N1cGVyc2V0U3R5bGVkU2VsZWN0LnRzeA==) | `74.44% <0.00%> (+1.11%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [9aef9c5...b366ff7](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712514309
Hi @eschutho Thanks for the work!
Yes #11220 and #11305 caused conflicts for this PR, and we can make this PR work with it. The idea of preventing co-edit is, when dashboard is loaded, it carries the last_modified_time which was stored in the database. When user edit dashboard and submit new updates to save, we send this last_modified_time with all the updates. `/save_dash/` endpoint compares the last_modified_time from incoming request with current database: if incoming last_modified_time < current data, this means the dashboard was changed **_since_** user open dashboard, so the save request will be rejected.
So for the Dashboard Properties modal, right now the problem is it didn't send last_modified_time information when it calling `/save_dash/` request, which happens after the first save is done. For other dashboard metadata change flow, we manage last_modified_time in redux store. Dashboard Properties modal didn't use redux store, but you can manually add this parameter like this:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/reducers/dashboardState.js#L110
this can make sure the incoming request always newer than database record.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-709596900
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/0e97c4f66c5449bfbf7b195073b02550a071de0d?el=desc) will **decrease** coverage by `0.33%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 61.56% 61.23% -0.34%
==========================================
Files 835 835
Lines 39653 39679 +26
Branches 3604 3619 +15
==========================================
- Hits 24414 24297 -117
- Misses 15058 15202 +144
+ Partials 181 180 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.90% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.24% <ø> (-0.67%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-0.82%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (+31.03%)` | :arrow_up: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `70.00% <55.17%> (+24.02%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.59% <0.00%> (-2.41%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [0e97c4f...657d4c5](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ea105bc23b355530a2abad51eb9a35bcecbf08b5?el=desc) will **decrease** coverage by `4.60%`.
> The diff coverage is `5.55%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.77% 61.17% -4.61%
==========================================
Files 816 816
Lines 38406 38533 +127
Branches 3607 3631 +24
==========================================
- Hits 25263 23571 -1692
- Misses 13035 14776 +1741
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.89% <5.55%> (+0.16%)` | :arrow_up: |
| #python | `60.74% <ø> (-0.67%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-17.50%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `38.23% <0.00%> (-22.48%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `55.17% <ø> (-41.38%)` | :arrow_down: |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `50.84% <16.66%> (-47.31%)` | :arrow_down: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [181 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [ea105bc...c0e053a](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=h1) Report
> Merging [#10834](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d95b7c2a73ecd0bf0f44434d80ab8496d9cb6311?el=desc) will **decrease** coverage by `4.53%`.
> The diff coverage is `48.57%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10834 +/- ##
==========================================
- Coverage 65.88% 61.35% -4.54%
==========================================
Files 827 827
Lines 39070 39076 +6
Branches 3676 3691 +15
==========================================
- Hits 25741 23974 -1767
- Misses 13222 14922 +1700
- Partials 107 180 +73
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.46% <48.57%> (+0.22%)` | :arrow_up: |
| #python | `60.68% <ø> (-0.67%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `34.84% <0.00%> (-16.32%)` | :arrow_down: |
| [...explore/components/controls/ColorSchemeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvclNjaGVtZUNvbnRyb2wuanN4) | `86.20% <ø> (-10.35%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `71.02% <55.17%> (+10.31%)` | :arrow_up: |
| [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <100.00%> (-15.72%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [183 more](https://codecov.io/gh/apache/incubator-superset/pull/10834/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=footer). Last update [d95b7c2...be343c2](https://codecov.io/gh/apache/incubator-superset/pull/10834?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-698625718
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho edited a comment on pull request #10834: [WIP] fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-693004442
yeah, we sorted it yesterday. I'm going to pick up the features that they wanted to add to this branch as well. **EDIT- the new features/refactor are going into a separate PR. This just fixes the existing bugs.
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-707508315
@eschutho and @mistercrunch Thanks for the work, but this save flow really confused me. See how `Set auto-refresh frequency`, `Set filter mapping` etc works,
- when user clicks `Save` button in the modal, it is only close Modal, but changes are not sent to server-side.
- when user clicks `Save` button on dashboard header, the changes are sent to server-side and saved into dashboards table.
But this `Edit dashboard properties` flow, saved changes twice:
- when user clicks `Save` button in the modal, it saved changes by calling `/api/v1`
- when user clicks `Save` button on dashboard header, it saved changes by calling `/save_dashboard/`
Why does it have to save twice and calls different API?
I think we should have single API do `save dashboard metadata` job, and all the items in the edit dropdown list should behave 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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-712514309
Hi @eschutho Thanks for the work!
Yes #11220 and #11305 caused conflicts for this PR, and we can make this PR work with it. The idea of preventing co-edit is, when dashboard is loaded, it carries the last_modified_time which was stored in the database. When user edit dashboard and submit new updates to save, we send this last_modified_time with all the updates. `/save_dash/` endpoint compares the last_modified_time from incoming request with current database: if incoming last_modified_time < current data, this means the dashboard was changed **_since_** user open dashboard, so the save request will be rejected.
So for the Dashboard Properties modal, right now the problem is it didn't send last_modified_time information when it calling `/save_dash/` request, which happens after the first save is done. Dashboard Properties modal didn't use redux store, but you can manually add this parameter like this:
https://github.com/apache/incubator-superset/blob/8863c939ad01fe1e065ce60edc93a683e6828d80/superset-frontend/src/dashboard/reducers/dashboardState.js#L110
this can make sure the incoming request always newer than database record.
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat merged pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat merged pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on pull request #10834: use conditional operator for lookup
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-690817969
when running cypress locally/dev/docker we still need to change the baseUrl in cypress.json to hit port 8088. Will look into ways to make this configurable.
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho closed pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho closed pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho commented on a change in pull request #10834: fix dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#discussion_r488283584
##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js
##########
@@ -16,55 +16,118 @@
* specific language governing permissions and limitations
* under the License.
*/
+
+import shortid from 'shortid';
import readResponseBlob from '../../utils/readResponseBlob';
import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper';
describe('Dashboard save action', () => {
- let dashboardId;
-
beforeEach(() => {
cy.server();
cy.login();
cy.visit(WORLD_HEALTH_DASHBOARD);
+ });
- cy.get('#app').then(data => {
- const bootstrapData = JSON.parse(data[0].dataset.bootstrap);
- const dashboard = bootstrapData.dashboard_data;
- dashboardId = dashboard.id;
- cy.route('POST', `/superset/copy_dash/${dashboardId}/`).as('copyRequest');
- });
+ // it('should save as new dashboard', () => {
Review comment:
👍 I uncommented it. Thanks for catching!
----------------------------------------------------------------
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.
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] [incubator-superset] eschutho edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-702480353
----------------------------------------------------------------
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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-707508315
@eschutho and @mistercrunch Thanks for the work, but this `save dashboard flow` really confused me. See how `Set auto-refresh frequency`, `Set filter mapping` etc works:
- when user clicks `Save` button in the modal, it is only close Modal, but changes are not sent to server-side.
- when user clicks `Save` button on dashboard header, the changes are sent to server-side and saved into dashboards table.
But this `Edit dashboard properties` flow, it saved changes twice:
- when user clicks `Save` button in the modal, it saved changes by calling `/api/v1`
- when user clicks `Save` button on dashboard header, it saved changes again, but calling `/save_dashboard/`
Why does it have to save twice and calls different API?
I think we should have single API do `save dashboard metadata` job, and all the items in the edit dropdown list should behave 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.
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] [incubator-superset] graceguo-supercat edited a comment on pull request #10834: fix: dashboard edit/save errors
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10834:
URL: https://github.com/apache/incubator-superset/pull/10834#issuecomment-713095207
> Sounds great. Should we add your suggestion regarding timestamps in this PR or a future one?
should be in this PR. Otherwise after user save changes in Dashboard Properties modal, they will see error message (even changes are actually saved).
----------------------------------------------------------------
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.
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