Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Issue#62 update user api endpoint #74

Merged

Conversation

mtreacy002
Copy link
Member

@mtreacy002 mtreacy002 commented Jul 9, 2020

Description

Create PUT /user/personal_details api endpoint and test cases

Fixes #73 as sub-issue of #62

Type of Change:

  • Code

Code/Quality Assurance Only

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

How Has This Been Tested?

  • run the app manually with ms-bitschema-backend-server as MS server and this PR branch as BIT backend server.
  • As this is the first time you connect to ms-bitschema-backend-server, you must create a new user first, login then you can update the user profile.

User profile before update (called using GET/user/personal_details):
Screen Shot 2020-07-09 at 9 04 40 pm

NOTE: with the current bug reported on issue #596 MS backend, I don't have any other option but to update ONLY the username.

Successful update message (on PUT/user/personal_details):
Screen Shot 2020-07-09 at 9 05 11 pm

Check to make sure user profile gets updated (by calling GET/user/personal_details):
Screen Shot 2020-07-09 at 9 05 31 pm

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

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)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@mtreacy002
Copy link
Member Author

Note @anitab-org/bridgeintech-maintainers .
There is a bug currently causing the internal server error. This is caused by MS schema User model has organization as one of the field with string data type.

Screen Shot 2020-07-09 at 4 11 54 pm

The probleem is BIT schema uses Organization table link to MS User table as FK. So MS User model should not have organization stated as one of the field. This confused SQLAlchemy and raise the following error:

Screen Shot 2020-07-09 at 4 04 09 pm

I'll try to rename organization field on User model to something else (like current organization and see if that change the outcome.

@mtreacy002 mtreacy002 force-pushed the issue#62-update-user-api-endpoint branch from e8b9165 to 4df201a Compare July 9, 2020 11:13
@mtreacy002
Copy link
Member Author

Update @anitab-org/bridgeintech-maintainers .
I've fixed the bug by doing the followings:

  • removed migration script (at the moment it's not quite right so will revisit later in the future as part of nice to have/future feature
  • renamed organization field on user model to current_organization (incl any place where it's being used)
  • refactored MS backend code to accommodate the field name changed.

This PR is now complete and ready to be reviewed.

🔊 Note for reviewer:

If you'd like to test this Update user personal details manually, you need to run MS server from ms-bitschema-backend-server branch from my fork repo. This branch already linked to the same database as BIT server (MS+API REST APIs and one database structure as the main target of the project)

@mtreacy002 mtreacy002 requested review from ramitsawhney27 and a team July 9, 2020 11:31
@mtreacy002 mtreacy002 self-assigned this Jul 9, 2020
@mtreacy002 mtreacy002 added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Program: GSOC Related to work completed during the Google Summer of Code Program. labels Jul 9, 2020
@mtreacy002 mtreacy002 changed the title WIP: Feat: Issue#62 update user api endpoint Feat: Issue#62 update user api endpoint Jul 9, 2020
@mtreacy002 mtreacy002 force-pushed the issue#62-update-user-api-endpoint branch 3 times, most recently from 8ee18a7 to c50a12b Compare July 9, 2020 12:52
@mtreacy002 mtreacy002 force-pushed the issue#62-update-user-api-endpoint branch from c50a12b to 2de1a87 Compare July 9, 2020 13:14
@mtreacy002
Copy link
Member Author

mtreacy002 commented Jul 9, 2020

Update @anitab-org/bridgeintech-maintainers .
I've fixed the bug by doing the followings:

  • removed migration script (at the moment it's not quite right so will revisit later in the future as part of nice to have/future feature
  • renamed organization field on user model to current_organization (incl all other places where it's being used)
  • refactored MS backend code to accommodate the field name changed.

This PR is now complete and ready to be reviewed.

🔊 Note for reviewer:

If you'd like to test this Update user personal details manually, you need to run MS server from ms-bitschema-backend-server branch from my fork repo. This branch already linked to the same database as BIT server (MS+API REST APIs and one database structure as the main target of the project)

@isabelcosta and @anitab-org/bridgeintech-maintainers . The renaming User table organization field atm only done on my copy of MS backend. However, to make it sync, the actual MS repo will also need to be adjusted. I'm aware it's not ideal to change anything in the current MS tables especially since it's so close to production already.
The other option (rather than changing column field on MS User table), is to change back BIT Organization table name to Company. Otherwise, I cannot think of another way to solve this.

@foongminwong foongminwong merged commit 5421677 into anitab-org:develop Jul 14, 2020
@foongminwong
Copy link

The gif was too large to upload, this PR was tested successfully.

https://giphy.com/gifs/U1mItXgfG1MEKsuYsk (a little blurry)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Program: GSOC Related to work completed during the Google Summer of Code Program.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dev: Update user's Personal Details
3 participants