Fix setup wizard selection of network devices for sign up or import

kolibri/core/assets/src/views/sync/SelectDeviceModalGroup/SelectDeviceForm.vue

@@ -38,10 +38,10 @@
             :key="idx"
             v-model="selectedDeviceId"
             class="radio-button"
-            :value="canLearnerSignUp(d.id) ? d.id : false"
+            :value="d.id"
             :label="d.nickname"
             :description="d.base_url"
-            :disabled="!canLearnerSignUp(d.id) || formDisabled || !isDeviceAvailable(d.id)"
+            :disabled="formDisabled || !isDeviceAvailable(d.id)"
           />
           <KButton
             :key="`forget-${idx}`"
@@ -65,13 +65,10 @@
             :key="d.id"
             v-model="selectedDeviceId"
             class="radio-button"
-            :value="canLearnerSignUp(d.id) ? d.instance_id : false"
+            :value="d.instance_id"
             :label="formatNameAndId(d.device_name, d.id)"
             :description="formatBaseDevice(d)"
-            :disabled="!canLearnerSignUp(d.id)
-              || formDisabled
-              || fetchFailed
-              || !isDeviceAvailable(d.id)"
+            :disabled="formDisabled || fetchFailed || !isDeviceAvailable(d.id)"
           />
         </div>
       </template>
@@ -136,21 +133,21 @@
 
 <script>
 
-  import { computed, ref } from 'kolibri.lib.vueCompositionApi';
-  import { useLocalStorage, get, computedAsync } from '@vueuse/core';
+  import { computed } from 'kolibri.lib.vueCompositionApi';
+  import { useLocalStorage, get } from '@vueuse/core';
   import find from 'lodash/find';
   import UiAlert from 'kolibri-design-system/lib/keen/UiAlert';
   import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';
   import commonSyncElements from 'kolibri.coreVue.mixins.commonSyncElements';
   import { UnreachableConnectionStatuses } from './constants';
   import useDeviceDeletion from './useDeviceDeletion.js';
-  import useDevices, {
-    useDevicesWithChannel,
-    useDevicesWithFacility,
-    useDevicesForLearnOnlyDevice,
+  import {
+    useDevicesWithFilter,
+    useDeviceChannelFilter,
+    useDeviceFacilityFilter,
+    useDeviceMinimumVersionFilter,
   } from './useDevices.js';
   import useConnectionChecker from './useConnectionChecker.js';
-  import { deviceFacilityCanSignUp } from './api.js';
 
   export default {
     name: 'SelectDeviceForm',
@@ -159,25 +156,26 @@
     },
     mixins: [commonCoreStrings, commonSyncElements],
     setup(props, context) {
-      // We don't have a use case for combining these at the moment
-      if (
-        (props.filterByChannelId !== null && props.filterByFacilityId !== null) ||
-        ((props.filterByChannelId !== null || props.filterByFacilityId !== null) &&
-          props.filterLODAvailable)
-      ) {
-        throw new Error('Filtering for LOD and having channel or facility is not implemented');
-      }
+      const apiParams = {};
+      const deviceFilters = [];
 
-      let useDevicesResult = null;
       if (props.filterByChannelId !== null) {
-        useDevicesResult = useDevicesWithChannel(props.filterByChannelId);
-      } else if (props.filterByFacilityId !== null) {
-        // This is inherently filtered to full-facility devices
-        useDevicesResult = useDevicesWithFacility({ facilityId: props.filterByFacilityId });
-      } else if (props.filterLODAvailable) {
-        useDevicesResult = useDevicesForLearnOnlyDevice();
-      } else {
-        useDevicesResult = useDevices({ subset_of_users_device: false });
+        deviceFilters.push(useDeviceChannelFilter({ id: props.filterByChannelId }));
+      }
+
+      if (props.filterByFacilityId !== null || props.filterByFacilityCanSignUp !== null) {
+        apiParams.subset_of_users_device = false;
+        deviceFilters.push(
+          useDeviceFacilityFilter({
+            id: props.filterByFacilityId,
+            learner_can_sign_up: props.filterByFacilityCanSignUp,
+          })
+        );
+      }
+
+      if (props.filterLODAvailable) {
+        apiParams.subset_of_users_device = false;
+        deviceFilters.push(useDeviceMinimumVersionFilter(0, 15, 0));
       }
 
       const {
@@ -186,7 +184,7 @@
         hasFetched,
         fetchFailed,
         forceFetch,
-      } = useDevicesResult;
+      } = useDevicesWithFilter(apiParams, deviceFilters);
 
       const { devices, isDeleting, hasDeleted, deletingFailed, doDelete } = useDeviceDeletion(
         _devices,
@@ -200,24 +198,6 @@
       const discoveredDevices = computed(() => get(devices).filter(d => d.dynamic));
       const savedDevices = computed(() => get(devices).filter(d => !d.dynamic));
 
-      const isLoading = ref(false);
-
-      const lodsWithSignupFacility = computedAsync(
-        async () => {
-          const devicesAvailable = get(devices);
-          const allDevices = {};
-          for (const i of devicesAvailable) {
-            const canSignUp = await deviceFacilityCanSignUp(i.id);
-            if (canSignUp) {
-              allDevices[i.id] = true;
-            }
-          }
-          return allDevices;
-        },
-        {},
-        isLoading
-      );
-
       return {
         // useDevices
         devices,
@@ -237,7 +217,6 @@
         discoveredDevices,
         savedDevices,
         storageDeviceId,
-        lodsWithSignupFacility,
       };
     },
     props: {
@@ -258,6 +237,12 @@
         type: Boolean,
         default: false,
       },
+      // When looking for devices for which a learner can sign up
+      // eslint-disable-next-line kolibri/vue-no-unused-properties
+      filterByFacilityCanSignUp: {
+        type: Boolean,
+        default: null,
+      },
       // If an ID is provided, that device's radio button will be automatically selected
       selectedId: {
         type: String,
@@ -298,7 +283,6 @@
             this.isDeleting ||
             this.fetchFailed ||
             this.isSubmitChecking ||
-            !this.canLearnerSignUp(this.selectedDeviceId) ||
             !this.isDeviceAvailable(this.selectedDeviceId) ||
             this.availableDeviceIds.length === 0
         );
@@ -391,9 +375,6 @@
           this.$emit('removed_address');
         });
       },
-      canLearnerSignUp(id) {
-        return this.lodsWithSignupFacility && id in this.lodsWithSignupFacility;
-      },
     },
     $trs: {
       deletingFailedText: {

kolibri/core/assets/src/views/sync/SelectDeviceModalGroup/__test__/SelectDeviceForm.spec.js

@@ -1,52 +1,61 @@
 import { shallowMount } from '@vue/test-utils';
 import SelectDeviceForm from '../SelectDeviceForm';
 import { fetchDevices, updateConnectionStatus } from '../api';
+import { ConnectionStatus } from '../constants';
 
 jest.mock('../api.js', () => ({
   fetchDevices: jest.fn(),
   deleteDevice: jest.fn().mockResolvedValue(),
   updateConnectionStatus: jest.fn(),
-  deviceFacilityCanSignUp: jest.fn().mockResolvedValue(true),
 }));
 
 const devices = [
   {
     id: '1',
     instance_id: '1',
     nickname: 'Available Server',
+    device_name: 'Available Server',
     base_url: 'http://localhost:8000',
     application: 'kolibri',
     available: true,
+    is_local: true,
+    since_last_accessed: 0,
+    subset_of_users_device: false,
+    kolibri_version: '0.16.0',
   },
   {
     id: '2',
     instance_id: '2',
     nickname: 'Unavailable Server',
+    device_name: 'Unavailable Server',
     base_url: 'http://localhost:8001',
     application: 'kolibri',
     available: false,
+    is_local: true,
+    since_last_accessed: 0,
+    subset_of_users_device: false,
+    kolibri_version: '0.16.0',
   },
   {
     id: '3',
     instance_id: '3',
     nickname: 'Content-less Server',
+    device_name: 'Content-less Server',
     base_url: 'http://localhost:8001',
     application: 'kolibri',
     available: true,
+    is_local: true,
+    since_last_accessed: 0,
+    subset_of_users_device: false,
+    kolibri_version: '0.16.0',
   },
 ];
 
 const staticDevices = devices.map(a => ({ ...a, dynamic: false }));
 const dynamicDevices = devices.map(a => ({ ...a, dynamic: true }));
 
 function makeWrapper() {
-  const deviceIdMap = devices.reduce((acc, device) => {
-    acc[device.id] = device;
-    return acc;
-  }, {});
-  const wrapper = shallowMount(SelectDeviceForm, {
-    mocks: { lodsWithSignupFacility: deviceIdMap },
-  });
+  const wrapper = shallowMount(SelectDeviceForm);
   // prettier-ignore
   const els = {
     KModal: () => wrapper.findComponent({ name: 'KModal' }),
@@ -59,15 +68,29 @@ function makeWrapper() {
 }
 
 describe('SelectDeviceForm', () => {
+  let devices = [];
   beforeEach(() => {
+    devices = staticDevices.concat(dynamicDevices);
     fetchDevices.mockReset();
-    fetchDevices.mockResolvedValue(staticDevices.concat(dynamicDevices));
+    fetchDevices.mockResolvedValue(devices);
+    updateConnectionStatus.mockImplementation(device => {
+      const updatedDevice = devices.find(d => d.id === device.id);
+      if (!updatedDevice) {
+        return Promise.resolve({
+          ...device,
+          available: false,
+          connection_status: ConnectionStatus.Unknown,
+        });
+      }
+      return Promise.resolve(device);
+    });
   });
 
   it('shows one device for each one fetched', async () => {
     const { els, wrapper } = makeWrapper();
     await wrapper.vm.$nextTick();
     await wrapper.vm.$nextTick();
+    await wrapper.vm.$nextTick();
     expect(wrapper.vm.hasFetched).toBe(true);
     expect(els.radioButtons()).toHaveLength(6);
     const server1 = els.radioButtons().at(0);
@@ -81,6 +104,7 @@ describe('SelectDeviceForm', () => {
     const { els, wrapper } = makeWrapper();
     await wrapper.vm.$nextTick();
     await wrapper.vm.$nextTick();
+    await wrapper.vm.$nextTick();
     expect(els.radioButtons()).toHaveLength(0);
     expect(wrapper.text()).toContain('There are no devices yet');
     expect(els.KModal().props().submitDisabled).toEqual(true);
@@ -94,6 +118,8 @@ describe('SelectDeviceForm', () => {
         .at(n)
         .props().disabled;
     }
+
+    await wrapper.vm.$nextTick();
     await wrapper.vm.$nextTick();
     await wrapper.vm.$nextTick();
     expect(radioButtonNIsDisabled(0)).toEqual(false);
@@ -115,6 +141,7 @@ describe('SelectDeviceForm', () => {
     els.KModal().vm.$emit('submit');
     await wrapper.vm.$nextTick();
     await wrapper.vm.$nextTick();
+    await wrapper.vm.$nextTick();
     expect(wrapper.emitted().submit[0][0].id).toEqual('1');
   });
 

kolibri/core/assets/src/views/sync/SelectDeviceModalGroup/api.js

@@ -1,3 +1,4 @@
+import matches from 'lodash/matches';
 import {
   NetworkLocationResource,
   RemoteChannelResource,
@@ -24,13 +25,20 @@ export function fetchDevices(params = {}) {
 }
 
 /**
- * @param {string} facilityId
+ * @typedef {Object} FacilityFilter
+ * @property {string} [id]
+ * @property {boolean} [learner_can_sign_up]
+ */
+
+/**
  * @param {NetworkLocation} device
+ * @param {FacilityFilter} facility
  * @return {Promise<boolean>}
  */
-export function facilityIsAvailableAtDevice(facilityId, device) {
+export function deviceHasMatchingFacility(device, facility) {
+  // TODO: ideally we could pass along the filters directly to the API
   return NetworkLocationResource.fetchFacilities(device.id).then(({ facilities }) => {
-    return Boolean(facilities.find(({ id }) => id === facilityId));
+    return Boolean(facilities.find(matches(facility)));
   });
 }
 
@@ -58,20 +66,3 @@ export function channelIsAvailableAtDevice(channelId, device) {
 export function updateConnectionStatus(device) {
   return NetworkLocationResource.updateConnectionStatus(device.id);
 }
-
-/**
- * @param {string} deviceId
- * @return {Promise<boolean>}
- */
-export function deviceFacilityCanSignUp(deviceId) {
-  return NetworkLocationResource.fetchFacilities(deviceId).then(({ device_id, facilities }) => {
-    if (deviceId === device_id) {
-      for (const facility of facilities) {
-        if (facility.learner_can_sign_up) {
-          return true;
-        }
-      }
-    }
-    return false;
-  });
-}

kolibri/core/assets/src/views/sync/SelectDeviceModalGroup/index.vue

@@ -11,6 +11,7 @@
       :filterByChannelId="filterByChannelId"
       :filterByFacilityId="filterByFacilityId"
       :filterLODAvailable="filterLODAvailable"
+      :filterByFacilityCanSignUp="filterByFacilityCanSignUp"
       :selectedId="addedAddressId"
       :formDisabled="$attrs.selectAddressDisabled"
       @click_add_address="goToAddAddress"
@@ -49,6 +50,11 @@
         type: Boolean,
         default: false,
       },
+      // When looking for devices for which a learner can sign up
+      filterByFacilityCanSignUp: {
+        type: Boolean,
+        default: null,
+      },
     },
     data() {
       return {