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

feat(api): Accepting all types for attributes values #102

Merged
merged 4 commits into from
Sep 14, 2018

Conversation

mfahadahmed
Copy link
Contributor

No description provided.

@@ -458,6 +458,39 @@ public void TestActivateExperimentNotRunning()

Assert.IsNull(variationkey);
}

[Test]
public void TestActivateWithTypedAttributes()
Copy link
Contributor

Choose a reason for hiding this comment

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

This test case is not verifying the payload generated from EventBuilder is returned correctly or not.
Create an expectedEventBuilder class and call EventBuilder, and compare it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added payload comparison test case for typed attributes in EventBuilderTest.cs.

@@ -71,7 +71,7 @@ public bool Evaluate(JToken conditions, UserAttributes userAttributes)

string conditionName = conditions["name"].ToString();
return userAttributes != null && userAttributes.ContainsKey(conditionName)
&& userAttributes[conditionName] == conditions["value"].ToString();
&& userAttributes[conditionName].ToString() == conditions["value"].ToString();
Copy link
Contributor

Choose a reason for hiding this comment

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

This may cause some problem when any value other than string is converted into string. @oakbani

Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

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

Seems easy. I've run this against the Compat tests and they were successful. Approving.

@mikeproeng37 mikeproeng37 merged commit ed28c56 into master Sep 14, 2018
@mfahadahmed mfahadahmed deleted the fahad/attribute-types branch September 17, 2018 08:00
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