Skip to content

Commit c03ce23

Browse files
committed
merge: merge branch 'THU/superset_embedding_bugfixes'
2 parents 3eb95c4 + 05e7e30 commit c03ce23

File tree

3 files changed

+34
-25
lines changed

3 files changed

+34
-25
lines changed

src/charts/DashboardPlaceholder/useDashboardPlaceholder.js

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,11 @@ const DEFAULT_LABELS = {
1414
inProgress: { label: 'Scenario run in progress...' },
1515
hasErrors: { label: 'An error occured during the scenario run' },
1616
hasUnknownStatus: {
17-
label: 'This scenario has an unknown state, if the problem persists, please, contact your administrator',
17+
label: 'This scenario has an unknown state. If the problem persists, please contact your administrator.',
1818
},
1919
resultsDisplayDisabled: 'Display of results is disabled',
2020
downloadButton: 'Download logs',
21-
errors: {
22-
unknown: 'Unknown error',
23-
details: 'Something went wrong when fetching PowerBI reports info',
24-
},
21+
noTokenForBI: 'Authentication failed. If the problem persists, please contact your administrator.',
2522
};
2623

2724
const forgeDashboardPlaceholder = ({
@@ -32,6 +29,7 @@ const forgeDashboardPlaceholder = ({
3229
labels: tmpLabels,
3330
scenario,
3431
scenarioDTO,
32+
hasTokenForBI,
3533
}) => {
3634
const labels = { ...DEFAULT_LABELS, ...tmpLabels };
3735

@@ -42,23 +40,22 @@ const forgeDashboardPlaceholder = ({
4240
const hasError = scenarioLastRunStatus === RUNNER_RUN_STATE.FAILED;
4341
const hasUnknownStatus = scenarioLastRunStatus === RUNNER_RUN_STATE.UNKNOWN;
4442

45-
if (alwaysShowReports) return null;
43+
if (alwaysShowReports && !disabled) return null;
4644
if (noScenario) return <DashboardPlaceholderLayout label={labels.noScenario.label} title={labels.noScenario.title} />;
47-
if (noRun) return <DashboardPlaceholderLayout label={labels.noRun.label} title={labels.noRun.title} />;
48-
if (runInProgress)
49-
return <DashboardPlaceholderLayout label={labels.inProgress.label} title={labels.inProgress.title} inProgress />;
45+
if (noRun) return <DashboardPlaceholderLayout label={labels.noRun.label} />;
46+
if (runInProgress) return <DashboardPlaceholderLayout label={labels.inProgress.label} inProgress />;
5047
if (hasError)
5148
return (
5249
<DashboardPlaceholderLayout
5350
label={labels.hasErrors.label}
54-
title={labels.hasErrors.title}
5551
downloadLogsFile={downloadLogsFile}
5652
downloadLabel={labels.downloadButton}
5753
/>
5854
);
5955
if (hasUnknownStatus) return <DashboardPlaceholderLayout label={labels.hasUnknownStatus.label} />;
6056
if (disabled) return <DashboardPlaceholderLayout label={labels.resultsDisplayDisabled} />;
6157
if (noDashboardConfigured) return <DashboardPlaceholderLayout label={labels.noDashboard.label} />;
58+
if (!hasTokenForBI) return <DashboardPlaceholderLayout label={labels.noTokenForBI} />;
6259
return null;
6360
};
6461

src/charts/PowerBIReport/PowerBIReport.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export const PowerBIReport = ({
7070
const tokenType = useAAD ? 0 : 1;
7171
// PowerBI Report object (received via callback)
7272
const [report, setReport] = useState();
73-
const [disabled, setDisabled] = useState(false);
73+
const [canTriggerRefresh, setCanTriggerRefresh] = useState(true);
7474
const [embedConfig, setEmbedConfig] = useState({
7575
type: 'report',
7676
id: reportId,
@@ -91,20 +91,24 @@ export const PowerBIReport = ({
9191
[dynamicFilters, scenarioDTO]
9292
);
9393

94+
const disabled = useMemo(() => reports?.status === 'DISABLED', [reports]);
95+
const hasTokenFetchFailed = useMemo(() => reports?.status === 'ERROR', [reports]);
9496
const { placeholder } = useMemo(() => {
9597
return getDashboardPlaceholder({
9698
alwaysShowReports,
97-
disabled: reports?.status === 'DISABLED',
99+
disabled,
98100
downloadLogsFile,
99101
noDashboardConfigured: reportConfiguration[index] == null,
100102
scenario,
101103
scenarioDTO,
102104
labels,
105+
hasTokenForBI: embedConfig?.accessToken != null && embedConfig.accessToken.length > 0,
103106
});
104107
}, [
108+
disabled,
109+
embedConfig.accessToken,
105110
getDashboardPlaceholder,
106111
alwaysShowReports,
107-
reports,
108112
downloadLogsFile,
109113
reportConfiguration,
110114
index,
@@ -140,10 +144,8 @@ export const PowerBIReport = ({
140144
report.refresh();
141145

142146
if (triggerTimeout) {
143-
setDisabled(true);
144-
setTimeout(() => {
145-
setDisabled(false);
146-
}, refreshTimeout);
147+
setCanTriggerRefresh(false);
148+
setTimeout(() => setCanTriggerRefresh(true), refreshTimeout);
147149
}
148150
},
149151
[refreshTimeout, report]
@@ -164,7 +166,7 @@ export const PowerBIReport = ({
164166
<PowerBIEmbed cssClassName={classes.report} embedConfig={embedConfig} getEmbeddedComponent={setReport} />
165167
);
166168
} catch (error) {
167-
console.log('Error when intializing the PowerBIEmbed component.');
169+
console.log('Error when initializing the PowerBIEmbed component.');
168170
console.error(error);
169171
return null;
170172
}
@@ -176,7 +178,7 @@ export const PowerBIReport = ({
176178
(scenarioLastRunStatus === undefined || scenarioLastRunStatus === RUNNER_RUN_STATE.SUCCESSFUL) && !noScenario;
177179

178180
const divContainerStyle = {};
179-
if (!isReady && !alwaysShowReports) {
181+
if (disabled || hasTokenFetchFailed || (!isReady && !alwaysShowReports)) {
180182
divContainerStyle.display = 'none';
181183
}
182184

@@ -195,7 +197,7 @@ export const PowerBIReport = ({
195197
<FadingTooltip title={labels.refreshTooltip}>
196198
<IconButton
197199
aria-label="refresh"
198-
disabled={!report || disabled}
200+
disabled={!report || !canTriggerRefresh}
199201
color="primary"
200202
onClick={refreshReport}
201203
size="large"
@@ -331,6 +333,7 @@ PowerBIReport.propTypes = {
331333
label: PropTypes.string,
332334
}),
333335
resultsDisplayDisabled: PropTypes.string,
336+
noTokenForBI: PropTypes.string,
334337
downloadButton: PropTypes.string.isRequired,
335338
refreshTooltip: PropTypes.string.isRequired,
336339
errors: PropTypes.shape({

src/charts/SupersetReport/SupersetReport.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ export const SupersetReport = ({
2626
report,
2727
scenario,
2828
style,
29-
visibleScenarios,
29+
visibleScenarios = [],
3030
}) => {
3131
const containerRef = useRef(null);
3232
const tokenRef = useRef(null);
33+
const previousUiConfigRef = useRef(null);
34+
3335
const [isEmbedded, setIsEmbedded] = useState(false);
3436
const [dashboard, setDashboard] = useState(null);
3537

@@ -45,15 +47,19 @@ export const SupersetReport = ({
4547
if (!report?.id || !options?.supersetUrl) return;
4648

4749
const loadSuperset = async () => {
48-
if (isEmbedded || guestToken?.status !== SUPERSET_GUEST_TOKEN_STATUS.SUCCESS) return;
50+
if (guestToken?.status !== SUPERSET_GUEST_TOKEN_STATUS.SUCCESS) return;
51+
52+
const forceRefresh = JSON.stringify(report?.uiConfig) !== JSON.stringify(previousUiConfigRef.current);
53+
if (isEmbedded && !forceRefresh) return;
4954

55+
previousUiConfigRef.current = report?.uiConfig;
5056
try {
5157
const embedded = await embedDashboard({
5258
id: report.id,
5359
supersetDomain: options.supersetUrl,
5460
mountPoint: containerRef.current,
5561
fetchGuestToken: async () => tokenRef.current,
56-
dashboardUiConfig: report?.uiConfig || {},
62+
dashboardUiConfig: report?.uiConfig ?? {},
5763
});
5864
setDashboard(embedded);
5965
setIsEmbedded(true);
@@ -73,7 +79,7 @@ export const SupersetReport = ({
7379
if (dashboard?.destroy) dashboard.destroy();
7480
};
7581
// eslint-disable-next-line react-hooks/exhaustive-deps
76-
}, [report.id, options.supersetUrl, guestToken]);
82+
}, [isEmbedded, report?.id, report?.uiConfig, options.supersetUrl, guestToken]);
7783

7884
const { placeholder, showPlaceholder } = useMemo(() => {
7985
const scenarioDTO = PowerBIUtils.constructScenarioDTO(scenario, visibleScenarios);
@@ -85,8 +91,10 @@ export const SupersetReport = ({
8591
scenario,
8692
scenarioDTO,
8793
labels,
94+
hasTokenForBI: guestToken?.status === SUPERSET_GUEST_TOKEN_STATUS.SUCCESS,
8895
});
8996
}, [
97+
guestToken?.status,
9098
getDashboardPlaceholder,
9199
alwaysShowReports,
92100
disabled,
@@ -101,11 +109,12 @@ export const SupersetReport = ({
101109
const containerHeight = isReportVisible ? (report?.height ?? style?.height ?? '800px') : '100%';
102110
const containerWidth = isReportVisible ? (report?.width ?? style?.width ?? '100%') : '100%';
103111
const reportContainerDisplay = isReportVisible ? undefined : 'none';
112+
const isWaitingForToken = guestToken?.status === SUPERSET_GUEST_TOKEN_STATUS.LOADING && guestToken?.value === '';
104113
const showLoadingSpinner =
105114
!isParentLoading &&
106115
guestToken?.status !== SUPERSET_GUEST_TOKEN_STATUS.ERROR &&
107116
placeholder == null &&
108-
(!isEmbedded || guestToken?.status === SUPERSET_GUEST_TOKEN_STATUS.LOADING);
117+
(!isEmbedded || isWaitingForToken);
109118

110119
return (
111120
<Box sx={{ position: 'relative', width: containerWidth, height: containerHeight, overflow: 'hidden' }}>

0 commit comments

Comments
 (0)