rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2766008459
##########
superset-frontend/src/dashboard/components/Dashboard.tsx:
##########
@@ -36,72 +37,109 @@ import { areObjectsEqual } from '../../reduxUtils';
import getLocationHash from '../util/getLocationHash';
import isDashboardEmpty from '../util/isDashboardEmpty';
+import type {
+ AppliedCrossFilterType,
+ AppliedNativeFilterType,
+ Filter,
+} from '@superset-ui/core';
import { getAffectedOwnDataCharts } from '../util/charts/getOwnDataCharts';
import { getRelatedCharts } from '../util/getRelatedCharts';
+import type {
+ ActiveFilters,
+ ChartConfiguration,
+ DashboardLayout,
+ DatasourcesState,
+ LayoutItem,
+} from '../types';
-const propTypes = {
- actions: PropTypes.shape({
- addSliceToDashboard: PropTypes.func.isRequired,
- removeSliceFromDashboard: PropTypes.func.isRequired,
- triggerQuery: PropTypes.func.isRequired,
- logEvent: PropTypes.func.isRequired,
- clearDataMaskState: PropTypes.func.isRequired,
- }).isRequired,
- dashboardId: PropTypes.number.isRequired,
- editMode: PropTypes.bool,
- isPublished: PropTypes.bool,
- hasUnsavedChanges: PropTypes.bool,
- slices: PropTypes.objectOf(slicePropShape).isRequired,
- activeFilters: PropTypes.object.isRequired,
- chartConfiguration: PropTypes.object,
- datasources: PropTypes.object.isRequired,
- ownDataCharts: PropTypes.object.isRequired,
- layout: PropTypes.object.isRequired,
- impressionId: PropTypes.string.isRequired,
- timeout: PropTypes.number,
- userId: PropTypes.string,
- children: PropTypes.node,
-};
-
-const defaultProps = {
- timeout: 60,
- userId: '',
-};
-
-class Dashboard extends PureComponent {
+type RelatedChartsFilter =
+ | AppliedNativeFilterType
+ | AppliedCrossFilterType
+ | Filter;
+
+interface DashboardActions {
+ addSliceToDashboard: (id: number, component: LayoutItem | undefined) => void;
+ removeSliceFromDashboard: (id: number) => void;
+ triggerQuery: (value: boolean, id: number | string) => void;
+ logEvent: (eventName: string, eventData: Record<string, unknown>) => void;
+ clearDataMaskState: () => void;
+ clearAllChartStates: () => void;
+ setDatasources: (datasources: unknown) => void;
+}
+
+interface DashboardProps {
+ actions: DashboardActions;
+ dashboardId: number;
+ editMode?: boolean;
+ isPublished?: boolean;
+ hasUnsavedChanges?: boolean;
+ slices: Record<string, Slice>;
+ activeFilters: ActiveFilters;
+ chartConfiguration?: ChartConfiguration;
+ datasources: DatasourcesState;
+ ownDataCharts: JsonObject;
+ layout: DashboardLayout;
+ impressionId: string;
+ timeout?: number;
+ userId?: string;
+ children?: ReactNode;
+}
+
+interface VisibilityEventData {
+ start_offset: number;
+ ts: number;
+}
+
+class Dashboard extends PureComponent<DashboardProps> {
static contextType = PluginContext;
- static onBeforeUnload(hasChanged) {
+ // Use type assertion when accessing context instead of declare field
+ // to avoid babel transformation issues in Jest
+
+ static defaultProps = {
+ timeout: 60,
+ userId: '',
+ };
+
+ appliedFilters: ActiveFilters;
+
+ appliedOwnDataCharts: JsonObject;
+
+ visibilityEventData: VisibilityEventData;
+
+ static onBeforeUnload(hasChanged: boolean): void {
if (hasChanged) {
window.addEventListener('beforeunload', Dashboard.unload);
} else {
window.removeEventListener('beforeunload', Dashboard.unload);
}
}
- static unload() {
+ static unload(): string {
const message = t('You have unsaved changes.');
- window.event.returnValue = message; // Gecko + IE
+ // Gecko + IE: returnValue is typed as boolean but historically accepts
string
+ (window.event as BeforeUnloadEvent).returnValue = message;
return message; // Gecko + Webkit, Safari, Chrome etc.
}
- constructor(props) {
+ constructor(props: DashboardProps) {
super(props);
this.appliedFilters = props.activeFilters ?? {};
this.appliedOwnDataCharts = props.ownDataCharts ?? {};
+ this.visibilityEventData = { start_offset: 0, ts: 0 };
this.onVisibilityChange = this.onVisibilityChange.bind(this);
}
- componentDidMount() {
+ componentDidMount(): void {
const bootstrapData = getBootstrapData();
const { editMode, isPublished, layout } = this.props;
- const eventData = {
+ const eventData: Record<string, unknown> = {
is_soft_navigation: Logger.timeOriginOffset > 0,
is_edit_mode: editMode,
mount_duration: Logger.getTimestamp(),
is_empty: isDashboardEmpty(layout),
is_published: isPublished,
- bootstrap_data_length: bootstrapData.length,
+ bootstrap_data_length: JSON.stringify(bootstrapData).length,
Review Comment:
This change is intentional and a bugfix. getBootstrapData() returns a
BootstrapData object, not an array. The old code bootstrapData.length was
always undefined because plain objects don't have a .length property.
JSON.stringify(bootstrapData).length gives the actual serialized byte
size, which is what the bootstrap_data_length metric name implies.
--
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]
