Moodle
  1. Moodle
  2. MDL-14055

code uses hard-coded integer values instead of defined, human-readable strings

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Authentication
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      What does "2" mean? I don't know. Ah – look at the HTML:

      <option value="0" selected="selected">Keep internal</option>
      <option value="1">Suspend internal</option>
      <option value="2">Full delete internal</option>

      The code should use, e.g., AUTH_REMOVEUSER_KEEP, AUTH_REMOVEUSER_SUSPEND, and AUTH_REMOVEUSER_FULLDELETE instead of 0, 1, and 2.

      I'll submit a patch for 2.0 shortly, because I suppose it's not worth changing released versions. This will NOT change functionality, it will only improve code readability (which should cut down on future bug introduction, developer frustration, etc.)

        Gliffy Diagrams

          Activity

          Hide
          Matt Oquist added a comment -

          Here's a patch against HEAD. Warning: this is untested, though a visual inspection of the diff shows the hard-coded values replaced cleanly with the define()d tokens, so I don't know how anything could go wrong.

          I'm just about to start testing user removal with external db auth on my 1.8.3+ test system, and I made this patch from the changes I've already made there. I'll definitely comment here if I run into any problems.

          Show
          Matt Oquist added a comment - Here's a patch against HEAD. Warning: this is untested, though a visual inspection of the diff shows the hard-coded values replaced cleanly with the define()d tokens, so I don't know how anything could go wrong. I'm just about to start testing user removal with external db auth on my 1.8.3+ test system, and I made this patch from the changes I've already made there. I'll definitely comment here if I run into any problems.
          Hide
          Martin Dougiamas added a comment -

          Thanks Matt! Dongsheng can you check in to dev ?

          Show
          Martin Dougiamas added a comment - Thanks Matt! Dongsheng can you check in to dev ?
          Hide
          Andrew Davis added a comment -

          verified. closing. While there i noticed this same problem elsewhere. MDL-21066

          Show
          Andrew Davis added a comment - verified. closing. While there i noticed this same problem elsewhere. MDL-21066

            People

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

              Dates

              • Created:
                Updated:
                Resolved: