Planning: Higher level API endpoints and CRUD functions about bridge-in-tech-backend

[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]

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.

[mtreacy002]

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