TNO-2159: Additional fix for reports missing content sometimes #1408
Conversation
| @@ -129,7 +129,7 @@ export const ReportAdmin: React.FC<IReportAdminProps> = ({ path: defaultPath = ' | |||
| if (existingReport) { | |||
| setReport(toForm(existingReport)); | |||
| } else { | |||
| getReport(reportId, true) | |||
| getReport(reportId) | |||
There was a problem hiding this comment.
I reversed this back to how it was before as we don't need report content in ReportAdmin page
| if (open && !existingReport?.instances?.length) { | ||
| fetchReport(report.id); | ||
| } | ||
| }} |
There was a problem hiding this comment.
Another optimization, whereas prior to this we were re-fetching the report data every time someone expanded the ReportCard, even if the report had already been fetched
Now we only fetch the report + instance data if it's not present
There was a problem hiding this comment.
This should work for most cases, but doesn't account for the background processes which run auto-reports.
| React.useEffect(() => { | ||
| const reportId = parseInt(id ?? '0'); | ||
| if (!!reportId && myReports?.length) { | ||
| const existingReport = myReports.find((r) => r.id === reportId); | ||
| if (existingReport) { | ||
| const hasFetchedContent = existingReport?.instances.some((r) => r.content?.length); |
There was a problem hiding this comment.
This is the best I can do given the current data structure. I am assuming that if there's any content present for any of the instances, that means we have fetched content for this report before, so there's no need to refetch
Here's the rough breakdown of what data is present & when:
- fetchMyReports() -> returns reports with no instance data, so
instances: [] - getReport(id) -> returns report with instances data but NO content,
instances: [{ ...instanceData, content: [] }] - getReport(id, true) -> returns report + instance + content
- generateReport(id) -> reports instance + content
As long as one of getReport(id, true) or generateReport(id) have been called by this point. All content data needed should be present.
There was a problem hiding this comment.
I think it's a reasonable approach.
| if (open && !existingReport?.instances?.length) { | ||
| fetchReport(report.id); | ||
| } | ||
| }} |
There was a problem hiding this comment.
This should work for most cases, but doesn't account for the background processes which run auto-reports.
| React.useEffect(() => { | ||
| const reportId = parseInt(id ?? '0'); | ||
| if (!!reportId && myReports?.length) { | ||
| const existingReport = myReports.find((r) => r.id === reportId); | ||
| if (existingReport) { | ||
| const hasFetchedContent = existingReport?.instances.some((r) => r.content?.length); |
There was a problem hiding this comment.
I think it's a reasonable approach.
No description provided.