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

Fixes CourseDailyMetricsSerializer when average_progress is 1.00 #230

Merged
merged 3 commits into from
Jun 26, 2020

Conversation

johnbaldwin
Copy link
Contributor

@johnbaldwin johnbaldwin commented Jun 26, 2020

Updated CourseDailyMetricsSerializer.average_progress max_digits to
allow 3 digits. Was previously 2 digits which failed the serializer
validation when the average_progress value was 1.00.

Also updated the average_progress field definition to set min and max
values. Unfortunately, this validation is not being tested. Did some
initial timeboxed investigation, but stopped at time limit of 1 hour.
Note here that we need to investigation and make sure validation works.

Updated the serializer tests

Updated CourseDailyMetricsSerializer.average_progress `max_digits` to
allow 3 digits. Was previously 2 digits which failed the serializer
validation when the `average_progress` value was 1.00.

Also updated the `average_progress` field definition to set min and max
values. Unfortunately, this validation is not being tested. Did some
initial timeboxed investigation, but stopped at time limit of 1 hour.
Note here that we need to investigation and make sure validation works.

Updated the serializer tests
@johnbaldwin johnbaldwin marked this pull request as ready for review June 26, 2020 04:42
@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2020

Codecov Report

Merging #230 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #230   +/-   ##
=======================================
  Coverage   91.29%   91.29%           
=======================================
  Files          38       38           
  Lines        1953     1953           
=======================================
  Hits         1783     1783           
  Misses        170      170           
Impacted Files Coverage Δ
figures/serializers.py 94.37% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bc1acd...b3c8af9. Read the comment docs.

@johnbaldwin johnbaldwin merged commit 0f4e22e into master Jun 26, 2020
@OmarIthawi OmarIthawi deleted the john/fix-serializers branch June 26, 2020 16:16
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