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

Non-editing teachers are unable to use JS on assign grading table view

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.6.1, 2.7, FRONTEND
    • Component/s: Assignment
    • Labels:
      None

      Description

      If logged in as non-editing teacher, go into assign grading page (ensure that submission comments are on, e.g. no team submission, etc), and try to expand submission comments. Nothing will happen, while you will get the comments expanded being logged in as a teacher. It works on other view (grading particular student, etc).

      If you take a look on the browser's console, you will notice this:
      Uncaught TypeError: Cannot call method 'on' of null at module.js:153

        Gliffy Diagrams

          Activity

          Hide
          pavel.m.sokolov Pavel Sokolov added a comment - - edited

          The problem arises because of bug within mod/assign/module.js. Since non-editing teachers cannot choose active enrolment, it breaks JS on Line 153.

          The following should be changed:

          FROM:
          showonlyactiveenrolelement.on('change', function(e)

          { Y.one('form.gradingoptionsform').submit(); }

          );

          TO:
          if (showonlyactiveenrolelement) {
          showonlyactiveenrolelement.on('change', function(e)

          { Y.one('form.gradingoptionsform').submit(); }

          );
          }

          I can submit a patch via branch, if you want.

          Show
          pavel.m.sokolov Pavel Sokolov added a comment - - edited The problem arises because of bug within mod/assign/module.js. Since non-editing teachers cannot choose active enrolment, it breaks JS on Line 153. The following should be changed: FROM: showonlyactiveenrolelement.on('change', function(e) { Y.one('form.gradingoptionsform').submit(); } ); TO: if (showonlyactiveenrolelement) { showonlyactiveenrolelement.on('change', function(e) { Y.one('form.gradingoptionsform').submit(); } ); } I can submit a patch via branch, if you want.
          Hide
          danmarsden Dan Marsden added a comment -

          Hi Pavel - if you add a link here to a git branch with a fix that would be great! - If you haven't done this before you might want to take a look here:
          http://docs.moodle.org/dev/Commit_cheat_sheet

          Show
          danmarsden Dan Marsden added a comment - Hi Pavel - if you add a link here to a git branch with a fix that would be great! - If you haven't done this before you might want to take a look here: http://docs.moodle.org/dev/Commit_cheat_sheet
          Hide
          pavel.m.sokolov Pavel Sokolov added a comment -

          Hi Dan,
          I attached a git branch with a fix (originally cloned from master).

          Show
          pavel.m.sokolov Pavel Sokolov added a comment - Hi Dan, I attached a git branch with a fix (originally cloned from master).
          Hide
          danmarsden Dan Marsden added a comment -

          thanks Pavel - any chance you could modify that commit message to follow the guidelines here:
          http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages

          a better commit message would look like this:

          MDL-43199 assign: fix js error when expanding comments for non editing teachers in assign grading table 
          

          I haven't looked at the code in context so passing this up for peer review - if you get a chance to fix that commit message it would be great! - you will need to update the Pull Master Diff URL above after doing that as the commitid will change.

          Show
          danmarsden Dan Marsden added a comment - thanks Pavel - any chance you could modify that commit message to follow the guidelines here: http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages a better commit message would look like this: MDL-43199 assign: fix js error when expanding comments for non editing teachers in assign grading table I haven't looked at the code in context so passing this up for peer review - if you get a chance to fix that commit message it would be great! - you will need to update the Pull Master Diff URL above after doing that as the commitid will change.
          Hide
          danmarsden Dan Marsden added a comment -

          bouncing this up for peer review - I haven't looked closely at the code myself.

          Show
          danmarsden Dan Marsden added a comment - bouncing this up for peer review - I haven't looked closely at the code myself.
          Hide
          pavel.m.sokolov Pavel Sokolov added a comment -

          Fixed commit message and updated Pull Master Diff URL. Thank you for guidance.

          Show
          pavel.m.sokolov Pavel Sokolov added a comment - Fixed commit message and updated Pull Master Diff URL. Thank you for guidance.
          Hide
          danmarsden Dan Marsden added a comment -

          great! - hopefully someone will peer review this soon.

          Show
          danmarsden Dan Marsden added a comment - great! - hopefully someone will peer review this soon.
          Hide
          damyon Damyon Wiese added a comment -

          Thanks - the patch looks perfect. I just updated the branches so there are 26 and master branches so it's clear for the integrator. This does not affect 25.

          Nice fix - pushing for integration!

          Show
          damyon Damyon Wiese added a comment - Thanks - the patch looks perfect. I just updated the branches so there are 26 and master branches so it's clear for the integrator. This does not affect 25. Nice fix - pushing for integration!
          Hide
          marina Marina Glancy added a comment -

          Thanks Pavel, Damyon, this was integrated in 2.6 and master

          Show
          marina Marina Glancy added a comment - Thanks Pavel, Damyon, this was integrated in 2.6 and master
          Hide
          dmonllao David Monllaó added a comment -

          All working as expected, no JS errors.

          Show
          dmonllao David Monllaó added a comment - All working as expected, no JS errors.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks for the code, its now upstream!

          Heres a fun trick to try in the spirit of Friday the 13th.
          I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks for the code, its now upstream! Heres a fun trick to try in the spirit of Friday the 13th. I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                13/Jan/14