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

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

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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
          moquist 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
          moquist 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
          dougiamas Martin Dougiamas added a comment -

          Thanks Matt! Dongsheng can you check in to dev ?

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

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

          Show
          andyjdavis 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:
                Fix Release Date:
                24/Nov/10