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.

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

fix(contacts): handle optional scim fields #3827

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

sreenara
Copy link
Contributor

COMPLETES # NA

This pull request addresses

Optional fields in the SCIM API response. Not all contacts will have all the fields populated and the SCIM API mandates only 3 mandatory fields. This causes a crash in our code.

by making the following changes

Mark all fields except 3 mandatory ones as optional. Handle contact resolution logic accordingly.
Added new UTs to test this optional field logic in the resolution.

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios where tested

  1. Create a custom contact and verify on Webex
  2. Create a cloud contact and verify on Webex
  3. Test the optional fields and minimal contact response from SCIM using new UTs

I certified that

  • I have read and followed contributing guidelines

  • I discussed changes with code owners prior to submitting this pull request

  • I have not skipped any automated checks

  • All existing and new tests passed

  • I have updated the documentation accordingly


Make sure to have followed the contributing guidelines before submitting.

@sreenara sreenara requested a review from a team as a code owner September 13, 2024 06:11
@sreenara sreenara added the validated If the pull request is validated for automation. label Sep 13, 2024
if (photos?.length) {
avatarURL = photos[0].value;
let sipAddresses;
if (filteredContact[SCIM_WEBEXIDENTITY_USER]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Maybe we can use ternary operator here also.. but just a suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adhmenon the ternary operator doesn't work here. Something to do with the way typescript recognizes this object member. I initially tried with ternary but kept getting errors that SCIM_WEBEXIDENTITY_USER could be undefined.

Copy link
Contributor

@adhmenon adhmenon left a comment

Choose a reason for hiding this comment

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

LGTM. Just a small minro comment.

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3827.d3m3l2kee0btzx.amplifyapp.com

@sreenara sreenara merged commit 8573ece into webex:next Sep 13, 2024
11 checks passed
@sreenara sreenara deleted the handle-scim-response branch September 13, 2024 08:44
pagour98 pushed a commit to pagour98/webex-js-sdk that referenced this pull request Sep 27, 2024
pagour98 pushed a commit to pagour98/webex-js-sdk that referenced this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validated If the pull request is validated for automation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants