Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29141

After turning on show question text in the question bank, you can't turn it off again

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.4, 2.1.1, 2.2
    • Fix Version/s: 2.0.5, 2.1.2
    • Component/s: Questions
    • Labels:
      None
    • Testing Instructions:
      Hide

      1. Go to the question bank in any course that has questions.
      2. Tick the Show question text checkbox. Verify that
      a. the page reloads, and the question text appears, and
      b. the checkbox is now checked.
      3. Now uncheck the checkbox. Verify that
      a. the page reloads, and the question text is now hidden, and
      b. the checkbox is now unchecked.

      Show
      1. Go to the question bank in any course that has questions. 2. Tick the Show question text checkbox. Verify that a. the page reloads, and the question text appears, and b. the checkbox is now checked. 3. Now uncheck the checkbox. Verify that a. the page reloads, and the question text is now hidden, and b. the checkbox is now unchecked.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      This is a follow-up to MDL-27631.

      After you tick the Show question text checkbox, the page reloads, and the question text is shown.

      However, on that page, the checkbox is not correct, so you cannot un-check it to hide the question text.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            Note that the fix on the 2.0 branch includes back-porting the fix for MDL-27631 to that branch. Once this bug has been integrated, it might be nice to mark that bug as fixed in 2.0.x

            Show
            timhunt Tim Hunt added a comment - Note that the fix on the 2.0 branch includes back-porting the fix for MDL-27631 to that branch. Once this bug has been integrated, it might be nice to mark that bug as fixed in 2.0.x
            Hide
            timhunt Tim Hunt added a comment -

            By the way, I am aware how badly it sucks that the same optional param calls for qbshowtext happens in question_edit_setup and question_bank_view. Perhaps I will have time to fix this before Moodle 2.2.

            Note that the back-port to 2.0 is not a clean cherry-pick. It had to be fixed up by hand because of coding-style and other changes.

            Show
            timhunt Tim Hunt added a comment - By the way, I am aware how badly it sucks that the same optional param calls for qbshowtext happens in question_edit_setup and question_bank_view. Perhaps I will have time to fix this before Moodle 2.2. Note that the back-port to 2.0 is not a clean cherry-pick. It had to be fixed up by hand because of coding-style and other changes.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Tim, this line:

            +    if(($showquestiontext = optional_param('qbshowtext', -1, PARAM_BOOL)) != -1) {

            Could you:

            1) Split it into 2, one for getting the param and another for the condition... and
            2) add one space after "if"

            Really it's unreadable in its current incarnation. Surely no a matter of rejecting this, but I prefer to give you a chance to amend it.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Tim, this line: + if(($showquestiontext = optional_param('qbshowtext', -1, PARAM_BOOL)) != -1) { Could you: 1) Split it into 2, one for getting the param and another for the condition... and 2) add one space after "if" Really it's unreadable in its current incarnation. Surely no a matter of rejecting this, but I prefer to give you a chance to amend it. TIA and ciao
            Hide
            timhunt Tim Hunt added a comment -

            I am not going to change it.

            1. That idiom is used in many places in that file. I am not going to change one without changing them all, and code clean-up like that should not happen on stable branches, only on master.

            2. In the past you complained and told me not to fix coding style when making small changes in existing code, because it made the diff harder to read. Now you are saying that I should make that sort of style fix. Would you like to decide what you want, then document it?

            Show
            timhunt Tim Hunt added a comment - I am not going to change it. 1. That idiom is used in many places in that file. I am not going to change one without changing them all, and code clean-up like that should not happen on stable branches, only on master. 2. In the past you complained and told me not to fix coding style when making small changes in existing code, because it made the diff harder to read. Now you are saying that I should make that sort of style fix. Would you like to decide what you want, then document it?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Tim:

            1) Agree: I would recommend you to perform the split to 2 lines only into master no problem with that.
            2) Don't agree: I never have said that lines being part of the solution must not be coding-style fixed. Instead I always have said that we should not be doing coding-style fixing in unrelated lines (but create separate cleanup issue for that). And you know it, so don't try to mislead me, please!

            Said that, oki, I'm going to integrate this, I hope you will cleanup it some day. Thanks and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Tim: 1) Agree: I would recommend you to perform the split to 2 lines only into master no problem with that. 2) Don't agree: I never have said that lines being part of the solution must not be coding-style fixed. Instead I always have said that we should not be doing coding-style fixing in unrelated lines (but create separate cleanup issue for that). And you know it, so don't try to mislead me, please! Said that, oki, I'm going to integrate this, I hope you will cleanup it some day. Thanks and ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            And note that you are contradicting yourself completely as far as you have already introduced 100% unrelated changes at least in the 20_STABLE branch, so guy... surely a bit of thinking before stating (false) things/principles would be greatly welcome.

            PS: All this said 100% joking, eh! 1000% in fact. I hope all problems are like this, lol.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - And note that you are contradicting yourself completely as far as you have already introduced 100% unrelated changes at least in the 20_STABLE branch, so guy... surely a bit of thinking before stating (false) things/principles would be greatly welcome. PS: All this said 100% joking, eh! 1000% in fact. I hope all problems are like this, lol.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Passed as bro!

            Show
            samhemelryk Sam Hemelryk added a comment - Passed as bro!
            Hide
            timhunt Tim Hunt added a comment -

            Welcome home to New Zealand Sam!

            Show
            timhunt Tim Hunt added a comment - Welcome home to New Zealand Sam!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay!

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay! Closing, ciao

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Oct/11