Skip to content

Fix "randomize question order" field not being synced when toggling its checkbox in EditModal#2956

Merged
rtibbles merged 3 commits into
learningequality:hotfixesfrom
jonboiser:sync-randomize-field
Feb 17, 2021
Merged

Fix "randomize question order" field not being synced when toggling its checkbox in EditModal#2956
rtibbles merged 3 commits into
learningequality:hotfixesfrom
jonboiser:sync-randomize-field

Conversation

@jonboiser
Copy link
Copy Markdown
Contributor

Description

  1. Adds randomize to ExtraFieldsSerializer
  2. Fixes diffing code to properly handle the case when going from randomized = true -> randomized = false

Issue Addressed (if applicable)

Fixes #2864

Steps to Test

  • Edit an exercise. All the fields should synchronize as they are modified

@jonboiser jonboiser requested review from MisRob and rtibbles February 16, 2021 23:29
Copy link
Copy Markdown
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Code changes look good - a regression test would be great.

mastery_model = ChoiceField(
choices=exercises.MASTERY_MODELS, allow_null=True, required=False
)
randomize = BooleanField()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you either add this to this test here: https://github.com/learningequality/studio/blob/hotfixes/contentcuration/contentcuration/tests/viewsets/test_contentnode.py#L534 or create a similar test specifically for randomize?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added a section for randomize in th existing unit test

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 17, 2021

Codecov Report

Merging #2956 (1a66bf3) into hotfixes (28587f8) will increase coverage by 4.78%.
The diff coverage is 93.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           hotfixes    #2956      +/-   ##
============================================
+ Coverage     80.80%   85.59%   +4.78%     
============================================
  Files           281      299      +18     
  Lines         12659    15893    +3234     
============================================
+ Hits          10229    13603    +3374     
+ Misses         2430     2290     -140     
Impacted Files Coverage Δ
contentcuration/contentcuration/decorators.py 56.60% <50.00%> (-37.60%) ⬇️
...ntentcuration/contentcuration/db/models/manager.py 91.20% <90.98%> (-8.80%) ⬇️
contentcuration/contentcuration/forms.py 82.85% <94.31%> (+34.40%) ⬆️
contentcuration/contentcuration/api.py 92.06% <100.00%> (+1.43%) ⬆️
...tentcuration/contentcuration/context_processors.py 100.00% <100.00%> (ø)
...ontentcuration/contentcuration/db/advisory_lock.py 100.00% <100.00%> (ø)
...tcuration/contentcuration/db/models/expressions.py 93.33% <100.00%> (-6.67%) ⬇️
...entcuration/contentcuration/db/models/functions.py 100.00% <100.00%> (ø)
...ation/contentcuration/tests/test_rest_framework.py 36.93% <0.00%> (-63.07%) ⬇️
contentcuration/contentcuration/utils/format.py 21.05% <0.00%> (-49.54%) ⬇️
... and 181 more

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 9edb358...27afd30. Read the comment docs.

@rtibbles rtibbles merged commit b533936 into learningequality:hotfixes Feb 17, 2021
@jonboiser jonboiser deleted the sync-randomize-field branch February 17, 2021 18:45
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.

"Randomize question order for learners" doesn't persist after the page reload

2 participants