It would be good to have something like a green light/red light visual indicator that settings on the settings page are valid.

That is, test that chosen directories exist, that directories are writable, etc.. and provide visual indication.

Also could check that wget and tar are available.

Comments

nterbogt’s picture

nterbogt commented

I don't know if it's possible or not, but it would be nice to have this geen/red light status go directly into admin/logs/status page rather than the individual module settings page.

drawk’s picture

drawk commented

It would certainly be possible to set the warnings in the watchdog pages. I'm not sure if that would be better than doing it while validating the settings, though.

Maybe both?

pobster’s picture

pobster commented

Meh if it were me I'd just add this to the settings code page;

if (!is_dir(variable_get('module_installer_path_download', 'files'))) form_set_error('module_installer_path_download', t('The directory %directory does not exist.', array('%directory' => variable_get('module_installer_path_download', 'files'))));

Or perhaps a more Drupally;

file_check_directory(variable_get('module_installer_path_download', 'files'), 0, 'module_installer_path_download');

Neither work very well as Drupal settings are broken imo - if the path can't be found you can't resave the settings, so if you make a mistake you're stuck with it. That's Drupal broken not the module... Maybe they'll fix it, maybe they won't... Who knows.

Pobster

drawk’s picture

drawk commented

Hmm, isn't this what the validation hook is intended for though? If an invalid directory is given, then we can catch it before the change is committed and warn the user.

pobster’s picture

pobster commented

Ah yes :o) But I'm using the 4.7 branch of your module (well... I'm using both, but the 4.7 branch is on my live site and the 5.0 is on my test site) and you don't get the luxury of a validation hook for your settings in 4.7...

...I guess for 4.7 you could always not set a form_error and just return an error drupal_set_message instead? Up to you! Your module! ;o) I myself don't think any of this is necessary, this is an administration tool and as such if its wrong when you try to use it then it tells you what the problem is. In my eyes this is sufficient?

Pobster