From e5ea2187a17ca0c13ec613cfb5b6db00d7f4ccf7 Mon Sep 17 00:00:00 2001
From: Richard Tibbles <richard@learningequality.org>
Date: Tue, 10 May 2022 13:47:18 -0700
Subject: [PATCH 1/3] Sort facility user viewset to prevent errors in
 pagination.

---
 kolibri/core/auth/api.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kolibri/core/auth/api.py b/kolibri/core/auth/api.py
index 1b14162e923..3d49cfc8fbc 100644
--- a/kolibri/core/auth/api.py
+++ b/kolibri/core/auth/api.py
@@ -298,6 +298,8 @@ class FacilityUserViewSet(ValuesViewset):
         DjangoFilterBackend,
         filters.SearchFilter,
     )
+    order_by_field = "username"
+
     queryset = FacilityUser.objects.all()
     serializer_class = FacilityUserSerializer
     filter_class = FacilityUserFilter
@@ -338,6 +340,7 @@ def consolidate(self, items, queryset):
                     roles.append(role)
             item["roles"] = roles
             output.append(item)
+        output = sorted(output, key=lambda x: x[self.order_by_field])
         return output
 
     def set_password_if_needed(self, instance, serializer):

From 60f98f984533c0a230349d0b57475c97fda3ddd6 Mon Sep 17 00:00:00 2001
From: Richard Tibbles <richard@learningequality.org>
Date: Tue, 10 May 2022 13:47:52 -0700
Subject: [PATCH 2/3] Clean up pagination implementation.

---
 .../src/modules/userManagement/handlers.js    |   9 +-
 .../PaginatedListContainerWithBackend.vue     | 157 +++---------------
 .../assets/src/views/UserPage/index.vue       |  95 ++++++++++-
 3 files changed, 118 insertions(+), 143 deletions(-)

diff --git a/kolibri/plugins/facility/assets/src/modules/userManagement/handlers.js b/kolibri/plugins/facility/assets/src/modules/userManagement/handlers.js
index 9aed40c4940..393de407cf8 100644
--- a/kolibri/plugins/facility/assets/src/modules/userManagement/handlers.js
+++ b/kolibri/plugins/facility/assets/src/modules/userManagement/handlers.js
@@ -1,3 +1,4 @@
+import pickBy from 'lodash/pickBy';
 import { FacilityUserResource } from 'kolibri.resources';
 import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator';
 import { _userState } from '../mappers';
@@ -6,7 +7,13 @@ export function showUserPage(store, toRoute) {
   store.dispatch('preparePage');
   const facilityId = toRoute.params.facility_id || store.getters.activeFacilityId;
   return FacilityUserResource.fetchCollection({
-    getParams: { member_of: facilityId, page_size: 30 },
+    getParams: pickBy({
+      member_of: facilityId,
+      page: toRoute.query.page || 1,
+      page_size: toRoute.query.page_size || 30,
+      search: toRoute.query.search && toRoute.query.search.trim(),
+      user_type: toRoute.query.user_type,
+    }),
     force: true,
   }).only(
     samePageCheckGenerator(store),
diff --git a/kolibri/plugins/facility/assets/src/views/PaginatedListContainerWithBackend.vue b/kolibri/plugins/facility/assets/src/views/PaginatedListContainerWithBackend.vue
index 610fe36750c..ea2ea1e08f7 100644
--- a/kolibri/plugins/facility/assets/src/views/PaginatedListContainerWithBackend.vue
+++ b/kolibri/plugins/facility/assets/src/views/PaginatedListContainerWithBackend.vue
@@ -9,32 +9,29 @@
         :layout12="{ span: 5, alignment: 'right' }"
         class="text-filter"
       >
-        <FilterTextbox v-model="filterInput" :placeholder="filterPlaceholder" />
+        <slot name="filter"></slot>
       </KGridItem>
     </KGrid>
 
     <div>
-      <slot
-        :items="userList"
-        :filterInput="filterInput"
-      >
+      <slot>
       </slot>
     </div>
 
     <nav class="pagination-nav">
       <span dir="auto" class="pagination-label">
-        {{ $tr('pagination', { visibleStartRange, visibleEndRange, numFilteredItems }) }}
+        {{ $translate('pagination', { visibleStartRange, visibleEndRange, numFilteredItems }) }}
       </span>
       <KButtonGroup>
         <KIconButton
-          :ariaLabel="$tr('previousResults')"
+          :ariaLabel="$translate('previousResults')"
           :disabled="previousButtonDisabled"
           size="small"
           icon="back"
           @click="changePage(-1)"
         />
         <KIconButton
-          :ariaLabel="$tr('nextResults')"
+          :ariaLabel="$translate('nextResults')"
           :disabled="nextButtonDisabled"
           size="small"
           icon="forward"
@@ -50,172 +47,64 @@
 <script>
 
   import clamp from 'lodash/clamp';
-  import FilterTextbox from 'kolibri.coreVue.components.FilterTextbox';
-  import { FacilityUserResource } from 'kolibri.resources';
-  import store from 'kolibri.coreVue.vuex.store';
-  import { _userState } from '../modules/mappers';
+  import PaginatedListContainer from 'kolibri.coreVue.components.PaginatedListContainer';
+  import { crossComponentTranslator } from 'kolibri.utils.i18n';
 
   export default {
     name: 'PaginatedListContainerWithBackend',
-    components: {
-      FilterTextbox,
-    },
     props: {
-      // The entire list of items
-      items: {
-        type: Array,
-        required: true,
-      },
-      filterPlaceholder: {
-        type: String,
-        required: true,
-      },
       itemsPerPage: {
         type: Number,
-        required: false,
-        default: 30,
+        required: true,
       },
       totalPageNumber: {
         type: Number,
-        required: false,
-        default: 1,
+        required: true,
       },
-      totalUsers: {
+      value: {
         type: Number,
         required: true,
       },
-      roleFilter: {
-        type: Object,
-        required: false,
-        default: null,
-      },
-      excludeMemberOf: {
-        type: String,
-        required: false,
-        default: '',
-      },
-      userAssignmentType: {
-        type: String,
-        required: false,
-        default: '',
+      numFilteredItems: {
+        type: Number,
+        required: true,
       },
     },
-    data() {
-      return {
-        filterInput: '',
-        currentPage: 1,
-        userList: this.items,
-        totalPageNumbers: this.totalPageNumber,
-        usersCount: this.totalUsers,
-      };
-    },
     computed: {
-      numFilteredItems() {
-        return this.usersCount;
-      },
       startRange() {
-        return (this.currentPage - 1) * this.itemsPerPage;
+        return (this.value - 1) * this.itemsPerPage;
       },
       visibleStartRange() {
-        // return this.currentPage;
         return Math.min(this.startRange + 1, this.numFilteredItems);
       },
       endRange() {
-        return this.currentPage * this.itemsPerPage;
+        return this.value * this.itemsPerPage;
       },
       visibleEndRange() {
         return Math.min(this.endRange, this.numFilteredItems);
       },
       previousButtonDisabled() {
-        return this.currentPage === 1 || this.numFilteredItems === 0;
+        return this.value === 1 || this.numFilteredItems === 0;
       },
       nextButtonDisabled() {
         return (
-          this.totalPageNumbers === 1 ||
-          this.currentPage === this.totalPageNumbers ||
+          this.totalPageNumber === 1 ||
+          this.value === this.totalPageNumber ||
           this.numFilteredItems === 0
         );
       },
     },
-    watch: {
-      filterInput: {
-        handler() {
-          this.currentPage = 1;
-          this.get_users();
-        },
-      },
-      roleFilter: {
-        handler() {
-          this.currentPage = 1;
-          this.get_users();
-        },
-      },
-      items: {
-        handler() {
-          this.get_users();
-        },
-      },
+    beforeCreate() {
+      this.$translator = crossComponentTranslator(PaginatedListContainer);
     },
     methods: {
-      get_users() {
-        const facilityId = store.getters.activeFacilityId;
-        FacilityUserResource.fetchCollection({
-          getParams: {
-            member_of: facilityId,
-            page_size: this.itemsPerPage,
-            page: this.currentPage,
-            search: this.filterInput,
-            exclude_member_of: !this.excludeMemberOf ? '' : this.excludeMemberOf,
-            user_type:
-              !this.roleFilter || this.roleFilter.value === 'all' ? '' : this.roleFilter.value,
-            exclude_user_type: this.userAssignmentType === 'coaches' ? 'learner' : '',
-          },
-          force: true,
-        }).then(
-          users => {
-            this.currentPage = users.page;
-            this.userList = users.results.map(_userState);
-            this.totalPageNumbers = users.total_pages;
-            this.usersCount = users.count;
-          },
-          error => {
-            // check if this error is raised by the api because of currentPage is more than
-            // the total pages
-            if (
-              error.response.status === 404 &&
-              error.response.data &&
-              error.response.data[0].id === 'NOT_FOUND'
-            ) {
-              // set the currentPage to 1 and recall the api
-              this.currentPage = 1;
-              this.get_users();
-            } else {
-              store.dispatch('handleApiError', error);
-            }
-          }
-        );
-      },
       changePage(change) {
         // Clamp the newPage number between the bounds if browser doesn't correctly
         // disable buttons (see #6454 issue with old versions of MS Edge)
-        this.currentPage = clamp(this.currentPage + change, 1, this.totalPageNumbers);
-        this.get_users();
-      },
-    },
-    $trs: {
-      previousResults: {
-        message: 'Previous results',
-        context:
-          'Text which indicates the previous page of results when a user makes a search query.\n',
-      },
-      nextResults: {
-        message: 'Next results',
-        context: 'Text which indicates the next page of results when a user makes a search query.',
+        this.$emit('input', clamp(this.value + change, 1, this.totalPageNumber));
       },
-      pagination: {
-        message:
-          '{ visibleStartRange, number } - { visibleEndRange, number } of { numFilteredItems, number }',
-        context: "Refers to pagination. Only translate the word \"of''.",
+      $translate(msg, params) {
+        return this.$translator.$tr(msg, params);
       },
     },
   };
diff --git a/kolibri/plugins/facility/assets/src/views/UserPage/index.vue b/kolibri/plugins/facility/assets/src/views/UserPage/index.vue
index 6b75948497c..7ba48afd654 100644
--- a/kolibri/plugins/facility/assets/src/views/UserPage/index.vue
+++ b/kolibri/plugins/facility/assets/src/views/UserPage/index.vue
@@ -24,11 +24,12 @@
     </KGrid>
 
     <PaginatedListContainerWithBackend
+      v-model="currentPage"
       :items="facilityUsers"
-      :filterPlaceholder="$tr('searchText')"
+      :itemsPerPage="itemsPerPage"
       :totalPageNumber="totalPages"
       :roleFilter="roleFilter"
-      :totalUsers="usersCount"
+      :numFilteredItems="usersCount"
     >
       <template #otherFilter>
         <KSelect
@@ -40,11 +41,15 @@
         />
       </template>
 
-      <template #default="{ items, filterInput }">
+      <template #filter>
+        <FilterTextbox v-model="search" :placeholder="$tr('searchText')" />
+      </template>
+
+      <template>
         <UserTable
           class="move-down user-roster"
-          :users="items"
-          :emptyMessage="emptyMessageForItems(items, filterInput)"
+          :users="facilityUsers"
+          :emptyMessage="emptyMessageForItems(facilityUsers, search)"
           :showDemographicInfo="true"
         >
           <template #action="userRow">
@@ -83,8 +88,11 @@
 <script>
 
   import { mapState, mapGetters } from 'vuex';
+  import debounce from 'lodash/debounce';
+  import pickBy from 'lodash/pickBy';
   import { UserKinds } from 'kolibri.coreVue.vuex.constants';
   import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';
+  import FilterTextbox from 'kolibri.coreVue.components.FilterTextbox';
   import cloneDeep from 'lodash/cloneDeep';
   import PaginatedListContainerWithBackend from '../PaginatedListContainerWithBackend';
   import UserTable from '../UserTable';
@@ -102,6 +110,7 @@
       };
     },
     components: {
+      FilterTextbox,
       ResetUserPasswordModal,
       DeleteUserModal,
       UserTable,
@@ -110,7 +119,6 @@
     mixins: [commonCoreStrings],
     data() {
       return {
-        roleFilter: null,
         selectedUser: null,
         modalShown: null,
       };
@@ -128,9 +136,67 @@
           { label: this.$tr('superAdmins'), value: UserKinds.SUPERUSER },
         ];
       },
+      roleFilter: {
+        get() {
+          return (
+            this.userKinds.find(k => k.value === this.$route.query.user_type) || this.userKinds[0]
+          );
+        },
+        set(value) {
+          value = value.value;
+          if (value === ALL_FILTER) {
+            value = null;
+          }
+          this.$router.push({
+            ...this.$route,
+            query: pickBy({
+              ...this.$route.query,
+              user_type: value,
+              page: null,
+            }),
+          });
+        },
+      },
+      search: {
+        get() {
+          return this.$route.query.search || '';
+        },
+        set(value) {
+          this.debouncedSearchTerm(value);
+        },
+      },
+      currentPage: {
+        get() {
+          return Number(this.$route.query.page || 1);
+        },
+        set(value) {
+          this.$router.push({
+            ...this.$route,
+            query: pickBy({
+              ...this.$route.query,
+              page: value,
+            }),
+          });
+        },
+      },
+      itemsPerPage: {
+        get() {
+          return this.$route.query.page_size || 30;
+        },
+        set(value) {
+          this.$router.push({
+            ...this.$route,
+            query: pickBy({
+              ...this.$route.query,
+              page_size: value,
+              page: null,
+            }),
+          });
+        },
+      },
     },
-    beforeMount() {
-      this.roleFilter = this.userKinds[0];
+    created() {
+      this.debouncedSearchTerm = debounce(this.emitSearchTerm, 500);
     },
     methods: {
       emptyMessageForItems(items, filterText) {
@@ -183,6 +249,19 @@
         // Otherwise, only non-SUs can be edited.
         return this.isSuperuser || !user.is_superuser;
       },
+      emitSearchTerm(value) {
+        if (value === '') {
+          value = null;
+        }
+        this.$router.push({
+          ...this.$route,
+          query: pickBy({
+            ...this.$route.query,
+            search: value,
+            page: null,
+          }),
+        });
+      },
     },
     $trs: {
       searchText: {

From 73f9f14ae866cea9352a9c5866188ae85f4bd8f9 Mon Sep 17 00:00:00 2001
From: Richard Tibbles <richard@learningequality.org>
Date: Thu, 12 May 2022 11:27:40 -0700
Subject: [PATCH 3/3] Add router to fix behaviour in tests.

---
 .../src/views/UserPage/__test__/UserPage.spec.js      | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kolibri/plugins/facility/assets/src/views/UserPage/__test__/UserPage.spec.js b/kolibri/plugins/facility/assets/src/views/UserPage/__test__/UserPage.spec.js
index 5ad717eb215..63517a0d730 100644
--- a/kolibri/plugins/facility/assets/src/views/UserPage/__test__/UserPage.spec.js
+++ b/kolibri/plugins/facility/assets/src/views/UserPage/__test__/UserPage.spec.js
@@ -1,5 +1,6 @@
 import mock from 'xhr-mock';
 import { mount, RouterLinkStub } from '@vue/test-utils';
+import VueRouter from 'vue-router';
 import makeStore from '../../../../test/makeStore';
 import UserPage from '../index';
 
@@ -7,12 +8,22 @@ jest.mock('kolibri.lib.logging');
 jest.mock('kolibri.urls');
 jest.mock('lockr');
 
+const router = new VueRouter({
+  routes: [
+    {
+      path: '/userpage/',
+      name: 'UserPage',
+    },
+  ],
+});
+
 UserPage.computed.newUserLink = () => ({});
 
 function makeWrapper() {
   const store = makeStore();
   const wrapper = mount(UserPage, {
     store,
+    router,
     stubs: {
       RouterLink: RouterLinkStub,
     },