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.

Sign up for GitHub

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

Jump to bottom

Feat: Update user personal background #61

Merged
merged 5 commits into from
Aug 10, 2020
Merged

Feat: Update user personal background #61

merged 5 commits into from
Aug 10, 2020

Conversation

mtreacy002
Copy link
Member

@mtreacy002 mtreacy002 commented Aug 2, 2020 •
edited
Loading

Description

Allow user to update their personal background

Fixes #49

Type of Change:

  • Code
  • User Interface

Code/Quality Assurance Only

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

How Has This Been Tested?

  • login then go to My Space > 'Personal Details' > Personal Background
  • If this is the first time user view the Personal Background page, it will show the message below:

Screen Shot 2020-08-02 at 10 08 40 pm

  • Fill in the personal background information. On successful process, see message below

Screen Shot 2020-08-04 at 8 34 52 am

If user click Save button without selecting anything, the default value is saved "Prefeer not to say"

Screen Shot 2020-08-04 at 8 35 19 am

  • Now change the details and on successful update see message below

Screen Shot 2020-08-04 at 8 35 45 am

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
  • New and existing unit tests pass locally with my changes

foongminwong
foongminwong reviewed Aug 2, 2020
View reviewed changes
Copy link

@foongminwong foongminwong left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mtreacy002 , I was able to login and go to the Personal Background page.

test-pr-61-server-unavailable-when-login

However, it shows an error that The server is currently unavailable. Try again later after successful login while running both MS & BIT locally.

Prior to this, I did populate the personal background for user bopogow on Swagger UI (see below). So when I click on the Personal Background page, I should expect the page should have the populated information. Hmm any fix on this?

test-pr-61-already-populate-personal-bg

Other: Remember to resolve the conflicting files below

@mtreacy002 mtreacy002 force-pushed the issue49-update-personal-background branch from af05496 to 227d1a1 Compare August 2, 2020 14:12
@mtreacy002
Copy link
Member Author

mtreacy002 commented Aug 2, 2020 •
edited
Loading

Hi @foongminwong. I have resolved the merge conflict issue. Can you please try again by pulling the latest code change? Maybe that will fix the issue? Also, you don't need to create the personal background on backend bcoz the frontend should be able to create or update using the same Save button.
Please let me know how you go.

@mtreacy002
Feat Update user additional info for Timezone
7c87442 
add default value to is_organization_rep

Fix if else and fetch bug

Fix POST and PUT additional info

Fix bug defaultChecked checkbox is_organization_rep

fix bug timezone not selected

Refactor api requests to PUT for both create and update additional info
@foongminwong
Copy link

foongminwong commented Aug 3, 2020 •
edited
Loading

Hi @foongminwong. I have resolved the merge conflict issue. Can you please try again by pulling the latest code change? Maybe that will fix the issue? Also, you don't need to create the personal background on backend bcoz the frontend should be able to create or update using the same Save button.
Please let me know how you go.

@mtreacy002 Hmm I pulled the latest codes but I still have the error The server is currently unavailable. Try again later for a user who has created their personal background (bg) on the backend.

I created the personal bg for user bopogow on the backend a while ago when tested this PR, so as a user, I assume their personal bg automatically GET from backend, when I clicked Personal Background on the frontend. Let me know your thoughts!

@meenakshi-dhanani
Copy link
Contributor

Inside the Personal Background file, if you have already checked a is_public and the other value, then for any of the other fields, if !value then PreferNotToAnswer.
you don't need to do that if with the other too many fields.

@mtreacy002 mtreacy002 force-pushed the issue49-update-personal-background branch from 227d1a1 to 14dc1a7 Compare August 3, 2020 22:39
@mtreacy002
Copy link
Member Author

Hi @foongminwong. I have resolved the merge conflict issue. Can you please try again by pulling the latest code change? Maybe that will fix the issue? Also, you don't need to create the personal background on backend bcoz the frontend should be able to create or update using the same Save button.
Please let me know how you go.

@mtreacy002 Hmm I pulled the latest codes but I still have the error The server is currently unavailable. Try again later for a user who has created their personal background (bg) on the backend.

I created the personal bg for user bopogow on the backend a while ago when tested this PR, so as a user, I assume their personal bg automatically GET from backend, when I clicked Personal Background on the frontend. Let me know your thoughts!

@foongminwong, that should be the case (if data already there). I'm not sure why you have this issue since I wasn't able to replicate the bug at my end. Do you have time sometime today to meet on hangout to troubleshoot this with me? Leet me know when you're available.

@mtreacy002
Copy link
Member Author

Inside the Personal Background file, if you have already checked a is_public and the other value, then for any of the other fields, if !value then PreferNotToAnswer.
you don't need to do that if with the other too many fields.

@meenakshi-dhanani . The reason why I need to specify each enum fields since there are only 10 out 19 fields has default value "Prefer not to say". The other 8 default value is empty string. The is_public is the only boolean value.
Screen Shot 2020-08-04 at 8 35 19 am

@mtreacy002
Feat: View members list
5ab20f7 
Add availabilities column

add tba on Program completed and Current Availability columns

Fix service unavailable constant
@mtreacy002
Feat: update user personal background
63f8956 
Refactor POST PUT call to PUT only
@mtreacy002 mtreacy002 force-pushed the issue49-update-personal-background branch from 14dc1a7 to 63f8956 Compare August 4, 2020 03:17
@meenakshi-dhanani
Copy link
Contributor

Inside the Personal Background file, if you have already checked a is_public and the other value, then for any of the other fields, if !value then PreferNotToAnswer.
you don't need to do that if with the other too many fields.

@meenakshi-dhanani . The reason why I need to specify each enum fields since there are only 10 out 19 fields has default value "Prefer not to say". The other 8 default value is empty string. The is_public is the only boolean value.
Screen Shot 2020-08-04 at 8 35 19 am

What is the case for the other fields, in case their value is not present? I'm trying to see if there's a different answer for other fields that we use if their value is not available. Or can we generalize saying, if value is not there, then it is "Prefer not to answer". In that case you won't have to mention all fields

Meenakshi Dhanani and others added 2 commits August 8, 2020 20:30
Set personal background options with Prefer Not to Say if they are empty
712e744
@mtreacy002
Merge pull request #2 from meenakshi-dhanani/issue49-update-personal-…
f7f161e 
…background-fix-selected

Set personal background options with Prefer Not to Say if they are empty
@mtreacy002
Copy link
Member Author

@meenakshi-dhanani , I've checked and merged the PR you raised for the update personal background. It looks good and make sense. Thanks for your help. 😉

foongminwong
foongminwong approved these changes Aug 9, 2020
View reviewed changes
Copy link

@foongminwong foongminwong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Code review - Done

  2. All possible responses were tested as below:

    • Test PR 61 - Update user personal background
      Screenshot/gif/url:

    test-bit-web-pr-61-update-personal-bg-successfully

    Expected Result: As a user, I should be able to update personal background.
    Actual Result: Same as expected.

  3. OS Version: Windows 10

@meenakshi-dhanani meenakshi-dhanani merged commit 09b4b4c into anitab-org:develop Aug 10, 2020
@mtreacy002 mtreacy002 deleted the issue49-update-personal-background branch September 12, 2020 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meenakshi-dhanani meenakshi-dhanani meenakshi-dhanani approved these changes

@foongminwong foongminwong foongminwong approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update User Personal Background
3 participants
@mtreacy002 @foongminwong @meenakshi-dhanani