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

          Attachments

            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: