Merge branch 'develop' into SWAP-4929-user-officer-experiment-table-filter

Author: yoganandaness

apps/backend/package-lock.json

@@ -96,11 +96,6 @@ const instrumentDataSource = container.resolve(
   Tokens.InstrumentDataSource
 ) as InstrumentDataSource;
 
-const mockRemoveScientistFromInstruments = jest.spyOn(
-  instrumentDataSource,
-  'removeScientistFromInstruments'
-);
-
 const mockAssignScientistToInstruments = jest.spyOn(
   instrumentDataSource,
   'assignScientistToInstruments'
@@ -132,7 +127,6 @@ beforeAll(() => {
 
 beforeEach(() => {
   mockAssignScientistToInstruments.mockClear();
-  mockRemoveScientistFromInstruments.mockClear();
   mockGetRequiredInstrumentForRole.mockClear();
 });
 
@@ -230,108 +224,23 @@ test('When a user requests multiple instruments, only new existing ones are adde
   expect(result).toEqual([lsfInstrument.id]);
 });
 
-// getInstrumentsToRemove
-test('When a user has an instrument they have not requested, the instrument is removed', async () => {
-  const requiredInstruments: string[] = [];
-  const currentInstruments = [isisInstrument];
-
-  const result = await userAuthorization.getInstrumentsToRemove(
-    requiredInstruments,
-    currentInstruments
-  );
-
-  expect(result).toEqual([isisInstrument.id]);
-});
-
-test('When a user has an instrument they have requested, no instrument is removed', async () => {
-  const requiredInstruments = [isisInstrument.name];
-  const currentInstruments = [isisInstrument];
-
-  const result = await userAuthorization.getInstrumentsToRemove(
-    requiredInstruments,
-    currentInstruments
-  );
-
-  expect(result).toEqual([]);
-});
-
-test('When a user does not have an instrument they requested, no instrument is removed', async () => {
-  const requiredInstruments = [isisInstrument.name];
-  const currentInstruments: Instrument[] = [];
-
-  const result = await userAuthorization.getInstrumentsToRemove(
-    requiredInstruments,
-    currentInstruments
-  );
-
-  expect(result).toEqual([]);
-});
-
-test('When a user requests a nonexisting instrument, all other instruments are removed', async () => {
-  const requiredInstruments = [nonExistingInstrumentName];
-  const currentInstruments = [isisInstrument, lsfInstrument];
-
-  const result = await userAuthorization.getInstrumentsToRemove(
-    requiredInstruments,
-    currentInstruments
-  );
-
-  expect(result).toEqual([isisInstrument.id, lsfInstrument.id]);
-});
-
-test('When a user requests multiple instrument, only non requested ones are removed', async () => {
-  const requiredInstruments = [isisInstrument.name];
-  const currentInstruments: Instrument[] = [isisInstrument, lsfInstrument];
-
-  const result = await userAuthorization.getInstrumentsToRemove(
-    requiredInstruments,
-    currentInstruments
-  );
-
-  expect(result).toEqual([lsfInstrument.id]);
-});
-
 //autoAssignRemoveInstruments
-test('When a user requires an instrument but does not have it, the instrument is assigned and no instrument are removed', async () => {
-  await userAuthorization.autoAssignRemoveInstruments(
-    0,
-    [isisInstrument.name],
-    [],
-    true
-  );
+test('When a user requires an instrument but does not have it, the instrument is assigned', async () => {
+  await userAuthorization.autoAssignInstruments(0, [isisInstrument.name], []);
 
   expect(mockAssignScientistToInstruments).toHaveBeenCalledWith(0, [
     isisInstrument.id,
   ]);
-  expect(mockRemoveScientistFromInstruments).toHaveBeenCalledTimes(0);
 });
 
-test('When a user does not require an instrument but has it, no instrument is assigned and the instrument is removed', async () => {
-  await userAuthorization.autoAssignRemoveInstruments(
-    0,
-    [],
-    [isisInstrument],
-    true
-  );
-
-  expect(mockAssignScientistToInstruments).toHaveBeenCalledTimes(0);
-  expect(mockRemoveScientistFromInstruments).toHaveBeenCalledWith(0, [
-    isisInstrument.id,
-  ]);
-});
-
-test('When a user requires an instrument but has a different one, the requested instrument is assigned and the current instrument is removed', async () => {
-  await userAuthorization.autoAssignRemoveInstruments(
+test('When a user requires an instrument but has a different one, the requested instrument is assigned and the current instrument stays the same', async () => {
+  await userAuthorization.autoAssignInstruments(
     0,
     [isisInstrument.name],
-    [lsfInstrument],
-    true
+    [lsfInstrument]
   );
 
   expect(mockAssignScientistToInstruments).toHaveBeenCalledWith(0, [
     isisInstrument.id,
   ]);
-  expect(mockRemoveScientistFromInstruments).toHaveBeenCalledWith(0, [
-    lsfInstrument.id,
-  ]);
 });

apps/backend/src/auth/StfcUserAuthorization.spec.ts

@@ -102,26 +102,10 @@ export class StfcUserAuthorization extends UserAuthorization {
     return instrumentsToAssign.map((instrument) => instrument.id);
   }
 
-  async getInstrumentsToRemove(
-    requiredInstrumentNames: string[],
-    currentInstruments: Instrument[]
-  ) {
-    // Remove instrument scientist from any instruments they are assigned to,
-    // but shouldn't be.
-    // We don't need to check if the instruments exist here,
-    // because we fetch the list of currently assigned instruments from the DB
-    return currentInstruments
-      .filter(
-        (instrument) => !requiredInstrumentNames.includes(instrument.name)
-      )
-      .map((instrument) => instrument.id);
-  }
-
-  async autoAssignRemoveInstruments(
+  async autoAssignInstruments(
     userId: number,
     requiredInstrumentNames: string[],
-    currentInstruments: Instrument[],
-    removeInstruments: boolean
+    currentInstruments: Instrument[]
   ) {
     // Assign any instruments which aren't currently assigned
     const instrumentsToAdd = await this.getInstrumentsToAdd(
@@ -142,28 +126,6 @@ export class StfcUserAuthorization extends UserAuthorization {
         instrumentsToAdd
       );
     }
-
-    if (removeInstruments) {
-      // Remove any instruments which are currently assigned, but not required
-      const instrumentsToRemove = await this.getInstrumentsToRemove(
-        requiredInstrumentNames,
-        currentInstruments
-      );
-
-      if (instrumentsToRemove.length > 0) {
-        logger.logInfo(
-          'Auto-removing STFC instrument scientist from instruments',
-          {
-            instruments: instrumentsToRemove,
-          }
-        );
-
-        this.instrumentDataSource.removeScientistFromInstruments(
-          userId,
-          instrumentsToRemove
-        );
-      }
-    }
   }
 
   async externalTokenLogin(
@@ -250,16 +212,10 @@ export class StfcUserAuthorization extends UserAuthorization {
         await this.instrumentDataSource.getUserInstruments(userNumber);
 
       // Don't remove instruments from ISIS Staff and User officers as these will be manually assigned
-      this.autoAssignRemoveInstruments(
+      this.autoAssignInstruments(
         userNumber,
         requiredInstruments,
-        currentUserInstruments,
-        !uniqueRoles.find(
-          (role) =>
-            role.name === 'ISIS Instrument Scientist' ||
-            role.name === 'CLF LSF Link Scientist' ||
-            role.name === 'User Officer'
-        )
+        currentUserInstruments
       );
     }
 

apps/backend/src/auth/StfcUserAuthorization.ts

@@ -82,7 +82,10 @@ const CreateUpdateInstrument = ({
 
     try {
       await api()
-        .getUsers({ filter: value, userRole: UserRole.INSTRUMENT_SCIENTIST })
+        .getUsers({
+          userRole: UserRole.INSTRUMENT_SCIENTIST,
+          searchText: value,
+        })
         .then((data) => {
           if (data.users?.totalCount == 0) {
             setFieldError(

apps/frontend/src/components/instrument/CreateUpdateInstrument.tsx

@@ -82,7 +82,7 @@ const CreateUpdateInternalReview = ({
 
     try {
       await api()
-        .getUsers({ filter: value, userRole: UserRole.INTERNAL_REVIEWER })
+        .getUsers({ searchText: value, userRole: UserRole.INTERNAL_REVIEWER })
         .then((data) => {
           if (data.users?.totalCount == 0) {
             setFieldError(