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

      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.

        Gliffy Diagrams

        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: