LT has to use the "Require e-mail verification when a visitor creates an account" setting from core to make it's decisions, because it leverages the core login block. however, core's idea of email verification and LT's idea of email validation are pretty much polar opposites, which makes this setting very confusing to configure correctly for LT to function properly.

currently, we mirror the setting as 'Set password' in the LT settings, and flip it's display from the core setting, which makes a lot more sense for the way LT works. This however still leaves the core setting at admin/user/settings displayed, which still results in confusion.

all that to say that i think it would be a good idea if we did some form_alter magic at admin/user/settings to remove that checkbox, replace it with a static form value for the setting, and display something like "Control for user set passwords at registration has been moved to the Logintoboggan settings page as the 'Set password' option"

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hunmonk’s picture

Category: task » feature

please see http://drupal.org/project/logintoboggan#commitment for more information on how task/feature requests are handled in the module.

sbrattla’s picture

FileSize
1.45 KB

Hi,

I thought that was a good idea. I've attached a patch which simply adds a little code to logintoboggan_form_alter and modifies the 'user_admin_settings' form. The checkbox 'Require e-mail verification when a visitor creates an account.' simply gets disabled, and the description explains that LoginToboggan now controls this functionality.

Not sure if there are any pitfalls around here, but if LoginToboggan merely mirrors the setting (value flipped), then I can't see any harm done by disabling the ability to modify the setting in core and moving it to LoginToboggan?

hunmonk’s picture

Status: Active » Needs work

this is a good start, but needs some corrections/further investigation:

  1. comments should have a space after the //.
  2. wether -> whether
  3. i'm pretty sure that we don't need to set #value on a disabled form item any more in 7.x. please remove that attribute and test to see if the setting is in fact preserved. if not, then use the same logic as the #default_value setting in the original form item, the ternary expression is not needed.

    or, alternatively:

    i'm wondering if a cleaner approach here would be to simply form_alter the checkbox item into a '#type' => 'item', and override the description. pretty sure that would look good, and since a form item is not an input type, the system settings form wouldn't try to save the value.

  4. i think the implementation of t() for the link to LT settings is wrong. as far as i remember it should be '<a href="!link">LoginToboggan settings page</a>', then use url() for !link, to aid in proper translation of the string.
sbrattla’s picture

Hi,

I've created a new patch where a space has been added after slashes, and I've corrected the 'whether' spelling mistake. Furthermore, the link has been corrected according to your explanation about using url() and not l(). Thanks for your comments!

I'd be happy to change the checkbox to an item, but my only worries is that it might lead to confusion when someone will be looking for a setting which all by a sudden has been removed by another module. People rarely read descriptions, so by letting the checkbox remain as disabled might avoid confusions. What do you think?

hunmonk’s picture

looking better. i can get behind your reasoning for making it a disabled checkbox.

in which case, list item 3 from comment #3 above still needs to be addressed, then i'm happy to test/commit.

sbrattla’s picture

Hi,

I've updated the patch according to comment #5. Sorry for the delay!

hunmonk’s picture

Title: hide core's "Require e-mail verification when a visitor creates an account" » disable core 'Require e-mail verification when a visitor creates an account' setting
Status: Needs work » Fixed

committed to 7.x-1.x-dev, thanks!

Status: Fixed » Closed (fixed)

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