feat: Issue24 registration api backend

[mtreacy002]

Description

Create BIT API Users service with Register functionality so that a new user can register to BIT.

Fixes #24 and replacement of PR #25
NOTE: As this PR also carries code changes on PR #25, the changes needed in related to PR#25 is only made to this PR (#26). The PR#25 is to be closed without merging.

Type of Change:

• Code
• Documentation

Code/Quality Assurance Only

• New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Tested sending POST /register request to MS API with successful response.

TO DO:

• modify Register high-fidelity mockup to just having MS input fields and remove BIT fields. (Confirm with Mentors)

Checklist:

• My PR follows the style guidelines of this project
• I have performed a self-review of my own code or materials
• I have commented my code or provided relevant documentation, particularly in hard-to-understand areas

Code/Quality Assurance Only

• My changes generate no new warnings
• My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected) - hence the TO DO above

[mtreacy002]

Note: This is not a final work. The TO DO statement needs to be confirmed with mentors before moving forward. It'll clear issue with getting user_id on registration form mentioned here and here

[mtreacy002]

update:
Hi @anitab-org/bridgeintech-maintainers.
Here's BIT Heroku link that is connected to BIT db (has both BIT and MS schemas). This link can be used as BIT remote server/API to test BIT development.

https://bridge-in-tech-bit-test.herokuapp.com

Important: Currently the db has mock data populated using add_db_mock.py as initial setup (so not using MS/BIT API). This line of code MUST be commented out for the rest of GSoC since it will mess up Heroku and its elephantSQL instance otherwise.

As for the MS API Heroku that is connected to MS db (only has MS schema), below is the link for it

https://bridge-in-tech-ms-test.herokuapp.com/

The ms heroku link above is only going to be used as the base_ms_api_url for BIT API. No UI interaction should be made using this ms heroku link except for the purpose of debugging.

[mtreacy002]

Also @ramitsawhney27 , for Wrong password and/or username, MS is returning 404 instead of 401. I put 401 in BIT. But should I open an issue on MS backend so someone can change it there?

[mtreacy002]

Update:
Hi @anitab-org/bridgeintech-maintainers. So, I've made the changes requested but will wait for further feedback before changing error code 401 HTTPStatus.Unauthorized and print to console line.

[meenakshi-dhanani]

Check out the comment on this - #15

[mtreacy002]

Update: @anitab-org/bridgeintech-maintainers
added pylint and .pylintrc to check code quality.

Now it's 9.96/10 🎉

[mtreacy002]

PS: please note that the recent code change is including .travis.yml and deploy.sh files

[meenakshi-dhanani]

Maya can you please move Travis changes to its own issue?

[isabelcosta]

@mtreacy002 do you understand this change regarding comment #26 (comment)?

@ramitsawhney27 For the sake of time and planning, could we have this change as technical debt, as an issue to be taken up later? either my Maya or any other contributor. I'm saying this because of the planning set out for Maya for GSoC. There were some changes here that we didn't predict with baggage coming from MS code, so I see no problem with leaving some changes for other issues.

[SanketDG]

@mtreacy002 Could you kindly resolve all the comments above that are not relevant anymore? This helps in other people go through and review the PR smoothly.

[mtreacy002]

@mtreacy002 Could you kindly resolve all the comments above that are not relevant anymore? This helps in other people go through and review the PR smoothly.

@SanketDG Are you sure I'm allowed to do that (mark comments as resolved)? Wouldn't the mentor need to give their approval and state it as resolve? I mean, I've done the changes but the mentor needs to confirm they are happy with the changes I've made. cc @ramitsawhney27

[mtreacy002]

Update @anitab-org/bridgeintech-maintainers .

I've fixed the http error responses:

• error code 500 for Internal server and Connection errors:
  

• error 400 when email is invalid:

Note that for error 409: Conflict, as the PR#620 on mentorship-backend that will fix the error code returned is still in process, atm it is still returned as 400: Bad request

[mtreacy002]

PS @anitab-org/bridgeintech-maintainers . I'm not sure how to write test case for Register API.

I noticed on mentorship backend this is done by querying the database directly.

But for BIT, this shouldn't be the case since BIT is only allowed to send request to MS API not querying database directly.

The issue is, how do I assertTrue random API responses that might be returned by MS API??

Can mentors please help give me some advice? Ta

[meenakshi-dhanani]

I have a couple of points:

• We are using pylint, to ensure your code is linted, you should add it to your Travis script (Can be done as another issue if you like)

• We are running tests, by actually setting up another database for test, against which a test will run. There are usually a lot of embedded db options, you shouldn't have to work so hard to run tests. (This can be a separate concern as well)

