Planning: Higher level API endpoints and CRUD functions

[mtreacy002]

As per proposal:

Higher Level API (CRUD operations)

REST API endpoints examples

POST /user/personal_background
=> this will add personal background information to the user profile.
GET, PUT, DELETE /user/personal_background/{user_id}
=> this will get/update/delete personal background information of the user which id is specified.
GET /users/find_by_physical_ability/{physical_ability}
=> this will get a list of all users which have the physical ability match the type specified in the ‘physical_ability’ passed as parameter.
GET /users/find_by_criteria/{criteria}
=> this will get a list of users that met the specific search criteria set in ‘criteria’ parameter which is a map of key-value (or dictionary). This way we don’t need to write an extensive list of GETs to filter users by each of their attributes (such as GET /users/find_by_physical_ability).
POST /company
=> this will create a company.
GET, PUT, DELETE /company/{company_id}
=> this will get/update/delete the details of the company which id is specified.
GET /companies
=> this will get the list of all companies.
GET /companies/{company_id}
=> this will get the company which id is specified.
GET /companies/find_by_country/{country}
=> this will get a list of companies in the specified country.
GET /companies/find_by_criteria/{criteria}
=> this will get a list of companies that met the specific search criteria set in ‘criteria’ parameter which is a map of key-value (or dictionary).
POST /program
=> this will create a program.
GET/PUT/DELETE /program/{program_id}
=> this will get/update/delete the details of the program with the specified id.
GET /programs
=> this will get the list of all programs.
GET /programs/{program_id}
=> this will get the program which id is specified.
GET /programs/find_by_contact_type/{contact_type}
=> this will get a list of programs that have contact_type specified by the parameter.
GET /programs/find_by_criteria/{criteria}
=> this will get a list of programs that met the specific search criteria set in ‘criteria’ parameter which is a map of key-value (~ dictionary).
POST /mentorship_relation/send_request
=> this will create a mentoring relation request as well as send email notification to recipient about MR request
GET, DELETE /mentorship_relation/{mentorship_relation_id}
=> this will get/delete the mentorship_relation with the specified id.
GET /mentorship_relations
=> this will get the list of all MRs.
PUT /mentorship_relations/{mentorship_relation_id}
/accept
=> this will allow the Company/Mentor/Mentee to accept an MR request.
PUT /mentorship_relations/{mentorship_relation_id}
/reject
=> this will allow the Company/Mentor/Mentee to reject an MR request.
PUT /mentorship_relations/{mentorship_relation_id}
/cancel
=> this will allow the Company/Mentor/Mentee to cancel an MR request.
GET / mentorship_relations/current
=> this will get the list of current MR regardless of their ‘state’ for the logged in user.
GET / mentorship_relations/find_by_criteria/{criteria}
=> this will get the list of MRs based on the ‘criteria’ passed on as the parameter.

[mtreacy002]

Hi @anitab-org/bridgeintech-maintainers, below is the screenshot of what I have in mind for API endpoints and their related CRUD operations in relation to Users API service. Please give me feedback to whether or not this makes sense to you all.

Link to file

Note: I haven't decided on the API endpoints for Home and Dashboard yet because it'll come after I get more picture on the whole API endpoints

[ramitsawhney27]

For the initial few endpoints, as you start implementing them, you should be documenting the error codes. For example: 400 bad request - and when would you return this?

[mtreacy002]

Update:
Hi @anitab-org/bridgeintech-maintainers. Here's the updated API endpoints and error handlings related to Users service

[ramitsawhney27]

These look good. What if there is an internal server error?

[mtreacy002]

I'm thinking to mask it with response 500 but display the error message as it would catch by the HTTPError exception. As mentioned in the below

Would this be Ok, @ramitsawhney27? If you agree to this, I would add error code 500 on it as well with unspecified message (coz it'll be whatever returned by the response)

[mtreacy002]

Update:
I've added error response 500 in all API requests because of the possibilities of internal error can happen anywhere in the request.
Below just an example of what I've added.

[meenakshi-dhanani]

Hey Maya,
Everything looks good. Just a question to think about the login api. So if there's any issue with authentication it makes sense to return a 401. Find explanation in the RFC here.

And in case the credentials a wrong, if you send back a 404, it might give a tip to the malicious users, to try combinations. A 401 should work for any invalid credentials.

[mtreacy002]

Hi @meenakshi-dhanani. That also makes sense to me, tbh (although I'm not sure about the 404 giving more tip to malicious user more than 401 does, 😃). However, in terms of error code, as BIT is using MS API service for this one (login), I don't know if it's a good idea to change the error code from 404 (given by the MS API) to 401 on BIT UI to client. Shouldn't BIT only passing whatever value is returned by MS API?
@ramitsawhney27 and @isabelcosta, is there any reason to use 404 instead of 401 on MS API on this matter? Is BIT allowed to change this?

[meenakshi-dhanani]

Couple of points: 401 can take care of any invalid credentials (credentials do not exist, or they are invalid). So 401 can be a blanket status code for that. If there is another response, say 404. Assuming I am a malicious user, I attempt to login with some credentials, if I get a 404 I know that these do not exist, so I'll try with another pair of credentials and might get a 401 and then I can continue and try till I get the right password for it. Since, 401 can take care of invalid I say we return only a 401 for any issue with credentials. Does that make sense?

…

On Sat, May 30, 2020 at 3:20 AM Maya Treacy ***@***.***> wrote: Hi @meenakshi-dhanani <https://github.com/meenakshi-dhanani>. That also makes sense to me, tbh (although I'm not sure about the 404 giving more tip to malicious user more than 401 does, 😃). However, in terms of error code, as BIT is using MS API service for this one (login), I don't know if it's a good idea to change the error code from 404 (given by the MS API) to 401 on BIT UI to client. Shouldn't BIT only passing whatever value is returned by MS API? @ramitsawhney27 <https://github.com/ramitsawhney27> and @isabelcosta <https://github.com/isabelcosta>, is there any reason to use 404 instead of 401 on MS API on this matter? Is BIT allowed to change this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADM6DFMU27RUEIR4HCIOKMLRUAU27ANCNFSM4M5SYKWA> .

[mtreacy002]

Update:
Here's the API endpoints and errors handling for Organizations service

[mtreacy002]

Update:

So, Here're the latest version for API endpoints

User service

[mtreacy002]

Update:

Organizations service

[mtreacy002]

Update:

Programs service

[mtreacy002]

Update:
API endpoints for Mentorship_Relations service

[ramitsawhney27]

Hi @mtreacy002, your error codes and specs look great. Thank you for the great work with them. One small suggestion, could we use "201-Created", rather than "200-OK" as the error code if something new is created? This is as per our previous discussions in weekly meetings.

[mtreacy002]

Sorry, haven't updated the docs yet, but have actually applied your suggestion in the code base for User Registration (PR#26).

Will change the docs now... 😉

[mtreacy002]

Hi @anitab-org/bridgeintech-maintainers. I have updated the docs for the REST API Endpoints and errors handling here. Also, I highlighted 18 error codes that I think would require changes to be done on Mentorship System backend side. This is because if we change it on MS backend, it only needs to change the word selection on HTTPStatus itself whereas if we want to keep MS as it is and mask the codes on BIT side, we would need to write extra lines of codes (e.g. if condition and even sometimes doing for loop) for each of the error points.
Please let me know what you think.

[meenakshi-dhanani]

Ideally, if MS has the logic of the API, then MS error codes need to be appropriate. Let's open issues on MS, if we can pick them up because this affects BIT too, let's do that. If we don't fix MS, there will be some unnecessary code on BIT, which might have to be removed once MS is fixed, so let's fix the source(because we can). Create them as issues on MS I say, if we can let's help someone who picks it up or let's pick it up. (Don't be blocked by it though)

[mtreacy002]

I agree with @meenakshi-dhanani .
@ramitsawhney27, @isabelcosta and @foongminwong. would you be ok if I open the issues on MS backend to fix the misrepresented http error codes?
Note that the 18 highlighted error codes that need to be replaced must be represented in separate issue (individually), or at least per function (e.g. user register function) so not to block the BIT development coz the changes can be done gradually.

[mtreacy002]

Update @anitab-org/bridgeintech-maintainers . Closing this as discussion on API endpoints will be ongoing on BridgeInTech Proposal Review doc