The vuex method for getting the current user's lists ('lists/getMyLists') uses a hack of grabbing the username to pass to the backend API from context.rootState.user.user.username. When deep linking to /lists or hard reloading on that view it is very likely that the mounted() method will be called before the App.vue level 'user/getUserInfo' vuex call has completed resulting in passing a username of 'undefined' to the backend and an empty set of lists as the response.
Description
Details
Event Timeline
According to the Vuex docs https://vuex.vuejs.org/guide/actions.html#composing-actions, it is possible to compose actions, as long as the action that needs to complete first returns a Promise (which all actions that make API calls now do thanks to @Raymond_Ndibe).
Here is what composing actions could look like, although there are other options, such as async/await.
getMyLists( context, page ) { return context.dispatch( 'user/getUserInfo', { vm: this }, { root: true } ).then( () => { const params = new URLSearchParams( [ [ 'user', context.rootState.user.user.username ], [ 'page', page ] ] ); const request = { url: '/api/lists/?' + params.toString(), method: 'GET', headers: { 'Content-Type': 'application/json' } }; makeApiCall( context, request ).then( ( success ) => { const data = success.body; context.commit( 'MY_LISTS', data ); }, ( failure ) => { displayErrorNotification.call( this, failure ); } ); } ); }
This seems to work, but so does the current code, as I haven't yet run into the problem of erroneously getting an empty set of lists as a response. How would we test for possible failure cases?
Indeed. This ability was actually the reason we wanted to update things to return Promises.
This seems to work, but so does the current code, as I haven't yet run into the problem of erroneously getting an empty set of lists as a response. How would we test for possible failure cases?
I can reproduce the call to http://localhost:8000/api/lists/?user=undefined&page=1 and empty result every time with a forced reload of http://localhost:8000/lists.
The only reason it doesn't happen in the unit tests for getMyLists is that we have supplied a stub rootState there which includes a user.user.username value.
I would like to try and make the dispatch to 'user/getUserInfo' more boring. The vm argument there is used to update the $ability plugin with the collected rights of the user. I think we can hide that from the caller with a little work.
Change 765576 had a related patch set uploaded (by BryanDavis; author: Bryan Davis):
[wikimedia/toolhub@main] ui: ensure user is fetched before fetching user's lists
Change 765576 merged by jenkins-bot:
[wikimedia/toolhub@main] ui: ensure user is fetched before fetching user's lists
Change 770638 had a related patch set uploaded (by BryanDavis; author: Bryan Davis):
[operations/deployment-charts@master] toolhub: Bump container version to 2022-03-15-002555-production
Change 770638 merged by jenkins-bot:
[operations/deployment-charts@master] toolhub: Bump container version to 2022-03-15-002555-production