Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: force visualization fetch to ensure recording for offline #3196

Conversation

edoardo
Copy link
Member

@edoardo edoardo commented Jan 28, 2025

Key features

  1. ensure the visualization request is cached for offline

Description

The visualization request stopped being recorded after recent changes. The componentDidUpdate method was taking care of the recording, but it doesn't work as before.
This change ensures the visualization is fetched when the dashboard is marked for offline so that the request is properly cached.

The visualization request stopped being recorded after recent changes.
The componentDidUpdate method was taking care of the recording, but it
doesn't work as before.
This change ensures the visualization is fetched when the dashboard is
marked for offline so that the request is properly cached.
@edoardo edoardo changed the base branch from master to feat/custom-plugins-rebased-latest January 28, 2025 09:15
@dhis2-bot
Copy link
Contributor

dhis2-bot commented Jan 28, 2025

🚀 Deployed on https://pr-3196.dashboard.netlify.dhis2.org

@dhis2-bot dhis2-bot temporarily deployed to netlify January 28, 2025 09:17 Inactive
@@ -96,6 +96,11 @@ class Item extends Component {
await apiFetchVisualization(this.props.item)
)

// force fetch when recording to allow caching of the visualizations request
if (this.props.isRecording) {
apiFetchVisualization(this.props.item)
Copy link
Collaborator

@jenniferarnesen jenniferarnesen Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to await this request?

It does look a bit weird to have the same request 5 lines above.

Also, no chance that this request would get triggered twice?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to await because we are not using the returned value.
The other fetch above needs the returned value for setting it in the Redux store.

I don't think it can be triggered twice because the recording can only happen once the dashboard is loaded.
But this needs testing.

…st' into fix/missing-offline-request-recording
@dhis2-bot dhis2-bot temporarily deployed to netlify January 28, 2025 11:05 Inactive
@jenniferarnesen jenniferarnesen merged commit 42aa33a into feat/custom-plugins-rebased-latest Jan 28, 2025
24 of 29 checks passed
@jenniferarnesen jenniferarnesen deleted the fix/missing-offline-request-recording branch January 28, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants