You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/12/17 17:57:24 UTC
[GitHub] [superset] JosueLugaro opened a new pull request #17807: Josue/sqleditorleftbar to functional
JosueLugaro opened a new pull request #17807:
URL: https://github.com/apache/superset/pull/17807
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
My conversion to a functional component is causing an infinite re render, unsure of where exactly this is taking place
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r787149772
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -51,98 +51,35 @@ const StyledScrollbarContent = styled.div`
height: ${props => props.contentHeight}px;
`;
-export default class SqlEditorLeftBar extends React.PureComponent {
- constructor(props) {
- super(props);
- this.resetState = this.resetState.bind(this);
- this.onSchemaChange = this.onSchemaChange.bind(this);
- this.onSchemasLoad = this.onSchemasLoad.bind(this);
- this.onTablesLoad = this.onTablesLoad.bind(this);
- this.onDbChange = this.onDbChange.bind(this);
- this.getDbList = this.getDbList.bind(this);
- this.onTableChange = this.onTableChange.bind(this);
- this.onToggleTable = this.onToggleTable.bind(this);
- }
-
- onSchemaChange(schema) {
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
- }
-
- onSchemasLoad(schemas) {
- this.props.actions.queryEditorSetSchemaOptions(
- this.props.queryEditor,
- schemas,
- );
- }
-
- onTablesLoad(tables) {
- this.props.actions.queryEditorSetTableOptions(
- this.props.queryEditor,
- tables,
- );
- }
-
- onDbChange(db) {
- this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- db.id,
- );
- }
-
- onTableChange(tableName, schemaName) {
+export default function SqlEditorLeftBar({
Review comment:
Please change this to an arrow function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a change in pull request #17807: Josue/sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r771588221
##########
File path: superset-frontend/src/components/DatabaseSelector/index.tsx
##########
@@ -205,6 +205,7 @@ export default function DatabaseSelector({
[formMode, getDbList, handleError, sqlLabMode],
);
+ console.log(db, schema, 'INSIDE DATABASE SELECTOR');
Review comment:
Make sure this is deleted in the final :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a change in pull request #17807: Josue/sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r771607063
##########
File path: superset-frontend/src/components/DatabaseSelector/index.tsx
##########
@@ -205,6 +205,7 @@ export default function DatabaseSelector({
[formMode, getDbList, handleError, sqlLabMode],
);
+ console.log(db, schema, 'INSIDE DATABASE SELECTOR');
Review comment:
No worries! And its just in draft form so its all good. Just something to keep in mind for when it goes into review (I am guilty of this one 0_0 )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999039329
# [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17807](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11563b) into [master](https://codecov.io/gh/apache/superset/commit/c49545aec1ecddc6ee0e6a55597d8f26c2680009?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c49545a) will **increase** coverage by `0.00%`.
> The diff coverage is `72.22%`.
[](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17807 +/- ##
=======================================
Coverage 67.03% 67.03%
=======================================
Files 1609 1609
Lines 64877 64856 -21
Branches 6863 6861 -2
=======================================
- Hits 43488 43479 -9
+ Misses 19525 19512 -13
- Partials 1864 1865 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.78% <72.22%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/SqlLab/components/SqlEditorLeftBar/index.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIvaW5kZXguanN4) | `67.74% <72.22%> (+12.18%)` | :arrow_up: |
| [...et-frontend/src/components/EditableTitle/index.tsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS9pbmRleC50c3g=) | `71.79% <0.00%> (-1.29%)` | :arrow_down: |
| [...end/src/components/Datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YXNvdXJjZS9EYXRhc291cmNlRWRpdG9yLmpzeA==) | `68.93% <0.00%> (-0.17%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c49545a...e11563b](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] JosueLugaro commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
JosueLugaro commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999208175
> This is great @JosueLugaro, thank you! I left some feedback, but lmk if you have any questions.
Thank you! I will get right on 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-1016868761
Ephemeral environment shutdown and build artifacts deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] JosueLugaro commented on a change in pull request #17807: Josue/sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
JosueLugaro commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r771589622
##########
File path: superset-frontend/src/components/DatabaseSelector/index.tsx
##########
@@ -205,6 +205,7 @@ export default function DatabaseSelector({
[formMode, getDbList, handleError, sqlLabMode],
);
+ console.log(db, schema, 'INSIDE DATABASE SELECTOR');
Review comment:
Of course, I apologize, I was trying to find them all and evidently missed 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999054051
/testenv 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r787149772
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -51,98 +51,35 @@ const StyledScrollbarContent = styled.div`
height: ${props => props.contentHeight}px;
`;
-export default class SqlEditorLeftBar extends React.PureComponent {
- constructor(props) {
- super(props);
- this.resetState = this.resetState.bind(this);
- this.onSchemaChange = this.onSchemaChange.bind(this);
- this.onSchemasLoad = this.onSchemasLoad.bind(this);
- this.onTablesLoad = this.onTablesLoad.bind(this);
- this.onDbChange = this.onDbChange.bind(this);
- this.getDbList = this.getDbList.bind(this);
- this.onTableChange = this.onTableChange.bind(this);
- this.onToggleTable = this.onToggleTable.bind(this);
- }
-
- onSchemaChange(schema) {
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
- }
-
- onSchemasLoad(schemas) {
- this.props.actions.queryEditorSetSchemaOptions(
- this.props.queryEditor,
- schemas,
- );
- }
-
- onTablesLoad(tables) {
- this.props.actions.queryEditorSetTableOptions(
- this.props.queryEditor,
- tables,
- );
- }
-
- onDbChange(db) {
- this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- db.id,
- );
- }
-
- onTableChange(tableName, schemaName) {
+export default function SqlEditorLeftBar({
Review comment:
Please change this to an arrow function
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +98,73 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = height - 130; // 130 is the height of the selects above
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={database}
+ dbId={queryEditor.dbId}
+ getDbList={actions.setDatabases}
+ handleError={actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={actions.queryEditorSetSchema}
+ onSchemasLoad={actions.queryEditorSetSchemaOptions}
+ onTableChange={onTableChange}
+ onTablesLoad={actions.queryEditorSetTableOptions}
+ schema={queryEditor.schema}
+ sqlLabMode
+ />
+ <div className="divider" />
+ <StyledScrollbarContainer>
+ <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
+ <Collapse
+ activeKey={tb
+ .filter(({ expanded }) => expanded)
+ .map(({ id }) => id)}
+ css={theme => css`
Review comment:
These style definitions are a little long, can you move them to an external variable? As per [the style guide](https://github.com/apache/superset/issues/15264).
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +98,73 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = height - 130; // 130 is the height of the selects above
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={database}
+ dbId={queryEditor.dbId}
+ getDbList={actions.setDatabases}
+ handleError={actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={actions.queryEditorSetSchema}
+ onSchemasLoad={actions.queryEditorSetSchemaOptions}
+ onTableChange={onTableChange}
+ onTablesLoad={actions.queryEditorSetTableOptions}
+ schema={queryEditor.schema}
+ sqlLabMode
+ />
+ <div className="divider" />
+ <StyledScrollbarContainer>
+ <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
Review comment:
```suggestion
<div
css={css`
height: ${props => props.contentHeight}px;
`}
contentHeight={tableMetaDataHeight}
>
```
Since this style definition is only one line, it doesn't need to be in an external variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999039329
# [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17807](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f8f91fa) into [master](https://codecov.io/gh/apache/superset/commit/88db2cc0ab425ff1c8298cbbc677d42da90a40c9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (88db2cc) will **increase** coverage by `0.00%`.
> The diff coverage is `54.16%`.
[](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17807 +/- ##
=======================================
Coverage 66.36% 66.36%
=======================================
Files 1570 1570
Lines 61767 61751 -16
Branches 6243 6242 -1
=======================================
- Hits 40990 40981 -9
+ Misses 19178 19170 -8
- Partials 1599 1600 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `50.91% <54.16%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/SqlLab/components/SqlEditorLeftBar/index.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIvaW5kZXguanN4) | `54.28% <54.16%> (-1.27%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `56.52% <0.00%> (+0.27%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [88db2cc...f8f91fa](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r773528654
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +112,74 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = props.height - 130; // 130 is the height of the selects above
+ const qe = props.queryEditor;
Review comment:
you can destructure this line as well: `const {queryEditor: qe}= props.queryEditor`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] removed a comment on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
github-actions[bot] removed a comment on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999017240
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams merged pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams merged pull request #17807:
URL: https://github.com/apache/superset/pull/17807
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r788026014
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +112,74 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = props.height - 130; // 130 is the height of the selects above
+ const qe = props.queryEditor;
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={props.database}
+ dbId={qe.dbId}
+ getDbList={() => getDbList}
+ handleError={props.actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={onSchemaChange}
+ onSchemasLoad={() => onSchemasLoad}
Review comment:
You're correct @JosueLugaro. I realized that below. It looks like you got it, though!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a change in pull request #17807: Josue/sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r773314600
##########
File path: superset-frontend/src/components/TableSelector/index.tsx
##########
@@ -33,6 +33,7 @@ import DatabaseSelector, {
import RefreshLabel from 'src/components/RefreshLabel';
import CertifiedBadge from 'src/components/CertifiedBadge';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
+import { ConsoleSqlOutlined } from '@ant-design/icons';
Review comment:
Why is this change in here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a change in pull request #17807: Josue/sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r773360435
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +112,77 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = props.height - 130; // 130 is the height of the selects above
+ const qe = props.queryEditor;
+
+ // console.log(props.database, 'DATABASE IN EDITOR LEFT');
Review comment:
please remove these console logs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi removed a comment on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi removed a comment on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999016868
/testenv 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999017240
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r773531374
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -51,98 +51,49 @@ const StyledScrollbarContent = styled.div`
height: ${props => props.contentHeight}px;
`;
-export default class SqlEditorLeftBar extends React.PureComponent {
- constructor(props) {
- super(props);
- this.resetState = this.resetState.bind(this);
- this.onSchemaChange = this.onSchemaChange.bind(this);
- this.onSchemasLoad = this.onSchemasLoad.bind(this);
- this.onTablesLoad = this.onTablesLoad.bind(this);
- this.onDbChange = this.onDbChange.bind(this);
- this.getDbList = this.getDbList.bind(this);
- this.onTableChange = this.onTableChange.bind(this);
- this.onToggleTable = this.onToggleTable.bind(this);
- }
-
- onSchemaChange(schema) {
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
- }
-
- onSchemasLoad(schemas) {
- this.props.actions.queryEditorSetSchemaOptions(
- this.props.queryEditor,
- schemas,
- );
- }
-
- onTablesLoad(tables) {
- this.props.actions.queryEditorSetTableOptions(
- this.props.queryEditor,
- tables,
- );
- }
-
- onDbChange(db) {
- this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- db.id,
- );
- }
-
- onTableChange(tableName, schemaName) {
+export default function SqlEditorLeftBar(props) {
+ const onSchemaChange = schema => {
+ props.actions.queryEditorSetSchema(props.queryEditor, schema);
+ };
+
+ const onSchemasLoad = schemas => {
+ props.actions.queryEditorSetSchemaOptions(props.queryEditor, schemas);
+ };
+
+ const onTablesLoad = tables => {
+ props.actions.queryEditorSetTableOptions(props.queryEditor, tables);
+ };
Review comment:
I think we can remove these three functions as well and just call them below.
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -51,98 +51,49 @@ const StyledScrollbarContent = styled.div`
height: ${props => props.contentHeight}px;
`;
-export default class SqlEditorLeftBar extends React.PureComponent {
- constructor(props) {
- super(props);
- this.resetState = this.resetState.bind(this);
- this.onSchemaChange = this.onSchemaChange.bind(this);
- this.onSchemasLoad = this.onSchemasLoad.bind(this);
- this.onTablesLoad = this.onTablesLoad.bind(this);
- this.onDbChange = this.onDbChange.bind(this);
- this.getDbList = this.getDbList.bind(this);
- this.onTableChange = this.onTableChange.bind(this);
- this.onToggleTable = this.onToggleTable.bind(this);
- }
-
- onSchemaChange(schema) {
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
- }
-
- onSchemasLoad(schemas) {
- this.props.actions.queryEditorSetSchemaOptions(
- this.props.queryEditor,
- schemas,
- );
- }
-
- onTablesLoad(tables) {
- this.props.actions.queryEditorSetTableOptions(
- this.props.queryEditor,
- tables,
- );
- }
-
- onDbChange(db) {
- this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- db.id,
- );
- }
-
- onTableChange(tableName, schemaName) {
+export default function SqlEditorLeftBar(props) {
+ const onSchemaChange = schema => {
+ props.actions.queryEditorSetSchema(props.queryEditor, schema);
+ };
+
+ const onSchemasLoad = schemas => {
+ props.actions.queryEditorSetSchemaOptions(props.queryEditor, schemas);
+ };
+
+ const onTablesLoad = tables => {
+ props.actions.queryEditorSetTableOptions(props.queryEditor, tables);
+ };
+
+ const onDbChange = db => {
+ props.actions.queryEditorSetDb(props.queryEditor, db.id);
+ props.actions.queryEditorSetFunctionNames(props.queryEditor, db.id);
+ };
+
+ const onTableChange = (tableName, schemaName) => {
if (tableName && schemaName) {
- this.props.actions.addTable(
- this.props.queryEditor,
- tableName,
- schemaName,
- );
+ props.actions.addTable(props.queryEditor, tableName, schemaName);
}
- }
+ };
- onToggleTable(tables) {
- this.props.tables.forEach(table => {
+ const onToggleTable = tables => {
+ props.tables.forEach(table => {
if (!tables.includes(table.id.toString()) && table.expanded) {
- this.props.actions.collapseTable(table);
+ props.actions.collapseTable(table);
} else if (tables.includes(table.id.toString()) && !table.expanded) {
- this.props.actions.expandTable(table);
+ props.actions.expandTable(table);
}
});
- }
-
- getDbList(dbs) {
- this.props.actions.setDatabases(dbs);
- }
-
- dbMutator(data) {
- const options = data.result.map(db => ({
- value: db.id,
- label: db.database_name,
- }));
- this.props.actions.setDatabases(data.result);
- if (data.result.length === 0) {
- this.props.actions.addDangerToast(
- t("It seems you don't have access to any database"),
- );
- }
- return options;
- }
+ };
- resetState() {
- this.props.actions.resetState();
- }
+ const getDbList = dbs => {
+ props.actions.setDatabases(dbs);
+ };
Review comment:
same as below..
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +112,74 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = props.height - 130; // 130 is the height of the selects above
+ const qe = props.queryEditor;
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={props.database}
+ dbId={qe.dbId}
+ getDbList={() => getDbList}
+ handleError={props.actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={onSchemaChange}
+ onSchemasLoad={() => onSchemasLoad}
+ onTableChange={onTableChange}
+ onTablesLoad={() => onTablesLoad}
Review comment:
same here.. don't need the extra function wrapper. Actually this line and the above will just return the function and not instantiate it. Let me know if the above recommendation doesn't work.
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +112,74 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = props.height - 130; // 130 is the height of the selects above
+ const qe = props.queryEditor;
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={props.database}
+ dbId={qe.dbId}
+ getDbList={() => getDbList}
+ handleError={props.actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={onSchemaChange}
+ onSchemasLoad={() => onSchemasLoad}
+ onTableChange={onTableChange}
+ onTablesLoad={() => onTablesLoad}
+ schema={qe.schema}
+ sqlLabMode
+ />
+ <div className="divider" />
+ <StyledScrollbarContainer>
+ <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
+ <Collapse
+ activeKey={props.tables
+ .filter(({ expanded }) => expanded)
+ .map(({ id }) => id)}
+ css={theme => css`
+ .ant-collapse-item {
+ margin-bottom: ${theme.gridUnit * 3}px;
+ }
+ .ant-collapse-header {
+ padding: 0px !important;
+ display: flex;
+ align-items: center;
+ }
+ .ant-collapse-content-box {
+ padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
+ }
+ .ant-collapse-arrow {
+ top: ${theme.gridUnit * 2}px !important;
+ color: ${theme.colors.primary.dark1} !important;
+ &: hover {
+ color: ${theme.colors.primary.dark2} !important;
Review comment:
normally, we wouldn't put these importants in here, but we can fix that later, since they were in the original file and out of scope for this fix.
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +112,74 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = props.height - 130; // 130 is the height of the selects above
+ const qe = props.queryEditor;
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={props.database}
+ dbId={qe.dbId}
+ getDbList={() => getDbList}
+ handleError={props.actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={onSchemaChange}
+ onSchemasLoad={() => onSchemasLoad}
Review comment:
this is equivalent to `onSchemasLoad={onSchemasLoad}`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999016868
/testenv 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999039329
# [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17807](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d9bb646) into [master](https://codecov.io/gh/apache/superset/commit/cfd851aa13e1f9642cce014e84669293e65c38bb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cfd851a) will **increase** coverage by `0.00%`.
> The diff coverage is `65.62%`.
[](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17807 +/- ##
=======================================
Coverage 67.09% 67.09%
=======================================
Files 1609 1609
Lines 64863 64854 -9
Branches 6863 6861 -2
=======================================
- Hits 43520 43517 -3
+ Misses 19479 19473 -6
Partials 1864 1864
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.77% <65.62%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/SqlLab/components/SqlEditorLeftBar/index.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIvaW5kZXguanN4) | `64.44% <65.62%> (+8.88%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.28% <0.00%> (-0.58%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cfd851a...d9bb646](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] JosueLugaro commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
JosueLugaro commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r773539042
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +112,74 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = props.height - 130; // 130 is the height of the selects above
+ const qe = props.queryEditor;
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={props.database}
+ dbId={qe.dbId}
+ getDbList={() => getDbList}
+ handleError={props.actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={onSchemaChange}
+ onSchemasLoad={() => onSchemasLoad}
Review comment:
I'm not sure that they are equivalent. When I initially converted to a functional component, the sql lab and left bar were re rendering infinitely. Arash debugged the issue and found that utilizing "() => onSchemasLoad" along with adding "() =>" to other functions that were passed as props fixed the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] JosueLugaro commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
JosueLugaro commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-1016823724
> Thanks @JosueLugaro looks great!
Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999039329
# [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17807](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11563b) into [master](https://codecov.io/gh/apache/superset/commit/88db2cc0ab425ff1c8298cbbc677d42da90a40c9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (88db2cc) will **increase** coverage by `0.67%`.
> The diff coverage is `72.22%`.
> :exclamation: Current head e11563b differs from pull request most recent head f8f91fa. Consider uploading reports for the commit f8f91fa to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17807 +/- ##
==========================================
+ Coverage 66.36% 67.03% +0.67%
==========================================
Files 1570 1609 +39
Lines 61767 64856 +3089
Branches 6243 6861 +618
==========================================
+ Hits 40990 43479 +2489
- Misses 19178 19512 +334
- Partials 1599 1865 +266
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.78% <72.22%> (+2.85%)` | :arrow_up: |
| presto | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/SqlLab/components/SqlEditorLeftBar/index.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIvaW5kZXguanN4) | `67.74% <72.22%> (+12.18%)` | :arrow_up: |
| [...tend/src/filters/components/Select/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvY29udHJvbFBhbmVsLnRz) | `58.33% <0.00%> (-41.67%)` | :arrow_down: |
| [...eModal/DatabaseConnectionForm/CommonParameters.tsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0RhdGFiYXNlQ29ubmVjdGlvbkZvcm0vQ29tbW9uUGFyYW1ldGVycy50c3g=) | `51.42% <0.00%> (-34.29%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/data/database/state.ts](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9zdGF0ZS50cw==) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [...t-frontend/src/components/PopoverSection/index.tsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUG9wb3ZlclNlY3Rpb24vaW5kZXgudHN4) | `73.33% <0.00%> (-26.67%)` | :arrow_down: |
| [...t-frontend/src/components/MessageToasts/actions.ts](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVzc2FnZVRvYXN0cy9hY3Rpb25zLnRz) | `72.00% <0.00%> (-21.75%)` | :arrow_down: |
| [...hboard/components/menu/BackgroundStyleDropdown.tsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL21lbnUvQmFja2dyb3VuZFN0eWxlRHJvcGRvd24udHN4) | `81.81% <0.00%> (-18.19%)` | :arrow_down: |
| [.../CRUD/data/database/DatabaseModal/ExtraOptions.tsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0V4dHJhT3B0aW9ucy50c3g=) | `65.85% <0.00%> (-17.48%)` | :arrow_down: |
| [superset-frontend/src/dataMask/actions.ts](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFNYXNrL2FjdGlvbnMudHM=) | `55.55% <0.00%> (-17.18%)` | :arrow_down: |
| [...nd/src/explore/components/DatasourcePanel/types.ts](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwvdHlwZXMudHM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
| ... and [546 more](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [88db2cc...f8f91fa](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] JosueLugaro commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
JosueLugaro commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-1016920682
> Looks great, thanks for working on this! ✨
Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999822536
@AAfghahi Ephemeral environment spinning up at http://35.160.28.49:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999820154
/testenv 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r773527693
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -51,98 +51,49 @@ const StyledScrollbarContent = styled.div`
height: ${props => props.contentHeight}px;
`;
-export default class SqlEditorLeftBar extends React.PureComponent {
- constructor(props) {
- super(props);
- this.resetState = this.resetState.bind(this);
- this.onSchemaChange = this.onSchemaChange.bind(this);
- this.onSchemasLoad = this.onSchemasLoad.bind(this);
- this.onTablesLoad = this.onTablesLoad.bind(this);
- this.onDbChange = this.onDbChange.bind(this);
- this.getDbList = this.getDbList.bind(this);
- this.onTableChange = this.onTableChange.bind(this);
- this.onToggleTable = this.onToggleTable.bind(this);
- }
-
- onSchemaChange(schema) {
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
- }
-
- onSchemasLoad(schemas) {
- this.props.actions.queryEditorSetSchemaOptions(
- this.props.queryEditor,
- schemas,
- );
- }
-
- onTablesLoad(tables) {
- this.props.actions.queryEditorSetTableOptions(
- this.props.queryEditor,
- tables,
- );
- }
-
- onDbChange(db) {
- this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- db.id,
- );
- }
-
- onTableChange(tableName, schemaName) {
+export default function SqlEditorLeftBar(props) {
Review comment:
I would suggest as an improvement here, to destructure the props into each individual property. Just one level is probably fine, like `actions`, `tables`, `queryEditor`, etc. That will clean up a lot of the `props.` code in this file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #17807: Josue/sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r771591377
##########
File path: superset-frontend/src/components/DatabaseSelector/index.tsx
##########
@@ -205,6 +205,7 @@ export default function DatabaseSelector({
[formMode, getDbList, handleError, sqlLabMode],
);
+ console.log(db, schema, 'INSIDE DATABASE SELECTOR');
Review comment:
I told him it'd be okay to leave the code a little messy if he's just pushing it up to a draft PR for us to help debug, that's on me 😁
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999055100
@AAfghahi Ephemeral environment spinning up at http://54.202.40.124:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r773528143
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -51,98 +51,49 @@ const StyledScrollbarContent = styled.div`
height: ${props => props.contentHeight}px;
`;
-export default class SqlEditorLeftBar extends React.PureComponent {
- constructor(props) {
- super(props);
- this.resetState = this.resetState.bind(this);
- this.onSchemaChange = this.onSchemaChange.bind(this);
- this.onSchemasLoad = this.onSchemasLoad.bind(this);
- this.onTablesLoad = this.onTablesLoad.bind(this);
- this.onDbChange = this.onDbChange.bind(this);
- this.getDbList = this.getDbList.bind(this);
- this.onTableChange = this.onTableChange.bind(this);
- this.onToggleTable = this.onToggleTable.bind(this);
- }
-
- onSchemaChange(schema) {
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
- }
-
- onSchemasLoad(schemas) {
- this.props.actions.queryEditorSetSchemaOptions(
- this.props.queryEditor,
- schemas,
- );
- }
-
- onTablesLoad(tables) {
- this.props.actions.queryEditorSetTableOptions(
- this.props.queryEditor,
- tables,
- );
- }
-
- onDbChange(db) {
- this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- db.id,
- );
- }
-
- onTableChange(tableName, schemaName) {
+export default function SqlEditorLeftBar(props) {
+ const onSchemaChange = schema => {
+ props.actions.queryEditorSetSchema(props.queryEditor, schema);
+ };
+
+ const onSchemasLoad = schemas => {
+ props.actions.queryEditorSetSchemaOptions(props.queryEditor, schemas);
+ };
+
+ const onTablesLoad = tables => {
+ props.actions.queryEditorSetTableOptions(props.queryEditor, tables);
+ };
+
+ const onDbChange = db => {
+ props.actions.queryEditorSetDb(props.queryEditor, db.id);
+ props.actions.queryEditorSetFunctionNames(props.queryEditor, db.id);
+ };
+
+ const onTableChange = (tableName, schemaName) => {
if (tableName && schemaName) {
- this.props.actions.addTable(
- this.props.queryEditor,
- tableName,
- schemaName,
- );
+ props.actions.addTable(props.queryEditor, tableName, schemaName);
}
- }
+ };
- onToggleTable(tables) {
- this.props.tables.forEach(table => {
+ const onToggleTable = tables => {
+ props.tables.forEach(table => {
if (!tables.includes(table.id.toString()) && table.expanded) {
- this.props.actions.collapseTable(table);
+ props.actions.collapseTable(table);
} else if (tables.includes(table.id.toString()) && !table.expanded) {
- this.props.actions.expandTable(table);
+ props.actions.expandTable(table);
}
});
- }
-
- getDbList(dbs) {
- this.props.actions.setDatabases(dbs);
- }
-
- dbMutator(data) {
- const options = data.result.map(db => ({
- value: db.id,
- label: db.database_name,
- }));
- this.props.actions.setDatabases(data.result);
- if (data.result.length === 0) {
- this.props.actions.addDangerToast(
- t("It seems you don't have access to any database"),
- );
- }
- return options;
- }
+ };
- resetState() {
- this.props.actions.resetState();
- }
+ const getDbList = dbs => {
+ props.actions.setDatabases(dbs);
+ };
- changeTable(tableOpt) {
- if (!tableOpt) {
- return;
- }
- const schemaName = tableOpt.value.schema;
- const tableName = tableOpt.value.table;
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schemaName);
- this.props.actions.addTable(this.props.queryEditor, tableName, schemaName);
- }
+ const resetState = () => {
+ props.actions.resetState();
+ };
Review comment:
this function is probably redundant. Can we just call the action?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17807: refactor: sqleditorleftbar to functional
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17807:
URL: https://github.com/apache/superset/pull/17807#issuecomment-999039329
# [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17807](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d9bb646) into [master](https://codecov.io/gh/apache/superset/commit/c49545aec1ecddc6ee0e6a55597d8f26c2680009?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c49545a) will **increase** coverage by `0.06%`.
> The diff coverage is `65.62%`.
> :exclamation: Current head d9bb646 differs from pull request most recent head e11563b. Consider uploading reports for the commit e11563b to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17807 +/- ##
==========================================
+ Coverage 67.03% 67.09% +0.06%
==========================================
Files 1609 1609
Lines 64877 64854 -23
Branches 6863 6861 -2
==========================================
+ Hits 43488 43517 +29
+ Misses 19525 19473 -52
Partials 1864 1864
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.77% <65.62%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/SqlLab/components/SqlEditorLeftBar/index.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIvaW5kZXguanN4) | `64.44% <65.62%> (+8.88%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.28% <0.00%> (-0.58%)` | :arrow_down: |
| [...uperset-frontend/src/explore/exploreUtils/index.js](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvZXhwbG9yZVV0aWxzL2luZGV4Lmpz) | `66.42% <0.00%> (-0.48%)` | :arrow_down: |
| [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `96.61% <0.00%> (-0.32%)` | :arrow_down: |
| [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `91.22% <0.00%> (-0.07%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `77.80% <0.00%> (-0.02%)` | :arrow_down: |
| [superset-frontend/src/chart/Chart.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0LmpzeA==) | `29.62% <0.00%> (ø)` | |
| [superset/datasets/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `58.33% <0.00%> (ø)` | |
| [...t-frontend/src/explore/reducers/getInitialState.ts](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLnRz) | `33.33% <0.00%> (ø)` | |
| [...ntend/src/explore/components/ExploreChartPanel.jsx](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC5qc3g=) | `14.66% <0.00%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/superset/pull/17807/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c49545a...e11563b](https://codecov.io/gh/apache/superset/pull/17807?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org