carbon-for-ibm-dotcom
carbon-for-ibm-dotcom copied to clipboard
feat(masthead-v2): consolidate Cloud auth methods with profileAPI & update docs
Related Ticket(s)
https://github.com/carbon-design-system/carbon-for-ibm-dotcom/issues/9291
Description
- This PR consolidates the Cloud authentication methods and ensures that we deliver the right utility menu items based on the value defined in the auth-method prop/attribute.
- Add different storybook previews per masthead endpoint (eg,
with V2 Data
,with Cloud Data
), since the endpoint knob does not update the source string of the endpoint consistently.
Changelog
Changed
- Move Cloud auth methods/functions into the Profile services within the
@carbon/ibmdotcom-services
package and adjust@carbon/ibmdotcom-services-store
- Add
has-contact
knob to show adopters that they can optionally add/removecontact
CTA/button.
To do
- [x] Update the
docs
tab to include info about the different auth method types (eg.profile-api
,cookie
, anddocs-api
). Will pick this up tomorrow morning.
Deploy preview created for package React
:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/9312/index.html
Built with commit: 2701bb4488bb418f170e91bae25887b73840431e
Deploy preview created for package Web Components
:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/9312/index.html
Built with commit: 2701bb4488bb418f170e91bae25887b73840431e
Deploy preview created for package Web Components (React wrapper)
:
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/9312/index.html
Built with commit: 2701bb4488bb418f170e91bae25887b73840431e
Everything seems fine, nothing in the code is jumping out as needing addressed, but I also can't really test it in the browser. Could/should we get some e2e tests in place to ensure the v2 and cloud mastheads are both authenticating & updating properly?
@andy-blum I'm able to verify that the cookie
auth method is working fine in the browser (see attached), but I do agree that we need to have some sort of automatic testing for these auth types. There's already a unit test written for the profileAPI
, so we should be good on that front.
As for the cookie
and docs-api
, @huntermacd wrote some when we did the Cloud Masthead (see cloudAccountAuth.test.ts and CloudAccountAuth.test.js ) and we just need to consolidate it to the global profile tests. I'll open a new issue for this work. However, in the meantime, I want to get the changes in PR re-review so that we can merge them.
Everything seems fine, nothing in the code is jumping out as needing addressed, but I also can't really test it in the browser. Could/should we get some e2e tests in place to ensure the v2 and cloud mastheads are both authenticating & updating properly?
Right just like @proeung mentioned, we shifted all the cloud authentication logic onto the ProfileAPI
service. The ProfileAPI
service now handles the logic of which authentication method to execute and is being utilized for the masthead-v2. The existing tests for ProfileAPI
will cover the default authentication, but we'll definitely have to copy over the tests from CloudAccountAuth
over later
Thanks all for the review! I think this PR is good for merging and a new CDN alpha build can get started once the changes have been merged down.
Also, here's the issue for cleaning up the unit tests (https://github.com/carbon-design-system/carbon-for-ibm-dotcom/issues/9314). @andy-blum or @jkaeser feel free to pick up this issue.