Moodle
  1. Moodle
  2. MDL-39740

Updating grade_item idnumber doesn't update course_module idnumber

    Details

    • Testing Instructions:
      Hide
      1. Create an assignment
      2. Go to the gradebook, categories and items
      3. Edit topic total calculation
      4. Add id number “assign1” to assignment
      5. Click “add ID numbers”
      6. Enter grade calculation formula: =[[assign1]]
      7. Save changes
      8. Edit assignment settings
      9. Make sure ID number “assign1” appears under common module settings
      10. Change description
      11. Save and return to topic
      12. Go back to gradebook, categories and items
      13. Edit topic total calculation
      14. Make sure ID number “assign1” appears in list
      15. Make sure calculation formula displays correctly

      (note this effects all modules, but the test cases cover the assign module)

      Show
      Create an assignment Go to the gradebook, categories and items Edit topic total calculation Add id number “assign1” to assignment Click “add ID numbers” Enter grade calculation formula: =[ [assign1] ] Save changes Edit assignment settings Make sure ID number “assign1” appears under common module settings Change description Save and return to topic Go back to gradebook, categories and items Edit topic total calculation Make sure ID number “assign1” appears in list Make sure calculation formula displays correctly (note this effects all modules, but the test cases cover the assign module)
    • Workaround:
      Hide

      Set the module idnumber via the module settings form rather than via the gradebook. This will push the idnumber to the grade item.

      Show
      Set the module idnumber via the module settings form rather than via the gradebook. This will push the idnumber to the grade item.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-39740_grade_item_idnumber_fix

      Description

      If a module's grade item idnumber is set (via the Grades / ▶ Edit / ▶ Edit calculation page), and the associated course module does not yet have an idnumber set, it is supposed to push the new idnumber value to the course module's idnumber setting. This is failing due to a bug in grade_item::add_idnumber().

      As a result, when editing a module's settigs and saving, it will blow away the idnumber set on the grade item which effects the custom calculation set.

      This appears to be due to a check which is doing a strict type comparison, expecting an integer, but records back from the DB contain the value as a string.

      Steps to reproduce:

      1. Create an assignment
      2. Go to the gradebook, categories and items, full view
      3. Edit topic total calculation (click the calculator icon)
      4. Add id number “assign1” to assignment
      5. Click “add ID numbers”
      6. Enter grade calculation formula: =[[assign1]]
      7. Save changes
      8. Edit assignment settings
      9. Change description
      10. Save and return to topic
      11. Go back to gradebook, categories and items
      12. Edit topic total calculation
      13. Assignment ID number has disappeared from list
      14. Calculation formula now displays incorrectly, eg =##giXXX##

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Michael de Raadt added a comment -

            This duplicates a couple of pre-exisiting issues, but as this is more complete, this can be our go-to issue.

            If you would like to push this to peer review, please do so.

            Show
            Michael de Raadt added a comment - This duplicates a couple of pre-exisiting issues, but as this is more complete, this can be our go-to issue. If you would like to push this to peer review, please do so.
            Hide
            Damyon Wiese added a comment -

            This fix looks correct to me - added Andrew as a watcher to get his +1.

            The only thing I can see missing is the 24 branch - but it looks like an easy cherry-pick.

            [Y] Syntax
            [Y] Whitespace
            [-] Output
            [-] Language
            [-] Databases
            [Y] Testing (instructions and automated tests)
            [-] Security
            [-] Documentation
            [N] Git - Missing 24 git branch
            [Y] Sanity check

            Show
            Damyon Wiese added a comment - This fix looks correct to me - added Andrew as a watcher to get his +1. The only thing I can see missing is the 24 branch - but it looks like an easy cherry-pick. [Y] Syntax [Y] Whitespace [-] Output [-] Language [-] Databases [Y] Testing (instructions and automated tests) [-] Security [-] Documentation [N] Git - Missing 24 git branch [Y] Sanity check
            Hide
            Andrew Davis added a comment -

            +1. Just make sure to run this through all the grade related unit tests if you haven't already.

            Show
            Andrew Davis added a comment - +1. Just make sure to run this through all the grade related unit tests if you haven't already.
            Hide
            CiBoT added a comment -

            Results for MDL-39740

            • Error: the repository field is empty. Nothing was checked.
            Show
            CiBoT added a comment - Results for MDL-39740 Error: the repository field is empty. Nothing was checked.
            Hide
            Dan Poltawski added a comment -

            Sending all 'waiting for peer review' issues for integration review. The integration team are doing this to ensure any 'integratable issues' will not got missed for freeze.

            Note: We will prioritise peer reviewed issues and may not spend as much time examining non-integratable, non peer-reviewed issues.

            This is a present from the iTeam - it means that peer review is not working well enough! We really do not want to do this again! Lets improve our peer review process!

            Show
            Dan Poltawski added a comment - Sending all 'waiting for peer review' issues for integration review. The integration team are doing this to ensure any 'integratable issues' will not got missed for freeze. Note: We will prioritise peer reviewed issues and may not spend as much time examining non-integratable, non peer-reviewed issues. This is a present from the iTeam - it means that peer review is not working well enough! We really do not want to do this again! Lets improve our peer review process!
            Hide
            Dan Poltawski added a comment -

            Thanks Ashley - not sure how this got stuck, but its been integrated now!

            Show
            Dan Poltawski added a comment - Thanks Ashley - not sure how this got stuck, but its been integrated now!
            Hide
            Adrian Greeve added a comment -

            Tested on 2.5, 2.6, and master integration branches.
            I had an initial hiccough with the idnumber not being updated, but I haven't been able to reproduce it.
            Seems okay. Test passed.

            Show
            Adrian Greeve added a comment - Tested on 2.5, 2.6, and master integration branches. I had an initial hiccough with the idnumber not being updated, but I haven't been able to reproduce it. Seems okay. Test passed.
            Hide
            Dan Poltawski added a comment -

            Thanks for your efforts, this change is now part of Moodle!

            Show
            Dan Poltawski added a comment - Thanks for your efforts, this change is now part of Moodle!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: