korbit-ai[bot] commented on code in PR #34319:
URL: https://github.com/apache/superset/pull/34319#discussion_r2231510386
##########
superset-frontend/src/features/datasets/metadataBar/DatasetMetadataBar.skipped-stories.tsx:
##########
@@ -52,7 +52,23 @@ export default {
export const DatasetSpecific = () => {
SupersetClient.reset();
SupersetClient.configure({ csrfToken: '1234' }).init();
- const { metadataBar } = useDatasetMetadataBar({ datasetId: 1 });
+
+ const mockDataset = {
Review Comment:
### Undocumented mock dataset structure <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The mockDataset object lacks documentation explaining what it represents and
why specific mock values were chosen.
###### Why this matters
Future developers might modify the mock values without understanding their
significance in demonstrating the component's features.
###### Suggested change ∙ *Feature Preview*
```typescript
// Mock dataset representing a typical dataset entity with all required
// fields for metadata bar display
const mockDataset = {
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7ee0996a-5af0-40a1-8aec-cecaaed6607a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7ee0996a-5af0-40a1-8aec-cecaaed6607a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7ee0996a-5af0-40a1-8aec-cecaaed6607a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7ee0996a-5af0-40a1-8aec-cecaaed6607a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7ee0996a-5af0-40a1-8aec-cecaaed6607a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:26609def-565b-4d4a-b80f-9a4d523045fd -->
[](26609def-565b-4d4a-b80f-9a4d523045fd)
##########
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts:
##########
@@ -40,6 +40,7 @@ export enum FeatureFlag {
EmbeddableCharts = 'EMBEDDABLE_CHARTS',
EmbeddedSuperset = 'EMBEDDED_SUPERSET',
EnableAdvancedDataTypes = 'ENABLE_ADVANCED_DATA_TYPES',
+ DrillingInEmbedded = 'DRILLING_IN_EMBEDDED',
Review Comment:
### Feature Flag Not Alphabetically Ordered <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The new feature flag DrillingInEmbedded is not added in alphabetical order
as required by the code comment '// PLEASE KEEP THE LIST SORTED ALPHABETICALLY'.
###### Why this matters
Breaking the alphabetical order makes it harder to locate specific feature
flags and could lead to confusion when maintaining the codebase, especially
when dealing with a large number of flags.
###### Suggested change ∙ *Feature Preview*
Move the feature flag to maintain alphabetical order between 'DrillBy' and
'DynamicPlugins':
```typescript
DrillBy = 'DRILL_BY',
DrillingInEmbedded = 'DRILLING_IN_EMBEDDED',
DynamicPlugins = 'DYNAMIC_PLUGINS',
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cb827a67-267a-465c-804b-084327270cef/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cb827a67-267a-465c-804b-084327270cef?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cb827a67-267a-465c-804b-084327270cef?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cb827a67-267a-465c-804b-084327270cef?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cb827a67-267a-465c-804b-084327270cef)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:a350b125-4af6-4a5f-934f-516ae4f97d30 -->
[](a350b125-4af6-4a5f-934f-516ae4f97d30)
##########
superset/datasets/schemas.py:
##########
@@ -19,14 +19,29 @@
from dateutil.parser import isoparse
from flask_babel import lazy_gettext as _
-from marshmallow import fields, pre_load, Schema, validates_schema,
ValidationError
+from marshmallow import (
+ fields,
+ post_dump,
+ pre_load,
+ Schema,
+ validates_schema,
+ ValidationError,
+)
from marshmallow.validate import Length, OneOf
+from superset import security_manager
+from superset.connectors.sqla.models import SqlaTable
from superset.exceptions import SupersetMarshmallowValidationError
from superset.utils import json
get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}}
get_export_ids_schema = {"type": "array", "items": {"type": "integer"}}
+get_drill_info_schema = {
+ "type": "object",
+ "properties": {
+ "dashboard_id": {"type": "integer"},
+ },
+}
Review Comment:
### Missing Required Field Specification in Schema <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The schema for dashboard_id in get_drill_info_schema lacks the 'required'
field specification
###### Why this matters
Without specifying if dashboard_id is required, the API may accept invalid
requests without this crucial parameter, potentially causing drill
functionality to fail
###### Suggested change ∙ *Feature Preview*
Add the 'required' field to explicitly define whether dashboard_id is
mandatory:
```python
get_drill_info_schema = {
"type": "object",
"properties": {
"dashboard_id": {"type": "integer"},
},
"required": ["dashboard_id"]
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e99813f2-75e2-4618-9ab0-7a8768f977c3/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e99813f2-75e2-4618-9ab0-7a8768f977c3?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e99813f2-75e2-4618-9ab0-7a8768f977c3?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e99813f2-75e2-4618-9ab0-7a8768f977c3?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e99813f2-75e2-4618-9ab0-7a8768f977c3)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:b7a64e65-ce08-4155-9594-fa5394022bd5 -->
[](b7a64e65-ce08-4155-9594-fa5394022bd5)
##########
superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:
##########
@@ -93,6 +94,8 @@ const ModalFooter = ({ formData, closeModal }:
ModalFooterProps) => {
const [datasource_id, datasource_type] = formData.datasource.split('__');
useEffect(() => {
+ // short circuit if the user is embedded as explore is not available
+ if (isEmbedded()) return;
Review Comment:
### Incomplete State Handling in Embedded Mode <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code prematurely returns when in embedded mode without setting the URL
state, which might lead to incorrect button state handling.
###### Why this matters
Without setting the URL state to a known value in embedded mode, the
'isEditDisabled' logic might behave inconsistently, as it relies on the URL
state.
###### Suggested change ∙ *Feature Preview*
Set the URL state to an explicit value when in embedded mode:
```typescript
if (isEmbedded()) {
setUrl('');
return;
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5963815c-f2a8-449c-a347-e17dc804360a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5963815c-f2a8-449c-a347-e17dc804360a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5963815c-f2a8-449c-a347-e17dc804360a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5963815c-f2a8-449c-a347-e17dc804360a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5963815c-f2a8-449c-a347-e17dc804360a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:a7752cd0-044f-40fa-86f8-0ae0937dc191 -->
[](a7752cd0-044f-40fa-86f8-0ae0937dc191)
##########
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx:
##########
@@ -138,12 +153,73 @@ const ChartContextMenu = (
const showDrillToDetail =
isFeatureEnabled(FeatureFlag.DrillToDetail) &&
canDrillToDetail &&
- isDisplayed(ContextMenuItem.DrillToDetail);
+ (!isEmbedded() ||
+ (isEmbedded() && isFeatureEnabled(FeatureFlag.DrillingInEmbedded)));
+ isDisplayed(ContextMenuItem.DrillToDetail);
const showDrillBy =
isFeatureEnabled(FeatureFlag.DrillBy) &&
canDrillBy &&
- isDisplayed(ContextMenuItem.DrillBy);
+ (!isEmbedded() ||
+ (isEmbedded() && isFeatureEnabled(FeatureFlag.DrillingInEmbedded)));
+ isDisplayed(ContextMenuItem.DrillBy);
+
+ useEffect(() => {
+ async function fetchDataset() {
+ if (!visible || dataset || (!showDrillBy && !showDrillToDetail)) return;
+
+ const datasetId = Number(formData.datasource.split('__')[0]);
+ try {
+ setIsLoadingDataset(true);
+ let response;
+
+ if (loadDrillByOptions) {
+ response = await loadDrillByOptions(datasetId, formData);
+ } else {
+ const endpoint =
`/api/v1/dataset/${datasetId}/drill_info/?q=(dashboard_id:${dashboardId})`;
+ response = await cachedSupersetGet({ endpoint });
+ }
+
+ const { json } = response;
+ const { result } = json;
+
+ // Filter columns for drill-by if needed
+ if (showDrillBy && result.columns) {
+ const filteredColumns = ensureIsArray(result.columns).filter(
+ column =>
+ // If using extensions, also filter by column.groupby since
extensions might not do this
+ (!loadDrillByOptions || column.groupby) &&
+ !ensureIsArray(
+ formData[filters?.drillBy?.groupbyFieldName ?? ''],
+ ).includes(column.column_name) &&
+ column.column_name !== formData.x_axis &&
+ ensureIsArray(additionalConfig?.drillBy?.excludedColumns)?.every(
+ excludedCol => excludedCol.column_name !== column.column_name,
+ ),
+ );
+ result.columns = filteredColumns;
+ }
+
+ setDataset(result);
+ } catch (error) {
+ logging.error('Failed to load dataset:', error);
+ supersetGetCache.delete(`/api/v1/dataset/${datasetId}/drill_info/`);
Review Comment:
### Insufficient Error Context <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The error logging does not include important contextual information like the
datasetId or dashboardId that could help with debugging.
###### Why this matters
When troubleshooting production issues, having detailed error context is
crucial for quick problem resolution. Without the dataset and dashboard
identifiers, it would be harder to trace the exact failure.
###### Suggested change ∙ *Feature Preview*
```typescript
} catch (error) {
logging.error(`Failed to load dataset. datasetId: ${datasetId},
dashboardId: ${dashboardId}`, error);
supersetGetCache.delete(`/api/v1/dataset/${datasetId}/drill_info/`);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6359d328-0a34-4219-b795-764823b1e77e/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6359d328-0a34-4219-b795-764823b1e77e?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6359d328-0a34-4219-b795-764823b1e77e?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6359d328-0a34-4219-b795-764823b1e77e?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6359d328-0a34-4219-b795-764823b1e77e)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:460a9b65-9c59-4e2a-a496-9503e70d7fa1 -->
[](460a9b65-9c59-4e2a-a496-9503e70d7fa1)
##########
superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx:
##########
@@ -104,18 +78,16 @@
openNewModal = true,
open,
onDrillBy,
+ dataset,
+ isLoadingDataset = false,
...rest
}: DrillByMenuItemsProps) => {
const theme = useTheme();
- const { addDangerToast } = useToasts();
- const [isLoadingColumns, setIsLoadingColumns] = useState(true);
const [searchInput, setSearchInput] = useState('');
const [debouncedSearchInput, setDebouncedSearchInput] = useState('');
- const [dataset, setDataset] = useState<Dataset>();
- const [columns, setColumns] = useState<Column[]>([]);
const ref = useRef<InputRef>(null);
- const showSearch =
- loadDrillByOptions || columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;
+ const columns = dataset ? ensureIsArray(dataset.columns) : [];
Review Comment:
### Missing Column Filtering Logic <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code doesn't filter the columns based on groupby and excludedColumns
criteria, which was present in the original code.
###### Why this matters
Without proper column filtering, users might see and select columns that
should not be available for drill-by operations, leading to incorrect
functionality.
###### Suggested change ∙ *Feature Preview*
Add the filtering logic back:
```typescript
const columns = dataset
? ensureIsArray(dataset.columns)
.filter(column => column.groupby)
.filter(
column =>
!ensureIsArray(
formData[drillByConfig?.groupbyFieldName ?? ''],
).includes(column.column_name) &&
column.column_name !== formData.x_axis &&
ensureIsArray(excludedColumns)?.every(
excludedCol => excludedCol.column_name !== column.column_name,
),
)
: [];
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1c01f743-21fb-4ac9-9028-05046a795afa/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1c01f743-21fb-4ac9-9028-05046a795afa?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1c01f743-21fb-4ac9-9028-05046a795afa?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1c01f743-21fb-4ac9-9028-05046a795afa?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1c01f743-21fb-4ac9-9028-05046a795afa)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:36052f13-2e27-47c1-ad2c-fa5f79d9c4c1 -->
[](36052f13-2e27-47c1-ad2c-fa5f79d9c4c1)
##########
superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx:
##########
@@ -104,18 +78,16 @@ export const DrillByMenuItems = ({
openNewModal = true,
open,
onDrillBy,
+ dataset,
+ isLoadingDataset = false,
...rest
}: DrillByMenuItemsProps) => {
const theme = useTheme();
- const { addDangerToast } = useToasts();
- const [isLoadingColumns, setIsLoadingColumns] = useState(true);
const [searchInput, setSearchInput] = useState('');
const [debouncedSearchInput, setDebouncedSearchInput] = useState('');
- const [dataset, setDataset] = useState<Dataset>();
- const [columns, setColumns] = useState<Column[]>([]);
const ref = useRef<InputRef>(null);
- const showSearch =
- loadDrillByOptions || columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;
+ const columns = dataset ? ensureIsArray(dataset.columns) : [];
+ const showSearch = columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;
Review Comment:
### Unmemoized Length Calculation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The columns.length computation is performed unnecessarily on every render
since it's not memoized.
###### Why this matters
Recalculating columns.length on every render when the columns array hasn't
changed impacts performance, especially when dealing with large datasets.
###### Suggested change ∙ *Feature Preview*
Memoize the showSearch calculation using useMemo to prevent unnecessary
recalculations:
```typescript
const showSearch = useMemo(
() => columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD,
[columns]
);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d43e1a6b-6bce-4538-9e32-72ac55b6dea9/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d43e1a6b-6bce-4538-9e32-72ac55b6dea9?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d43e1a6b-6bce-4538-9e32-72ac55b6dea9?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d43e1a6b-6bce-4538-9e32-72ac55b6dea9?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d43e1a6b-6bce-4538-9e32-72ac55b6dea9)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:83ad64ce-b0ae-4cc1-ae59-ea25966d498d -->
[](83ad64ce-b0ae-4cc1-ae59-ea25966d498d)
##########
superset/datasets/schemas.py:
##########
@@ -373,3 +388,48 @@
"description": "A list of each chart's warmup status and errors if
any"
},
)
+
+
+class DatasetColumnDrillInfo(Schema):
+ column_name = fields.String(required=True)
+ verbose_name = fields.String(required=False)
+
+
+class UserSchema(Schema):
+ first_name = fields.String()
+ last_name = fields.String()
+
+
+class DatasetDrillInfo(Schema):
+ id = fields.Integer()
+ columns = fields.List(fields.Nested(DatasetColumnDrillInfo))
+ table_name = fields.String()
+ owners = fields.List(fields.Nested(UserSchema))
+ created_by = fields.Nested(UserSchema)
+ created_on_humanized = fields.String()
+ changed_by = fields.Nested(UserSchema)
+ changed_on_humanized = fields.String()
+
+ # pylint: disable=unused-argument
+ @post_dump(pass_original=True)
+ def post_dump(
+ self, serialized: dict[str, Any], obj: SqlaTable, **kwargs: Any
+ ) -> dict[str, Any]:
Review Comment:
### Missing Null Safety Checks in post_dump <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The post_dump method may fail if obj is None or doesn't have the expected
columns attribute
###### Why this matters
Lack of null checks could cause the application to crash when processing
invalid or incomplete dataset objects
###### Suggested change ∙ *Feature Preview*
Add null safety checks:
```python
@post_dump(pass_original=True)
def post_dump(
self, serialized: dict[str, Any], obj: SqlaTable, **kwargs: Any
) -> dict[str, Any]:
if not obj:
return serialized
dimensions = {
col.column_name
for col in getattr(obj, "columns", [])
if col and getattr(col, "groupby", False)
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/435be054-62c1-472a-a28a-063ac6961e5a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/435be054-62c1-472a-a28a-063ac6961e5a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/435be054-62c1-472a-a28a-063ac6961e5a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/435be054-62c1-472a-a28a-063ac6961e5a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/435be054-62c1-472a-a28a-063ac6961e5a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:8efcc130-e7e4-4954-b5c5-98c51ed82b05 -->
[](8efcc130-e7e4-4954-b5c5-98c51ed82b05)
##########
superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx:
##########
@@ -16,49 +16,31 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { useEffect, useMemo, useState } from 'react';
+import { useMemo } from 'react';
import { css, t, useTheme } from '@superset-ui/core';
-import { Alert } from '@superset-ui/core/components';
import { Dataset } from 'src/components/Chart/types';
import MetadataBar from '@superset-ui/core/components/MetadataBar';
import {
ContentType,
MetadataType,
} from '@superset-ui/core/components/MetadataBar/ContentType';
-import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
-import { cachedSupersetGet } from 'src/utils/cachedSupersetGet';
+import { isEmbedded } from 'src/dashboard/util/isEmbedded';
+
+export interface UseDatasetMetadataBarProps {
+ dataset?: Dataset;
+}
-export type UseDatasetMetadataBarProps =
- | { datasetId?: undefined; dataset: Dataset }
- | { datasetId: number | string; dataset?: undefined };
export const useDatasetMetadataBar = ({
- dataset: datasetProps,
- datasetId,
-}: UseDatasetMetadataBarProps) => {
+ dataset,
+}: UseDatasetMetadataBarProps): { metadataBar: React.ReactElement | null } => {
const theme = useTheme();
- const [result, setResult] = useState<Dataset>();
- const [status, setStatus] = useState<ResourceStatus>(
- datasetProps ? ResourceStatus.Complete : ResourceStatus.Loading,
- );
-
- useEffect(() => {
- if (!datasetProps && datasetId) {
- cachedSupersetGet({
- endpoint: `/api/v1/dataset/${datasetId}`,
- })
- .then(({ json: { result } }) => {
- setResult(result);
- setStatus(ResourceStatus.Complete);
- })
- .catch(() => {
- setStatus(ResourceStatus.Error);
- });
- }
- }, [datasetId, datasetProps]);
const metadataBar = useMemo(() => {
+ // Short-circuit for embedded users - they don't need metadata bar
Review Comment:
### Unclear embedded context comment <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The inline comment doesn't explain why embedded users don't need the
metadata bar.
###### Why this matters
Future developers might remove this optimization without understanding its
purpose, potentially causing unnecessary renders.
###### Suggested change ∙ *Feature Preview*
// Skip metadata bar for embedded views to reduce visual noise and improve
performance in embedded contexts
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4731c6b1-325f-463c-b4ba-364c3c3aee3a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4731c6b1-325f-463c-b4ba-364c3c3aee3a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4731c6b1-325f-463c-b4ba-364c3c3aee3a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4731c6b1-325f-463c-b4ba-364c3c3aee3a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4731c6b1-325f-463c-b4ba-364c3c3aee3a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:bef1bf60-d88a-4863-967a-a0f59e7a7f7b -->
[](bef1bf60-d88a-4863-967a-a0f59e7a7f7b)
##########
superset/config.py:
##########
@@ -279,10 +279,11 @@ def _try_json_readsha(filepath: str, length: int) -> str
| None:
# Add endpoints that need to be exempt from CSRF protection
WTF_CSRF_EXEMPT_LIST = [
- "superset.views.core.log",
- "superset.views.core.explore_json",
"superset.charts.data.api.data",
"superset.dashboards.api.cache_dashboard_screenshot",
+ "superset.views.core.explore_json",
+ "superset.views.core.log",
+ "superset.views.datasource.views.samples",
]
Review Comment:
### Unjustified CSRF Protection Bypass <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Adding a new endpoint 'superset.views.datasource.views.samples' to the CSRF
exempt list without proper security justification increases the attack surface
for CSRF attacks.
###### Why this matters
CSRF exempt endpoints bypass important protection against Cross-Site Request
Forgery attacks. Each exempted endpoint needs strong justification and
alternative protections. Adding new exemptions increases security risk.
###### Suggested change ∙ *Feature Preview*
Remove the new endpoint from CSRF exempt list unless there is a critical
technical requirement and alternative protections are in place:
```python
WTF_CSRF_EXEMPT_LIST = [
"superset.charts.data.api.data",
"superset.dashboards.api.cache_dashboard_screenshot",
"superset.views.core.explore_json",
"superset.views.core.log",
]
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c3e4949d-2361-4891-927d-76c80d631aa2/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c3e4949d-2361-4891-927d-76c80d631aa2?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c3e4949d-2361-4891-927d-76c80d631aa2?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c3e4949d-2361-4891-927d-76c80d631aa2?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c3e4949d-2361-4891-927d-76c80d631aa2)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:d06e9eeb-84fb-4ac7-b25b-9e5a59dfa5fa -->
[](d06e9eeb-84fb-4ac7-b25b-9e5a59dfa5fa)
--
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]
