Refactor device filtering for optimal flexibility

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

@@ -141,10 +141,11 @@
   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';
 
@@ -155,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 {
@@ -182,7 +184,7 @@
         hasFetched,
         fetchFailed,
         forceFetch,
-      } = useDevicesResult;
+      } = useDevicesWithFilter(apiParams, deviceFilters);
 
       const { devices, isDeleting, hasDeleted, deletingFailed, doDelete } = useDeviceDeletion(
         _devices,
@@ -235,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,
@@ -275,7 +283,6 @@
             this.isDeleting ||
             this.fetchFailed ||
             this.isSubmitChecking ||
-            !this.canLearnerSignUp(this.selectedDeviceId) ||
             !this.isDeviceAvailable(this.selectedDeviceId) ||
             this.availableDeviceIds.length === 0
         );

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)));
   });
 }
 

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 {

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

@@ -1,8 +1,9 @@
+import isArray from 'lodash/isArray';
 import { ref, reactive, computed, onBeforeUnmount, watch } from 'kolibri.lib.vueCompositionApi';
 import { get, set, useMemoize, useTimeoutPoll } from '@vueuse/core';
 
 import useMinimumKolibriVersion from 'kolibri.coreVue.composables.useMinimumKolibriVersion';
-import { fetchDevices, channelIsAvailableAtDevice, facilityIsAvailableAtDevice } from './api';
+import { fetchDevices, channelIsAvailableAtDevice, deviceHasMatchingFacility } from './api';
 
 /**
  * @param {{}} apiParams
@@ -55,21 +56,25 @@ export default function useDevices(apiParams = {}) {
 /**
  *
  * @param {{}} apiParams
- * @param {function(NetworkLocation): Promise<bool>} filterFunction
+ * @param {
+ *  function(NetworkLocation): Promise<bool>|[function(NetworkLocation): Promise<bool>]
+ * } filterFunctionOrFunctions
  * @return {{
  *  devices: Ref<NetworkLocation[]>, hasFetched: Ref<bool>, isFetching: Ref<bool>,
  *  filterFailed: Ref<bool>, forceFetch: (function(): Promise<void>)
  * }}
  */
-function useDevicesWithFilter(apiParams, filterFunction) {
+export function useDevicesWithFilter(apiParams, filterFunctionOrFunctions) {
   const isFiltering = ref(false);
   const filteringFailed = ref(false);
   const hasFiltered = ref(false);
   const availableIds = ref([]);
   const unavailableIds = ref([]);
   const { devices, isFetching, fetchFailed, hasFetched, forceFetch } = useDevices(apiParams);
 
-  const getIsAvailable = useMemoize(filterFunction, { getKey: device => device.id });
+  const filterFunctions = isArray(filterFunctionOrFunctions)
+    ? filterFunctionOrFunctions
+    : [filterFunctionOrFunctions];
 
   // await for changes in devices array
   watch(devices, async devices => {
@@ -81,7 +86,13 @@ function useDevicesWithFilter(apiParams, filterFunction) {
       get(devices).map(async device => {
         try {
           // result is memoized once successful
-          const isAvailable = await getIsAvailable(device);
+          let isAvailable = true;
+          for (const filterFunction of filterFunctions) {
+            if (!(await filterFunction(device))) {
+              isAvailable = false;
+              break;
+            }
+          }
 
           // Put into refs to trigger reactive behavior in computed devices
           if (isAvailable) {
@@ -94,13 +105,7 @@ function useDevicesWithFilter(apiParams, filterFunction) {
             }
           }
         } catch (e) {
-          // clear cache to try again on next poll
-          getIsAvailable.cache.delete(device.id);
-
-          // If 404, don't mark as failed
-          if (e.response.status !== 404) {
-            failed = true;
-          }
+          failed = true;
         }
       })
     );
@@ -129,51 +134,83 @@ function useDevicesWithFilter(apiParams, filterFunction) {
 }
 
 /**
- * Filters devices to those that also have the specified channel
- * @param channelId
- * @return {{
- *  devices: Ref<NetworkLocation[]>, hasFetched: Ref<bool>, isFetching: Ref<bool>,
- *  filterFailed: Ref<bool>, forceFetch: (function(): Promise<void>)
- * }}
+ * Produces a memoized function that returns a Promise resolving with a boolean for filtering
+ * devices, and automatically clears memoized result if it fails, unless it is a 404
+ *
+ * @param {function(NetworkLocation): Promise<boolean>} filterFunction
+ * @return {(function(NetworkLocation): Promise<boolean>)}
  */
-export function useDevicesWithChannel(channelId = '') {
-  // If channelId is not provided, then we are at top-level import workflow and do not
-  // disable any devices unless it is unavailable
-  const filterDevices =
-    channelId === '' ? () => Promise.resolve(true) : channelIsAvailableAtDevice.bind({}, channelId);
+function useAsyncDeviceFilter(filterFunction) {
+  const memoized = useMemoize(filterFunction, { getKey: device => device.id });
+
+  return async function deviceFilter(device) {
+    try {
+      return await memoized(device);
+    } catch (e) {
+      // If not 404 clear cache to try again on next poll
+      if (e?.response?.status !== 404) {
+        memoized.cache.delete(device.id);
+        throw e;
+      }
 
-  return useDevicesWithFilter({}, filterDevices);
+      return false;
+    }
+  };
 }
 
 /**
- * Filters devices to those that also have the specified facility
- * @param facilityId
- * @return {{
- *  devices: Ref<NetworkLocation[]>, hasFetched: Ref<bool>, isFetching: Ref<bool>,
- *  filterFailed: Ref<bool>, forceFetch: (function(): Promise<void>)
- * }}
+ * Produces a function that resolves with a boolean for a device that has the specified facility
+ * @param {string|null} [id]
+ * @param {bool|null} [learner_can_sign_up]
+ * @return {function(NetworkLocation): Promise<boolean>}
+ */
+export function useDeviceFacilityFilter({ id = null, learner_can_sign_up = null }) {
+  const filters = {};
+
+  // If `id` is an empty string, we don't want to filter by that
+  if (id) {
+    filters.id = id;
+  }
+
+  if (learner_can_sign_up !== null) {
+    filters.learner_can_sign_up = learner_can_sign_up;
+  }
+
+  if (Object.keys(filters).length === 0) {
+    return () => Promise.resolve(true);
+  }
+
+  return useAsyncDeviceFilter(function deviceFacilityFilter(device) {
+    return deviceHasMatchingFacility(device, filters);
+  });
+}
+
+/**
+ * Produces a function that resolves with a boolean for a device that has the specified channel
+ * @param {string|null} id
+ * @return {function(NetworkLocation): Promise<boolean>}
  */
-export function useDevicesWithFacility({ facilityId = '', soud = false } = {}) {
-  // If facilityId is not provided, then we are at the initial Facility Import workflow
-  // disable any devices unless it is unavailable/offline
-  const filterDevices =
-    facilityId === ''
-      ? () => Promise.resolve(true)
-      : facilityIsAvailableAtDevice.bind({}, facilityId);
-
-  return useDevicesWithFilter({ subset_of_users_device: soud }, filterDevices);
+export function useDeviceChannelFilter({ id = null }) {
+  if (!id) {
+    return () => Promise.resolve(true);
+  }
+
+  return useAsyncDeviceFilter(function deviceChannelFilter(device) {
+    return channelIsAvailableAtDevice(id, device);
+  });
 }
 
 /**
- * Filters devices to those that are not SoUDs/LOD, since SoUD/LOD sync to full-facility devices
- * @return {{
- *  devices: Ref<NetworkLocation[]>, hasFetched: Ref<bool>, isFetching: Ref<bool>,
- *  filterFailed: Ref<bool>, forceFetch: (function(): Promise<void>)
- * }}
+ * Produces a function that resolves with a boolean if Kolibri version is at least the specified
+ * @param {number} major
+ * @param {number} minor
+ * @param {number} patch
+ * @return {function(NetworkLocation): Promise<boolean>}
  */
-export function useDevicesForLearnOnlyDevice() {
-  const { isMinimumKolibriVersion } = useMinimumKolibriVersion(0, 15, 0);
-  return useDevicesWithFilter({ subset_of_users_device: false }, async device => {
+export function useDeviceMinimumVersionFilter(major, minor, patch) {
+  const { isMinimumKolibriVersion } = useMinimumKolibriVersion(major, minor, patch);
+
+  return async function deviceMinimumVersionFilter(device) {
     return isMinimumKolibriVersion(device.kolibri_version);
-  });
+  };
 }

kolibri/core/assets/src/views/sync/syncComponentSet.js

@@ -11,9 +11,10 @@ export { default as SelectSourceModal } from './SelectSourceModal';
 export { default as SyncFacilityModalGroup } from './SyncFacilityModalGroup';
 export {
   default as useDevices,
-  useDevicesForLearnOnlyDevice,
-  useDevicesWithFacility,
-  useDevicesWithChannel,
+  useDevicesWithFilter,
+  useDeviceChannelFilter,
+  useDeviceFacilityFilter,
+  useDeviceMinimumVersionFilter,
 } from './SelectDeviceModalGroup/useDevices';
 export { default as useDeviceDeletion } from './SelectDeviceModalGroup/useDeviceDeletion';
 export { default as useConnectionChecker } from './SelectDeviceModalGroup/useConnectionChecker';

kolibri/plugins/setup_wizard/assets/src/views/JoinOrNewLOD.vue

@@ -19,6 +19,7 @@
     />
     <SelectDeviceModalGroup
       v-if="showSelectAddressModal"
+      :filterByFacilityCanSignUp="selected === Options.JOIN ? true : null"
       @cancel="showSelectAddressModal = false"
       @submit="handleContinueImport"
     />

packages/kolibri-common/components/SyncSchedule/ManageSyncSchedule.vue

@@ -126,7 +126,8 @@
   import commonSyncElements from 'kolibri.coreVue.mixins.commonSyncElements';
   import {
     SyncFacilityModalGroup,
-    useDevicesWithFacility,
+    useDeviceFacilityFilter,
+    useDevicesWithFilter,
   } from 'kolibri.coreVue.componentSets.sync';
   import { TaskTypes } from 'kolibri.utils.syncTaskUtils';
   import { KDP_ID, oneHour, oneDay, oneWeek, twoWeeks, oneMonth } from './constants';
@@ -142,7 +143,13 @@
     extends: ImmersivePage,
     mixins: [commonCoreStrings, commonSyncElements],
     setup(props) {
-      const { devices } = useDevicesWithFacility({ facilityId: props.facilityId });
+      const deviceFilter = useDeviceFacilityFilter({ id: props.facilityId });
+      const { devices } = useDevicesWithFilter(
+        {
+          subset_of_users_device: false,
+        },
+        deviceFilter
+      );
       const devicesById = computed(() => {
         return devices.value.reduce(
           (acc, device) => {