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
    • Rank:
      36446

      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.)

        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: