search

ELTE-Clean / Clean-Administration-Platform

2 stars 1 forks source link

Add main database endpoints #55

Closed the4t4 closed 2 years ago

the4t4 commented 2 years ago

Added the following endpoints:

None of them have been tested yet because the frontend is failing the application. The database has been updated, but not yet completely to match the requirements of #52

the4t4 commented 2 years ago

@Abdulla-Alkhulaqui the frontend is failing to build with this error:

9 45.62 npm ERR! code EBADPLATFORM

9 45.64 npm ERR! notsup Unsupported platform for @next/swc-win32-ia32-msvc@12.0.10: wanted {"os":"win32","arch":"ia32"} (current: {"os":"linux","arch":"x64"})

9 45.64 npm ERR! notsup Valid OS: win32

9 45.64 npm ERR! notsup Valid Arch: ia32

9 45.64 npm ERR! notsup Actual OS: linux

9 45.64 npm ERR! notsup Actual Arch: x64

Do you have any idea what this is?

Mohido commented 2 years ago

@the4t4 Just three main things to point out: 1) The first one is we still need the exports for all the utils functions since the backend can utilize whatever it needs from this database_utils.ts. 2) The call of a promised function in the utility requires the await keyword before it since they return promises. Promises can be thought of as individual threads. The behavior of the database endpoints that you defined is unpredictable. So for instance, in insertIntoTable('section', req.body); and the line underneath it: res.status(200).send(JSON.stringify({message: "New section added"}));. This will return status 200 no matter if the insertIntoTable() is valid or not. The paradigm I use is that I always return a result (QryResult) that has the result or the error. 3) In the future, there must be validation layers such that a demonstrator, only, can add a section to his assigned group and no other group. However, the current getUserKeycloak is not returning the roles. Therefore, I skipped that part. However, if you want to make sure that this is almost complete, you need to add at least a validation layer that the user (no matter the role for now) can alter only his specified groups/sections/tasks.

the4t4 commented 2 years ago

@Mohido

  1. I exported them again but I don't see the point since we added the layer of abstraction. I don't see any use case for execQry execTrans etc because now we have a better method for them which handles everything.
  2. Yeah I missed to put await/async calls to for the select statements but for the others, it will return 200 no matter what as I have pointed out in my other PR.
  3. I have left the validation out because these endpoints aren't even tested yet.
Mohido commented 2 years ago

@the4t4 1) The most useful use-case would be if we need to implement some other utility that relies on this utility to function. That utility will be in TS since its the standard for implementing our utilities (more robust). Other things are minor use-cases. However, for me I would love to export everything in the utilities to the backend and the backend developer can deal with it (Unless it is a secret configuration maybe?) . Having this standardized help the other developers that will come after us.

2) Yes you are correct regarding that one. It will always return 200. Not a big deal for now, since the front-end was off for testing

3) Gotcha mate

the4t4 commented 2 years ago

@Mohido ready for review

Mohido commented 2 years ago

For now, this looks all fine. I have no comments other than the distribution of the API endpoints must be function-specific and not server-application-specific.. So we only have: 1 file for /users/ and 1 file for /tasks/ and another one for /sections/ Instead of db.js. However, code ordering can come in the next PR. For now let's merge this one to the development branch.