carbon-for-ibm-dotcom icon indicating copy to clipboard operation
carbon-for-ibm-dotcom copied to clipboard

feat(masthead-v2): consolidate Cloud auth methods with profileAPI & update docs

Open proeung opened this issue 2 years ago • 6 comments

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/remove contact CTA/button.

To do

  • [x] Update the docs tab to include info about the different auth method types (eg. profile-api, cookie, and docs-api ). Will pick this up tomorrow morning.

proeung avatar Aug 29 '22 21:08 proeung

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

ibmdotcom-bot avatar Aug 29 '22 21:08 ibmdotcom-bot

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

ibmdotcom-bot avatar Aug 29 '22 21:08 ibmdotcom-bot

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

ibmdotcom-bot avatar Aug 29 '22 22:08 ibmdotcom-bot

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.

Screen Shot 2022-08-30 at 4 52 57 PM

proeung avatar Aug 30 '22 21:08 proeung

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

annawen1 avatar Aug 30 '22 21:08 annawen1

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.

proeung avatar Aug 30 '22 21:08 proeung