Moodle
  1. Moodle
  2. MDL-26835

Suspended Students appear in Gradebook, indistinguishable from Active Students

    Details

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

      Description

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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          Elena Ivanova added a comment -

          They should appear, due to MDL-26373

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

          Bringing this into the new sprint.

          Show
          Andrew Davis added a comment - Bringing this into the new sprint.
          Hide
          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
          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
          Andrew Davis added a comment -

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

          Show
          Andrew Davis added a comment - Ok, Ive committed a version that avoids calling is_enrolled() on every user. Peer review time.
          Hide
          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
          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
          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
          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
          Andrew Davis added a comment -

          PULL-689 PULL-690

          Show
          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: