Skip to content

fix: ODP - getQualifiedSegments should return null when fetch fails #496

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 6, 2023
Merged

fix: ODP - getQualifiedSegments should return null when fetch fails #496

merged 3 commits into from
Jan 6, 2023

Conversation

zashraf1985
Copy link
Contributor

Summary

getQualifiedSegments always returns empty list when fetch fails. Modified it to return null in case of failure. It will make it easy to differentiate between a fetch failure and a legit empty list response from the backed.

Test plan

All existing tests pass

Issues

FSSDK-8728

@zashraf1985 zashraf1985 requested a review from a team as a code owner December 21, 2022 03:58
@zashraf1985 zashraf1985 changed the title fix: getQualifiedSegments should return null when fetch fails fix: ODP - getQualifiedSegments should return null when fetch fails Dec 21, 2022
@mnoman09 mnoman09 closed this Dec 29, 2022
@mnoman09 mnoman09 reopened this Dec 29, 2022
@NomanShoaib
Added a function to create a copy of userContext, this will avoid sen…
7f87456 
…ding odpEvent upon creating just a copy of UserContext
jaeopt
jaeopt approved these changes Jan 3, 2023
View reviewed changes
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

LGTM

@jaeopt
Copy link
Contributor

jaeopt commented Jan 5, 2023

@mnoman09 I see you changed to createUserContextCopy() to avoid unnecessary identify event sending for legacy apis.
I found the same bug in swift-sdk as well and also expect the same in other sdks too. Can you add a FSC test case to cover this for all sdks? @msohailhussain

@msohailhussain
Copy link
Contributor

We found this case using FSC. :)

@jaeopt jaeopt merged commit f63cff5 into master Jan 6, 2023
@jaeopt jaeopt deleted the zeeshan/ats-get-qualified-segments-null-FSSDK-8728 branch January 6, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaeopt jaeopt jaeopt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zashraf1985 @jaeopt @msohailhussain @mnoman09 @NomanShoaib