Moodle

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

Details

  • Type: Improvement Improvement
  • Status: Closed 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.)

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

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: