learningequality / kolibri

Kolibri Learning Platform: the offline app for universal education
https://learningequality.org/kolibri/
MIT License
770 stars 647 forks source link

Use existence checks for page load blocking checks rather than loading full data payloads #7240

Open jonboiser opened 4 years ago

jonboiser commented 4 years ago

In the Coach, User, and Facility plugins, we make substantial changes to the layout and behavior of each plugins' Single-Page app depending on whether or not the device has 1 or more facilities.

This data (the facilities on the device) need to be available as soon as the app renders, but is (usually) requested asynchronously inside of a beforeEnter guard in the Vue Router, or perhaps somewhere else in a Vuex action. There is one bug #7222 that appears to be the result of the facilities not being loaded in time.

The ask: we bootstrap the facilities information into the SPA as we do for other crucial information, so it is always guaranteed that this data is available when the page renders.

indirectlylit commented 4 years ago

this should be fairly straightforward. adding to 0.14 as a P2

rtibbles commented 4 years ago

Also, this should be much more cache safe thanks to @nucleogenesis's work on etag generation for the SPA pages.

indirectlylit commented 4 years ago

Need to watch out for edge case situation where there are many dozens of facilities

rtibbles commented 4 years ago

Dozens is still not particularly problematic - thousands might be though.

rtibbles commented 4 years ago

The main thing would only be adding this on a per SPA basis (rather than being tempted to add it to the base template).

nucleogenesis commented 3 years ago

After fixing, should create follow-up issue for integrating the bootstrapped data into the SPAs (removing beforeEnter route guards and so on)

rtibbles commented 3 years ago

One thing to note is that we did previously do this: https://github.com/learningequality/kolibri/blob/release-v0.7.x/kolibri/core/templatetags/kolibri_tags.py#L110 and leveraged the frontend API Resource cache so that the beforeEnter hooks did not need to change.

However, I don't think we should revert to this behaviour, but we could add some 'state hydration' logic to the APIResource layer to accomplish this instead for these specific use cases.

nucleogenesis commented 3 years ago

Another thing we might consider is that we could bootstrap it using JS in the app.js for any individual plugin using the stateSetters method of the KolibriApp class that all of our plugins are extended from.

https://github.com/learningequality/kolibri/blob/7f76b3b2caa9b7866b318c7140d3a34704e3962d/kolibri/core/assets/src/kolibri_app.js#L38-L49

rtibbles commented 2 years ago

I think the preferable solution here to bootstrapping all the data into the page is probably to do some smarter data loading, and only loading what we need.

Previously we would load all classes on first page load in Learn just to see if a learner was a member in any classes.

Here, I changed that to have a special state endpoint that reported just the data needed for learn rendering: https://github.com/learningequality/kolibri/blob/1fd1fc08c92eae64634126a757c9e1af442870c2/kolibri/plugins/learn/viewsets.py#L46 just doing an exists check on classroom memberships for the user.

We could do a similar thing here, just loading the facility that the user wants and also returning information indicating whether there are any others. We can also load the facility dataset at the same time and avoid waiting for that in a subsequent request.