Skip to content

feat(ForcedDecisions): adding SetForcedDecision support in decide api. #324

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

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

yasirfolio3
Copy link
Contributor

Summary

Added SetForcedDecision support in decide api.

Test plan

  • unit tests for the new APIs
  • FSC tests with new test cases

properties:
flagKey:
type: string
ruleKey:
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please check if this accepts optional as well.

@@ -12,7 +12,7 @@ require (
github.com/go-kit/kit v0.9.0
github.com/google/uuid v1.1.1
github.com/lestrrat-go/jwx v0.9.0
github.com/optimizely/go-sdk v1.7.0
github.com/optimizely/go-sdk v1.7.1-0.20211102215133-82644c6670e8
Copy link
Contributor

Choose a reason for hiding this comment

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

bump the correct version once go-sdk is released.

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 with a type name change suggested

required:
- userId
OptimizelyForcedDecision:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the name? We have other public definition of OptimizelyForcedDecision and it can be confusing.
Just "ForcedDecision"?

}

// OptimizelyForcedDecision defines Forced Decision
type OptimizelyForcedDecision struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here. Change to "ForcedDecision"?

@yasirfolio3 yasirfolio3 reopened this Nov 22, 2021
@msohailhussain msohailhussain merged commit 733ff2d into master Nov 24, 2021
@msohailhussain msohailhussain deleted the yasir/forced-decision branch November 24, 2021 06:08
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 this pull request may close these issues.

3 participants