Commit d5b77d2c by Tyler Hallada Committed by GitHub

Merge pull request #509 from edx/thallada/metric-range-edge-cases

Fix edge cases for metric ranges
parents e75e489c 7b26686c
...@@ -94,7 +94,9 @@ define(function (require) { ...@@ -94,7 +94,9 @@ define(function (require) {
return null; return null;
} }
return [ return [
_.isNull(range[0]) ? -Infinity : range[0], // A null element is interpreted as infinity
// Note: Something like [null, 0] is invalid. The API should not return that.
_.isNull(range[0]) ? Infinity : range[0],
_.isNull(range[1]) ? Infinity : range[1] _.isNull(range[1]) ? Infinity : range[1]
]; ];
}); });
...@@ -134,14 +136,20 @@ define(function (require) { ...@@ -134,14 +136,20 @@ define(function (require) {
* in-range. * in-range.
* - When the range is infinite in the positive dimension, infinity is * - When the range is infinite in the positive dimension, infinity is
* considered in-range. * considered in-range.
* - When the range has the same value on both sides, that value is
* considered in-range, but only that value.
* *
* @param value Value in question. * @param value Value in question.
* @param range Array of min and max. May be null. * @param range Array of min and max. May be null.
*/ */
inMetricRange: function(value, range) { inMetricRange: function(value, range) {
return _.isNull(range) ? false : if (_.isNull(range)) {
(value === Infinity && range[1] === Infinity) ? true : return false;
value >= range[0] && value < range[1]; } else if ((value === Infinity && range[1] === Infinity) ||
(value === range[0] && range[0] === range[1])) {
return true;
}
return value >= range[0] && value < range[1];
} }
}); });
......
...@@ -67,15 +67,22 @@ define(function (require) { ...@@ -67,15 +67,22 @@ define(function (require) {
var courseMetadata = new CourseMetadataModel({ var courseMetadata = new CourseMetadataModel({
engagement_ranges: { engagement_ranges: {
problems_attempted: { problems_attempted: {
below_average: [null, 0], below_average: [0, 10],
average: [0, null], average: [10, null],
above_average: null above_average: null
},
problem_attempts_per_completed: {
below_average: [0, 10],
average: [10, null],
above_average: [null, null]
} }
} }
}, {parse: true}); }, {parse: true});
expect(courseMetadata.get('engagement_ranges').problems_attempted.below_average).toEqual([-Infinity, 0]); expect(courseMetadata.get('engagement_ranges').problems_attempted.below_average).toEqual([0, 10]);
expect(courseMetadata.get('engagement_ranges').problems_attempted.average).toEqual([0, Infinity]); expect(courseMetadata.get('engagement_ranges').problems_attempted.average).toEqual([10, Infinity]);
expect(courseMetadata.get('engagement_ranges').problems_attempted.above_average).toEqual(null); expect(courseMetadata.get('engagement_ranges').problems_attempted.above_average).toEqual(null);
expect(courseMetadata.get('engagement_ranges').problem_attempts_per_completed.above_average)
.toEqual([Infinity, Infinity]);
}); });
it('does not parse engagement date range', function () { it('does not parse engagement date range', function () {
...@@ -112,6 +119,12 @@ define(function (require) { ...@@ -112,6 +119,12 @@ define(function (require) {
var courseMetadata = new CourseMetadataModel(); var courseMetadata = new CourseMetadataModel();
expect(courseMetadata.inMetricRange(Infinity, [0, Infinity])).toBe(true); expect(courseMetadata.inMetricRange(Infinity, [0, Infinity])).toBe(true);
}); });
it('value is included in ranges where value is specified on both sides', function () {
var courseMetadata = new CourseMetadataModel();
expect(courseMetadata.inMetricRange(0, [0, 0])).toBe(true);
expect(courseMetadata.inMetricRange(Infinity, [Infinity, Infinity])).toBe(true);
});
}); });
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment