Moodle
  1. Moodle
  2. MDL-32192

Rename the string invalidgradeitmeid to invalidgradeitemid

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.5, 2.2.2, 2.3
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Gradebook
    • Labels:
    • Testing Instructions:
      Hide

      Testing this is a little bit bogus. The easiest way is to go into grade/report/grader/lib.php Comment out the continue at line 198. Also comment out the if around print_error('invalidgradeitemid');

      Now viewing the grader report should display a message saying "Incorrect grade item id"

      Remove your code changes and check that you can view the grader report without error.

      Turn editing on, enter a grade for a student, click update. Check that the grade was saved.

      Show
      Testing this is a little bit bogus. The easiest way is to go into grade/report/grader/lib.php Comment out the continue at line 198. Also comment out the if around print_error('invalidgradeitemid'); Now viewing the grader report should display a message saying "Incorrect grade item id" Remove your code changes and check that you can view the grader report without error. Turn editing on, enter a grade for a student, click update. Check that the grade was saved.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-32192_rename_string
    • Rank:
      38945

      Description

      What is an itme ID? It should be item ID. Rename the string. It is used in a few places and some amos code will also be necessary.

      The string is in the grades lang file.

        Activity

        Hide
        Andrew Davis added a comment -

        Added a solution and testing instructions. Putting this up for peer review.

        Show
        Andrew Davis added a comment - Added a solution and testing instructions. Putting this up for peer review.
        Hide
        Dan Poltawski added a comment -

        Hi Andrew,

        Looks good. Only thing is that I think that it would look better (and may even be necessary for AMOS) to move the ' AMOS BEGIN' off the first line of commit message onto its own line.

        Show
        Dan Poltawski added a comment - Hi Andrew, Looks good. Only thing is that I think that it would look better (and may even be necessary for AMOS) to move the ' AMOS BEGIN' off the first line of commit message onto its own line.
        Hide
        Andrew Davis added a comment -

        Split the commit message out to several lines and added the various branches.

        Show
        Andrew Davis added a comment - Split the commit message out to several lines and added the various branches.
        Hide
        Dan Poltawski 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.

        TIA and ciao

        Show
        Dan Poltawski 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. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Hi Andrew,

        Sending this back sorry, we don't remove strings from STABLE branches.

        The master branch is spot on however in the two stable branches you need to add the new string without removing the old string and use AMOS CPY instead of MOV.

        There are two reasons we don't remove strings from stable branches, a) so that those who upgrade moodle but have code still using the old key arn't left without a string, and b) so that those who upgrade their lang files but don't upgrade moodle arn't left with broken strings.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Andrew, Sending this back sorry, we don't remove strings from STABLE branches. The master branch is spot on however in the two stable branches you need to add the new string without removing the old string and use AMOS CPY instead of MOV. There are two reasons we don't remove strings from stable branches, a) so that those who upgrade moodle but have code still using the old key arn't left without a string, and b) so that those who upgrade their lang files but don't upgrade moodle arn't left with broken strings. Cheers Sam
        Hide
        Andrew Davis added a comment -

        I'm adding this into the current stable sprint to prevent me from forgetting it (again).

        Show
        Andrew Davis added a comment - I'm adding this into the current stable sprint to prevent me from forgetting it (again).
        Hide
        Andrew Davis added a comment -

        Updated the 2.2 and 2.1 stable branches. Submitting for integration.

        Show
        Andrew Davis added a comment - Updated the 2.2 and 2.1 stable branches. Submitting for integration.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I think CPY on stables is ok, and MOV only master.

        Anyway, the point I'm not sure is if "error" is a valid component name or it should be, perhaps "core_error" instead. I have not been able to find any previous use neither documentation about that.

        I'm halting this until getting some confirmation from David.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - I think CPY on stables is ok, and MOV only master. Anyway, the point I'm not sure is if "error" is a valid component name or it should be, perhaps "core_error" instead. I have not been able to find any previous use neither documentation about that. I'm halting this until getting some confirmation from David. Ciao
        Hide
        David Mudrak added a comment -

        Yup, Sam's explanation is perfect. MOV on master, CPY on stables. While I personally prefer frankenstyle in both get_string() calls and AMOScript in the commit message, AMOS will cope with the "error" component in [invalidgradeitemid,error] pretty well. Thanks Andrew for the fix and you guys for your eagle's eyes.

        Show
        David Mudrak added a comment - Yup, Sam's explanation is perfect. MOV on master, CPY on stables. While I personally prefer frankenstyle in both get_string() calls and AMOScript in the commit message, AMOS will cope with the "error" component in [invalidgradeitemid,error] pretty well. Thanks Andrew for the fix and you guys for your eagle's eyes.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks! (21, 22 and master)

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (21, 22 and master)
        Hide
        Rajesh Taneja added a comment -

        Works Great

        Thanks for fixing this Andrew.

        Show
        Rajesh Taneja added a comment - Works Great Thanks for fixing this Andrew.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        This has been near becoming rejected, because it's not the best code you are able to produce.

        But, luckily, at the end, it has landed and has been spread to all repos out there.

        Many thanks and, don't forget it, keep improving your skills, you can!

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - This has been near becoming rejected, because it's not the best code you are able to produce. But, luckily, at the end, it has landed and has been spread to all repos out there. Many thanks and, don't forget it, keep improving your skills, you can! Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: