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

Fix version 1 exams missing counterInExercise field #5431

Merged

Conversation

jonboiser
Copy link
Contributor

@jonboiser jonboiser commented Apr 26, 2019

Summary

  1. Adds a conversion function that adds a missing field to some Version 1 Exams
  2. Combines conversion functions into one that handles V0 -> v2 and V1 -> V2 conversion.
  3. Updates tests with examples from both V0 and V1 (Fixes make sure unit test examples include all exam versions #4940 )

Reviewer guidance

  1. Make some exams using the current 0.12 branch, which should create exams that are V1. Edit the Exam models in the shell, or via a REST tool to create Exams whose question_sources items are missing the counterInExercise field and also have a data_model_version field equal 0. Confirm that the problem in 'NaN' in titles of difficult questions #5427 occurs
  2. Update to this fork and verify that it is fixed.

References

Fixes #5427


Contributor Checklist

PR process:

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Testing:

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

Reviewer Checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@jonboiser jonboiser added the TODO: needs review Waiting for review label Apr 26, 2019
@jonboiser jonboiser added this to the 0.12.3 milestone Apr 26, 2019
@codecov
Copy link

codecov bot commented Apr 26, 2019

Codecov Report

Merging #5431 into release-v0.12.x will increase coverage by 0.13%.
The diff coverage is 71.42%.

Impacted Files Coverage Δ
...oach/assets/src/modules/questionDetail/handlers.js 0% <ø> (ø) ⬆️
...s/coach/assets/src/modules/questionList/actions.js 0% <0%> (ø) ⬆️
...rc/views/plan/CreateExamPage/CreateExamPreview.vue 0% <0%> (ø) ⬆️
...ns/learn/assets/src/modules/examViewer/handlers.js 0% <0%> (ø) ⬆️
...assets/src/modules/examCreation/selectQuestions.js 85.71% <100%> (+0.52%) ⬆️
kolibri/core/exams/serializers.py 98.93% <100%> (+6.15%) ⬆️
kolibri/core/exams/models.py 97.72% <100%> (ø) ⬆️
kolibri/core/assets/src/exams/utils.js 45.54% <80%> (+22.08%) ⬆️
kolibri/core/assets/src/state/mappers.js 85% <0%> (+45%) ⬆️

@jonboiser jonboiser force-pushed the exam-exercise-counter branch 2 times, most recently from 3ebc876 to ed64854 Compare April 26, 2019 21:28
@indirectlylit
Copy link
Contributor

my guess/hope is that this will also address #5425

@nucleogenesis
Copy link
Member

nucleogenesis commented Apr 30, 2019

@jonboiser

I tested the latest commits and they resolved the issues we discussed on Slack. When there are no counter_in_exercise fields then the numbers are displayed as expected.

There appears to be a failing test (which I get locally as well running yarn test): https://travis-ci.org/learningequality/kolibri/jobs/526240872 - once that's resolved then this should be ready to roll.

Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

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

@jonboiser Looks great!

@rtibbles rtibbles merged commit f7e4056 into learningequality:release-v0.12.x Apr 30, 2019
@jonboiser jonboiser deleted the exam-exercise-counter branch May 1, 2019 00:54
@jonboiser jonboiser removed the TODO: needs review Waiting for review label Sep 9, 2019
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.

4 participants