On the admin/settings/signup page, the "Reset to defaults" button is broken, since we're not really using system_settings_form() correctly. because we define our own submit handler for this form, the reset-to-defaults behavior from system_settings_form_submit() isn't happening. this is all fubar:
a) we should add our own submit handler for the settings form, but not nuke the system submit handler, like so:
$form['#submit']['signup_settings_page_submit'] = array();
$form['#submit']['system_settings_form_submit'] = array();
b) in our submit handler, we shouldn't duplicate any of the work to save stuff in the {variable} table. ours should only deal with the default signup junk we're storing in {signup} for nid==0.
c) our submit handler should also check for the operation (submit vs. reset), and in the case of reset, it should reset what's in {signup} for nid 0. obviously, it'd be nice to somehow share this code with what's in signup.install. perhaps it's not too evil (since reset happens so infrequently) that if we happen to hit that case, we just include_once() signup.install, and move the query to populate nid 0 into a separate function that could be called by both the settings reset handler and signup.install. it's already lame we duplicate this same query for mysql vs. postgres.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | signup_reset_settings_47x.patch_1.txt | 5.91 KB | dww |
| #2 | signup_reset_settings_47x.patch.txt | 3.46 KB | dww |
| #1 | signup_reset_settings_5x.patch.txt | 5.88 KB | dww |
Comments
Comment #1
dwwComment #2
dwwhere's a patch for 4.7.x. however, after i did the patch, i realized that the "reset to defaults" stuff is really all new in 5.x. ;) so, in terms of 4.7.x, this patch would be a new feature request. i might just commit it, anyway, but maybe i'll wait for the 4.7.x-2.* (assuming i ever commit new features there, instead of just going to 5.x-2.* for my new development). but, since i did the work, i thought i should share it.
Comment #3
dwwwhoops, previous 4.7.x patch left out the changes to signup.install. try this one, instead.
Comment #4
dwwcommitted the 5x patch to HEAD and DRUPAL-5.
this really is a new feature for 4.7.x, so i'm not going to commit to DRUPAL-4-7. however, if/when i create the DRUPAL-4-7--2 branch and open new feature development, i'll commit this there.
Comment #5
dwwComment #6
dwwbah, it was a pain to keep this patch separate, and it's well tested. so, i relaxed my no-new-features-in-the-stable-branch policy a little and just committed this to DRUPAL-4-7.
Comment #7
(not verified) commented