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

Rename the string invalidgradeitmeid to invalidgradeitemid

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Activity

          Hide
          andyjdavis Andrew Davis added a comment -

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

          Show
          andyjdavis Andrew Davis added a comment - Added a solution and testing instructions. Putting this up for peer review.
          Hide
          poltawski 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
          poltawski 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
          andyjdavis Andrew Davis added a comment -

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

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

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

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

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

          Show
          andyjdavis Andrew Davis added a comment - Updated the 2.2 and 2.1 stable branches. Submitting for integration.
          Hide
          stronk7 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
          stronk7 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
          mudrd8mz 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
          mudrd8mz 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
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks! (21, 22 and master)

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

          Works Great

          Thanks for fixing this Andrew.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Works Great Thanks for fixing this Andrew.
          Hide
          stronk7 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
          stronk7 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:
                Fix Release Date:
                14/May/12