Moodle
  1. Moodle
  2. MDL-39155

User picture shown during quiz should be large version not small one

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.6, 2.4.3
    • Fix Version/s: 2.6
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      1.Go to Quiz settings page and select one of the 3 options for user image.
      2.Small image on selection gives small image, none image on selection gives no image and large image on selection gives large image.
      3.Login as student and attempt the quiz.
      4.You will see the user picture being displayed according to your selection.

      Show
      1.Go to Quiz settings page and select one of the 3 options for user image. 2.Small image on selection gives small image, none image on selection gives no image and large image on selection gives large image. 3.Login as student and attempt the quiz. 4.You will see the user picture being displayed according to your selection.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      When the option is set to display user picture during quiz, it displays the tiny thumbnail version of the image. This is not suitable for its intended use. An invigilator would need to get so close to the display for the image to be recognisable. Could the large version of the picture be used instead? Or at least an option (e.g. Display user picture: No/Large/Small).

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Tim Hunt added a comment -

            Good idea. I think changing it to a No/Small/Large setting is the way to go.

            Show
            Tim Hunt added a comment - Good idea. I think changing it to a No/Small/Large setting is the way to go.
            Show
            Tim Hunt added a comment - Reviewing https://github.com/jacks92/moodle/compare/master...MDL-39155 .
            Hide
            Tim Hunt added a comment -

            Your approach to fixing this is basically correct. Well done.

            There is one significant problem

            1. In mod/quiz/attemptlib.php. You have copied and pasted a chunk of code, so now there are two coplies. Don't do things like that. You need to find a better way to write this bit.

            Then there are some minor things that show this code is not really finished and ready for review:

            2. you have a commit relating to another bug in https://github.com/jacks92/moodle/compare/master...MDL-39155. The branch needs to be cleaned up so there is a single commit.

            3.There are blank PHPdoc comments in mod/quiz/locallib.php. Those need to be fixed.

            4. There is incorrect whitespace and coding style in several places.

            Show
            Tim Hunt added a comment - Your approach to fixing this is basically correct. Well done. There is one significant problem 1. In mod/quiz/attemptlib.php. You have copied and pasted a chunk of code, so now there are two coplies. Don't do things like that. You need to find a better way to write this bit. Then there are some minor things that show this code is not really finished and ready for review: 2. you have a commit relating to another bug in https://github.com/jacks92/moodle/compare/master...MDL-39155 . The branch needs to be cleaned up so there is a single commit. 3.There are blank PHPdoc comments in mod/quiz/locallib.php. Those need to be fixed. 4. There is incorrect whitespace and coding style in several places.
            Hide
            Jayesh Anandani added a comment -

            1. Sir there are 3 possibilities so i have taken 3 if loops for them.And all contain different code.
            There is a slight difference between 2 and 3rd loop.In 3rd loop the size parameter is also being passed while it is not so in case of 2nd loop.So this was best possible way i could have done that.Any alternate way are most welcome.

            2,3,4: Solved them.Checked them through codechecker,inserted comments and finally my link has only one commit.

            Show
            Jayesh Anandani added a comment - 1. Sir there are 3 possibilities so i have taken 3 if loops for them.And all contain different code. There is a slight difference between 2 and 3rd loop.In 3rd loop the size parameter is also being passed while it is not so in case of 2nd loop.So this was best possible way i could have done that.Any alternate way are most welcome. 2,3,4: Solved them.Checked them through codechecker,inserted comments and finally my link has only one commit.
            Hide
            Tim Hunt added a comment -

            Much less duplication:

            $user = $DB->get_record('user', array('id' => $this->attemptobj->get_userid()));
            $userpicture = new user_picture($user);
            $userpicture->courseid = $this->attemptobj->get_courseid();
            if ($this->attemptobj->get_quiz()->showuserpicture==QUIZ_SHOWIMAGE_LARGE) {
                $userpicture->size = true;
            }
            return $userpicture;
            

            There are still whitespace errors, even if code-checker does not notice. I would put a space either side of ==. In the lang file, all the other lines have a space either side of =. You should not have a space between the PHPdoc comment and the define. In the form, you should indent the lines starting QUIZ_ to make it clear they continue the line before.

            Show
            Tim Hunt added a comment - Much less duplication: $user = $DB->get_record('user', array('id' => $this->attemptobj->get_userid())); $userpicture = new user_picture($user); $userpicture->courseid = $this->attemptobj->get_courseid(); if ($this->attemptobj->get_quiz()->showuserpicture==QUIZ_SHOWIMAGE_LARGE) { $userpicture->size = true; } return $userpicture; There are still whitespace errors, even if code-checker does not notice. I would put a space either side of ==. In the lang file, all the other lines have a space either side of =. You should not have a space between the PHPdoc comment and the define. In the form, you should indent the lines starting QUIZ_ to make it clear they continue the line before.
            Hide
            Jayesh Anandani added a comment -

            Would make the changes and update it.

            Show
            Jayesh Anandani added a comment - Would make the changes and update it.
            Hide
            Jayesh Anandani added a comment -

            All the errors along with code duplication checked and updated.

            Show
            Jayesh Anandani added a comment - All the errors along with code duplication checked and updated.
            Hide
            Tim Hunt added a comment -

            Well, some of the whitespace errors fixed. Surely it is not that hard to get the whitespace right. Look at other examples of code in the same files.

            Show
            Tim Hunt added a comment - Well, some of the whitespace errors fixed. Surely it is not that hard to get the whitespace right. Look at other examples of code in the same files.
            Hide
            Jayesh Anandani added a comment -

            Except that space, are the other indentations and code properly formated?
            I took into account mistakes pointed and again checked them with codechecker.

            Show
            Jayesh Anandani added a comment - Except that space, are the other indentations and code properly formated? I took into account mistakes pointed and again checked them with codechecker.
            Hide
            Jayesh Anandani added a comment -

            Well, yes sir those whitespace aren't to hard to get.I will try to minimize the error as much as possible.

            Show
            Jayesh Anandani added a comment - Well, yes sir those whitespace aren't to hard to get.I will try to minimize the error as much as possible.
            Hide
            Jayesh Anandani added a comment -

            Patch modified and testing instructions included.

            Show
            Jayesh Anandani added a comment - Patch modified and testing instructions included.
            Hide
            Tim Hunt added a comment -

            I have tested and it works.

            I just spotted two more issues:

            1. We need a corresponding chagne in settings.php

            2. " not ' in the lang file.

            Show
            Tim Hunt added a comment - I have tested and it works. I just spotted two more issues: 1. We need a corresponding chagne in settings.php 2. " not ' in the lang file.
            Hide
            Jayesh Anandani added a comment -

            1. Changes done to settings.php

            2. " replaced by ' in lang file.

            3. Provided space between == .

            Show
            Jayesh Anandani added a comment - 1. Changes done to settings.php 2. " replaced by ' in lang file. 3. Provided space between == .
            Hide
            Tim Hunt added a comment -

            Yay! we got there. Submitting for integration now.

            Show
            Tim Hunt added a comment - Yay! we got there. Submitting for integration now.
            Hide
            Ian McNaught added a comment -

            Thanks all for addressing this issue. Having been a Blackboard guy for 6 years, it is so refreshing to see how quickly even trivial things like this get addressed in the Moodle community.

            Show
            Ian McNaught added a comment - Thanks all for addressing this issue. Having been a Blackboard guy for 6 years, it is so refreshing to see how quickly even trivial things like this get addressed in the Moodle community.
            Hide
            Dan Poltawski added a comment -

            Thanks Jayesh!

            I've integrated this to master

            Show
            Dan Poltawski added a comment - Thanks Jayesh! I've integrated this to master
            Hide
            Jason Fowler added a comment -

            Thanks Jayesh and Tim, great work!

            Show
            Jason Fowler added a comment - Thanks Jayesh and Tim, great work!
            Hide
            Marina Glancy added a comment -

            Thanks for your awesome work! This has now become a part of Moodle.

            Closing as fixed!

            Show
            Marina Glancy added a comment - Thanks for your awesome work! This has now become a part of Moodle. Closing as fixed!
            Hide
            Mary Cooch added a comment -

            Added a QA test for this here MDLQA-5737 ready for inclusion in 2.6 testing.

            Show
            Mary Cooch added a comment - Added a QA test for this here MDLQA-5737 ready for inclusion in 2.6 testing.
            Hide
            Mary Cooch added a comment -

            Removing docs_required label as I mentioned it here http://docs.moodle.org/26/en/Quiz_settings

            Show
            Mary Cooch added a comment - Removing docs_required label as I mentioned it here http://docs.moodle.org/26/en/Quiz_settings

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: