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

Suspended Students appear in Gradebook, indistinguishable from Active Students

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: Gradebook
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE

      Description

      there should be a way to distinguish/separate suspended students from active students in the Gradebook.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            andyjdavis Andrew Davis added a comment - - edited

            MDL-20946 (once integrated) will lay the foundation for this issue to be fixed. Is it preferable to have suspended students hidden entirely or for them to be visible but marked in some way?

            Its certainly very easy for me to simply limit the gradebook to only show users with an active (non-suspended) enrollment but is that desirable?

            Show
            andyjdavis Andrew Davis added a comment - - edited MDL-20946 (once integrated) will lay the foundation for this issue to be fixed. Is it preferable to have suspended students hidden entirely or for them to be visible but marked in some way? Its certainly very easy for me to simply limit the gradebook to only show users with an active (non-suspended) enrollment but is that desirable?
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            Ive made some changes that will prevent suspended users from appearing at all. I'm not saying thats what we want to do but if we do then this may be how we do it

            repo: git://github.com/andyjdavis/moodle.git
            branch: MDL-26835_20_STABLE_grader_show_suspended
            diff: https://github.com/andyjdavis/moodle/compare/MOODLE_20_STABLE...MDL-26835_20_STABLE_grader_show_suspended

            Show
            andyjdavis Andrew Davis added a comment - - edited Ive made some changes that will prevent suspended users from appearing at all. I'm not saying thats what we want to do but if we do then this may be how we do it repo: git://github.com/andyjdavis/moodle.git branch: MDL-26835 _20_STABLE_grader_show_suspended diff: https://github.com/andyjdavis/moodle/compare/MOODLE_20_STABLE...MDL-26835_20_STABLE_grader_show_suspended
            Hide
            andyjdavis Andrew Davis added a comment -

            I've posted in the forums (http://moodle.org/mod/forum/discuss.php?d=172365) to get some more feedback on which option is preferable.

            Show
            andyjdavis Andrew Davis added a comment - I've posted in the forums ( http://moodle.org/mod/forum/discuss.php?d=172365 ) to get some more feedback on which option is preferable.
            Hide
            elenaivanova Elena Ivanova added a comment -

            They should appear, due to MDL-26373

            Show
            elenaivanova Elena Ivanova added a comment - They should appear, due to MDL-26373
            Hide
            andyjdavis Andrew Davis added a comment -

            Bringing this into the new sprint.

            Show
            andyjdavis Andrew Davis added a comment - Bringing this into the new sprint.
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            Ok, it seems like the consensus is that suspended users should be displayed but be flagged somehow.

            Ive committed a really preliminary version that flags users that are suspended using both css and an icon. There is a fairly significant problem with this in that it involves calling is_enrolled() on every user which is going to be fairly slow. Is there a better way to put a flag on each user to indicate that they are suspended?

            repo: git://github.com/andyjdavis/moodle.git
            branch: MDL-26835_20_STABLE_grader_highlight_suspended_users
            diff: https://github.com/andyjdavis/moodle/compare/MOODLE_20_STABLE...MDL-26835_20_STABLE_grader_highlight_suspended_users

            Show
            andyjdavis Andrew Davis added a comment - - edited Ok, it seems like the consensus is that suspended users should be displayed but be flagged somehow. Ive committed a really preliminary version that flags users that are suspended using both css and an icon. There is a fairly significant problem with this in that it involves calling is_enrolled() on every user which is going to be fairly slow. Is there a better way to put a flag on each user to indicate that they are suspended? repo: git://github.com/andyjdavis/moodle.git branch: MDL-26835 _20_STABLE_grader_highlight_suspended_users diff: https://github.com/andyjdavis/moodle/compare/MOODLE_20_STABLE...MDL-26835_20_STABLE_grader_highlight_suspended_users
            Hide
            andyjdavis Andrew Davis added a comment -

            Ok, Ive committed a version that avoids calling is_enrolled() on every user. Peer review time.

            Show
            andyjdavis Andrew Davis added a comment - Ok, Ive committed a version that avoids calling is_enrolled() on every user. Peer review time.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Andrew,

            I've just been looking at this now.
            I think you should get Petr to have a quick look a the SQL, he has a better eye for the enrolments stuff than me.
            I did spot a couple of things however:

            1. The SQL query is counting ue.id but it's not actually being used anywhere. It looks like it could be removed.
            2. Lines 412-416... is that meant to be $this->users[$user->id]>suspendedenrolment = !array_key_exists($user>id, $useractiveenrolments);
            3. Given you've already got an in_or_equal for the userid in the code above would it be pertinent to use it as part of this query... if its safe to I'm sure itd be better for performance.
            4. I think the icon should be renamed to something like suspendedenrolment.gif its much more obvious what it is being used for and less likely themer's will replace it with a media player pause button or the like.

            Other than that looks good to me.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Andrew, I've just been looking at this now. I think you should get Petr to have a quick look a the SQL, he has a better eye for the enrolments stuff than me. I did spot a couple of things however: The SQL query is counting ue.id but it's not actually being used anywhere. It looks like it could be removed. Lines 412-416... is that meant to be $this->users [$user->id] >suspendedenrolment = !array_key_exists($user >id, $useractiveenrolments); Given you've already got an in_or_equal for the userid in the code above would it be pertinent to use it as part of this query... if its safe to I'm sure itd be better for performance. I think the icon should be renamed to something like suspendedenrolment.gif its much more obvious what it is being used for and less likely themer's will replace it with a media player pause button or the like. Other than that looks good to me. Cheers Sam
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            fixed those 4 things.

            2.0 version
            repo: git://github.com/andyjdavis/moodle.git
            branch: MDL-26835_20_STABLE_grader_highlight_suspended_users
            diff: https://github.com/andyjdavis/moodle/compare/MOODLE_20_STABLE...MDL-26835_20_STABLE_grader_highlight_suspended_users

            master (2.1) version
            repo: git://github.com/andyjdavis/moodle.git
            branch: MDL-26835_MASTER_grader_highlight_suspended_users
            diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26835_MASTER_grader_highlight_suspended_users

            Show
            andyjdavis Andrew Davis added a comment - - edited fixed those 4 things. 2.0 version repo: git://github.com/andyjdavis/moodle.git branch: MDL-26835 _20_STABLE_grader_highlight_suspended_users diff: https://github.com/andyjdavis/moodle/compare/MOODLE_20_STABLE...MDL-26835_20_STABLE_grader_highlight_suspended_users master (2.1) version repo: git://github.com/andyjdavis/moodle.git branch: MDL-26835 _MASTER_grader_highlight_suspended_users diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26835_MASTER_grader_highlight_suspended_users
            Hide
            andyjdavis Andrew Davis added a comment -

            PULL-689 PULL-690

            Show
            andyjdavis Andrew Davis added a comment - PULL-689 PULL-690

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: