codeant-ai-for-open-source[bot] commented on code in PR #40912:
URL: https://github.com/apache/superset/pull/40912#discussion_r3382641971
##########
superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx:
##########
@@ -240,6 +241,24 @@ export const useHeaderActionsMenu = ({
key: MenuKeys.EditProperties,
label: t('Edit properties'),
});
+
+ // View lineage
+ if (dashboardId) {
+ menuItems.push(
+ createModalMenuItem(
+ MenuKeys.ViewLineage,
+ <LineageModal
+ entityType="dashboard"
+ entityId={dashboardId}
+ triggerNode={
+ <div data-test="view-lineage-menu-item">
+ {t('View lineage')}
+ </div>
+ }
+ />,
+ ),
+ );
+ }
Review Comment:
**Suggestion:** The lineage action is currently added inside the edit-only
branch, so users in normal dashboard view mode cannot access dashboard lineage
from the header actions menu. Move this menu item outside the edit-only
condition (with an appropriate read permission check) so lineage is available
consistently. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ View-mode header omits dashboard lineage entry.
- ⚠️ Read-only users lack header access to lineage.
- ⚠️ New lineage feature inconsistent across dashboards UI.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dashboard in view mode so `editMode` is false in `Header` (see
`superset-frontend/src/dashboard/components/Header/index.tsx:62-79`, where
`editMode` is
derived from `dashboardState.editMode`).
2. Note that the header actions dropdown is constructed via
`useHeaderActionsMenu` in
`Header` at
`superset-frontend/src/dashboard/components/Header/index.tsx:154-184`, passing
the current `editMode` and `dashboardInfo.id` as `dashboardId`.
3. Inside `useHeaderActionsDropdownMenu.tsx:204-59`, the lineage menu item
is only pushed
under the `if (editMode)` branch, specifically lines `236-261`, and the `//
View lineage`
block at `246-261` is nested inside this edit-only condition.
4. Because `editMode` is false in normal view mode and read-only users
cannot toggle it
(the "Edit dashboard" button is only shown when `userCanEdit` at
`Header/index.tsx:113-124`), the lineage menu item is never added to the
header actions
menu for view-mode users, so they cannot access dashboard lineage from the
header.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4411740aa8f24b8eb1689a56bef41273&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4411740aa8f24b8eb1689a56bef41273&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx
**Line:** 246:261
**Comment:**
*Incorrect Condition Logic: The lineage action is currently added
inside the edit-only branch, so users in normal dashboard view mode cannot
access dashboard lineage from the header actions menu. Move this menu item
outside the edit-only condition (with an appropriate read permission check) so
lineage is available consistently.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40912&comment_hash=b3c0ce8414655b73eedad32103424e50e76736144782e0959ae652e988535f1a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40912&comment_hash=b3c0ce8414655b73eedad32103424e50e76736144782e0959ae652e988535f1a&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/datasets/AddDataset/EditDataset/index.tsx:
##########
@@ -51,16 +53,19 @@ const TRANSLATIONS = {
USAGE_TEXT: t('Usage'),
COLUMNS_TEXT: t('Columns'),
METRICS_TEXT: t('Metrics'),
+ LINEAGE_TEXT: t('Lineage'),
};
const TABS_KEYS = {
COLUMNS: 'COLUMNS',
METRICS: 'METRICS',
USAGE: 'USAGE',
+ LINEAGE: 'LINEAGE',
};
const EditPage = ({ id }: EditPageProps) => {
const { usageCount } = useGetDatasetRelatedCounts(id);
+ const lineageResource = useDatasetLineage(id);
Review Comment:
**Suggestion:** The lineage API request is started immediately when the edit
page mounts, even if the user never opens the Lineage tab, which adds avoidable
latency and backend load. Move the hook call into a tab-pane component that
mounts only when the Lineage tab is active (or gate the fetch by active tab
key). [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dataset edit page always triggers lineage fetch.
- ⚠️ Extra lineage calls for users ignoring Lineage tab.
- ⚠️ Unnecessary backend load on dataset lineage endpoint.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Navigate to the dataset edit route, which renders `AddDataset`
(`superset-frontend/src/pages/DatasetCreation/index.tsx:86-119`) and, when a
numeric
`datasetId` param is present, shows `<EditPage id={id} />` at line 117.
2. In `EditPage`
(`superset-frontend/src/features/datasets/AddDataset/EditDataset/index.tsx:66-103`),
observe that `const lineageResource = useDatasetLineage(id);` at line 68 is
executed
unconditionally on mount, before any tab selection.
3. The `useDatasetLineage` hook in
`superset-frontend/src/hooks/apiResources/lineage.ts:20-21` delegates to
`useApiV1Resource<DatasetLineage>(\`/api/v1/dataset/${idOrUuid}/lineage\`)`,
which uses
`useApiResourceFullBody` (`apiResources.ts:87-135`) to issue a GET request
whenever the
`endpoint` string changes or the hook first mounts.
4. Although the lineage UI is only rendered inside the `LINEAGE` tab's
content
(`EditDataset/index.tsx:77-100`, `children: <LineageView
lineageResource={lineageResource}
entityType="dataset" />`), the lineage API call is already fired at step 2
for every
dataset edit page load, even if the user never switches to the Lineage tab,
causing
avoidable network and backend load.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a01a4c0c36ba4ffd8a1179c95611271c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a01a4c0c36ba4ffd8a1179c95611271c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/features/datasets/AddDataset/EditDataset/index.tsx
**Line:** 68:68
**Comment:**
*Performance: The lineage API request is started immediately when the
edit page mounts, even if the user never opens the Lineage tab, which adds
avoidable latency and backend load. Move the hook call into a tab-pane
component that mounts only when the Lineage tab is active (or gate the fetch by
active tab key).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40912&comment_hash=8084d1ed927b33153b226d4d1d140051a2bb472ebc5cded4011c12520272b842&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40912&comment_hash=8084d1ed927b33153b226d4d1d140051a2bb472ebc5cded4011c12520272b842&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/lineage/LineageModal.tsx:
##########
@@ -0,0 +1,79 @@
+/**
+ * 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 { FC, ReactNode } from 'react';
+import { t } from '@apache-superset/core/translation';
+import { ModalTrigger } from '@superset-ui/core/components';
+import {
+ useChartLineage,
+ useDashboardLineage,
+ useDatasetLineage,
+} from 'src/hooks/apiResources';
+import LineageView from './LineageView';
+
+export interface LineageModalProps {
+ entityType: 'dataset' | 'chart' | 'dashboard';
+ entityId: string | number;
+ triggerNode: ReactNode;
+}
+
+const LineageModal: FC<LineageModalProps> = ({
+ entityType,
+ entityId,
+ triggerNode,
+}) => {
+ const datasetLineage = useDatasetLineage(
+ entityType === 'dataset' ? entityId : '',
+ );
+ const chartLineage = useChartLineage(entityType === 'chart' ? entityId : '');
+ const dashboardLineage = useDashboardLineage(
+ entityType === 'dashboard' ? entityId : '',
+ );
Review Comment:
**Suggestion:** This modal calls all three lineage hooks on every render and
passes empty IDs for two of them, which still builds real endpoints like
`/api/v1/chart//lineage` and triggers unnecessary failing requests. Fetch only
the hook that matches the selected entity type (or add a skip mechanism) to
avoid extra API traffic and error-state churn. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Each lineage modal spawns two unused API requests.
- ⚠️ Requests hit invalid endpoints like `/api/v1/chart//lineage`.
- ⚠️ Background errors clutter network and server logs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a UI entry point that renders `LineageModal`, e.g. a dashboard card
in the list
view where `DashboardCard` adds a menu item with `<LineageModal
entityType="dashboard"
entityId={dashboard.id} ... />` at
`superset-frontend/src/features/dashboards/DashboardCard.tsx:15-29`, or the
chart actions
menu where `useExploreAdditionalActionsMenu` does `<LineageModal
entityType="chart"
entityId={slice.slice_id} ... />` at
`superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:34-47`,
or the dashboard header actions menu using `<LineageModal
entityType="dashboard"
entityId={dashboardId} ... />` at
`superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx:42-57`.
2. When any of those menus render the `LineageModal` component
(`superset-frontend/src/features/lineage/LineageModal.tsx:35-39`), React
mounts it
immediately as part of the menu label, regardless of whether the modal has
been opened
yet.
3. On mount, `LineageModal` calls all three hooks at lines `40-46`:
`useDatasetLineage(entityType === 'dataset' ? entityId : '')`,
`useChartLineage(entityType
=== 'chart' ? entityId : '')`, and `useDashboardLineage(entityType ===
'dashboard' ?
entityId : '')`, so for example with `entityType="dashboard"` two hooks
receive `''` as
`idOrUuid`.
4. Each hook builds its endpoint string in
`superset-frontend/src/hooks/apiResources/lineage.ts:20-35`, producing URLs
like
`/api/v1/dataset//lineage` and `/api/v1/chart//lineage` when given `''`, and
`useApiV1Resource` (`apiResources.ts:184-187`) then issues real GET requests
via
`useApiResourceFullBody` (`apiResources.ts:87-135`). These extra requests
fail on the
backend and only their `Resource` objects go into `Error` state (unused by
`LineageView`,
which consumes only the matching `lineageResource` at
`LineageModal.tsx:48-53`), but they
still add two failing API calls per lineage modal render.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1b4c05a9454d4e9b9bef62b01fe0fe8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1b4c05a9454d4e9b9bef62b01fe0fe8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/lineage/LineageModal.tsx
**Line:** 40:46
**Comment:**
*Performance: This modal calls all three lineage hooks on every render
and passes empty IDs for two of them, which still builds real endpoints like
`/api/v1/chart//lineage` and triggers unnecessary failing requests. Fetch only
the hook that matches the selected entity type (or add a skip mechanism) to
avoid extra API traffic and error-state churn.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40912&comment_hash=7725c386a2e6b7127367a3ae64e5c1d999dbd064c91070a4ddd99751846be196&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40912&comment_hash=7725c386a2e6b7127367a3ae64e5c1d999dbd064c91070a4ddd99751846be196&reaction=dislike'>👎</a>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
