bito-code-review[bot] commented on code in PR #38581:
URL: https://github.com/apache/superset/pull/38581#discussion_r3001955902
##########
superset-frontend/src/explore/actions/hydrateExplore.ts:
##########
@@ -161,10 +161,10 @@ export const hydrateExplore =
: findPermission('can_csv', 'Superset', user?.roles),
can_export_image: granularExport
? findPermission('can_export_image', 'Superset', user?.roles)
- : true,
+ : findPermission('can_csv', 'Superset', user?.roles),
can_copy_clipboard: granularExport
? findPermission('can_copy_clipboard', 'Superset', user?.roles)
- : true,
+ : findPermission('can_csv', 'Superset', user?.roles),
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect Permission Fallback</b></div>
<div id="fix">
When granularExport is disabled, can_export_image should default to true
(allowing image export) rather than requiring 'can_csv' permission, which is
specific to CSV exports. This change unnecessarily restricts access and doesn't
match the original behavior.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
can_export_image: granularExport
? findPermission('can_export_image', 'Superset', user?.roles)
: true,
can_copy_clipboard: granularExport
? findPermission('can_copy_clipboard', 'Superset', user?.roles)
: true,
````
</div>
</details>
</div>
<small><i>Code Review Run #7438c4</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx:
##########
@@ -79,8 +84,22 @@ export const TableControls = ({
`}
>
<RowCountLabel rowcount={rowcount} loading={isLoading} />
- {canDownload && (
+ {copyEnabled ? (
<CopyToClipboardButton data={formattedData} columns={columnNames} />
+ ) : (
+ <Tooltip title={t("You don't have permission to copy to clipboard")}>
+ <span
+ css={css`
+ opacity: 0.3;
+ cursor: not-allowed;
+ `}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Security: Disabled button still clickable</b></div>
<div id="fix">
The refactoring to conditional rendering removed the 'pointer-events: none'
style that was preventing clicks on the disabled copy button. Without it, users
without permission can still copy data by clicking the visually disabled
button, violating the access control. Add 'pointer-events: none;' to the span's
css to restore the intended behavior.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
css={css`
opacity: 0.3;
cursor: not-allowed;
pointer-events: none;
`}
````
</div>
</details>
</div>
<small><i>Code Review Run #7438c4</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/actions/hydrateExplore.ts:
##########
@@ -161,10 +161,10 @@ export const hydrateExplore =
: findPermission('can_csv', 'Superset', user?.roles),
can_export_image: granularExport
? findPermission('can_export_image', 'Superset', user?.roles)
- : true,
+ : findPermission('can_csv', 'Superset', user?.roles),
can_copy_clipboard: granularExport
? findPermission('can_copy_clipboard', 'Superset', user?.roles)
- : true,
+ : findPermission('can_csv', 'Superset', user?.roles),
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect Permission Fallback</b></div>
<div id="fix">
When granularExport is disabled, can_copy_clipboard should default to true
(allowing clipboard copy) rather than requiring 'can_csv' permission, which is
specific to CSV exports. This change unnecessarily restricts access and doesn't
match the original behavior.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
can_export_image: granularExport
? findPermission('can_export_image', 'Superset', user?.roles)
: true,
can_copy_clipboard: granularExport
? findPermission('can_copy_clipboard', 'Superset', user?.roles)
: true,
````
</div>
</details>
</div>
<small><i>Code Review Run #7438c4</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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]
