Copilot commented on code in PR #34606:
URL: https://github.com/apache/superset/pull/34606#discussion_r2267494595
##########
superset-frontend/src/explore/components/DataTablesPane/types.ts:
##########
@@ -16,40 +16,35 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { ReactElement } from 'react';
-import {
- Datasource,
- GenericDataType,
- JsonObject,
- QueryFormData,
-} from '@superset-ui/core';
-import { ExploreActions } from 'src/explore/actions/exploreActions';
-import { ChartStatus } from 'src/explore/types';
+import { GenericDataType, JsonObject, QueryFormData } from '@superset-ui/core';
+import type { ChartStatus, Datasource } from 'src/explore/types';
export enum ResultTypes {
Results = 'results',
Samples = 'samples',
}
+type SetForceQueryAction = (force: boolean) => void;
+
export interface DataTablesPaneProps {
- queryFormData: QueryFormData;
+ queryFormData: Partial<QueryFormData>;
Review Comment:
Making queryFormData a Partial<QueryFormData> may introduce runtime errors
if required properties are missing. Consider defining a more specific interface
that explicitly lists which QueryFormData properties are optional for this use
case.
```suggestion
queryFormData: DataTablesPaneQueryFormData;
```
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -122,7 +122,7 @@ export const useResultsPane = ({
columnNames={[]}
columnTypes={[]}
rowcount={0}
- datasourceId={queryFormData.datasource}
+ datasourceId={queryFormData.datasource ?? ''}
Review Comment:
Using an empty string as fallback for datasourceId could cause issues
downstream. Consider using a more explicit default value or handling the
undefined case more gracefully.
```suggestion
datasourceId={queryFormData.datasource ?? undefined}
```
##########
superset-frontend/src/explore/components/ExploreChartPanel/index.tsx:
##########
@@ -349,12 +368,14 @@ const ExploreChartPanel = ({
/>
)}
<ChartPills
- queriesResponse={chart.queriesResponse}
- chartStatus={chart.chartStatus}
chartUpdateStartTime={chart.chartUpdateStartTime}
- chartUpdateEndTime={chart.chartUpdateEndTime}
+ chartUpdateEndTime={chart.chartUpdateEndTime ?? 0}
Review Comment:
Using 0 as a fallback for chartUpdateEndTime timestamp could be misleading.
Consider using a more appropriate default like Date.now() or handling the
undefined case explicitly.
```suggestion
chartUpdateEndTime={chart.chartUpdateEndTime ?? Date.now()}
```
##########
superset-frontend/src/explore/components/ExploreChartPanel/index.tsx:
##########
@@ -349,12 +368,14 @@ const ExploreChartPanel = ({
/>
)}
<ChartPills
- queriesResponse={chart.queriesResponse}
- chartStatus={chart.chartStatus}
chartUpdateStartTime={chart.chartUpdateStartTime}
- chartUpdateEndTime={chart.chartUpdateEndTime}
+ chartUpdateEndTime={chart.chartUpdateEndTime ?? 0}
refreshCachedQuery={refreshCachedQuery}
- rowLimit={formData?.row_limit}
+ rowLimit={formData?.row_limit ?? 0}
Review Comment:
Using 0 as a fallback for rowLimit could result in no data being displayed.
Consider using a sensible default row limit value instead.
```suggestion
rowLimit={formData?.row_limit ?? 1000}
```
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -150,7 +150,7 @@ export const useResultsPane = ({
coltypes={result.coltypes}
rowcount={result.rowcount}
dataSize={dataSize}
- datasourceId={queryFormData.datasource}
+ datasourceId={queryFormData.datasource ?? ''}
Review Comment:
Using an empty string as fallback for datasourceId could cause issues
downstream. Consider using a more explicit default value or handling the
undefined case more gracefully.
```suggestion
datasourceId={queryFormData.datasource}
```
--
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]
