Merge pull request #10455 from nucleogenesis/fix--loading-states

Fixes loading states in general
learningequality · Jun 5, 2023 · 2a90b68 · 2a90b68
2 parents 011d8ee + c15cbe6
commit 2a90b68
Show file tree

Hide file tree

Showing 30 changed files with 327 additions and 282 deletions.
diff --git a/kolibri/core/assets/src/views/CorePage/AppBarPage.vue b/kolibri/core/assets/src/views/CorePage/AppBarPage.vue
@@ -54,6 +54,7 @@
 
   import { mapGetters } from 'vuex';
   import { get } from '@vueuse/core';
+  import _get from 'lodash/get';
   import LanguageSwitcherModal from 'kolibri.coreVue.components.LanguageSwitcherModal';
   import ScrollingHeader from 'kolibri.coreVue.components.ScrollingHeader';
   import useKResponsiveWindow from 'kolibri.coreVue.composables.useKResponsiveWindow';
@@ -100,7 +101,9 @@
       },
       loading: {
         type: Boolean,
-        default: null,
+        default() {
+          return _get(this, '$store.state.core.loading', false);
+        },
       },
     },
     data() {

diff --git a/kolibri/core/assets/src/views/CoreTable.vue b/kolibri/core/assets/src/views/CoreTable.vue
@@ -1,9 +1,14 @@
 <script>
 
   import get from 'lodash/get';
+  import logger from 'kolibri.lib.logging';
+  import KCircularLoader from 'kolibri-design-system/lib/loaders/KCircularLoader';
+
+  const logging = logger.getLogger(__filename);
 
   export default {
     name: 'CoreTable',
+    components: { KCircularLoader },
     props: {
       selectable: {
         type: Boolean,
@@ -14,6 +19,11 @@
         type: String,
         default: null,
       },
+      dataLoading: {
+        type: Boolean,
+        default: false,
+        required: false,
+      },
     },
     computed: {
       tHeadStyle() {
@@ -73,17 +83,27 @@
         }
       });
 
-      // Insert an empty message as a <p> at the end if it is provided and the
-      // table has no rows.
-      const showEmptyMessage = this.emptyMessage && !tableHasRows;
+      // If we have loaded the data, but have no empty message and no rows, we log an error.
+      if (!this.dataLoading && this.emptyMessage && !tableHasRows) {
+        logging.error('CoreTable: No rows in table, but no empty message provided.');
+      }
+
+      /*
+       * If data is still loading, then we show a loader. Otherwise, we show the
+       * empty message if there are no rows in the table. If we have loaded data, have
+       * an emptyMessage and have no rows. If we have rows, then we show the table alone
+       */
+      var dataStatusEl = this.dataLoading
+        ? createElement('p', [createElement(KCircularLoader)])
+        : !tableHasRows && createElement('p', this.emptyMessage); // Only show message if no rows
 
       return createElement('div', { class: 'core-table-container' }, [
         createElement('table', { class: 'core-table' }, [
           ...(this.$slots.default || []),
           theadEl,
           tbodyCopy,
         ]),
-        showEmptyMessage && createElement('p', this.emptyMessage),
+        dataStatusEl,
       ]);
     },
   };

diff --git a/kolibri/core/assets/src/views/ExamReport/index.vue b/kolibri/core/assets/src/views/ExamReport/index.vue
@@ -437,13 +437,14 @@
         MasteryLogResource.fetchMostRecentDiff(this.getParams())
           .then(currentTry => {
             this.currentTry = currentTry;
+            this.loading = false;
           })
           .catch(err => {
             if (err.response && err.response.status_code === 404) {
               this.$emit('noCompleteTries');
             }
-          })
-          .finally(() => (this.loading = false));
+            this.loading = false;
+          });
       },
       loadAllTries() {
         MasteryLogResource.fetchCollection({ getParams: this.getParams(), force: true }).then(

diff --git a/kolibri/core/assets/src/views/UserTable/__tests__/index.spec.js b/kolibri/core/assets/src/views/UserTable/__tests__/index.spec.js
@@ -1,9 +1,13 @@
-import { shallowMount, mount } from '@vue/test-utils';
+import { mount } from '@vue/test-utils';
 import { UserKinds } from 'kolibri.coreVue.vuex.constants';
+import { coreStoreFactory } from 'kolibri.coreVue.vuex.store';
 import UserTable from '../index';
 
 function makeWrapper({ propsData } = {}) {
+  const store = coreStoreFactory({});
+  store.dispatch('notLoading');
   return mount(UserTable, {
+    store,
     propsData,
   });
 }
@@ -36,7 +40,7 @@ const TEST_USERS = [
 
 describe(`UserTable`, () => {
   it(`smoke test`, () => {
-    const wrapper = shallowMount(UserTable);
+    const wrapper = makeWrapper();
     expect(wrapper.exists()).toBeTruthy();
   });
 

diff --git a/kolibri/core/assets/src/views/UserTable/index.vue b/kolibri/core/assets/src/views/UserTable/index.vue
@@ -17,7 +17,7 @@
       data-test="selectAllCheckbox"
       @change="selectAll($event)"
     />
-    <CoreTable>
+    <CoreTable :emptyMessage="emptyMessage" :dataLoading="dataLoading">
       <template #headers>
         <th data-test="fullNameHeader" :style="{ minWidth: '32px' }">
           <span v-if="selectable" class="visuallyhidden">
@@ -188,13 +188,6 @@
       </template>
     </CoreTable>
 
-    <p
-      v-if="!users || !users.length"
-      class="empty-message"
-    >
-      {{ emptyMessage }}
-    </p>
-
   </div>
 
 </template>
@@ -270,6 +263,10 @@
         type: String,
         default: '',
       },
+      dataLoading: {
+        type: Boolean,
+        default: false,
+      },
     },
     computed: {
       showSelectAllCheckbox() {

diff --git a/kolibri/plugins/coach/assets/src/modules/pluginModule.js b/kolibri/plugins/coach/assets/src/modules/pluginModule.js
@@ -21,7 +21,7 @@ const logging = logger.getLogger(__filename);
 export default {
   state() {
     return {
-      busy: false,
+      dataLoading: false,
       classList: [],
       pageName: '',
       toolbarRoute: {},
@@ -38,6 +38,9 @@ export default {
     SET_TOOLBAR_TITLE(state, title) {
       state.toolbarTitle = title;
     },
+    SET_DATA_LOADING(state, dataLoading) {
+      state.dataLoading = dataLoading;
+    },
     SET_TOOLBAR_ROUTE(state, toolbarRoute) {
       state.toolbarRoute = toolbarRoute;
     },
@@ -60,13 +63,19 @@ export default {
   },
   actions: {
     setClassList(store, facilityId) {
+      store.commit('SET_DATA_LOADING', true);
+      store.commit('SET_CLASS_LIST', []); // Reset the list if we're loading a new one
       return ClassroomResource.fetchCollection({
         getParams: { parent: facilityId || store.getters.currentFacilityId, role: 'coach' },
       })
         .then(classrooms => {
           store.commit('SET_CLASS_LIST', classrooms);
+          store.commit('SET_DATA_LOADING', false);
         })
-        .catch(error => store.dispatch('handleApiError', error));
+        .catch(error => {
+          store.dispatch('handleApiError', error);
+          store.commit('SET_DATA_LOADING', false);
+        });
     },
     /**
       * Handle coach page errors.

diff --git a/kolibri/plugins/coach/assets/src/views/CoachClassListPage.vue b/kolibri/plugins/coach/assets/src/views/CoachClassListPage.vue
@@ -17,18 +17,15 @@
       <h1>{{ coreString('classesLabel') }}</h1>
       <p>{{ $tr('classPageSubheader') }}</p>
 
-      <p v-if="classList.length === 0">
+      <p v-if="classList.length === 0 && !dataLoading">
         <KExternalLink
           v-if="isAdmin && createClassUrl"
           :text="$tr('noClassesDetailsForAdmin')"
           :href="createClassUrl"
         />
-        <span v-else>
-          {{ emptyStateDetails }}
-        </span>
       </p>
 
-      <CoreTable v-else>
+      <CoreTable v-else :dataLoading="dataLoading" :emptyMessage="emptyStateDetails">
         <template #headers>
           <th>{{ coreString('classNameLabel') }}</th>
           <th>{{ coreString('coachesLabel') }}</th>
@@ -83,7 +80,7 @@
     mixins: [commonCoach, commonCoreStrings],
     computed: {
       ...mapGetters(['isAdmin', 'isClassCoach', 'isFacilityCoach', 'userIsMultiFacilityAdmin']),
-      ...mapState(['classList']),
+      ...mapState(['classList', 'dataLoading']),
       // Message that shows up when state.classList is empty
       emptyStateDetails() {
         if (this.isClassCoach) {

diff --git a/kolibri/plugins/device/assets/src/modules/managePermissions/handlers.js b/kolibri/plugins/device/assets/src/modules/managePermissions/handlers.js
@@ -19,17 +19,20 @@ function fetchFacilityUsers() {
 
 export function showManagePermissionsPage(store) {
   const shouldResolve = samePageCheckGenerator(store);
+  store.commit('managePermissions/SET_LOADING_FACILITY_USERS', true);
   const promises = Promise.all([fetchFacilityUsers(store), fetchDevicePermissions()]);
   return promises
-    .then(function onSuccess([users, permissions]) {
+    .then(([users, permissions]) => {
       if (shouldResolve()) {
         store.commit('managePermissions/SET_STATE', {
           facilityUsers: users,
           permissions,
         });
       }
+      return store.commit('managePermissions/SET_LOADING_FACILITY_USERS', false);
     })
-    .catch(function onFailure(error) {
-      shouldResolve() ? store.dispatch('handleError', error) : null;
+    .catch(error => {
+      store.commit('managePermissions/SET_LOADING_FACILITY_USERS', false);
+      return shouldResolve() ? store.dispatch('handleError', error) : null;
     });
 }
diff --git a/kolibri/plugins/device/assets/src/modules/managePermissions/index.js b/kolibri/plugins/device/assets/src/modules/managePermissions/index.js
@@ -1,5 +1,6 @@
 function defaultState() {
   return {
+    loadingFacilityUsers: false,
     facilityUsers: [],
     permissions: {},
   };
@@ -15,5 +16,8 @@ export default {
     RESET_STATE(state) {
       Object.assign(state, defaultState());
     },
+    SET_LOADING_FACILITY_USERS(state, loadingFacilityUsers) {
+      state.loadingFacilityUsers = loadingFacilityUsers;
+    },
   },
 };
diff --git a/kolibri/plugins/device/assets/src/views/ManageContentPage/index.vue b/kolibri/plugins/device/assets/src/views/ManageContentPage/index.vue
@@ -45,20 +45,23 @@
           @clearall="handleClickClearAll"
         />
 
-        <p v-if="!channelsAreInstalled">
+        <p v-if="!channelsAreInstalled && !channelListLoading">
           {{ $tr('emptyChannelListMessage') }}
         </p>
 
         <div class="channels-list">
-          <ChannelPanel
-            v-for="channel in sortedChannels"
-            :key="channel.id"
-            :channel="channel"
-            :disabled="channelIsBeingDeleted(channel.id)"
-            :showNewLabel="showNewLabel(channel.id)"
-            @select_delete="deleteChannelId = channel.id"
-            @select_manage="handleSelectManage(channel.id)"
-          />
+          <KCircularLoader v-if="channelListLoading" />
+          <div v-else>
+            <ChannelPanel
+              v-for="channel in sortedChannels"
+              :key="channel.id"
+              :channel="channel"
+              :disabled="channelIsBeingDeleted(channel.id)"
+              :showNewLabel="showNewLabel(channel.id)"
+              @select_delete="deleteChannelId = channel.id"
+              @select_manage="handleSelectManage(channel.id)"
+            />
+          </div>
         </div>
 
         <SelectTransferSourceModal :pageName="pageName" />
@@ -134,6 +137,7 @@
         'managedTasks',
       ]),
       ...mapState('manageContent/wizard', ['pageName']),
+      ...mapState('manageContent', ['channelListLoading']),
       pageTitle() {
         return deviceString('deviceManagementTitle');
       },

diff --git a/kolibri/plugins/device/assets/src/views/ManagePermissionsPage/UserGrid.vue b/kolibri/plugins/device/assets/src/views/ManagePermissionsPage/UserGrid.vue
@@ -2,7 +2,7 @@
 
   <div>
 
-    <CoreTable :emptyMessage="emptyMessage">
+    <CoreTable :emptyMessage="emptyMessage" :dataLoading="dataLoading">
       <template #headers>
         <th>{{ coreString('fullNameLabel') }}</th>
         <th>{{ coreString('usernameLabel') }}</th>
@@ -82,6 +82,11 @@
         type: Function,
         default: () => null,
       },
+      dataLoading: {
+        type: Boolean,
+        default: false,
+        required: false,
+      },
     },
     computed: {
       ...mapGetters(['facilities', 'currentUserId']),

diff --git a/kolibri/plugins/device/assets/src/views/ManagePermissionsPage/index.vue b/kolibri/plugins/device/assets/src/views/ManagePermissionsPage/index.vue
@@ -15,6 +15,7 @@
       </div>
 
       <PaginatedListContainer
+        :dataLoading="loadingFacilityUsers"
         :items="usersFilteredByDropdown"
         :filterPlaceholder="$tr('searchPlaceholder')"
       >
@@ -44,6 +45,7 @@
         </template>
         <template #default="{ items, filterInput }">
           <UserGrid
+            :dataLoading="loadingFacilityUsers"
             :searchFilter="searchFilterText"
             :facilityUsers="items"
             :userPermissions="userPermissions"
@@ -98,6 +100,7 @@
       ...mapState('managePermissions', {
         facilityUsers: state => state.facilityUsers,
         userPermissions: state => userid => state.permissions[userid],
+        loadingFacilityUsers: state => state.loadingFacilityUsers,
       }),
       ...mapState({
         query: state => state.query,

diff --git a/kolibri/plugins/facility/assets/src/modules/classEditManagement/handlers.js b/kolibri/plugins/facility/assets/src/modules/classEditManagement/handlers.js
@@ -1,4 +1,3 @@
-import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator';
 import { ClassroomResource, FacilityUserResource } from 'kolibri.resources';
 import { localeCompare } from 'kolibri.utils.i18n';
 import { _userState } from '../mappers';
@@ -17,22 +16,17 @@ export function showClassEditPage(store, classId) {
     ClassroomResource.fetchModel({ id: classId, force: true }),
     ClassroomResource.fetchCollection({ getParams: { parent: facilityId }, force: true }),
   ];
-  const shouldResolve = samePageCheckGenerator(store);
-  Promise.all(promises).then(
-    ([facilityUsers, classroom, classrooms]) => {
-      if (shouldResolve()) {
-        store.commit('classEditManagement/SET_STATE', {
-          modalShown: false,
-          currentClass: classroom,
-          classes: classrooms,
-          classLearners: sortUsersByFullName(facilityUsers).map(_userState),
-          classCoaches: sortUsersByFullName(classroom.coaches).map(_userState),
-        });
-        store.commit('CORE_SET_PAGE_LOADING', false);
-      }
-    },
-    error => {
-      shouldResolve() ? store.dispatch('handleError', error) : null;
-    }
-  );
+  store.commit('classEditManagement/SET_DATA_LOADING', true);
+  Promise.all(promises)
+    .then(([facilityUsers, classroom, classrooms]) => {
+      store.commit('classEditManagement/SET_DATA_LOADING', false);
+      store.commit('classEditManagement/SET_STATE', {
+        modalShown: false,
+        currentClass: classroom,
+        classes: classrooms,
+        classLearners: sortUsersByFullName(facilityUsers).map(_userState),
+        classCoaches: sortUsersByFullName(classroom.coaches).map(_userState),
+      });
+    })
+    .catch(error => store.dispatch('handleError', error));
 }