Add Room Combobox to Initial Room Schedule page #557

natalynyu · 2022-08-04T15:55:52Z

This PR adds the initial Room Schedule tab/page, which includes a combobox that contains all of the rooms in the database. The previously existing location controller method getRoomAvailability was using the / route, and I updated the new controller method getRooms to use this route instead. This was because getRooms is more general and does not take parameters in order to find the room availability for a specific time (term/calendar year/day, etc).

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality
to change)

Checklist:

I have run eslint on the code
I have added JSDoc for all of my code (where applicable)
I have added tests to cover my changes.

Priority:

Normal
High

Related Issues:

Fixes #550

The new room schedule path was added so that it appears in the list of tabs in the app header.

The existing API that is getting the room availability was updated so that its path is /api/rooms/availability instead so that this more general retrieval of rooms without availability can have the path /api/rooms. In order to get the room information, we are querying the existing roomListingViewRepository. While we do not technically need the capacity or campus information to build the room dropdown, we can reuse this service for the future room admin tab.

The existing room API's path was updated from /api/rooms/ to /api/rooms/availability so that our more general querying of room information can use the path /api/rooms/, so this is fixing the existing tests checking the room availability path.

…tely

…g displayed

This room schedule component includes a combobox that has a filter function that uses the mark-one regex filter function which allows a case insensitive search. It also allows you to search the middle of a word (e.g. if the room is 1414 Massachusetts Avenue 230, you could search "ave" to see this room as one of the search results). Two NoteText components were also added to the menu that show the (for now) hardcoded semester and currently selected room.

These tests check the results of calling the room controller. All rooms are returned as a result of calling the controller when the user is authenticated, and unauthenticated users will receive a forbidden status.

codecov · 2022-08-04T16:08:12Z

Codecov Report

Merging #557 (68cdf9b) into develop (eee2d1b) will decrease coverage by 0.06%.
The diff coverage is 92.00%.

@@             Coverage Diff             @@
##           develop     #557      +/-   ##
===========================================
- Coverage    96.12%   96.06%   -0.07%     
===========================================
  Files          179      181       +2     
  Lines         3768     3812      +44     
  Branches       443      446       +3     
===========================================
+ Hits          3622     3662      +40     
- Misses          68       71       +3     
- Partials        78       79       +1

Impacted Files	Coverage Δ
src/client/components/layout/AppHeader.tsx	`92.00% <ø> (ø)`
src/client/components/layout/AppRouter.tsx	`85.00% <ø> (ø)`
.../client/components/pages/Schedule/ScheduleView.tsx	`97.14% <ø> (ø)`
src/common/dto/room/RoomResponse.dto.ts	`100.00% <ø> (ø)`
...ent/components/pages/RoomSchedule/RoomSchedule.tsx	`85.71% <85.71%> (ø)`
src/client/api/rooms.ts	`100.00% <100.00%> (ø)`
.../client/components/pages/Courses/RoomSelection.tsx	`100.00% <100.00%> (ø)`
...nt/components/pages/Courses/RoomSelectionTable.tsx	`100.00% <100.00%> (ø)`
.../components/pages/Schedule/blocks/SessionBlock.tsx	`90.90% <100.00%> (+0.58%)`	⬆️
src/client/components/pages/index.ts	`100.00% <100.00%> (ø)`
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…e to use it The RoomMeetingResponse DTO has all of the properties that the original RoomResponse DTO had. The existing code related to room availability was updated to use the RoomMeetingResponse DTO, and the code related to the room combobox on the RoomSchedule page uses the RoomResponse DTO, which has the propeties id, name, campus, and capacity.

rmainwork

This looks great, just a few small notes.

I'm also having some difficulty testing this in my browser - when I make a room selection, the room seems to have been successfully chosen, but the selection box is blank, the table is blank and no AJAX request seems to be being made to retrieve data. Is there something I'm doing wrong?

rmainwork · 2022-08-09T13:58:58Z

tests/integration/server/location/location.controller.test.ts

+      });
+      it('should return a nonempty array of data', function () {
+        strictEqual(Array.isArray(result), true);
+        notStrictEqual(result, 0);


Should this be result.length?

Yep, good call! I fixed this in dcd4821.

src/server/location/location.service.ts

rmainwork · 2022-08-09T14:11:05Z

src/server/location/location.service.ts

+      .createQueryBuilder()
+      .select('id')
+      .addSelect('name')
+      .addSelect('campus')
+      .addSelect('capacity')
+      .getRawMany();


I think you should be able to just replace this query builder call with Repository.find().

Suggested change

.createQueryBuilder()

.select('id')

.addSelect('name')

.addSelect('campus')

.addSelect('capacity')

.getRawMany();

.find({

select: ['name', 'campus', 'capacity']

});

This is good to know - thanks for the suggestion. I updated this in 68cdf9b.

rmainwork · 2022-08-09T14:35:12Z

tests/integration/server/location/location.controller.test.ts

+        const rawRooms = await locationRepo.createQueryBuilder('r')
+          .select('CONCAT_WS(\' \', b.name, r.name)', 'name')
+          .leftJoin('r.building', 'b')
+          .leftJoin('b.campus', 'c')


Since you're not selecting anything from the campus table, you can probably get rid of this JOIN.

Suggested change

.leftJoin('b.campus', 'c')

Ah, that's a good point.. This was removed in b8221b7

rmainwork · 2022-08-09T16:40:35Z

src/client/components/pages/RoomSchedule/RoomSchedule.tsx

+
+  const loadRooms = useCallback(async (): Promise<void> => {
+    try {
+      const roomList = await LocationAPI.getRooms();


Suggested change

const roomList = await LocationAPI.getRooms();

const roomList = await getRooms();

I'd suggest importing and using getRooms instead of LocationAPI.getRooms() here as the latter was producing a type error for me (likely due to the way things are exported/imported).

It looks like we updated all the client APIs to export the functions in this style in this commit as a result of no longer being able to stub free-standing functions in Typescript 3.9.2+:

Sinon stubbing failed after upgrade to 3.9.2 microsoft/TypeScript#38568

Possible to stub a standalone utility function? sinonjs/sinon#562

What type error are you seeing in importing with LocationAPI?

I was getting Unsafe call of an any typed value from @typescript-eslint/no-unsafe-call.

I wouldn't think this would be related to Sinon as this error was thrown in the production code. I think it has more to do with the way it's being imported here (from client/api/rooms) and the way it's being exported (or rather isn't being exported ) from client/api.

Adding export * from './rooms'; to src/client/api/index.ts and then changing the import statement to import { LocationApi } from 'client/api' fixes this error.

diff --git a/src/client/api/index.ts b/src/client/api/index.ts index f19ee6b4..4c61a767 100644 --- a/src/client/api/index.ts +++ b/src/client/api/index.ts @@ -6,3 +6,4 @@ export * from './nonClassMeeting'; export * from './meetings'; export * from './reports'; export * from './httpStatus'; +export * from './rooms'; diff --git a/src/client/components/pages/RoomSchedule/RoomSchedule.tsx b/src/client/components/pages/RoomSchedule/RoomSchedule.tsx index 3f9927cb..da1f7f57 100644 --- a/src/client/components/pages/RoomSchedule/RoomSchedule.tsx +++ b/src/client/components/pages/RoomSchedule/RoomSchedule.tsx @@ -1,4 +1,4 @@ -import { LocationAPI } from 'client/api/rooms'; +import { LocationAPI } from 'client/api'; import { AppMessage, MESSAGE_TYPE, MESSAGE_ACTION } from 'client/classes'; import { MenuFlex } from 'client/components/general'; import { VerticalSpace } from 'client/components/layout';

@rmainseas Ah ok thanks for looking into this. I can make the update, but for some reason I am not seeing this error - do you think we have different versions of the linter or something like that?

Yeah of course. Not sure, that's an interesting question and probably something we should look into....

This test was meant to check that the length of the data that is returned from the API was not 0.

Nothing is being queried in the campus table, so the join is not necessary.

…h of the columns individually

natalynyu · 2022-08-11T15:53:45Z

I'm also having some difficulty testing this in my browser - when I make a room selection, the room seems to have been successfully chosen, but the selection box is blank, the table is blank and no AJAX request seems to be being made to retrieve data. Is there something I'm doing wrong?

Thanks for reviewing this, Robert! As of now, the only thing that happens when selecting a room is that the text next to "Current Room:" is updated as an indication of what room was selected. The next ticket (#551) will implement the Room Schedule table for the selected room and the (for now) hardcoded semester.

rmainwork · 2022-08-11T15:56:14Z

Ah, gotcha. For some reason, I'd thought it was meant to populate a schedule view below the dropdown.

In that case, I'm going to approve this. Merge away.

natalynyu added 7 commits August 3, 2022 12:12

Add room-schedule link

5621dd7

The new room schedule path was added so that it appears in the list of tabs in the app header.

Add tests to check that the Room Schedule link is displayed appropria…

61ba755

…tely

Add App Router tests to check that the RoomSchedule component is bein…

4ad0d8b

…g displayed

Add tests to check the results of calling the location controller

1205417

These tests check the results of calling the room controller. All rooms are returned as a result of calling the controller when the user is authenticated, and unauthenticated users will receive a forbidden status.

natalynyu requested a review from rmainwork August 8, 2022 20:00

rmainwork suggested changes Aug 9, 2022

View reviewed changes

natalynyu added 4 commits August 10, 2022 14:53

Check the result's length rather than the result itself

dcd4821

This test was meant to check that the length of the data that is returned from the API was not 0.

Add docblock to getRoomList function

72c6a20

Remove unused join to campus table

b8221b7

Nothing is being queried in the campus table, so the join is not necessary.

Improve service method by just calling .find instead of selecting eac…

68cdf9b

…h of the columns individually

natalynyu requested a review from rmainwork August 11, 2022 15:54

rmainwork approved these changes Aug 11, 2022

View reviewed changes

natalynyu merged commit edbfb01 into develop Aug 11, 2022

natalynyu deleted the feature/550-initial-room-schedule-page branch August 11, 2022 15:57

natalynyu restored the feature/550-initial-room-schedule-page branch August 11, 2022 18:12

natalynyu mentioned this pull request Aug 11, 2022

Update import and export statements for the Location API #560

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Room Combobox to Initial Room Schedule page #557

Add Room Combobox to Initial Room Schedule page #557

natalynyu commented Aug 4, 2022 •

edited

Loading

codecov bot commented Aug 4, 2022 •

edited

Loading

rmainwork left a comment

rmainwork Aug 9, 2022

natalynyu Aug 11, 2022

rmainwork Aug 9, 2022

natalynyu Aug 11, 2022

rmainwork Aug 9, 2022

natalynyu Aug 11, 2022

rmainwork Aug 9, 2022 •

edited

Loading

natalynyu Aug 10, 2022

rmainwork Aug 11, 2022

rmainwork Aug 11, 2022

rmainwork Aug 11, 2022

natalynyu Aug 11, 2022

rmainwork Aug 11, 2022

natalynyu commented Aug 11, 2022

rmainwork commented Aug 11, 2022

	const roomList = await LocationAPI.getRooms();
	const roomList = await getRooms();

Add Room Combobox to Initial Room Schedule page #557

Add Room Combobox to Initial Room Schedule page #557

Conversation

natalynyu commented Aug 4, 2022 • edited Loading

Types of changes

Checklist:

Priority:

Related Issues:

codecov bot commented Aug 4, 2022 • edited Loading

Codecov Report

rmainwork left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmainwork Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natalynyu commented Aug 11, 2022

rmainwork commented Aug 11, 2022

natalynyu commented Aug 4, 2022 •

edited

Loading

codecov bot commented Aug 4, 2022 •

edited

Loading

rmainwork Aug 9, 2022 •

edited

Loading