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 2022/11/05 01:47:47 UTC
[GitHub] [superset] pkdotson opened a new pull request, #22043: feat: add tabs to edit dataset page
pkdotson opened a new pull request, #22043:
URL: https://github.com/apache/superset/pull/22043
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### 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] eschutho commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1054926017
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -106,6 +107,14 @@ export default function AddDataset() {
getDatasetsList();
}
}, [dataset?.schema]);
+ const [showEditPage, setShowEditPage] = useState(false);
Review Comment:
another wording suggestion: how about something like `editPageIsVisible` and `setEditPageIsVisible`? Just so it sounds like a boolean. You _could_ maybe leave out `is` if you wanted, too.
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by "lyndsiWilliams (via GitHub)" <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1095057732
##########
superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx:
##########
@@ -45,17 +46,19 @@ export default function DatasetLayout({
datasetPanel,
rightPanel,
footer,
+ editPageIsVisible,
}: DatasetLayoutProps) {
return (
<StyledLayoutWrapper data-test="dataset-layout-wrapper">
{header && <StyledLayoutHeader>{header}</StyledLayoutHeader>}
<OuterRow>
- <LeftColumn>
- {leftPanel && (
- <StyledLayoutLeftPanel>{leftPanel}</StyledLayoutLeftPanel>
- )}
- </LeftColumn>
-
+ {!editPageIsVisible && (
+ <LeftColumn>
+ {leftPanel && (
+ <StyledLayoutLeftPanel>{leftPanel}</StyledLayoutLeftPanel>
+ )}
+ </LeftColumn>
+ )}
Review Comment:
Good call, updated in [`this commit`](https://github.com/apache/superset/pull/22043/commits/c6a9b0766df0321ff07e78973aeef82dc32ffbef).
--
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1065058206
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx:
##########
@@ -104,7 +104,13 @@ function Footer({
if (typeof response === 'number') {
logEvent(LOG_ACTIONS_DATASET_CREATION_SUCCESS, datasetObject);
// When a dataset is created the response we get is its ID number
- goToPreviousUrl();
+
+ // Go to usage page if in testing mode. Will be changed
+ // to go to usage page when completed.
+ if (window.location.search === '?testing') {
+ window.location.href = `/dataset/${response}?testing`;
+ }
+ goToUrl(`/dataset/${response}?testing`);
Review Comment:
If #22610 is merged first, keep whatever is decided on for #22610.
--
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 #22043: feat: add tabs to edit dataset page
Posted by "lyndsiWilliams (via GitHub)" <gi...@apache.org>.
lyndsiWilliams merged PR #22043:
URL: https://github.com/apache/superset/pull/22043
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1054828612
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,86 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t, logging } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATiONS = {
Review Comment:
lowercase `i`?
--
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 #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on PR #22043:
URL: https://github.com/apache/superset/pull/22043#issuecomment-1355326774
/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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1046640254
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ margin-top: 34px;
+ padding-left: 16px;
+ // display: inline-block;
+ .ant-tabs-top > .ant-tabs-nav::before,
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: 200px;
+ }
+ .ant-tabs-nav-list > div:nth-last-child(2) {
+ // margin-right: 0px;
+ }
+`;
+
+const TabStyles = styled.div`
+ .ant-badge {
+ width: 32px;
+ margin-left: 10px;
Review Comment:
Addressed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/47a8e16dccf7a695bc45263d01177373e30c8f12).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1054901197
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx:
##########
@@ -104,7 +104,13 @@ function Footer({
if (typeof response === 'number') {
logEvent(LOG_ACTIONS_DATASET_CREATION_SUCCESS, datasetObject);
// When a dataset is created the response we get is its ID number
- goToPreviousUrl();
+
+ // Go to usage page if in testing mode. Will be changed
+ // to go to usage page when completed.
+ if (window.location.search === '?testing') {
+ window.location.href = `/dataset/${response}?testing`;
+ }
+ goToPreviousUrl(`/dataset/${response}?testing`);
Review Comment:
can we name this just `goToUrl` then if it's being used for more than the previous url?
--
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 #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22043:
URL: https://github.com/apache/superset/pull/22043#issuecomment-1307962521
# [Codecov](https://codecov.io/gh/apache/superset/pull/22043?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 [#22043](https://codecov.io/gh/apache/superset/pull/22043?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3970673) into [master](https://codecov.io/gh/apache/superset/commit/ba65f668972666dcd32602b718c858622c87dab6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba65f66) will **increase** coverage by `0.06%`.
> The diff coverage is `62.50%`.
```diff
@@ Coverage Diff @@
## master #22043 +/- ##
==========================================
+ Coverage 67.03% 67.10% +0.06%
==========================================
Files 1813 1819 +6
Lines 69424 69639 +215
Branches 7448 7502 +54
==========================================
+ Hits 46541 46732 +191
- Misses 20963 20965 +2
- Partials 1920 1942 +22
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.71% <62.50%> (+0.21%)` | :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/22043?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...iews/CRUD/data/dataset/AddDataset/Footer/index.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXQvRm9vdGVyL2luZGV4LnRzeA==) | `31.25% <0.00%> (-2.09%)` | :arrow_down: |
| [...d/src/views/CRUD/data/dataset/AddDataset/index.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXQvaW5kZXgudHN4) | `55.55% <50.00%> (-2.34%)` | :arrow_down: |
| [...CRUD/data/dataset/AddDataset/EditDataset/index.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXQvRWRpdERhdGFzZXQvaW5kZXgudHN4) | `75.00% <75.00%> (ø)` | |
| [...rc/views/CRUD/data/dataset/DatasetLayout/index.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0RhdGFzZXRMYXlvdXQvaW5kZXgudHN4) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `81.81% <0.00%> (-18.19%)` | :arrow_down: |
| [...s/superset-ui-core/src/components/SafeMarkdown.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY29tcG9uZW50cy9TYWZlTWFya2Rvd24udHN4) | `61.53% <0.00%> (-5.13%)` | :arrow_down: |
| [...-frontend/src/dashboard/reducers/dashboardState.js](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXNoYm9hcmRTdGF0ZS5qcw==) | `74.46% <0.00%> (-1.62%)` | :arrow_down: |
| [...rset-frontend/src/explore/components/SaveModal.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwudHN4) | `39.42% <0.00%> (-0.58%)` | :arrow_down: |
| [...ponents/ReportModal/HeaderReportDropdown/index.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUmVwb3J0TW9kYWwvSGVhZGVyUmVwb3J0RHJvcGRvd24vaW5kZXgudHN4) | `67.50% <0.00%> (-0.50%)` | :arrow_down: |
| [...perset-frontend/src/views/components/RightMenu.tsx](https://codecov.io/gh/apache/superset/pull/22043/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2NvbXBvbmVudHMvUmlnaHRNZW51LnRzeA==) | `78.23% <0.00%> (-0.25%)` | :arrow_down: |
| ... and [22 more](https://codecov.io/gh/apache/superset/pull/22043/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) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061039542
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -106,6 +107,14 @@ export default function AddDataset() {
getDatasetsList();
}
}, [dataset?.schema]);
+ const [showEditPage, setShowEditPage] = useState(false);
Review Comment:
I like that change, set to `editPageIsVisible` in [`this commit`](https://github.com/apache/superset/pull/22043/commits/77f5c7f40fe0c0269d5ef8a2f3e6ad2483d8e180).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1072727729
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -75,11 +76,36 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
};
}
-export const UseGetDatasetsList = (queryParams: string | undefined) =>
- SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- })
- .then(({ json }) => json)
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
+export const useGetDatasetsList = (queryParams: string | undefined) => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+
+ const getDatasetsList = () =>
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ })
+ .then(({ json }) => {
+ setDatasets(json?.result);
+ })
+ .catch(error =>
+ logging.error('There was an error fetching dataset', error),
+ );
+
+ const datasetNames = datasets?.map(dataset => dataset.table_name);
+
+ return { getDatasetsList, datasets, datasetNames };
+};
+
+export const useGetDatasetRelatedObjects = (id: string) => {
+ const [usageCount, setUsageCount] = useState(0);
+
+ const getDatasetRelatedObjects = () =>
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json }) => {
+ setUsageCount(json?.charts.count);
+ })
+ .catch(error => logging.error(error));
Review Comment:
#22610 changes all the errors to use toasts, once it's merged I will be bringing those changes into this PR.
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1072726435
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx:
##########
@@ -104,7 +104,13 @@ function Footer({
if (typeof response === 'number') {
logEvent(LOG_ACTIONS_DATASET_CREATION_SUCCESS, datasetObject);
// When a dataset is created the response we get is its ID number
- goToPreviousUrl();
+
+ // Go to usage page if in testing mode. Will be changed
+ // to go to usage page when completed.
+ if (window.location.search === '?testing') {
+ window.location.href = `/dataset/${response}?testing`;
+ }
+ goToUrl(`/dataset/${response}?testing`);
Review Comment:
#22610 changes this, once it's merged I will be bringing those changes into this PR.
--
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by "codyml (via GitHub)" <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1095213114
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -81,35 +83,97 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
/**
* Retrieves all pages of dataset results
*/
-export const UseGetDatasetsList = async (filters: object[]) => {
- let results: DatasetObject[] = [];
- let page = 0;
- let count;
-
- // If count is undefined or less than results, we need to
- // asynchronously retrieve a page of dataset results
- while (count === undefined || results.length < count) {
- const queryParams = rison.encode_uri({ filters, page });
- try {
- // eslint-disable-next-line no-await-in-loop
- const response = await SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- });
-
- // Reassign local count to response's count
- ({ count } = response.json);
-
- const {
- json: { result },
- } = response;
-
- results = [...results, ...result];
-
- page += 1;
- } catch (error) {
- addDangerToast(t('There was an error fetching dataset'));
- logging.error(t('There was an error fetching dataset'), error);
+export const useDatasetsList = (
+ db:
+ | (DatabaseObject & {
+ owners: [number];
+ })
+ | undefined,
+ schema: string | null | undefined,
+) => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+ const encodedSchema = schema ? encodeURIComponent(schema) : undefined;
+
+ const getDatasetsList = useCallback(async (filters: object[]) => {
+ let results: DatasetObject[] = [];
+ let page = 0;
+ let count;
+
+ // If count is undefined or less than results, we need to
+ // asynchronously retrieve a page of dataset results
+ while (count === undefined || results.length < count) {
+ const queryParams = rison.encode_uri({ filters, page });
+ try {
+ // eslint-disable-next-line no-await-in-loop
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ });
+
+ // Reassign local count to response's count
+ ({ count } = response.json);
+
+ const {
+ json: { result },
+ } = response;
+
+ results = [...results, ...result];
+
+ page += 1;
+ } catch (error) {
+ addDangerToast(t('There was an error fetching dataset'));
+ logging.error(t('There was an error fetching dataset'), error);
+ }
}
- }
- return results;
+
+ setDatasets(results);
+ return results;
Review Comment:
```suggestion
```
--
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 #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22043:
URL: https://github.com/apache/superset/pull/22043#issuecomment-1355336165
@AAfghahi Ephemeral environment spinning up at http://54.188.91.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] lyndsiWilliams commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1072715610
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -91,22 +90,22 @@ export default function AddDataset() {
})
: undefined;
- const getDatasetsList = async () => {
- await UseGetDatasetsList(queryParams)
- .then(json => {
- setDatasets(json?.result);
- })
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
- };
+ const { getDatasetsList, datasets, datasetNames } =
+ useGetDatasetsList(queryParams);
useEffect(() => {
if (dataset?.schema) {
getDatasetsList();
}
}, [dataset?.schema]);
Review Comment:
Thank you for the explanation! Fixed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/41fbaeb11bfd574a84a24732cadd2dae25e6f39d).
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -75,11 +76,36 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
};
}
-export const UseGetDatasetsList = (queryParams: string | undefined) =>
- SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- })
- .then(({ json }) => json)
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
+export const useGetDatasetsList = (queryParams: string | undefined) => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+
+ const getDatasetsList = () =>
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ })
+ .then(({ json }) => {
+ setDatasets(json?.result);
+ })
+ .catch(error =>
+ logging.error('There was an error fetching dataset', error),
+ );
Review Comment:
Fixed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/41fbaeb11bfd574a84a24732cadd2dae25e6f39d).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1072724332
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import fetchMock from 'fetch-mock';
+import { render, screen } from 'spec/helpers/testing-library';
+import EditDataset from './index';
+
+const DATASET_ENDPOINT = 'glob:*api/v1/dataset/1/related_objects';
+
+const mockedProps = {
+ id: '1',
+};
+
+fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } });
+
+test('should render edit dataset view with tabs', async () => {
+ fetchMock.calls(DATASET_ENDPOINT);
Review Comment:
Another good catch! Fixed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/41fbaeb11bfd574a84a24732cadd2dae25e6f39d).
--
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1063842495
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -75,11 +76,36 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
};
}
-export const UseGetDatasetsList = (queryParams: string | undefined) =>
- SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- })
- .then(({ json }) => json)
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
+export const useGetDatasetsList = (queryParams: string | undefined) => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+
+ const getDatasetsList = () =>
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ })
+ .then(({ json }) => {
+ setDatasets(json?.result);
+ })
+ .catch(error =>
+ logging.error('There was an error fetching dataset', error),
+ );
Review Comment:
```suggestion
const getDatasetsList = useCallback(
() =>
SupersetClient.get({
endpoint: `/api/v1/dataset/?q=${queryParams}`,
})
.then(({ json }) => {
setDatasets(json?.result);
})
.catch(error =>
logging.error('There was an error fetching dataset', error),
),
[queryParams],
);
```
If you wrap this function definition in `useCallback` you can include it in the dependency array in `index.tsx` above.
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -76,8 +76,7 @@ export default function AddDataset() {
Reducer<Partial<DatasetObject> | null, DSReducerActionType>
>(datasetReducer, null);
const [hasColumns, setHasColumns] = useState(false);
- const [datasets, setDatasets] = useState<DatasetObject[]>([]);
- const datasetNames = datasets.map(dataset => dataset.table_name);
+ const [editPageIsVisible, setEditPageIsVisible] = useState(false);
const encodedSchema = dataset?.schema
Review Comment:
I think there's a bug in the filtering below (lines 84-91). It doesn't filter by database, only schema, so if you have multiple databases with schemas of the same name and columns of the same name it'll say columns already have datasets when they don't if a different database has datasets based on those schema/column combinations:
https://user-images.githubusercontent.com/13007381/211111177-9d9c0c2f-f7a9-469e-9072-9a8c99d76969.mov
So, I think we need to add a filter for database ID there?
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -75,11 +76,36 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
};
}
-export const UseGetDatasetsList = (queryParams: string | undefined) =>
- SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- })
- .then(({ json }) => json)
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
+export const useGetDatasetsList = (queryParams: string | undefined) => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+
+ const getDatasetsList = () =>
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ })
+ .then(({ json }) => {
+ setDatasets(json?.result);
+ })
+ .catch(error =>
+ logging.error('There was an error fetching dataset', error),
+ );
+
+ const datasetNames = datasets?.map(dataset => dataset.table_name);
+
+ return { getDatasetsList, datasets, datasetNames };
+};
+
+export const useGetDatasetRelatedObjects = (id: string) => {
+ const [usageCount, setUsageCount] = useState(0);
+
+ const getDatasetRelatedObjects = () =>
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json }) => {
+ setUsageCount(json?.charts.count);
+ })
+ .catch(error => logging.error(error));
Review Comment:
Not blocking, but do we know how we want errors to appear if these requests fail? If they should be displayed to the user in toasts, we could include the `useToasts` hook and call `addDangerToast` here and in `getDatasetsList` above.
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -91,22 +90,22 @@ export default function AddDataset() {
})
: undefined;
- const getDatasetsList = async () => {
- await UseGetDatasetsList(queryParams)
- .then(json => {
- setDatasets(json?.result);
- })
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
- };
+ const { getDatasetsList, datasets, datasetNames } =
+ useGetDatasetsList(queryParams);
useEffect(() => {
if (dataset?.schema) {
getDatasetsList();
}
}, [dataset?.schema]);
+ const id = window.location.pathname.split('/')[2];
+ useEffect(() => {
+ if (!Number.isNaN(parseFloat(id))) {
Review Comment:
```suggestion
if (!Number.isNaN(parseInt(id, 10))) {
```
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import fetchMock from 'fetch-mock';
+import { render, screen } from 'spec/helpers/testing-library';
+import EditDataset from './index';
+
+const DATASET_ENDPOINT = 'glob:*api/v1/dataset/1/related_objects';
+
+const mockedProps = {
+ id: '1',
+};
+
+fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } });
+
+test('should render edit dataset view with tabs', async () => {
+ fetchMock.calls(DATASET_ENDPOINT);
Review Comment:
Does this do anything? We could do this at the end of the test to check if a request was made instead:
```ts
expect(fetchMock.called(DATASET_ENDPOINT)).toBeTruthy();
```
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -91,22 +90,22 @@ export default function AddDataset() {
})
: undefined;
- const getDatasetsList = async () => {
- await UseGetDatasetsList(queryParams)
- .then(json => {
- setDatasets(json?.result);
- })
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
- };
+ const { getDatasetsList, datasets, datasetNames } =
+ useGetDatasetsList(queryParams);
useEffect(() => {
if (dataset?.schema) {
getDatasetsList();
}
}, [dataset?.schema]);
Review Comment:
```suggestion
}, [dataset?.schema, getDatasetsList]);
```
Somewhat related: I'm getting a missing dependency lint error here. I'm guessing it was left out because it looks like if you add in the missing `getDatasetsList` dependency then go to the new Add Dataset flow, as soon as you select a database schema it calls `getDatasets` over and over, firing endless API calls. I think this is happening because in the `useGetDatasetsList` hook, `getDatasetsList` is defined as a new function every time the hook runs, which endlessly triggers the effect. I think the correct fix would be to put back the missing dependency here, then wrap the definition of `getDatasetsList` in a `useCallback` call so it's not redefined unless its dependencies change – see my suggested change in `hooks.ts`.
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -91,22 +90,22 @@ export default function AddDataset() {
})
: undefined;
- const getDatasetsList = async () => {
- await UseGetDatasetsList(queryParams)
- .then(json => {
- setDatasets(json?.result);
- })
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
- };
+ const { getDatasetsList, datasets, datasetNames } =
+ useGetDatasetsList(queryParams);
useEffect(() => {
if (dataset?.schema) {
getDatasetsList();
}
}, [dataset?.schema]);
+ const id = window.location.pathname.split('/')[2];
Review Comment:
Because we've defined the path for this page as `/dataset/:datasetId` in `routes.tsx`, React Router will parse out that param for us if we use [`useParams`](https://v5.reactrouter.com/web/api/Hooks/useparams) hook:
```suggestion
const { datasetId: id } = useParams<{ datasetId: string }>();
```
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx:
##########
@@ -104,7 +104,13 @@ function Footer({
if (typeof response === 'number') {
logEvent(LOG_ACTIONS_DATASET_CREATION_SUCCESS, datasetObject);
// When a dataset is created the response we get is its ID number
- goToPreviousUrl();
+
+ // Go to usage page if in testing mode. Will be changed
+ // to go to usage page when completed.
+ if (window.location.search === '?testing') {
+ window.location.href = `/dataset/${response}?testing`;
+ }
+ goToUrl(`/dataset/${response}?testing`);
Review Comment:
I'm a little foggy on what's going on here and above in `goToUrl` with the redirect URLs but as long as it works for testing and we update it to its final form before making it publicly accessible it's fine with 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] AAfghahi commented on pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on PR #22043:
URL: https://github.com/apache/superset/pull/22043#issuecomment-1355327131
Looks good, but I am going to do some quick visual testing
--
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 #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on PR #22043:
URL: https://github.com/apache/superset/pull/22043#issuecomment-1355512711
visual testing looks great! 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.
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061039133
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,86 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t, logging } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATiONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const [usageCount, setUsageCount] = useState(0);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id)
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json = {} }) => {
+ setUsageCount(json.charts.count);
+ })
+ .catch(err => logging.log(err));
+ }, [id]);
+
+ const UsageTab = (
+ <TabStyles>
+ <span>{TRANSLATiONS.USAGE_TEXT}</span>
+ <Badge count={usageCount + 1} />
Review Comment:
I think this was leftover testing code for the number pill, removed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/77f5c7f40fe0c0269d5ef8a2f3e6ad2483d8e180).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061038894
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,86 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t, logging } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATiONS = {
Review Comment:
Whoops! Got cause using the shift key instead of caps lock 🤣
Fixed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/77f5c7f40fe0c0269d5ef8a2f3e6ad2483d8e180).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by "lyndsiWilliams (via GitHub)" <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1095057474
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -82,35 +76,31 @@ export default function AddDataset() {
Reducer<Partial<DatasetObject> | null, DSReducerActionType>
>(datasetReducer, null);
const [hasColumns, setHasColumns] = useState(false);
- const [datasets, setDatasets] = useState<DatasetObject[]>([]);
- const datasetNames = datasets.map(dataset => dataset.table_name);
+ const [editPageIsVisible, setEditPageIsVisible] = useState(false);
const encodedSchema = dataset?.schema
? encodeURIComponent(dataset?.schema)
: undefined;
- const getDatasetsList = useCallback(async () => {
+ const { getDatasetsList, datasets, datasetNames } = useGetDatasetsList();
Review Comment:
Same as above, this is much better! Updated in [`this commit`](https://github.com/apache/superset/pull/22043/commits/c6a9b0766df0321ff07e78973aeef82dc32ffbef).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061761029
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, t } from '@superset-ui/core';
+import React, { useEffect } from 'react';
+import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATIONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const { getDatasetRelatedObjects, usageCount } =
+ useGetDatasetRelatedObjects(id);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id) {
+ getDatasetRelatedObjects();
+ }
+ }, [id]);
Review Comment:
Originally this was set up to only run depending on `id` changing. I added `getDatasetRelatedObjects` to the dependency array and it didn't change the functionality so I added it to remove the lint warning in [`this commit`](https://github.com/apache/superset/pull/22043/commits/a553903cbbf14114612c4690ea4291759f21bb6c). Good call, 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.
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1062401035
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, t } from '@superset-ui/core';
+import React, { useEffect } from 'react';
+import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATIONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const { getDatasetRelatedObjects, usageCount } =
+ useGetDatasetRelatedObjects(id);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id) {
+ getDatasetRelatedObjects();
+ }
+ }, [id, getDatasetRelatedObjects]);
+
+ const UsageTab = (
Review Comment:
(definitely not blocking, I can make this change in my next PR)
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061060022
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -83,3 +83,10 @@ export const UseGetDatasetsList = (queryParams: string | undefined) =>
.catch(error =>
logging.error('There was an error fetching dataset', error),
);
+
+export const UseGetDatasetRelatedObjects = (id: string) =>
Review Comment:
nit, but convention on any methods is to have them in camel case with the first letter lowercase. Can you fix the UseGetDatasetsList 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.
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061614573
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, t } from '@superset-ui/core';
+import React, { useEffect } from 'react';
+import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATIONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const { getDatasetRelatedObjects, usageCount } =
+ useGetDatasetRelatedObjects(id);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id) {
+ getDatasetRelatedObjects();
+ }
+ }, [id]);
Review Comment:
I'm getting a dependency array lint warning here; is there a reason `getDatasetRelatedObjects` isn't in the dependency array?
--
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061966533
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, t } from '@superset-ui/core';
+import React, { useEffect } from 'react';
+import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATIONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const { getDatasetRelatedObjects, usageCount } =
+ useGetDatasetRelatedObjects(id);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id) {
+ getDatasetRelatedObjects();
+ }
+ }, [id, getDatasetRelatedObjects]);
+
+ const UsageTab = (
Review Comment:
Nit: since this isn't a component, could we rename it to use pascalCase (`usageTab`)?
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1054829745
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,86 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t, logging } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATiONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const [usageCount, setUsageCount] = useState(0);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id)
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json = {} }) => {
+ setUsageCount(json.charts.count);
+ })
+ .catch(err => logging.log(err));
+ }, [id]);
+
+ const UsageTab = (
+ <TabStyles>
+ <span>{TRANSLATiONS.USAGE_TEXT}</span>
+ <Badge count={usageCount + 1} />
Review Comment:
why is it +1?
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1072717260
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -76,8 +76,7 @@ export default function AddDataset() {
Reducer<Partial<DatasetObject> | null, DSReducerActionType>
>(datasetReducer, null);
const [hasColumns, setHasColumns] = useState(false);
- const [datasets, setDatasets] = useState<DatasetObject[]>([]);
- const datasetNames = datasets.map(dataset => dataset.table_name);
+ const [editPageIsVisible, setEditPageIsVisible] = useState(false);
const encodedSchema = dataset?.schema
Review Comment:
Good catch! I added a filter for database ID in [`this commit`](https://github.com/apache/superset/pull/22043/commits/41fbaeb11bfd574a84a24732cadd2dae25e6f39d).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by "lyndsiWilliams (via GitHub)" <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1095056960
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -81,35 +81,65 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
/**
* Retrieves all pages of dataset results
*/
-export const UseGetDatasetsList = async (filters: object[]) => {
- let results: DatasetObject[] = [];
- let page = 0;
- let count;
-
- // If count is undefined or less than results, we need to
- // asynchronously retrieve a page of dataset results
- while (count === undefined || results.length < count) {
- const queryParams = rison.encode_uri({ filters, page });
- try {
- // eslint-disable-next-line no-await-in-loop
- const response = await SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- });
+export const useGetDatasetsList = () => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+
+ const getDatasetsList = useCallback(async (filters: object[]) => {
+ let results: DatasetObject[] = [];
+ let page = 0;
+ let count;
- // Reassign local count to response's count
- ({ count } = response.json);
+ // If count is undefined or less than results, we need to
+ // asynchronously retrieve a page of dataset results
+ while (count === undefined || results.length < count) {
+ const queryParams = rison.encode_uri({ filters, page });
+ try {
+ // eslint-disable-next-line no-await-in-loop
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ });
- const {
- json: { result },
- } = response;
+ // Reassign local count to response's count
+ ({ count } = response.json);
- results = [...results, ...result];
+ const {
+ json: { result },
+ } = response;
- page += 1;
- } catch (error) {
- addDangerToast(t('There was an error fetching dataset'));
- logging.error(t('There was an error fetching dataset'), error);
+ results = [...results, ...result];
+
+ page += 1;
+ } catch (error) {
+ addDangerToast(t('There was an error fetching dataset'));
+ logging.error(t('There was an error fetching dataset'), error);
+ }
}
- }
- return results;
+
+ setDatasets(results);
+ return results;
+ }, []);
+
+ const datasetNames = datasets?.map(dataset => dataset.table_name);
+
+ return { getDatasetsList, datasets, datasetNames };
+};
+
+export const useGetDatasetRelatedObjects = (id: string) => {
+ const [usageCount, setUsageCount] = useState(0);
+
+ const getDatasetRelatedObjects = () =>
Review Comment:
Good call on the useCallback and encapsulating the logic in the hooks! Updated in [`this commit`](https://github.com/apache/superset/pull/22043/commits/c6a9b0766df0321ff07e78973aeef82dc32ffbef).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by "lyndsiWilliams (via GitHub)" <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1095223816
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -81,35 +83,97 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
/**
* Retrieves all pages of dataset results
*/
-export const UseGetDatasetsList = async (filters: object[]) => {
- let results: DatasetObject[] = [];
- let page = 0;
- let count;
-
- // If count is undefined or less than results, we need to
- // asynchronously retrieve a page of dataset results
- while (count === undefined || results.length < count) {
- const queryParams = rison.encode_uri({ filters, page });
- try {
- // eslint-disable-next-line no-await-in-loop
- const response = await SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- });
-
- // Reassign local count to response's count
- ({ count } = response.json);
-
- const {
- json: { result },
- } = response;
-
- results = [...results, ...result];
-
- page += 1;
- } catch (error) {
- addDangerToast(t('There was an error fetching dataset'));
- logging.error(t('There was an error fetching dataset'), error);
+export const useDatasetsList = (
+ db:
+ | (DatabaseObject & {
+ owners: [number];
+ })
+ | undefined,
+ schema: string | null | undefined,
+) => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+ const encodedSchema = schema ? encodeURIComponent(schema) : undefined;
+
+ const getDatasetsList = useCallback(async (filters: object[]) => {
+ let results: DatasetObject[] = [];
+ let page = 0;
+ let count;
+
+ // If count is undefined or less than results, we need to
+ // asynchronously retrieve a page of dataset results
+ while (count === undefined || results.length < count) {
+ const queryParams = rison.encode_uri({ filters, page });
+ try {
+ // eslint-disable-next-line no-await-in-loop
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ });
+
+ // Reassign local count to response's count
+ ({ count } = response.json);
+
+ const {
+ json: { result },
+ } = response;
+
+ results = [...results, ...result];
+
+ page += 1;
+ } catch (error) {
+ addDangerToast(t('There was an error fetching dataset'));
+ logging.error(t('There was an error fetching dataset'), error);
+ }
}
- }
- return results;
+
+ setDatasets(results);
+ return results;
Review Comment:
Thank you! Fixed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/57ccf2c8b32a7aa34585228a6084c32010bd5b9c).
--
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 #22043: feat: add tabs to edit dataset page
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #22043:
URL: https://github.com/apache/superset/pull/22043#issuecomment-1414593490
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] lyndsiWilliams commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1046640375
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ margin-top: 34px;
+ padding-left: 16px;
+ // display: inline-block;
+ .ant-tabs-top > .ant-tabs-nav::before,
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: 200px;
+ }
+ .ant-tabs-nav-list > div:nth-last-child(2) {
+ // margin-right: 0px;
+ }
+`;
+
+const TabStyles = styled.div`
+ .ant-badge {
+ width: 32px;
+ margin-left: 10px;
+ }
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const EditPage = ({ id }: EditPageProps) => {
+ const [usageCount, setUsageCount] = useState(0);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id)
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json = {} }) => {
+ setUsageCount(json.charts.count);
+ })
+ .catch(err => console.log(err));
+ }, [id]);
+
+ const Tab = (
+ <TabStyles>
+ <span>{t('Usage')}</span>
+ <Badge count={usageCount + 1} />
+ </TabStyles>
+ );
+
+ return (
+ <>
+ <StyledTabs moreIcon={null} fullWidth={false}>
+ <Tabs.TabPane tab={t('Columns')} key="1" />
+ <Tabs.TabPane tab={t('Metrics')} key="2" />
+ <Tabs.TabPane tab={Tab} key="3">
+ <div>placeholder</div>
+ </Tabs.TabPane>
+ </StyledTabs>
+ </>
Review Comment:
Addressed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/47a8e16dccf7a695bc45263d01177373e30c8f12).
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ margin-top: 34px;
+ padding-left: 16px;
+ // display: inline-block;
+ .ant-tabs-top > .ant-tabs-nav::before,
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: 200px;
+ }
+ .ant-tabs-nav-list > div:nth-last-child(2) {
+ // margin-right: 0px;
+ }
+`;
+
+const TabStyles = styled.div`
+ .ant-badge {
+ width: 32px;
+ margin-left: 10px;
+ }
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const EditPage = ({ id }: EditPageProps) => {
+ const [usageCount, setUsageCount] = useState(0);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id)
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json = {} }) => {
+ setUsageCount(json.charts.count);
+ })
+ .catch(err => console.log(err));
+ }, [id]);
+
+ const Tab = (
+ <TabStyles>
+ <span>{t('Usage')}</span>
+ <Badge count={usageCount + 1} />
+ </TabStyles>
+ );
+
+ return (
+ <>
+ <StyledTabs moreIcon={null} fullWidth={false}>
+ <Tabs.TabPane tab={t('Columns')} key="1" />
+ <Tabs.TabPane tab={t('Metrics')} key="2" />
+ <Tabs.TabPane tab={Tab} key="3">
+ <div>placeholder</div>
Review Comment:
Addressed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/47a8e16dccf7a695bc45263d01177373e30c8f12).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1054829118
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,86 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t, logging } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATiONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const [usageCount, setUsageCount] = useState(0);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id)
+ SupersetClient.get({
Review Comment:
can we move this into a hook or a redux action? We're trying to keep api calls out of views.
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1062948150
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, t } from '@superset-ui/core';
+import React, { useEffect } from 'react';
+import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ ${({ theme }) => `
+ margin-top: ${theme.gridUnit * 8.5}px;
+ padding-left: ${theme.gridUnit * 4}px;
+
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: ${theme.gridUnit * 50}px;
+ }
+ `}
+`;
+
+const TabStyles = styled.div`
+ ${({ theme }) => `
+ .ant-badge {
+ width: ${theme.gridUnit * 8}px;
+ margin-left: ${theme.gridUnit * 2.5}px;
+ }
+ `}
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const TRANSLATIONS = {
+ USAGE_TEXT: t('Usage'),
+ COLUMNS_TEXT: t('Columns'),
+ METRICS_TEXT: t('Metrics'),
+};
+
+const EditPage = ({ id }: EditPageProps) => {
+ const { getDatasetRelatedObjects, usageCount } =
+ useGetDatasetRelatedObjects(id);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id) {
+ getDatasetRelatedObjects();
+ }
+ }, [id, getDatasetRelatedObjects]);
+
+ const UsageTab = (
Review Comment:
I think that's a good call! Changed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/6d188bc864bda58d451e7030a042a5d22804b5c9).
--
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1063860003
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import fetchMock from 'fetch-mock';
+import { render, screen } from 'spec/helpers/testing-library';
+import EditDataset from './index';
+
+const DATASET_ENDPOINT = 'glob:*api/v1/dataset/1/related_objects';
+
+const mockedProps = {
+ id: '1',
+};
+
+fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } });
+
+test('should render edit dataset view with tabs', async () => {
+ fetchMock.calls(DATASET_ENDPOINT);
Review Comment:
Does this do anything? We could use `called` at the end of the test to check if a request was made instead:
```ts
expect(fetchMock.called(DATASET_ENDPOINT)).toBeTruthy();
```
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061039320
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx:
##########
@@ -104,7 +104,13 @@ function Footer({
if (typeof response === 'number') {
logEvent(LOG_ACTIONS_DATASET_CREATION_SUCCESS, datasetObject);
// When a dataset is created the response we get is its ID number
- goToPreviousUrl();
+
+ // Go to usage page if in testing mode. Will be changed
+ // to go to usage page when completed.
+ if (window.location.search === '?testing') {
+ window.location.href = `/dataset/${response}?testing`;
+ }
+ goToPreviousUrl(`/dataset/${response}?testing`);
Review Comment:
Good thinking, changed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/77f5c7f40fe0c0269d5ef8a2f3e6ad2483d8e180).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1061125670
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -83,3 +83,10 @@ export const UseGetDatasetsList = (queryParams: string | undefined) =>
.catch(error =>
logging.error('There was an error fetching dataset', error),
);
+
+export const UseGetDatasetRelatedObjects = (id: string) =>
Review Comment:
I updated the dataset hooks as discussed in Slack in [`this commit`](https://github.com/apache/superset/pull/22043/commits/4c2efaff40ec02caf4c8ec918af06a0203b722d8).
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1072726723
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -91,22 +90,22 @@ export default function AddDataset() {
})
: undefined;
- const getDatasetsList = async () => {
- await UseGetDatasetsList(queryParams)
- .then(json => {
- setDatasets(json?.result);
- })
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
- };
+ const { getDatasetsList, datasets, datasetNames } =
+ useGetDatasetsList(queryParams);
useEffect(() => {
if (dataset?.schema) {
getDatasetsList();
}
}, [dataset?.schema]);
+ const id = window.location.pathname.split('/')[2];
Review Comment:
Fixed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/41fbaeb11bfd574a84a24732cadd2dae25e6f39d).
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -91,22 +90,22 @@ export default function AddDataset() {
})
: undefined;
- const getDatasetsList = async () => {
- await UseGetDatasetsList(queryParams)
- .then(json => {
- setDatasets(json?.result);
- })
- .catch(error =>
- logging.error('There was an error fetching dataset', error),
- );
- };
+ const { getDatasetsList, datasets, datasetNames } =
+ useGetDatasetsList(queryParams);
useEffect(() => {
if (dataset?.schema) {
getDatasetsList();
}
}, [dataset?.schema]);
+ const id = window.location.pathname.split('/')[2];
+ useEffect(() => {
+ if (!Number.isNaN(parseFloat(id))) {
Review Comment:
Fixed in [`this commit`](https://github.com/apache/superset/pull/22043/commits/41fbaeb11bfd574a84a24732cadd2dae25e6f39d).
--
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] codyml commented on a diff in pull request #22043: feat: add tabs to edit dataset page
Posted by "codyml (via GitHub)" <gi...@apache.org>.
codyml commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1093843914
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -81,35 +81,65 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
/**
* Retrieves all pages of dataset results
*/
-export const UseGetDatasetsList = async (filters: object[]) => {
- let results: DatasetObject[] = [];
- let page = 0;
- let count;
-
- // If count is undefined or less than results, we need to
- // asynchronously retrieve a page of dataset results
- while (count === undefined || results.length < count) {
- const queryParams = rison.encode_uri({ filters, page });
- try {
- // eslint-disable-next-line no-await-in-loop
- const response = await SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- });
+export const useGetDatasetsList = () => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+
+ const getDatasetsList = useCallback(async (filters: object[]) => {
+ let results: DatasetObject[] = [];
+ let page = 0;
+ let count;
- // Reassign local count to response's count
- ({ count } = response.json);
+ // If count is undefined or less than results, we need to
+ // asynchronously retrieve a page of dataset results
+ while (count === undefined || results.length < count) {
+ const queryParams = rison.encode_uri({ filters, page });
+ try {
+ // eslint-disable-next-line no-await-in-loop
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ });
- const {
- json: { result },
- } = response;
+ // Reassign local count to response's count
+ ({ count } = response.json);
- results = [...results, ...result];
+ const {
+ json: { result },
+ } = response;
- page += 1;
- } catch (error) {
- addDangerToast(t('There was an error fetching dataset'));
- logging.error(t('There was an error fetching dataset'), error);
+ results = [...results, ...result];
+
+ page += 1;
+ } catch (error) {
+ addDangerToast(t('There was an error fetching dataset'));
+ logging.error(t('There was an error fetching dataset'), error);
+ }
}
- }
- return results;
+
+ setDatasets(results);
+ return results;
+ }, []);
+
+ const datasetNames = datasets?.map(dataset => dataset.table_name);
+
+ return { getDatasetsList, datasets, datasetNames };
+};
+
+export const useGetDatasetRelatedObjects = (id: string) => {
+ const [usageCount, setUsageCount] = useState(0);
+
+ const getDatasetRelatedObjects = () =>
Review Comment:
This function is redefined on each render, triggering extra effect runs and thereby extra API calls in `EditDataset/index.tsx`. If you wrap it in a `useCallback`, it'll only call the API when `id` changes, which I think is what we want.
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx:
##########
@@ -82,35 +76,31 @@ export default function AddDataset() {
Reducer<Partial<DatasetObject> | null, DSReducerActionType>
>(datasetReducer, null);
const [hasColumns, setHasColumns] = useState(false);
- const [datasets, setDatasets] = useState<DatasetObject[]>([]);
- const datasetNames = datasets.map(dataset => dataset.table_name);
+ const [editPageIsVisible, setEditPageIsVisible] = useState(false);
const encodedSchema = dataset?.schema
? encodeURIComponent(dataset?.schema)
: undefined;
- const getDatasetsList = useCallback(async () => {
+ const { getDatasetsList, datasets, datasetNames } = useGetDatasetsList();
Review Comment:
Non-blocking, but I think this hook could also be rewritten to encapsulate the get-dataset-list process a little better so there's less back-and-forth between the hook and the main function body. I think you could rename it something like `useDatasetsList`, move the schema encoding and the effect inside of the hook, and have it return just `datasets` and `datasetNames`, so lines 80-96 would be replaced with a one-liner like this:
```javascript
const { datasets, datasetNames } = useDatasetsList(dataset?.db, dataset?.schema);
```
##########
superset-frontend/src/views/CRUD/data/hooks.ts:
##########
@@ -81,35 +81,65 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
/**
* Retrieves all pages of dataset results
*/
-export const UseGetDatasetsList = async (filters: object[]) => {
- let results: DatasetObject[] = [];
- let page = 0;
- let count;
-
- // If count is undefined or less than results, we need to
- // asynchronously retrieve a page of dataset results
- while (count === undefined || results.length < count) {
- const queryParams = rison.encode_uri({ filters, page });
- try {
- // eslint-disable-next-line no-await-in-loop
- const response = await SupersetClient.get({
- endpoint: `/api/v1/dataset/?q=${queryParams}`,
- });
+export const useGetDatasetsList = () => {
+ const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+
+ const getDatasetsList = useCallback(async (filters: object[]) => {
+ let results: DatasetObject[] = [];
+ let page = 0;
+ let count;
- // Reassign local count to response's count
- ({ count } = response.json);
+ // If count is undefined or less than results, we need to
+ // asynchronously retrieve a page of dataset results
+ while (count === undefined || results.length < count) {
+ const queryParams = rison.encode_uri({ filters, page });
+ try {
+ // eslint-disable-next-line no-await-in-loop
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
+ });
- const {
- json: { result },
- } = response;
+ // Reassign local count to response's count
+ ({ count } = response.json);
- results = [...results, ...result];
+ const {
+ json: { result },
+ } = response;
- page += 1;
- } catch (error) {
- addDangerToast(t('There was an error fetching dataset'));
- logging.error(t('There was an error fetching dataset'), error);
+ results = [...results, ...result];
+
+ page += 1;
+ } catch (error) {
+ addDangerToast(t('There was an error fetching dataset'));
+ logging.error(t('There was an error fetching dataset'), error);
+ }
}
- }
- return results;
+
+ setDatasets(results);
+ return results;
+ }, []);
+
+ const datasetNames = datasets?.map(dataset => dataset.table_name);
+
+ return { getDatasetsList, datasets, datasetNames };
+};
+
+export const useGetDatasetRelatedObjects = (id: string) => {
+ const [usageCount, setUsageCount] = useState(0);
+
+ const getDatasetRelatedObjects = () =>
Review Comment:
Also, non-blocking but if you're working on it anyway, I think it might provide better encapsulation if you renamed the hook to something like `useDatasetRelatedCounts`, brought the effect inside of the hook, and returned only the counts (just `usageCount` for now, but with the possibility to expand to more counts later).
##########
superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx:
##########
@@ -45,17 +46,19 @@ export default function DatasetLayout({
datasetPanel,
rightPanel,
footer,
+ editPageIsVisible,
}: DatasetLayoutProps) {
return (
<StyledLayoutWrapper data-test="dataset-layout-wrapper">
{header && <StyledLayoutHeader>{header}</StyledLayoutHeader>}
<OuterRow>
- <LeftColumn>
- {leftPanel && (
- <StyledLayoutLeftPanel>{leftPanel}</StyledLayoutLeftPanel>
- )}
- </LeftColumn>
-
+ {!editPageIsVisible && (
+ <LeftColumn>
+ {leftPanel && (
+ <StyledLayoutLeftPanel>{leftPanel}</StyledLayoutLeftPanel>
+ )}
+ </LeftColumn>
+ )}
Review Comment:
Since `leftPanel` will be `null` if `editPageIsVisible` is true, could we avoid passing `editPageIsVisible` as a prop and instead simplify to this?
```suggestion
{leftPanel && (
<LeftColumn>
<StyledLayoutLeftPanel>{leftPanel}</StyledLayoutLeftPanel>
</LeftColumn>
)}
```
--
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 diff in pull request #22043: feat: add tabs to edit dataset page
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22043:
URL: https://github.com/apache/superset/pull/22043#discussion_r1018217888
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ margin-top: 34px;
+ padding-left: 16px;
+ // display: inline-block;
+ .ant-tabs-top > .ant-tabs-nav::before,
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: 200px;
+ }
+ .ant-tabs-nav-list > div:nth-last-child(2) {
+ // margin-right: 0px;
+ }
+`;
+
+const TabStyles = styled.div`
+ .ant-badge {
+ width: 32px;
+ margin-left: 10px;
Review Comment:
```suggestion
margin-top: ${supersetTheme.gridUnit * 8.5}px;
padding-left: ${supersetTheme.gridUnit * 4}px;
.ant-tabs-top > .ant-tabs-nav::before,
.ant-tabs-top > .ant-tabs-nav::before {
width: ${supersetTheme.gridUnit * 50}px;
}
`;
const TabStyles = styled.div`
.ant-badge {
width: ${supersetTheme.gridUnit * 8}px;
margin-left: ${supersetTheme.gridUnit * 2.5}px;
```
These values should use the superset theme's gridunit, you can import `supersetTheme` from core-ui to use in this spot. Are these commented bits supposed to be here? I removed them in the code suggestion just in case. Also, I noticed the specificity on lines 28 and 29 look identical (`.ant-tabs-top > .ant-tabs-nav::before`), does this need to be specified twice?
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ margin-top: 34px;
+ padding-left: 16px;
+ // display: inline-block;
+ .ant-tabs-top > .ant-tabs-nav::before,
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: 200px;
+ }
+ .ant-tabs-nav-list > div:nth-last-child(2) {
+ // margin-right: 0px;
+ }
+`;
+
+const TabStyles = styled.div`
+ .ant-badge {
+ width: 32px;
+ margin-left: 10px;
+ }
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const EditPage = ({ id }: EditPageProps) => {
+ const [usageCount, setUsageCount] = useState(0);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id)
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json = {} }) => {
+ setUsageCount(json.charts.count);
+ })
+ .catch(err => console.log(err));
+ }, [id]);
+
+ const Tab = (
+ <TabStyles>
+ <span>{t('Usage')}</span>
+ <Badge count={usageCount + 1} />
+ </TabStyles>
+ );
+
+ return (
+ <>
+ <StyledTabs moreIcon={null} fullWidth={false}>
+ <Tabs.TabPane tab={t('Columns')} key="1" />
+ <Tabs.TabPane tab={t('Metrics')} key="2" />
+ <Tabs.TabPane tab={Tab} key="3">
+ <div>placeholder</div>
Review Comment:
Is there text that's supposed to go in place of this placeholder?
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx:
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import fetchMock from 'fetch-mock';
+import { render, screen } from 'spec/helpers/testing-library';
+import EditDataset from './index';
+
+const DATASET_ENDPOINT = 'glob:*api/v1/dataset/1/related_objects';
+
+const mockedProps = {
+ id: '1',
+};
+
+fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } });
+
+test('should render edit dataset view with tabs', () => {
+ fetchMock.calls(DATASET_ENDPOINT);
+ render(<EditDataset {...mockedProps} />);
+
+ const columnTab = screen.getByRole('tab', { name: /columns/i });
Review Comment:
```suggestion
test('should render edit dataset view with tabs', async () => {
fetchMock.calls(DATASET_ENDPOINT);
render(<EditDataset {...mockedProps} />);
const columnTab = await screen.findByRole('tab', { name: /columns/i });
```
This test has an act error, if you change these lines it will remove the error.
##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, SupersetClient, t } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import Badge from 'src/components/Badge';
+import Tabs from 'src/components/Tabs';
+
+const StyledTabs = styled(Tabs)`
+ margin-top: 34px;
+ padding-left: 16px;
+ // display: inline-block;
+ .ant-tabs-top > .ant-tabs-nav::before,
+ .ant-tabs-top > .ant-tabs-nav::before {
+ width: 200px;
+ }
+ .ant-tabs-nav-list > div:nth-last-child(2) {
+ // margin-right: 0px;
+ }
+`;
+
+const TabStyles = styled.div`
+ .ant-badge {
+ width: 32px;
+ margin-left: 10px;
+ }
+`;
+
+interface EditPageProps {
+ id: string;
+}
+
+const EditPage = ({ id }: EditPageProps) => {
+ const [usageCount, setUsageCount] = useState(0);
+ useEffect(() => {
+ // Todo: this useEffect should be used to call all count methods conncurently
+ // when we populate data for the new tabs. For right separating out this
+ // api call for building the usage page.
+ if (id)
+ SupersetClient.get({
+ endpoint: `/api/v1/dataset/${id}/related_objects`,
+ })
+ .then(({ json = {} }) => {
+ setUsageCount(json.charts.count);
+ })
+ .catch(err => console.log(err));
+ }, [id]);
+
+ const Tab = (
+ <TabStyles>
+ <span>{t('Usage')}</span>
+ <Badge count={usageCount + 1} />
+ </TabStyles>
+ );
+
+ return (
+ <>
+ <StyledTabs moreIcon={null} fullWidth={false}>
+ <Tabs.TabPane tab={t('Columns')} key="1" />
+ <Tabs.TabPane tab={t('Metrics')} key="2" />
+ <Tabs.TabPane tab={Tab} key="3">
+ <div>placeholder</div>
+ </Tabs.TabPane>
+ </StyledTabs>
+ </>
Review Comment:
```suggestion
<StyledTabs moreIcon={null} fullWidth={false}>
<Tabs.TabPane tab={t('Columns')} key="1" />
<Tabs.TabPane tab={t('Metrics')} key="2" />
<Tabs.TabPane tab={Tab} key="3">
<div>placeholder</div>
</Tabs.TabPane>
</StyledTabs>
```
This fragment can be removed since it's all inside of one `StyledTabs` div.
--
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