• So I ran the app on my local machine, and checked all the tables being created but I think none are being used as of now. I won't say much since you have already completed this work. But I'm not in favor of writing code that is unused, if we're working in an incremental fashion then you add things as and when you need them. I don't see any other tables being used as far as registration is concerned. Hence this PR is huge.

• Also a question, so when registration does become successful, does nothing get persisted in any of the BIT tables? Does it just save in MS?

• Last question, again since I have joined midway I'm slightly confused why do we have two schemas? one in public and one in bitschema? I might need to get on call with @mtreacy002 to understand this.

Since, I have a lot of basic questions, I feel I need to get up to speed on them before I add in my approval. However, @foongminwong if you feel all looks good, you can merge. I can catch up meanwhile.

[mtreacy002]

Hi @meenakshi-dhanani . Thanks for sharing your views. I'll try to answer them the best I can. But since I'm also a newbie and have no real-world exposure to the common/best practice, @ramitsawhney27 probably will be able to give you better advice on things related to db questions).
Anyway, here's my responses:

We are using pylint, to ensure your code is linted, you should add it to your Travis script (Can be done as another issue if you like)

Yup, agree, going to look at that on a separate issue

We are running tests, by actually setting up another database for test, against which a test will run. There are usually a lot of embedded db options, you shouldn't have to work so hard to run tests. (This can be a separate concern as well)

will look around for the options. Can you also help give me the link if you find one? Ta

So I ran the app on my local machine, and checked all the tables being created but I think none are being used as of now. I won't say much since you have already completed this work. But I'm not in favor of writing code that is unused, if we're working in an incremental fashion then you add things as and when you need them. I don't see any other tables being used as far as registration is concerned. Hence this PR is huge.

I understand your point. With my limited knowledge (only based on what I've learned so far from uni/self-learning), there are few things I consider when working with database:

1. database development is one of the things that is hard to adopt the agile process unlike other areas of software development (the changes to its schema is not something we can easily add on the fly).
2. Especially with the type of data we want to manage (persistent data instead of fluid), we have no choice but to design database in the fullness (trying to capture the potential scenarios which leads to what attributes needed along with its normalisation and what constraints to be applied right from the get go (on the initial planning stage).
3. With planning, comes proof of concept before we can start our development. Hence, all the tables in the schema need to be created with constraints (relationship) implemented which then need to be populated with mock data to test if the relationship/design work as we plan in the "smaller" scale. For this project this test of concept was done using db_add_mock.py followed by querying the database to see if the tables can talk to each other (within its own schema or between different schemas).

I'll try to answer the last 2 questions in one go:

Also a question, so when registration does become successful, does nothing get persisted in any of the BIT tables? Does it just save in MS?

Last question, again since I have joined midway I'm slightly confused why do we have two schemas? one in public and one in bitschema? I might need to get on call with @mtreacy002 to understand this.

First of all, the reason why we have 2 schemas MS and BIT is because we want the two system to be loosely coupled (able to run independently to some extent) in particular since MS is so close to production. Changing the database directly inside MS schema and white-label the project to adopt the features of BIT is not an ideal option at this time.

As discussed at the planning stage with issue#10, at the end of GSoC, what we're trying to achieve is having 2 APIs (BIT+MS) and one database (still with 2 schemas). However, with the challenges stated in my above answer (not easy to add columns/tables on the fly when persistent data involved) we need to take extra care not to disrupt MS development.

The unavoidable dependent of BIT towards MS development happens since the heart' of BIT is the MS system which is mentorship relation (meaning any features of MS would ideally be adopted to BIT features). Additional value of BIT is that it brings organizations participation into the playing field through offering mentoring programs so upskilling IT personnels no longer only sole responsibility of individuals (mentors and mentees) but also organizations.

For the BIT MS to be loosely coupled with the 2 APIs and one db scenario we're trying to achieve, MS code base also needs to be aware that BIT schema exist (this means adding BIT models inside MS repo) although the interactions between the 2 only will be done through sending request to each other's API. Again, this unlikely to happen immediately without disrupting MS development or GSoC schedule (aka cross-project issues - needs to open issues on MS repo for someone to work on this BIT schema injection and finally, MS database migration to become one db with BIT)

That's also why, in the initial stage of GSoC, we'll make do with having 2 db's and later/gradually during GSoC, we'll look at how we're going to tackle this (merge the dbs). Even though it means with Register atm we only saving data in MS db, not on BIT db, but the 2 schemas need to be there at the beginning since it is the db design we want to achieve and with design on persistent data, we aim to follow "waterfall" principal rather than "agile" so not to make too many changes on the design which will make data management messy.

Hopefully this helps answer your questions. Again, I have no experience in this matter and only based my opinion from uni and self-learning. But @ramitsawhney27 probably can share his point of view on this too. 😉

Hope this answers

[foongminwong]

The changes made in this PR were tested locally. Following are the results:

1. Code review - Done by @ramitsawhney27 @meenakshi-dhanani

2. All possible responses were tested as below:

• Testing PR 26
  Screenshot/gif/url:
  

  Expected Result: As a user, I should be able to register on BIT.
  Actual Result: Failed ⚠, probably I setup the local backend incorrectly.

3. Additional testcases covered: N/A

4. Additional Comments/ Questions:

• I probably miss something when setting up the BIT backend locally, which might lead to the errors shown in the GIF, also written below (Trying to fix now @mtreacy002 ):
  • The 🔴 red error 🔴 at the top that I'm having is Resolver error at paths./register.post.parameters.0.schema.$ref Could not resolve reference because of: Could not resolve pointer: /definitions/User%20registration%20model does not exist in document
  • The error that I got when trying to register a user is 500 Error: INTERNAL SERVER ERROR - AttributeError: 'NoneType' object has no attribute 'json'

5. Quick overview of tables under bit_schema on pgAdmin:
   

6. Input for registration testing:

{
  "name": "vibere",
  "username": "vibere",
  "password": "123456789",
  "email": "vibere6497@psk3n.com",
  "terms_and_conditions_checked": true,
  "need_mentoring": false,
  "available_to_mentor": true,
  "is_organization_rep": true,
  "timezone": "CENTRAL TIME"
}

7. Status of PR Changed to: N/A

8. OS Version: Windows 10

[meenakshi-dhanani]

About this point:
database development is one of the things that is hard to adopt the agile process unlike other areas of software development (the changes to its schema is not something we can easily add on the fly).

Unlike the belief or what is taught in uni, we can create the database on the fly. We do that with incremental migrations. This article will help think in that manner - https://martinfowler.com/articles/evodb.html
This is just to highlight there are ways to work incrementally. The article hopefully should give you an insight. Having said that, I am not hinting you change the way you've begun working. But this is just feedback and some learning I guess.

[mtreacy002]

Hi @foongminwong , just wondering.
Our registration no longer need the BIT schema fields for user_extension based on my update posted on issue#2 and issue#24 which was approved by mentors.
So, if you have the input as per your screenshot above

perhaps you've pulled code was outdated and you need to pull the latest code from the PR?

The latest code in the PR should have the similar fields on Swagger UI to the one for registration on MS API

[meenakshi-dhanani]

@mtreacy002 so about the 2 dbs, I get it as a temporary solution for maybe GSoC. So for this story, I tested with sending a POST request to create user. Should there be an entry in the MS tables for a user at least? I am getting the correct responses from the API, I just want to validate where the record is stored.

[mtreacy002]

@meenakshi-dhanani. You can check it by querying directly to the MS db you run in parallel with the BIT API when you did the test (I ran my own MS API on local port 4000). You can do this either on psql shell or pgadmin4 whichever one you prefer.

[foongminwong]

The changes made in this PR were tested locally again. Following are the results:

1. Code review - Done (see previous code reviews from other mentors)

2. All possible responses were tested as below:

   • Register a new user on BIT
     Screenshot/gif/url:
     

   Expected Result: As a user, I can register under BIT.
   Actual Result: Same as expected. After registering, the user will receive an automatic email sent by MS for account verification. The user is also able to login into MS with the created account credentials.

3. Additional testcases covered:

   • Register a user with existed username on BIT
     Screenshot/gif/url:
     

   Expected Result: It should throw an error A user with that username already exists.
   Actual Result: Same as expected/

   • Register a user with invalid password (password length less than 7 characters) on BIT
     Screenshot/gif/url:
     

   Expected Result: It should throw an error The password field has to be longer than 7 characters and shorter than 65 characters.
   Actual Result: Same as expected.

   • Login into MS using the account created on BIT
     Screenshot/gif/url:
     

   Expected Result: As a user, I can login into MS using the account credentials created in BIT.
   Actual Result: Same as expected.

4. Additional Comments:

   • Ready to Merge ✔ @anitab-org/bridgeintech-maintainers ?

5. Input for registration testing:

{
  "name": "vibere",
  "username": "vibere",
  "password": "123456789",
  "email": "vibere6497@psk3n.com",
  "terms_and_conditions_checked": true,
  "need_mentoring": true,
  "available_to_mentor": true
}

6. Status of PR Changed to: N/A

7. OS Version: Windows 10

[ramitsawhney27]

@meenakshi-dhanani - do you have any additional comments, or are we good to merge?

[mtreacy002]

Thank you @foongminwong for your detailed tests. And congratulations on your successful setup 🎉

[meenakshi-dhanani]

Had a call with Maya to validate if the call to API is adding the records in the MS database. It works.