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

Is GRADE_UPDATE_ITEM_DELETED useful or can it be deleted?

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.1
    • Fix Version/s: 2.3
    • Component/s: Gradebook
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      Check that you have debugging set to developer.

      Go into a course that has at least one student.

      Add an LTI activity that uses grades. (please see MDLQA-1419 - contains instructions to add a particular LTI 'dummy')

      Grade a student.

      Delete the activity.

      Check you get no errors.

      Show
      Check that you have debugging set to developer. Go into a course that has at least one student. Add an LTI activity that uses grades. (please see MDLQA-1419 - contains instructions to add a particular LTI 'dummy') Grade a student. Delete the activity. Check you get no errors.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-31362_unused_constant

      Description

      GRADE_UPDATE_ITEM_DELETED isnt really used in code. Its defined in /lib/grade/constants.php but never used.

      Is it meant to be returned when a grade item is deleted? Or is it meant to be returned when you try to update a grade item that has been deleted? Or is it pointless so it can be safely removed?

        Gliffy Diagrams

          Activity

          Hide
          andyjdavis Andrew Davis added a comment - - edited

          I'm guessing but I think there were plans back in around 2007 to have grade_update() return GRADE_UPDATE_ITEM_DELETED when grade_update() was used to delete a grade item. Instead GRADE_UPDATE_OK is being returned.

          I could change grade_update() to return GRADE_UPDATE_ITEM_DELETED but this is part of the grade API so changing its return value is non-trivial. The current behaviour is not really wrong so I think we should just stick with removing GRADE_UPDATE_ITEM_DELETED to avoid future confusion.

          Show
          andyjdavis Andrew Davis added a comment - - edited I'm guessing but I think there were plans back in around 2007 to have grade_update() return GRADE_UPDATE_ITEM_DELETED when grade_update() was used to delete a grade item. Instead GRADE_UPDATE_OK is being returned. I could change grade_update() to return GRADE_UPDATE_ITEM_DELETED but this is part of the grade API so changing its return value is non-trivial. The current behaviour is not really wrong so I think we should just stick with removing GRADE_UPDATE_ITEM_DELETED to avoid future confusion.
          Hide
          andyjdavis Andrew Davis added a comment -

          Adding a branch.

          I suspect this should only go into master. There's no real need to break anyone's contrib code in stable if they have done the same as the lti module and attempted to future proof their code.

          Show
          andyjdavis Andrew Davis added a comment - Adding a branch. I suspect this should only go into master. There's no real need to break anyone's contrib code in stable if they have done the same as the lti module and attempted to future proof their code.
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Hi Andrew,
          Looks safe to me as well to delete it.
          +1 to integrate to master only
          Thanks

          Show
          ankit_frenz Ankit Agarwal added a comment - Hi Andrew, Looks safe to me as well to delete it. +1 to integrate to master only Thanks
          Hide
          andyjdavis Andrew Davis added a comment -

          Submitting for master only integration

          Show
          andyjdavis Andrew Davis added a comment - Submitting for master only integration
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Some hours ago...

          the main moodle.git repository has 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.

          TIA and ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Some hours ago... the main moodle.git repository has 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. TIA and ciao
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Andrew this has been integrated now

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Andrew this has been integrated now
          Hide
          timb Tim Barker added a comment -

          Will finish this off tomorrow

          Show
          timb Tim Barker added a comment - Will finish this off tomorrow
          Hide
          timb Tim Barker added a comment -

          I can't test this until I have a URL for an LTI (that accepts grades), which I don't currently have, so for now testing is on hold.

          Show
          timb Tim Barker added a comment - I can't test this until I have a URL for an LTI (that accepts grades), which I don't currently have, so for now testing is on hold.
          Hide
          nebgor Aparup Banerjee added a comment -

          ok got me some LTI to eat "http://tracker.moodle.org/browse/MDLQA-1419 and http://tracker.moodle.org/browse/MDLQA-1418" - thanks MD

          Show
          nebgor Aparup Banerjee added a comment - ok got me some LTI to eat "http://tracker.moodle.org/browse/MDLQA-1419 and http://tracker.moodle.org/browse/MDLQA-1418 " - thanks MD
          Hide
          nebgor Aparup Banerjee added a comment -

          seems to work fine for me..
          tested on qa.moodle.net (integration.git) with http://www.imsglobal.org/developers/LTI/test/v1p1/tool.php

          sends and receives grades fine.
          deletes fine - no errors.

          Show
          nebgor Aparup Banerjee added a comment - seems to work fine for me.. tested on qa.moodle.net (integration.git) with http://www.imsglobal.org/developers/LTI/test/v1p1/tool.php sends and receives grades fine. deletes fine - no errors.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          FCT (fixed, closing, thanks). Ciao

          "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
          ~ Benjamin Disraeli

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                25/Jun/12