-
Notifications
You must be signed in to change notification settings - Fork 93
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: Update additional info functionality #57
Feat: Update additional info functionality #57
Conversation
Update @meenakshi-dhanani . This PR is still a WIP. I'm having trouble getting the selected value on Timezone when submitting the form. Everything else are there except the is_organization_rep. Can you please help me troubleshoot this? Because of the missing timezone value, backend responded with payload invalid. |
915bd26
to
ffbd145
Compare
Update @anitab-org/bridgeintech-maintainers . Here's the PR for Create and Update User Additional Information. Please review when you have time. Thanks |
ffbd145
to
1529a67
Compare
Update @anitab-org/bridgeintech-maintainers. I just pushed a bug fix for default checked for is_organization_rep checkbox. |
1529a67
to
e7f63ab
Compare
Update @anitab-org/bridgeintech-maintainers . I've just pushed the bug fix for when timezone is not selected. Now a default value will automatically added when user hit save if no value is selected. |
ff31de5
to
4eaed7d
Compare
}) | ||
.catch(() => setErrorMessage("The service is temporarily unavailable, please try again later.")); | ||
.catch(() => setResponseMessage(SERVICE_UNAVAILABLE_ERROR)); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't it be update all the time?
why does it try to create and then if conflict update? If it's on this page, it should always try to update correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@meenakshi-dhanani . Because when user retrieve the page for the first time, the additional info would have not yet been created. The first response that user will see is No additional information found with your data. Please provide them now
. Then users will need to fill their additional info. The first time they click Save
button, this will send POST /user/additional_info request to backend. With this request, the data will be created.
But since we don't know which login user will already have created the additional information or not, every time user go to this page and fill in the information, on the Save
button click the system would by default try to send POST request, which will return error message 409 Conflict, then will automatically send PUT request to continue with update information. Does this make sense?
Remember that we only have this one form/page to do both create (POST) and update (PUT) additional information. This POST and PUT will be similar later to Personal background. The Personal Details page Is the only one that is different since there is no POST /user/personal_details endpoint since the data for this is created through POST /register
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
4eaed7d
to
7c87442
Compare
Update @anitab-org/bridgeintech-maintainers . I've refactored this PR to reeflect the changes in backend api endpoint for POST and PUT to just PUT /user/additional_info ( PR 101 ). Please check when you have time. |
Update @meenakshi-dhanani and @anitab-org/bridgeintech-maintainers . I've modified this PR to reflect the changes in backend API endpoint (PR 101). Please re-review if you have time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There is a PR in the backend that this is based on. @ramitsawhney27 could you review the backend PR as well as this? I've reviewed both and they seem fine to me |
There was a problem hiding this 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:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a test case for future reference:
{
"is_organization_rep": true,
"timezone": "America/New_York",
"phone": "123-456-789",
"mobile": "123-444-555",
"personal_website": "https://africaa.com"
}
Description
Allow useer to update theeir additional information
Fixes #48
Type of Change:
Code/Quality Assurance Only
How Has This Been Tested?
My Space
>Personal Details
>Additional Info
Notice how Timezone is skipped by user (which could happen either accidentally or on purpose if user prefer not to disclose their timezone). If this happens, the system will automatically use the default value of GMT+00 as user timezone. To test this, click away to different page and come back to
![Screen Shot 2020-08-03 at 8 06 08 pm](https://user-images.githubusercontent.com/29667122/89172689-5e445980-d5c6-11ea-97d1-0f2b1098ad7d.png)
My Spacee
>Additional Info
and see GMT being the value displayed from GET /user/additional_info request.My Space
>Additional Info
and see the updated data (in this example, the timezone field)Checklist:
Code/Quality Assurance Only