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

          Attachments

            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