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 some multiple choice levels showing the question twice in the level details dialog #40864

Merged
merged 5 commits into from
Jun 2, 2021

Conversation

bethanyaconnor
Copy link
Contributor

@bethanyaconnor bethanyaconnor commented Jun 1, 2021

Fixes some multiple choice levels showing the question multiple times. This was happening because question and get_question_text were sometimes the same (but sometimes different which is why I didn't catch this initially). This new code models how the haml displays this info, first displaying content in the content{1-4} fields, (here from here) and then displaying localized_questions[0]["text"] here. Match levels use localized_questions for the answers, which is out of scope of this PR.

Multi level before:
Screen Shot 2021-06-01 at 8 44 03 AM

Multi level after:
Screen Shot 2021-06-01 at 8 44 28 AM

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@bethanyaconnor bethanyaconnor changed the title Multi questions double question Fix some multiple choice levels showing the question twice in the level details dialog Jun 1, 2021
@bethanyaconnor bethanyaconnor marked this pull request as ready for review June 1, 2021 19:57
@bethanyaconnor bethanyaconnor requested a review from a team June 1, 2021 19:57
super.merge(
{
question: question
content: content
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding a test for this summarize case?

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.

None yet

2 participants