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

Empty userId breaks gradual rollout 100% expected behavior. #215

Closed
karimkod opened this issue Apr 18, 2024 · 3 comments · Fixed by #216
Closed

Empty userId breaks gradual rollout 100% expected behavior. #215

karimkod opened this issue Apr 18, 2024 · 3 comments · Fixed by #216

Comments

@karimkod
Copy link
Contributor

Describe the bug
When passing an empty userId (string.Empty) the toggle is not enabled. Although the toggle have a gradual rollout strategy of 100%.
When passing a null or a non-empty value, the same toogle is enabled as expected.

To Reproduce
Steps to reproduce the behavior:
Create a toggle with a gradual rollout strategy.
Pass the userId as string.empty ("") :

  var unleashEnabled = unleash.IsEnabled("togglename", new UnleashContext()
  {
      
      UserId = string.Empty

  });

Expected behavior
The value of unleashedEnabled should be always true, since the gradual rollout is 100% and there is not constraints.
Screenshots
If applicable, add screenshots to help explain your problem.

**Version **

    <PackageReference Include="Unleash.Client" Version="4.1.8" />

Additional context
Did my own digging and I think that the source is the calculation of stickinessId https://github.com/Unleash/unleash-client-dotnet/blob/main/src/Unleash/Strategies/FlexibleRolloutStrategy.cs#L43
When userId is empty, it is returned as the first choice of stickinessId but then we do the if (!string.IsNullOrEmpty(stickinessId)) which will be false, always.

@karimkod
Copy link
Contributor Author

karimkod commented Apr 18, 2024

If that's a real issue, I'm ready to open a PR, thanks :)
Need to be iso with other sdks behavior.
Checked node sdk, it doesn't exclude empty userId: https://github.com/Unleash/unleash-client-node/blob/main/src/strategy/flexible-rollout-strategy.ts

@sighphyre
Copy link
Member

Hey @karimkod,

Thanks for reporting this! Yes, this appears to be a bug. Empty string should not be doing this!

Oh if you're willing to submit a PR that would be amazing!

@karimkod
Copy link
Contributor Author

@sighphyre here it is #216

@andreas-unleash andreas-unleash moved this from New to In Progress in Issues and PRs Apr 22, 2024
sighphyre pushed a commit that referenced this issue Apr 22, 2024
Co-authored-by: Abdelkrim Bourennane <abdelkrim.bourennane@consulting-for.edenred.com>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Issues and PRs Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants