search

david-fisher / 320-F20-Track-IV

BSD 3-Clause "New" or "Revised" License
2 stars 0 forks source link

Add: page-group (DB access, endpoints), simulation (endpoints), issue-coverage-matrix (endpoints), components (endpoints) #126

Closed hoon0422 closed 3 years ago

hoon0422 commented 3 years ago

Goal of this pull-request

First of all, we all know that the design of DB is so bad. While writing DB access points and endpoints, I can find so many problems DB contains. However, even if DB contains many problems, this is not the main problem; the main problem is that our mission is to finish backend server which connects frontend and DB. If we follow DB, we give up the integration with frontend, and vice versa.

In the dilemma, I decided to follow DB even if there are many problems in it since it is easier to integrate with frontend with newly created APIs than to fix DB of track 2. However, while following DB, I did my best to change endpoints as least as I can. Therefore, in this pull-request, there are lots of changes in the format of inputs and outputs of endpoints.

@aj8uppal - Please read the changes and the real codes carefully by comparing the previous version with this change. I changed a lot, so there must be you did not expect. Please review carefully. @TsarFox - I added a field userID in JWT token. I don't know it is safe to do that, but we need a method to find the primary key of a user (id or email) with a given token. If there is another method please tell me.

Changes

  1. page-group

    Each page has field order and each page needs to be added or deleted by this unit. So, I created wrapper functions of DB access points wrapping in terms of order. These wrapper functions are in server/db/page-group directory.

    Also, I created endpoints for each page-group. The URL path of the endpoints only offer GET method to read data, and POST method to create data of each page group. All the endpoints and wrapper functions return JSON object formatted like this:

    res.json({
})
    • [pageOrder.PLAIN]: a page object retrieved from table pages in DB.
    • [pageOrder.PROMPT]: an array of prompt objects retrieved from table prompt in DB. If a page group of this endpoint does not contain prompt in nature, this value is null. This is different from a page group with no prompts which can have prompts in the future (In this case, the value is an empty array).
    • [pageOrder.MCQ]: a mcq object retrieved from table mcq in DB. In the same manner of [pageOrder.PROMPT], it can have null value.
    • [pageOrder.CONV]: a conversation_task object retrieved from table conversation_task. In the same manner of [pageOrder.PROMPT], it can have null value.

  2. User(instructor) only can access scenarios she/he instructs

    Previously, any user can access any scenario. I change this by adding userID to JWT token on signing in, and retrieving it from the token when accessing scenarios.

  3. Endpoints for DB components

    The problem of the goal of following DB is that frontend may not use the proper APIs it needs. To prevent this situation, I created endpoints for all the DB components. By these endpoints, frontend can read or update data. However, APIs for creation and deletion are not offered since creation and deletion must be controlled by a group (ex. group of pages, group of conversations). By component APIs, frontend team can be offered most of the APIs it needs.

  4. issue coverage matrix

    I found that the format of inputs and outputs our team's endpoint for the issue coverage matrix cannot be compatible with the current DB design. So I created new formats for the matrix. For the inputs and outputs, please check route/pages/issue-coverage-matrix.js file.

TsarFox commented 3 years ago

@TsarFox - I added a field userID in JWT token. I don't know it is safe to do that, but we need a method to find the primary key of a user (id or email) with a given token. If there is another method please tell me.

That's fine, don't worry.

Ostensibly, the database shouldn't need to be storing information retrieved via the SAML callback, but that's a poor decision the other team made and we have to deal with it now.

We should be able to look up the users ID with their email.

TsarFox commented 3 years ago

So far, this looks okay. I'll make sure you didn't accidentally remove any of our endpoints with your refactoring before I sign off on it.

Endpoints for DB components

The problem of the goal of following DB is that frontend may not use the proper APIs it needs. To prevent this situation, I created endpoints for all the DB components. By these endpoints, frontend can read or update data. However, APIs for creation and deletion are not offered since creation and deletion must be controlled by a group (ex. group of pages, group of conversations). By component APIs, frontend team can be offered most of the APIs it needs.

This is probably fine to leave in, but for the frontend team to be able to use these new endpoints, they would need to understand what endpoints there are and what they do. These aren't in the documentation we handed them. Furthermore, the frontend shouldn't need to know how the database is implemented.

hoon0422 commented 3 years ago

So far, this looks okay. I'll make sure you didn't accidentally remove any of our endpoints with your refactoring before I sign off on it.

Endpoints for DB components The problem of the goal of following DB is that frontend may not use the proper APIs it needs. To prevent this situation, I created endpoints for all the DB components. By these endpoints, frontend can read or update data. However, APIs for creation and deletion are not offered since creation and deletion must be controlled by a group (ex. group of pages, group of conversations). By component APIs, frontend team can be offered most of the APIs it needs.

This is probably fine to leave in, but for the frontend team to be able to use these new endpoints, they would need to understand what endpoints there are and what they do. These aren't in the documentation we handed them. Furthermore, the frontend shouldn't need to know how the database is implemented.

True. I created those endpoints since we do not have much time and it may be impossible to offer decent APIs they need. So my strategy is to give low-level APIs that can access DB data without creating new data or deleting data which may cause unknown problems.