seas-computing / course-planner

SEAS Course Planning Application. JSDoc Docs at: https://seas-computing.github.io/course-planner/
0 stars 1 forks source link

Creation of a new room in existing multi-campus building causes error to (incorrectly) show #670

Open rmainwork opened 1 year ago

rmainwork commented 1 year ago

Description

Some buildings exist on multiple campuses due to the fact that some rooms in that building may be managed by different entities (i.e: some rooms in the one building might be managed by SEAS, some might be managed centrally by Harvard).

In order to make this work, the same building has to exist on multiple campuses like so:

- Allston
  - SEC
    - Dean's Suite <--- Managed by SEAS

- Non-SEAS
  - SEC  <-- Different record in the building table but refers to the same physical building
    - Tutorial Room 1 <--- Not managed by SEAS

To Reproduce

  1. Create the same building on multiple campuses (this may have to be done in the database as the application does not currently allow for this)
  2. Try to create a room in one of the newly created campuses
  3. The application will detect the other campus and throw an error

Expected Behavior

The room should be created

Actual Behavior

An error about duplicate buildings was shown

Screenshots

image

Additional Information

The root cause of the issue appears to be related to the structure of the data sent to the server by the client. Currently, the client sends and object of type CreateRoomRequest which looks a little something like this:

type CreateRoomRequest = {
  campus: string;
  building: string;
  name: string;
  capacity: number;
}

It's then up to the server to look up roomRequest.building to determine if the building is being created or an existing building is being used.

In this instance, the data being sent to the server is the following:

{
  "campus": "Allston",
  "building": "SEC",
  "name": "LL1.246",
  "capacity": 88
}

The server then examines this, searches for roomRequest.building in the database where campus != roomRequest.campus. Essentially something to the effect of:

SELECT * FROM building b
LEFT JOIN campus c ON b."campusId" = c.id 
WHERE b.name = 'SEC' AND c.name <> 'Allston'

This query returns the other SEC that belongs to the "Non-SEAS" campus and the API endpoint (incorrectly) shows an error to the user about creating a duplicate building. Unfortunately it's difficult to modify the query to take account of this, as this is exactly the type of situation it exists to prevent (multiple duplicate buildings across campuses).

One potential solution to this would be to (instead of providing just the building name) to the server, send something like this:

type CreateRoomRequest = {
  campus: string;
  building: Pick<Building, 'id', 'name'>;
  name: string;
  capacity: number;
}

Instead of looking up the building name to decide if the building is being created or not, the presence (or absence) of building.id could to determine this. As an unintended bonus, this eliminates an extra database call to look up the building ID. If we're creating a new building, we can continue to use the existing logic in place as we want to validate that a duplicate building isn't being created.