Moodle

Random Short-Answer Matching question is a bit broken

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 1.9.5
  • Fix Version/s: STABLE backlog
  • Component/s: Questions
  • Labels:
    None
  • Environment:
    LAMP
  • Database:
    MySQL
  • Difficulty:
    Moderate
  • Affected Branches:
    MOODLE_19_STABLE

Description

Create a Quizz
Add a Random Short-Answer Matching question (I had few short answer question)
Some php error message appears when preview the question and preview the quizz (check both of them)
Once all looks normal transfert all hard coded string into lang
If you can also have a look to display.html and refactor it a bit, all good

Activity

Hide
Jerome Mouneyrac added a comment - - edited

I had a better look, here is the situation:

  • when none of the selected random short answer questions have a question text => some notice/warning are displayed
  • when only one of the selected random short answer question has a question text => this question is displayed and you can choose the answer from this question and also the other selected but none displayed random short answer. This is obviously a wrong behavior but cannot be fixed as many people could have found this bug friendly and use it this way.
  • when all the selected random short answer questions have a question text => works the way it should (http://docs.moodle.org/en/Random_Short-Answer_Matching_question_type)

For the fix: I'm going to display a Moodle error message instead of the php warnings.

Code related issue: 'Random Short-Answer Matching question' extends 'Matching question'. I tested the 'Matching question' and if they are quite similar, I'm not sure it was the best thing to extend.

Show
Jerome Mouneyrac added a comment - - edited I had a better look, here is the situation:
  • when none of the selected random short answer questions have a question text => some notice/warning are displayed
  • when only one of the selected random short answer question has a question text => this question is displayed and you can choose the answer from this question and also the other selected but none displayed random short answer. This is obviously a wrong behavior but cannot be fixed as many people could have found this bug friendly and use it this way.
  • when all the selected random short answer questions have a question text => works the way it should (http://docs.moodle.org/en/Random_Short-Answer_Matching_question_type)
For the fix: I'm going to display a Moodle error message instead of the php warnings. Code related issue: 'Random Short-Answer Matching question' extends 'Matching question'. I tested the 'Matching question' and if they are quite similar, I'm not sure it was the best thing to extend.
Hide
Jerome Mouneyrac added a comment -

Tim, I remember I talked about that with you, so I don't think you want to look at it but I just put you as watcher to let you know.

Show
Jerome Mouneyrac added a comment - Tim, I remember I talked about that with you, so I don't think you want to look at it but I just put you as watcher to let you know.
Hide
Jerome Mouneyrac added a comment -

I attached a fix, I'll commit it end of the week.

Show
Jerome Mouneyrac added a comment - I attached a fix, I'll commit it end of the week.
Hide
Tim Hunt added a comment -

I don't think that solution is sufficient.

If you look in question_randomsamatch_qtype::create_session_and_responses, there are already several checks (which output non-translated errors!).

I the any additional checks that are required should be there. However, I don't think that function should be doing any output. Instead you should probably set something like $state->options->errorstr there, and the check that (instead of get_correct_responses) in print_question_formulation_and_controls, and display the error there.

Does that make sense?

Show
Tim Hunt added a comment - I don't think that solution is sufficient. If you look in question_randomsamatch_qtype::create_session_and_responses, there are already several checks (which output non-translated errors!). I the any additional checks that are required should be there. However, I don't think that function should be doing any output. Instead you should probably set something like $state->options->errorstr there, and the check that (instead of get_correct_responses) in print_question_formulation_and_controls, and display the error there. Does that make sense?
Hide
Jerome Mouneyrac added a comment -

I'll have a look to print_question_formulation_and_controls this week, thanks for the advice

Show
Jerome Mouneyrac added a comment - I'll have a look to print_question_formulation_and_controls this week, thanks for the advice
Hide
Jerome Mouneyrac added a comment -

Hi Tim,
as we are starting the new scrum process I reassign this Quizz issue to you.

Show
Jerome Mouneyrac added a comment - Hi Tim, as we are starting the new scrum process I reassign this Quizz issue to you.
Hide
Eloy Lafuente (stronk7) added a comment -

(reverting to 1.9.11, Tim's components don't use the STABLE/DEV backlogs thing for now)

Show
Eloy Lafuente (stronk7) added a comment - (reverting to 1.9.11, Tim's components don't use the STABLE/DEV backlogs thing for now)

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated: