Moodle
  1. Moodle
  2. MDL-34800

Turning AJAX on in Grader Report causes update button to disappear

    Details

    • Testing Instructions:
      Hide

      Check the ajax site setting is turned on.

      Go to the grader report for a course. Go to the grader report settings (they're in the gradebook navigation drop down).

      Turn grader report ajax on (if it isnt already).

      Go to the grader report. The "Update" button should disable once all the JS has loaded. Click in a cell and enter a new student grade. Check it saves.

      Turn off grader ajax report. On the grader report turn editing on if its off.

      The Update button should remain active. Enter a new student grade and click update. Check it saved.

      Show
      Check the ajax site setting is turned on. Go to the grader report for a course. Go to the grader report settings (they're in the gradebook navigation drop down). Turn grader report ajax on (if it isnt already). Go to the grader report. The "Update" button should disable once all the JS has loaded. Click in a cell and enter a new student grade. Check it saves. Turn off grader ajax report. On the grader report turn editing on if its off. The Update button should remain active. Enter a new student grade and click update. Check it saved.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-34800_ajax_submit_24
    • Pull Master Branch:
      MDL-34800_ajax_submit
    • Rank:
      43295

      Description

      With AJAX on in the Grader report, after you enter a grade, when the page reloads, the Update button is hidden.

      Replication steps:

      1. Log in as admin/teacher
      2. Navigate to a course that has some gradable activities
      3. Navigate to Settings > Course admin > Grades
      4. From the drop-down at the top of the page select Grader report
      5. Navigate to Settings > Grade admin > My report preferences > Grader report
      6. In the General section set Enable AJAX to Yes
      7. Click Save changes - you should be returned to the Grader report
      8. Enter a grade and click on Update - the page reloads without Update button
      9. Click the configuration icon for the grade item
      10. Check Locked or Clearing the Overridden status and click Save changes - the submit button reappear
      11. Enter another grade and click on Update - the page reloads without Update button
      12. Navigate back to the course then return to the Grader report - The Update button is still hidden

      Turning off AJAX in Grader Report resolves the issue

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that. Feel free to help us work on the issue.

          I was able to reproduce the problem. I've added a bit more detail to your replication steps. For some reason this button is being deliberately hidden using CSS.

          Show
          Michael de Raadt added a comment - Thanks for reporting that. Feel free to help us work on the issue. I was able to reproduce the problem. I've added a bit more detail to your replication steps. For some reason this button is being deliberately hidden using CSS.
          Hide
          Emma Richardson added a comment -

          I wondered about something like that because at times you can see it load before it disappears.

          Show
          Emma Richardson added a comment - I wondered about something like that because at times you can see it load before it disappears.
          Hide
          Andrew Davis added a comment -

          The behaviour is actually deliberate. Its because there's no need to click a button to send your data to the server. Its sent as you enter it. Perhaps it needs to be rethought.

          Show
          Andrew Davis added a comment - The behaviour is actually deliberate. Its because there's no need to click a button to send your data to the server. Its sent as you enter it. Perhaps it needs to be rethought.
          Hide
          Emma Richardson added a comment -

          But the grades don't update or save without the submit button! Are they meant to?

          Show
          Emma Richardson added a comment - But the grades don't update or save without the submit button! Are they meant to?
          Hide
          Andrew Davis added a comment -

          Yes, although Im not sure theres any visual indication that its being done. You'd need to manually reload the page and check whether or not your changes have been saved.

          Show
          Andrew Davis added a comment - Yes, although Im not sure theres any visual indication that its being done. You'd need to manually reload the page and check whether or not your changes have been saved.
          Hide
          Emma Richardson added a comment -

          You are right - it does work. I see two issues with that. Firstly, the update button appears before any grades are added so why is it not removed altogether. Secondly, I think you would have to have a recalculate or refresh button (which is essentially is the submit button!) to update the final grade. Having to reload the page to see the effect of the submitted grades on the final score does not make much sense to me.

          Show
          Emma Richardson added a comment - You are right - it does work. I see two issues with that. Firstly, the update button appears before any grades are added so why is it not removed altogether. Secondly, I think you would have to have a recalculate or refresh button (which is essentially is the submit button!) to update the final grade. Having to reload the page to see the effect of the submitted grades on the final score does not make much sense to me.
          Hide
          Andrew Davis added a comment -

          The button unfortunately cannot be removed altogether because its possible the user has Javascript turned off in their browser. In that case the page needs to fall back to its non-ajax behaviour.

          I'm linking MDL-27201 which sounds related.

          Show
          Andrew Davis added a comment - The button unfortunately cannot be removed altogether because its possible the user has Javascript turned off in their browser. In that case the page needs to fall back to its non-ajax behaviour. I'm linking MDL-27201 which sounds related.
          Hide
          Autumm added a comment - - edited

          The idea that this would be deliberate makes me angry- grrrr. If you have the unlimited grades setting set to yes under general settings for grades you are able to grade over 100% - say you have an item that is worth 35 points this setting allows you to grade someone at 36+. But the only way to do this is to turn editing on in the gradebook and then enter that 36 and hit the update button. If you don't do this and use the ajax entry then you get an error that states - Error Click this box to remove it The grade entered for This Assignment for Susie Student is more than the maximum allowed (of course "This Assignment" will be the name of whatever your assignment is and "Susie Student" will be the name of the student that you are entering the overage grade for). As far as I can tell using the update button is the only way to use the unlimited grades setting.

          Is there another way to use unlimited grades?

          Show
          Autumm added a comment - - edited The idea that this would be deliberate makes me angry- grrrr. If you have the unlimited grades setting set to yes under general settings for grades you are able to grade over 100% - say you have an item that is worth 35 points this setting allows you to grade someone at 36+. But the only way to do this is to turn editing on in the gradebook and then enter that 36 and hit the update button. If you don't do this and use the ajax entry then you get an error that states - Error Click this box to remove it The grade entered for This Assignment for Susie Student is more than the maximum allowed (of course "This Assignment" will be the name of whatever your assignment is and "Susie Student" will be the name of the student that you are entering the overage grade for). As far as I can tell using the update button is the only way to use the unlimited grades setting. Is there another way to use unlimited grades?
          Hide
          Andrew Davis added a comment -

          Autumm, the problem you're describing is being fixed in MDL-32076. Once that is done you will be able to enter unlimited grades through the ajax interface.

          Show
          Andrew Davis added a comment - Autumm, the problem you're describing is being fixed in MDL-32076 . Once that is done you will be able to enter unlimited grades through the ajax interface.
          Hide
          Andrew Davis added a comment - - edited

          I'm putting up a potential change. https://github.com/andyjdavis/moodle/compare/master...MDL-34800_ajax_submit
          All I've done is switch from hiding the update button to simply disabling it. Hopefully that is a little less off putting. As the page loads you can enter grades and, if you're really fast or if there's a problem, use the Update button. Once the Javascript code is all loaded the button disables and changes you make are automatically saved as you enter them.

          Show
          Andrew Davis added a comment - - edited I'm putting up a potential change. https://github.com/andyjdavis/moodle/compare/master...MDL-34800_ajax_submit All I've done is switch from hiding the update button to simply disabling it. Hopefully that is a little less off putting. As the page loads you can enter grades and, if you're really fast or if there's a problem, use the Update button. Once the Javascript code is all loaded the button disables and changes you make are automatically saved as you enter them.
          Hide
          Andrew Davis added a comment -

          Emma, regarding your comment (https://tracker.moodle.org/browse/MDL-34800?focusedCommentId=172793&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-172793) the course total should be being updated when you submit grades using the ajax interface provided the course total itself is not overriden.

          Currently however the overall average cells are not updated via ajax. MDL-27201 will fix that.

          Show
          Andrew Davis added a comment - Emma, regarding your comment ( https://tracker.moodle.org/browse/MDL-34800?focusedCommentId=172793&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-172793 ) the course total should be being updated when you submit grades using the ajax interface provided the course total itself is not overriden. Currently however the overall average cells are not updated via ajax. MDL-27201 will fix that.
          Hide
          Jason Fowler added a comment -

          [Y] Syntax
          [Y] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [N] Testing
          [-] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Just need some testing instructions there Andrew.

          Show
          Jason Fowler added a comment - [Y] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [N] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Just need some testing instructions there Andrew.
          Hide
          Damyon Wiese added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          Thanks!

          Show
          Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 24 and 23. Thanks Andrew

          Show
          Dan Poltawski added a comment - Integrated to master, 24 and 23. Thanks Andrew
          Hide
          Michael de Raadt added a comment -

          Test result: Unsuccessful

          I tested this on Master, 24 and 23. Oddly, it worked as expected in 24, but not in Master or 23.

          I enabled AJAX in the site level report settings and in the user's preference settings within Gradebook.

          In 23 and Master a JS error was presented and the Update button was not disabled. This was the error...

          23
          TypeError: this.report.grades is undefined
          http://michael.moodle.local/23_integration/lib/javascript.php/1361327125/grade/report/grader/module.js
          Line 40
          
          master
          TypeError: this.report.grades is undefined
          http://michael.moodle.local/master_integration/lib/javascript.php/1361333993/grade/report/grader/module.js
          Line 1
          

          The offending code appears to be in here somewhere (line 40 from 23)...

          if(allcomplete){this.pendingsubmissions=[];}};M.gradereport_grader.classes.ajax.prototype.display_submission_error=function(message,cell){var erroroverlay=new this.report.Y.Overlay({headerContent:'<div><strong class="error">'+M.str.gradereport_grader.ajaxerror+'</strong> <em>'+M.str.gradereport_grader.ajaxclicktoclose+'</em></div>',bodyContent:message,visible:false,zIndex:3});erroroverlay.set('xy',[cell.getX()+10,cell.getY()+10]);erroroverlay.render(this.report.table.ancestor('div'));erroroverlay.show();erroroverlay.get('boundingBox').on('click',function(){this.get('boundingBox').setStyle('visibility','hidden');this.hide();this.destroy();},erroroverlay);erroroverlay.get('boundingBox').setStyle('visibility','visible');};M.gradereport_grader.classes.existingfield=function(ajax,userid,itemid){this.report=ajax.report;this.userid=userid;this.itemid=itemid;this.editfeedback=ajax.showquickfeedback;this.grade=this.report.Y.one('#grade_'+userid+'_'+itemid);for(var i=0;i<this.report.grades.length;i++){if(this.report.grades[i]['user']==this.userid&&this.report.grades[i]['item']==this.itemid){this.oldgrade=this.report.grades[i]['grade'];}}
          
          Show
          Michael de Raadt added a comment - Test result: Unsuccessful I tested this on Master, 24 and 23. Oddly, it worked as expected in 24, but not in Master or 23. I enabled AJAX in the site level report settings and in the user's preference settings within Gradebook. In 23 and Master a JS error was presented and the Update button was not disabled. This was the error... 23 TypeError: this.report.grades is undefined http://michael.moodle.local/23_integration/lib/javascript.php/1361327125/grade/report/grader/module.js Line 40 master TypeError: this.report.grades is undefined http://michael.moodle.local/master_integration/lib/javascript.php/1361333993/grade/report/grader/module.js Line 1 The offending code appears to be in here somewhere (line 40 from 23)... if (allcomplete){ this .pendingsubmissions=[];}};M.gradereport_grader.classes.ajax.prototype.display_submission_error=function(message,cell){ var erroroverlay= new this .report.Y.Overlay({headerContent:'<div><strong class= "error" >'+M.str.gradereport_grader.ajaxerror+'</strong> <em>'+M.str.gradereport_grader.ajaxclicktoclose+'</em></div>',bodyContent:message,visible: false ,zIndex:3});erroroverlay.set('xy',[cell.getX()+10,cell.getY()+10]);erroroverlay.render( this .report.table.ancestor('div'));erroroverlay.show();erroroverlay.get('boundingBox').on('click',function(){ this .get('boundingBox').setStyle('visibility','hidden'); this .hide(); this .destroy();},erroroverlay);erroroverlay.get('boundingBox').setStyle('visibility','visible');};M.gradereport_grader.classes.existingfield=function(ajax,userid,itemid){ this .report=ajax.report; this .userid=userid; this .itemid=itemid; this .editfeedback=ajax.showquickfeedback; this .grade= this .report.Y.one('#grade_'+userid+'_'+itemid); for ( var i=0;i< this .report.grades.length;i++){ if ( this .report.grades[i]['user']== this .userid&& this .report.grades[i]['item']== this .itemid){ this .oldgrade= this .report.grades[i]['grade'];}}
          Hide
          Andrew Davis added a comment - - edited

          I can't for the life of me reproduce this in either integration or the main repository. Michael, can you check whether you see the same thing in the main repository? There's no guarantee that whatever is causing this is new code.

          Show
          Andrew Davis added a comment - - edited I can't for the life of me reproduce this in either integration or the main repository. Michael, can you check whether you see the same thing in the main repository? There's no guarantee that whatever is causing this is new code.
          Hide
          Dan Poltawski added a comment -

          Michael?

          Show
          Dan Poltawski added a comment - Michael?
          Hide
          Andrew Davis added a comment -

          fyi, I've tried this in Firefox and Chrome, both in Ubuntu. It works fine as either teacher or admin (for me).

          Michael, I know you're using Firefox but in what OS? It may or may not be relevant.

          Show
          Andrew Davis added a comment - fyi, I've tried this in Firefox and Chrome, both in Ubuntu. It works fine as either teacher or admin (for me). Michael, I know you're using Firefox but in what OS? It may or may not be relevant.
          Hide
          Michael de Raadt added a comment -

          Sorry. I will have another look now.

          Show
          Michael de Raadt added a comment - Sorry. I will have another look now.
          Hide
          Michael de Raadt added a comment -

          This is definitely still happening in the integration master. I will checkout a stable release and try this after lunch.

          Show
          Michael de Raadt added a comment - This is definitely still happening in the integration master. I will checkout a stable release and try this after lunch.
          Hide
          Michael de Raadt added a comment -

          I just checked this with a brand new Stable instance and it had the same problem. So it doesn't look like this change is responsible for the problem.

          I found one factor that you might be able to replicate for your testing. I am using a gradebook that has activities in it, but no actual grades. In other words, it is a fresh course restore. When I add some marks and click Update, the page refreshes and then things seem to work as expected. This might explain the difference I was seeing between versions before. After adding some grades in the stable version, the update button disappears as reported; in the integration version, the button is now disabled.

          I retested this with FF, Chrome and IE and they all reported a JS error. IE isolated it down to the following...

          SCRIPT5007: Unable to get property 'length' of undefined or null reference 
          module.js, line 1 character 12608
          

          ...which seems to revolve around this statement...

          for(var i=0;i<this.report.grades.length;i++)if(this.report.grades[i]...
          

          ...which leads back to the report FF gave (shown above).

          I hope this helps.

          Show
          Michael de Raadt added a comment - I just checked this with a brand new Stable instance and it had the same problem. So it doesn't look like this change is responsible for the problem. I found one factor that you might be able to replicate for your testing. I am using a gradebook that has activities in it, but no actual grades. In other words, it is a fresh course restore. When I add some marks and click Update, the page refreshes and then things seem to work as expected. This might explain the difference I was seeing between versions before. After adding some grades in the stable version, the update button disappears as reported; in the integration version, the button is now disabled. I retested this with FF, Chrome and IE and they all reported a JS error. IE isolated it down to the following... SCRIPT5007: Unable to get property 'length' of undefined or null reference module.js, line 1 character 12608 ...which seems to revolve around this statement... for ( var i=0;i< this .report.grades.length;i++) if ( this .report.grades[i]... ...which leads back to the report FF gave (shown above). I hope this helps.
          Hide
          Damyon Wiese added a comment -

          The reported JS error looks like a different issue to me - I vote for passing this.

          Show
          Damyon Wiese added a comment - The reported JS error looks like a different issue to me - I vote for passing this.
          Hide
          Michael de Raadt added a comment -

          While this error is a not caused by current changes, I believe it is in the scope of the current issue.

          Show
          Michael de Raadt added a comment - While this error is a not caused by current changes, I believe it is in the scope of the current issue.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          I think that if the problem addressed by Michael is reproducible without the patch (aka, current moodle.git = production), then it's a separate issue for sure.

          Only if the problem was introduced by this issue I'd consider them related.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited I think that if the problem addressed by Michael is reproducible without the patch (aka, current moodle.git = production), then it's a separate issue for sure. Only if the problem was introduced by this issue I'd consider them related. Ciao
          Hide
          Dan Poltawski added a comment -

          Michael has confirmed this is a pre-existing problem, so passing this based on integrator vote.

          Show
          Dan Poltawski added a comment - Michael has confirmed this is a pre-existing problem, so passing this based on integrator vote.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Cool, but is such problem reported?

          Show
          Eloy Lafuente (stronk7) added a comment - Cool, but is such problem reported?
          Hide
          Dan Poltawski added a comment -

          Yep: MDL-38149

          Show
          Dan Poltawski added a comment - Yep: MDL-38149
          Hide
          Andrew Davis added a comment -

          Thankyou everyone for dealing with this in my absence. I'm quite unwell right now but should hopefully be back on deck tomorrow.

          Show
          Andrew Davis added a comment - Thankyou everyone for dealing with this in my absence. I'm quite unwell right now but should hopefully be back on deck tomorrow.
          Hide
          Damyon Wiese added a comment -

          Thanks for your hard work - this issue has made it! Moodle is now a little bit better.

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Thanks for your hard work - this issue has made it! Moodle is now a little bit better. Regards, Damyon

            People

            • Votes:
              12 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: