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(profile): change profile api endpoint #4909
feat(profile): change profile api endpoint #4909
Conversation
Deploy preview created for package Built with commit: d292b39a8f4283a72b1c0337c2689409103d09a1 |
Deploy preview created for package Built with commit: d292b39a8f4283a72b1c0337c2689409103d09a1 |
Deploy preview created for package Built with commit: d292b39a8f4283a72b1c0337c2689409103d09a1 |
return async dispatch => { | ||
await ProfileAPI.monitorUserStatus((error: Error, status: UserStatus) => { |
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.
Seems that this PR changes ProfileAPI.monitorUserStatus()
from a callback-based function to an async function. And thus the call here should be changed accordingly?
Update: Realized that we haven't done such update yet, but I think we should. Given ProfileAPI.monitorUserStatus()
is a public API and direct change to there is breaking, we can probably introduce ProfileAPI.getUserStatus()
.
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.
Oh sorry didn't realize we had ProfileAPI.getUserStatus()
already... My apologies. In this case, we can rename monitorUserStatus()
Redux action to getUserStatus()
, change this implementation to use ProfileAPI.getUserStatus()
(instead of ProfileAPI.monitorUserStatus()
) and change masthead-container.ts
and masthead-composite.ts
accordingly.
Deploy preview created for package Built with commit: d292b39a8f4283a72b1c0337c2689409103d09a1 |
…/annawen1/ibm-dotcom-library into feat/change-profile-api-endpoint
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 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.
Thank you for the work @annawen1!
// it('dispatches the action to get user authentication status', async () => { | ||
// ProfileAPI.getUserStatus.mockResolvedValue({ user: 'Unauthenticated' }); | ||
// const store = mockStore(); | ||
// await store.dispatch(ProfileAPI.getUserStatus()); |
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.
Instead of ProfileAPI.getUserStatus()
(that makes a call to @carbon/ibmdotcom-services
module), we need to call a Redux action here. Here is a test in this category for reference: https://github.com/carbon-design-system/carbon-for-ibm-dotcom/blob/v1.14.0/packages/services-store/src/actions/__tests__/localeAPI.test.ts#L84
…g and unit tests (#4926) ### Related Ticket(s) Change profile endpoint on Carbon Masthead for signed in state #4701 ### Description Update naming and unit tests from feedback: #4909 ### Changelog **Changed** - update `getUserStatus` action to `loadUserStatus` in accordance to naming strategy - create `Unauthenticated` const in ProfileAPI type - use `request` instead of `status` for the `setRequestUserStatusInProgress` and `setUserStatus` actions <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "package: styles": Carbon Expressive --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
### Related Ticket(s) Change profile endpoint on Carbon Masthead for signed in state carbon-design-system#4701 ### Description Change profile endpoint which no longer allows for jsonp to get around CORS errors. Set the new preprod profile endpoint as default. ### Changelog **Changed** - use `axios` for the profile calls - set default host endpoint to preprod url **Removed** - `jsonp` package - no longer needed <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "package: styles": Carbon Expressive --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
…g and unit tests (carbon-design-system#4926) ### Related Ticket(s) Change profile endpoint on Carbon Masthead for signed in state carbon-design-system#4701 ### Description Update naming and unit tests from feedback: carbon-design-system#4909 ### Changelog **Changed** - update `getUserStatus` action to `loadUserStatus` in accordance to naming strategy - create `Unauthenticated` const in ProfileAPI type - use `request` instead of `status` for the `setRequestUserStatusInProgress` and `setUserStatus` actions <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "package: styles": Carbon Expressive --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
Related Ticket(s)
Change profile endpoint on Carbon Masthead for signed in state #4701
Description
Change profile endpoint which no longer allows for jsonp to get around CORS errors. Set the new preprod profile endpoint as default.
Changelog
Changed
axios
for the profile callsRemoved
jsonp
package - no longer needed