Do not use variable_get when validating the settings form

By using variable_get while validating the settings form, you are using the old value and not the value that the user entered to (for example) reference what the "max timeout" should be. If I get an error saying that "role x timeout must be an integer greater than 60, and less then 1800" and so I change the max to 10000 I still get the identical error when I resubmit the form even though I have acceptable values.

Do not validate role based timeouts if the user has unchecked "use role timeout"

These values should only be validated when they matter. Why make the user put in a timeout value for administrators that is less than "max" if the user is not using that type of timeout?

..oh, and I snuck in some more coding standards cleanup. Shhhhhhhhh

CommentFileSizeAuthor
#5 autologout-form_validation_fixes-1706540-1706540.patch6.28 KBjohnennew
#3 autologout-form_validation_fixes-1706540.patch7.43 KBjohnennew
autologout_validation.patch5.69 KBbleen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dandrews’s picture

dandrews CreditAttribution: dandrews commented
Status: Needs review » Needs work

Tried patching and ran into some issues, will work on it and post back.

rooby’s picture

rooby CreditAttribution: rooby commented

I just had a quick look and there are no calls to variable_get() in the settings validate function.

I think this issue is no longer relevant with the latest version of the module.

johnennew’s picture

johnennew CreditAttribution: johnennew commented
Status: Needs work » Needs review
FileSize
autologout-form_validation_fixes-1706540.patch7.43 KB

Hi all,

The default timeout is respecting the new form submitted value of max_timeout but the role time outs still do not. Attached patch fixes this as well as adding the removal of form validation of role timeouts which are disabled. Tests included describing the issue and proving working functionality.

johnennew’s picture

johnennew CreditAttribution: johnennew commented
Version: 7.x-4.x-dev » 6.x-4.x-dev
Status: Needs review » Needs work

Pretty sure this is good with the tests - committing to the 7.x-4.x and setting to 6.x-4.x for backport

johnennew’s picture

johnennew CreditAttribution: johnennew commented
Status: Needs work » Needs review
FileSize
autologout-form_validation_fixes-1706540-1706540.patch6.28 KB

6.x-4.x backport ready for committing.

johnennew’s picture

johnennew CreditAttribution: johnennew commented
Status: Needs review » Fixed

Patch committed to 6.x-4.x

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit a0e9d7d on 7.x-4.x, 8.x-1.x authored by bleen18, committed by ceng: 
    Issue #1706540 by bleen18 and ceng: Fixed form validation and add some...