Moodle
  1. Moodle
  2. MDL-19089

Ellipses incorrectly displayed for My Moodle page

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.4, 1.9.5, 2.0
    • Fix Version/s: 1.9.6
    • Component/s: My home
    • Labels:
      None
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      31726

      Description

      At the bottom of the my moodle page, ellipsis (...) is displayed when the user has more courses than the preference setting allows to be displayed (although it didn't really work that way). The way it really worked was this:

      The number of courses used to be hard coded at 21. There courses we displayed, and if the count was >20, then the ellipsis would be displayed. I get the felling this was originally only supposed to display the first 20 courses, and the 21 was to allow a marker that there are more. But the implementation showed all the courses returned (all 21). If you have 21 courses, they will all show plus you get the ellipsis, not a correct behavior.

      Now the 21 was changed to a preference setting in MDL-13224, but the ellipsis setting was never changed, so if the preference is set to 50, and 32 courses are displayed, the ellipsis will still show.

      The first attached diff should show how to correct that problem, but it doesn't correct the 1 off problem I described above.

      The second diff would correct both problems.

      Both diffs are off of current MOODLE_19_STABLE and I HAVE NOT tested them extensively yet.

      1. mymoodle1.diff
        0.3 kB
        Eric Merrill
      2. mymoodle2.diff
        0.6 kB
        Eric Merrill

        Issue Links

          Activity

          Hide
          Mike Churchward added a comment -

          Isn't all that is required to fix this, a change to the course count check, such as:

              if (count($courses) > $courses_limit) {
                  echo '<br />...';  
              }
          
          Show
          Mike Churchward added a comment - Isn't all that is required to fix this, a change to the course count check, such as: if (count($courses) > $courses_limit) { echo '<br />...'; }
          Hide
          Eric Merrill added a comment -

          Mike -

          No, because if you look above, at the line that fetches the course, it is:

          $courses = get_my_courses($USER->id, 'visible DESC,sortorder ASC', '*', false, $courses_limit);

          So the number of courses returned will never be greater than $course_limit. We also need to check for $course_limit==0, because that will return all courses.

          Show
          Eric Merrill added a comment - Mike - No, because if you look above, at the line that fetches the course, it is: $courses = get_my_courses($USER->id, 'visible DESC,sortorder ASC', '*', false , $courses_limit); So the number of courses returned will never be greater than $course_limit. We also need to check for $course_limit==0, because that will return all courses.
          Hide
          Mike Churchward added a comment -

          Okay. Got it.

          Your solution seems sound then.

          mike

          Show
          Mike Churchward added a comment - Okay. Got it. Your solution seems sound then. mike
          Hide
          Daniel Neis added a comment -

          Hello,

          this will be integrated to moodle code? The site http://moodle.ufsc.br is using this on production since the submission and it works good.

          Show
          Daniel Neis added a comment - Hello, this will be integrated to moodle code? The site http://moodle.ufsc.br is using this on production since the submission and it works good.
          Hide
          Martin Dougiamas added a comment -

          Eric, if you get a moment, could you check this in pretty please?

          Show
          Martin Dougiamas added a comment - Eric, if you get a moment, could you check this in pretty please?
          Hide
          Martin Dougiamas added a comment -

          (and also to HEAD of course)

          Show
          Martin Dougiamas added a comment - (and also to HEAD of course)
          Hide
          Eric Merrill added a comment -

          I have committed this change, but in the process discovered a but in get_my_courses.

          Due to this bug, two more courses then the amount set in the admin setting will be shown. When that ticket is correct, this bug should be fully resolved.

          Show
          Eric Merrill added a comment - I have committed this change, but in the process discovered a but in get_my_courses. Due to this bug, two more courses then the amount set in the admin setting will be shown. When that ticket is correct, this bug should be fully resolved.
          Hide
          Eric Merrill added a comment -

          With MDL-20472 resolved, this issue should also be fully corrected. Resolving the ticket..

          Show
          Eric Merrill added a comment - With MDL-20472 resolved, this issue should also be fully corrected. Resolving the ticket..
          Hide
          Andrew Davis added a comment -

          Closing

          Show
          Andrew Davis added a comment - Closing

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: