Skip to content

Fix crash when deleting a heading question#2665

Merged
richardebeling merged 2 commits intoe-valuation:mainfrom
karyon:karyon/fix-crash-when-deleting-heading-questions
Mar 10, 2026
Merged

Fix crash when deleting a heading question#2665
richardebeling merged 2 commits intoe-valuation:mainfrom
karyon:karyon/fix-crash-when-deleting-heading-questions

Conversation

@karyon
Copy link
Copy Markdown
Collaborator

@karyon karyon commented Mar 8, 2026

This got introduced in #2513. answer_class throws on heading questions.

The tests didn't explicitly cover deleting a heading question, but test_delete_question would still occasionally do it depending on the status of the RNG when running with --shuffle, because it didn't specify the question type so model baker would choose one at random. I saw this failing in a PR check for #2643.

karyon added 2 commits March 8, 2026 18:18
This got introduced in e-valuation#2513. The tests didn't explicitly cover deleting a heading question, but test_delete_question would still occasionally do it depending on the status of the RNG when running with --shuffle, because it didn't specify the question type so model baker would choose one at random.
Copy link
Copy Markdown
Collaborator

@Kakadus Kakadus left a comment

Choose a reason for hiding this comment

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

Thanks, I overlooked that answer_class only works correctly, if the question is not a heading.

Comment thread evap/evaluation/models.py
@niklasmohrin
Copy link
Copy Markdown
Member

not merging so that @Kakadus and @karyon can still discuss the comment, either option there is fine with me

Copy link
Copy Markdown
Collaborator

@Kakadus Kakadus left a comment

Choose a reason for hiding this comment

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

good2go for me like this

@richardebeling richardebeling merged commit 843e5d8 into e-valuation:main Mar 10, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants