rmainwork
commented
1 year ago After some investigation on this, it looks like it wouldn't be too difficult of a change to make (especially considering the extensive test suite we have).
diff --git a/src/client/api/__tests__/location.test.ts b/src/client/api/__tests__/location.test.ts
deleted file mode 100644
index 26ea69ad..00000000
--- a/src/client/api/__tests__/location.test.ts
+++ /dev/null
@@ -1,52 +0,0 @@
-import { deepStrictEqual, fail, strictEqual } from 'assert';
-import RoomAdminResponse from 'common/dto/room/RoomAdminResponse.dto';
-import { SinonStub, stub } from 'sinon';
-import { adminRoomsResponse, error } from 'testData';
-import request from '../request';
-import { LocationAPI } from '../rooms';
-
-describe('Location API', function () {
- let getAdminRoomsResponse: RoomAdminResponse[];
- let getStub: SinonStub;
- describe('GET /rooms/admin', function () {
- beforeEach(function () {
- getStub = stub(request, 'get');
- });
- afterEach(function () {
- getStub.restore();
- });
- describe('gets all room information', function () {
- context('when data fetch succeeds', function () {
- beforeEach(async function () {
- getStub.resolves({
- data: adminRoomsResponse,
- });
- getAdminRoomsResponse = await LocationAPI.getAdminRooms();
- });
- it('should call getAdminRooms', function () {
- strictEqual(getStub.callCount, 1);
- });
- it('should request /api/rooms/admin', function () {
- const [[path]] = getStub.args;
- strictEqual(path, '/api/rooms/admin');
- });
- it('should return the rooms information', function () {
- deepStrictEqual(getAdminRoomsResponse, adminRoomsResponse);
- });
- });
- context('when data fetch fails', function () {
- beforeEach(function () {
- getStub.rejects();
- });
- it('should throw an error', async function () {
- try {
- await LocationAPI.getAdminRooms();
- fail('Did not throw an error');
- } catch (err) {
- deepStrictEqual(err, error);
- }
- });
- });
- });
- });
-});
diff --git a/src/client/api/rooms.ts b/src/client/api/rooms.ts
index 5a69a5d9..3a63308d 100644
--- a/src/client/api/rooms.ts
+++ b/src/client/api/rooms.ts
@@ -1,4 +1,3 @@
-import RoomAdminResponse from 'common/dto/room/RoomAdminResponse.dto';
import RoomMeetingResponse from 'common/dto/room/RoomMeetingResponse.dto';
import RoomRequest from 'common/dto/room/RoomRequest.dto';
import RoomResponse from 'common/dto/room/RoomResponse.dto';
@@ -28,16 +27,7 @@ Promise<RoomMeetingResponse[]> => {
return response.data as RoomMeetingResponse[];
};
-/**
- * Retrieves all room numbers along with their building and campus info
- */
-export const getAdminRooms = async ():Promise<RoomAdminResponse[]> => {
- const response = await request.get('/api/rooms/admin');
- return response.data as RoomAdminResponse[];
-};
-
export const LocationAPI = {
getRoomAvailability,
getRooms,
- getAdminRooms,
};
diff --git a/src/client/components/pages/RoomAdmin/RoomAdmin.tsx b/src/client/components/pages/RoomAdmin/RoomAdmin.tsx
index 5274f85d..31f2d146 100644
--- a/src/client/components/pages/RoomAdmin/RoomAdmin.tsx
+++ b/src/client/components/pages/RoomAdmin/RoomAdmin.tsx
@@ -46,7 +46,7 @@ const RoomAdmin: FunctionComponent = (): ReactElement => {
*/
useEffect((): void => {
setFetching(true);
- LocationAPI.getAdminRooms()
+ LocationAPI.getRooms()
.then((rooms): void => {
setFullRoomList(rooms);
})
diff --git a/src/client/components/pages/RoomAdmin/__tests__/RoomAdminTable.test.tsx b/src/client/components/pages/RoomAdmin/__tests__/RoomAdminTable.test.tsx
index 7df5898e..db9355ec 100644
--- a/src/client/components/pages/RoomAdmin/__tests__/RoomAdminTable.test.tsx
+++ b/src/client/components/pages/RoomAdmin/__tests__/RoomAdminTable.test.tsx
@@ -34,7 +34,7 @@ describe('Room Admin Table', function () {
describe('data fetching', function () {
context('when room data fetch succeeds', function () {
beforeEach(function () {
- getStub = stub(LocationAPI, 'getAdminRooms');
+ getStub = stub(LocationAPI, 'getRooms');
getStub.resolves(adminRoomsResponse);
});
context('when there are room records', function () {
@@ -152,7 +152,7 @@ describe('Room Admin Table', function () {
let dispatchMessage: SinonStub;
beforeEach(function () {
dispatchMessage = stub();
- getStub = stub(LocationAPI, 'getAdminRooms');
+ getStub = stub(LocationAPI, 'getRooms');
getStub.rejects(error);
});
afterEach(function () {
diff --git a/src/common/dto/room/RoomResponse.dto.ts b/src/common/dto/room/RoomResponse.dto.ts
index 00afd854..d048bc1b 100644
--- a/src/common/dto/room/RoomResponse.dto.ts
+++ b/src/common/dto/room/RoomResponse.dto.ts
@@ -1,5 +1,28 @@
import { ApiProperty } from '@nestjs/swagger';
+abstract class CampusInfo {
+ @ApiProperty({
+ example: '4193a3e5-5987-4083-97cf-a949c146260f',
+ })
+ id: string;
+
+ @ApiProperty({
+ example: 'Cambridge',
+ })
+ name: string;
+}
+
+abstract class BuildingInfo {
+ @ApiProperty({ example: 'b5478231-478e-464d-b29c-45c98fa472b3' })
+ id: string;
+
+ @ApiProperty({ example: 'Maxwell Dworkin' })
+ name: string;
+
+ @ApiProperty({ type: CampusInfo })
+ campus: CampusInfo;
+}
+
export default abstract class RoomResponse {
/**
* The database uuid of the room
@@ -11,13 +34,10 @@ export default abstract class RoomResponse {
public id: string;
/**
- * The campus within the university where the room is located
+ * The building and campus name information of the room
*/
- @ApiProperty({
- type: 'string',
- example: 'Allston',
- })
- public campus: string;
+ @ApiProperty({ type: BuildingInfo })
+ public building: BuildingInfo;
/**
* The name of the building concatenated with the number of the room
diff --git a/src/server/location/location.controller.ts b/src/server/location/location.controller.ts
index bdbb07f2..64b1268b 100644
--- a/src/server/location/location.controller.ts
+++ b/src/server/location/location.controller.ts
@@ -61,6 +61,6 @@ export class LocationController {
isArray: true,
})
public async getAdminRooms(): Promise<RoomAdminResponse[]> {
- return this.locationService.getAdminRooms();
+ return this.locationService.getRoomList();
}
}
diff --git a/src/server/location/location.service.ts b/src/server/location/location.service.ts
index abfcdcf9..88822946 100644
--- a/src/server/location/location.service.ts
+++ b/src/server/location/location.service.ts
@@ -42,11 +42,21 @@ export class LocationService {
* Retrieves all rooms in the database along with their campus and capacity
* information.
*/
- public async getRoomList(): Promise<RoomResponse[]> {
- return this.roomListingViewRepository
- .find({
- select: ['id', 'name', 'campus', 'capacity'],
- });
+ public async getRoomList(): Promise<Room[]> {
+ return this.roomRepository
+ .createQueryBuilder('r')
+ .leftJoinAndSelect(
+ 'r.building',
+ 'building'
+ )
+ .leftJoinAndSelect(
+ 'building.campus',
+ 'campus'
+ )
+ .orderBy('campus.name', 'ASC')
+ .addOrderBy('building.name', 'ASC')
+ .addOrderBy('r.name', 'ASC')
+ .getMany();
}
/**
@@ -145,25 +155,4 @@ export class LocationService {
.filter((title) => !!title),
}));
}
-
- /**
- * Resolves with a list of rooms along with their associated campus and
- * building information. This is used to populate the Room Admin table.
- */
- public async getAdminRooms(): Promise<RoomAdminResponse[]> {
- return this.roomRepository
- .createQueryBuilder('r')
- .leftJoinAndSelect(
- 'r.building',
- 'building'
- )
- .leftJoinAndSelect(
- 'building.campus',
- 'campus'
- )
- .orderBy('campus.name', 'ASC')
- .addOrderBy('building.name', 'ASC')
- .addOrderBy('r.name', 'ASC')
- .getMany() as Promise<RoomAdminResponse[]>;
- }
}There is still a failing test that needs to be fixed, but other than that this should just about cover it.
This idea was brought up through PR #570.
There are a few different service methods that are querying for room information but in different shapes. We should look into whether there is a way to streamline the methods such that we have fewer methods to get the room information.