[CI 5057] Add value count support and tests #87
Conversation
| [JsonProperty("field")] | ||
| public string Field { get; set; } | ||
| [JsonProperty("value")] | ||
| public string CountedValue { get; set; } |
There was a problem hiding this comment.
I think we should keep to how the API names fields in case value is used in another aggregation type in the future. wdyt?
| public string CountedValue { get; set; } | |
| public string Value { get; set; } |
There was a problem hiding this comment.
separately, should we support other value types like int and boolean as well? I think the current implementation doesn't allow counting of those types of values atm. Maybe we can use generics or object instead?
There was a problem hiding this comment.
The data type change makes sense.
In regards to the name, .NET has a limitation where the property can't have the same name as the class it's within, and changing the class name at this point would be a bigger change than planned
Mudaafi
left a comment
Mudaafi
left a comment
There was a problem hiding this comment.
lgtm, thanks for making the changes
Mudaafi
deleted the
ci-5057-net-add-value_count-support-to-variations_map
branch
Add value_count params for variations_map https://docs.constructor.com/reference/constructor-concepts-variations-mapping