Commit fd657370 authored by Lisa Seeto's avatar Lisa Seeto Committed by Robert Lyon
Browse files

Bug 1844076: Only 1 error message shown even though there are 2 on "Site options"



- refactor networkingform & siteoptionform to not use _fail funcs
- siteoptionform now has its own validation func

behatnotneeded

Change-Id: I219ec6cbb56aa559b60a9cd97226601e2008305d
Signed-off-by: default avatarLisa Seeto <lisaseeto@catalyst.net.nz>
parent 1babdfd1
Loading
Loading
Loading
Loading
+7 −32
Original line number Diff line number Diff line
@@ -78,6 +78,7 @@ $networkingform = pieform(
                'title'        => get_string('enablenetworking','admin'),
                'description'  => get_string('enablenetworkingdescription','admin'),
                'defaultvalue' => get_config('enablenetworking'),
                'disabled'     => in_array('enablenetworking', $OVERRIDDEN),
            ),
            'promiscuousmode' => array(
                'type'         => 'switchbox',
@@ -85,6 +86,7 @@ $networkingform = pieform(
                'title'        => get_string('promiscuousmode','admin'),
                'description'  => get_string('promiscuousmodedescription','admin'),
                'defaultvalue' => get_config('promiscuousmode'),
                'disabled'     => in_array('promiscuousmode', $OVERRIDDEN),
            ),
            'submitbuttons' => array(
                'type' => 'fieldset',
@@ -112,17 +114,8 @@ $networkingform = pieform(
    )
);

function networkingform_fail(Pieform $form) {
    $form->reply(PIEFORM_ERR, array(
        'message' => get_string('enablenetworkingfailed','admin'),
        'goto'    => '/admin/site/networking.php',
    ));
}


function networkingform_submit(Pieform $form, $values) {
    $reply = '';

    if ($form->get_submitvalue() === 'deletekey') {
        global $SESSION;
        $openssl = OpenSslRepo::singleton();
@@ -135,33 +128,15 @@ function networkingform_submit(Pieform $form, $values) {
    }

    if (get_config('enablenetworking') != $values['enablenetworking']) {
        if (!set_config('enablenetworking', $values['enablenetworking'])) {
            networkingform_fail($form);
        }
        else {
            if (empty($values['enablenetworking'])) {
                $reply .= get_string('networkingdisabled','admin');
            }
            else {
                $reply .= get_string('networkingenabled','admin');
        $values['enablenetworking'] ? $reply .= get_string('networkingenabled','admin') : $reply .= get_string('networkingdisabled','admin');
    }
        }
    }

    if (get_config('promiscuousmode') != $values['promiscuousmode']) {
        if (!set_config('promiscuousmode', $values['promiscuousmode'])) {
            networkingform_fail($form);
        }
        else {
            if (empty($values['promiscuousmode'])) {
                $reply .= get_string('promiscuousmodedisabled','admin');
            }
            else {
                $reply .= get_string('promiscuousmodeenabled','admin');
            }
        }
        $values['promiscuousmode'] ? $reply .= get_string('promiscuousmodeenabled','admin') : $reply .= get_string('promiscuousmodedisabled','admin');
    }

    set_config('enablenetworking', $values['enablenetworking']);
    set_config('promiscuousmode', $values['promiscuousmode']);

    $form->reply(PIEFORM_OK, array(
        'message' => ($reply == '') ? get_string('networkingunchanged','admin') : $reply,
        'goto'    => '/admin/site/networking.php',
+39 −54
Original line number Diff line number Diff line
@@ -43,6 +43,8 @@ $siteoptionform = array(
    'renderer'   => 'div',
    'plugintype' => 'core',
    'pluginname' => 'admin',
    'validatecallback' => 'siteoptions_validate',
    'successcallback' => 'siteoptions_submit',
    'jssuccesscallback' => 'checkReload',
    'elements'   => array(
        'sitesettings' => array(
@@ -845,11 +847,29 @@ $siteoptionform['elements']['submit'] = array(

$siteoptionform = pieform($siteoptionform);

function siteoptions_fail(Pieform $form, $field) {
    $form->reply(PIEFORM_ERR, array(
        'message' => get_string('setsiteoptionsfailed', 'admin', get_string($field, 'admin')),
        'goto'    => '/admin/site/options.php',
    ));
function siteoptions_validate(Pieform $form, $values) {

    if (get_config('searchplugin') != $values['searchplugin']) {
        // Call the new search plugin's can connect
        safe_require('search', $values['searchplugin']);
        $connect = call_static_method(generate_class_name('search', $values['searchplugin']), 'can_connect');
        if (!$connect) {
            $form->set_error('searchplugin', get_string('searchconfigerror1', 'admin', $values['searchplugin']));
        }
    }
    if ($values['viruschecking'] == true) {
        $pathtoclam = escapeshellcmd(trim(get_config('pathtoclam')));
        if (!$pathtoclam ) {
            $form->set_error('viruschecking', get_string('clamnotset', 'mahara', $pathtoclam));
        }
        else if (!file_exists($pathtoclam) && !is_executable($pathtoclam)) {
            $form->set_error('viruschecking', get_string('clamlost', 'mahara', $pathtoclam));
        }
    }

    if ($values['recaptchaonregisterform'] && !($values['recaptchapublickey'] && $values['recaptchaprivatekey'])) {
        $form->set_error('recaptchaonregisterform', get_string('recaptchakeysmissing1', 'admin'));
    }
}

function siteoptions_submit(Pieform $form, $values) {
@@ -959,14 +979,17 @@ function siteoptions_submit(Pieform $form, $values) {
    }
    // Turn homepageredirecturl into string
    $values['homepageredirecturl'] = !empty($values['homepageredirecturl']) ? $values['homepageredirecturl'][0] : '';
    $oldsearchplugin = get_config('searchplugin');
    $oldlanguage = get_config('lang');
    $oldtheme = get_config('theme');
    $fieldsfailed = 0;
    foreach ($fields as $field) {
        if (!set_config($field, $values[$field])) {
            siteoptions_fail($form, $field);
            $form->set_error($field, get_string('setsiteoptionsfailed1', 'admin'));
            $fieldsfailed += 1;
        }
    }
    $oldsearchplugin = get_config('searchplugin');
    $oldlanguage = get_config('lang');
    $oldtheme = get_config('theme');

    if ($oldlanguage != $values['lang']) {
        safe_require('artefact', 'file');
        ArtefactTypeFolder::change_public_folder_name($oldlanguage, $values['lang']);
@@ -982,63 +1005,25 @@ function siteoptions_submit(Pieform $form, $values) {
        safe_require('search', $values['searchplugin']);
        $initialize = call_static_method(generate_class_name('search', $values['searchplugin']), 'initialize_sitewide');
        if (!$initialize) {
            $form->reply(PIEFORM_ERR, array(
                'message' => get_string('searchconfigerror1', 'admin', $values['searchplugin']),
                'goto'    => '/admin/site/options.php',
            ));
            $form->set_error('searchplugin', get_string('searchconfigerror1', 'admin', $values['searchplugin']));
            $fieldsfailed += 1;
        }
    }
    // Call the new search plugin's can connect
    safe_require('search', $values['searchplugin']);
    $connect = call_static_method(generate_class_name('search', $values['searchplugin']), 'can_connect');
    if (!$connect) {
    if ($fieldsfailed > 0) {
        $form->reply(PIEFORM_ERR, array(
            'message' => get_string('searchconfigerror1', 'admin', $values['searchplugin']),
            'message' => get_string('setsiteoptionsfailednotice', 'admin', $fieldsfailed),
            'goto'    => '/admin/site/options.php',
        ));
    }

    // submitted sessionlifetime is in minutes; db entry session_timeout is in seconds
    if (!set_config('session_timeout', $values['sessionlifetime'] * 60)) {
        siteoptions_fail($form, 'sessionlifetime');
    }
    set_config('session_timeout', $values['sessionlifetime'] * 60);

    // Submitted value is on/off; database entry should be 1/0
    foreach(array('viruschecking', 'usersallowedmultipleinstitutions') as $checkbox) {
        if (!set_config($checkbox, (int) ($values[$checkbox] == 'on'))) {
            siteoptions_fail($form, $checkbox);
        }
        set_config($checkbox, (int) ($values[$checkbox] == 'on'));
    }

    if ($values['viruschecking'] == 'on') {
        $pathtoclam = escapeshellcmd(trim(get_config('pathtoclam')));
        if (!$pathtoclam ) {
            $form->reply(PIEFORM_ERR, array(
                'message' => get_string('clamnotset', 'mahara', $pathtoclam),
                'goto'    => '/admin/site/options.php',
            ));
        }
        else if (!file_exists($pathtoclam) && !is_executable($pathtoclam)) {
            $form->reply(PIEFORM_ERR, array(
                'message' => get_string('clamlost', 'mahara', $pathtoclam),
                'goto'    => '/admin/site/options.php',
            ));
        }
    }

    if (get_config('recaptchaonregisterform')
            && !(
                    get_config('recaptchapublickey')
                    && get_config('recaptchaprivatekey')
            )
    ) {
        $form->reply(
            PIEFORM_ERR,
            array(
                'message' => get_string('recaptchakeysmissing1', 'admin'),
                'goto' => '/admin/site/options.php',
            )
        );
    }
    // Need to clear the cached menus in case site config changes effect them.
    clear_menu_cache();

+2 −1
Original line number Diff line number Diff line
@@ -452,7 +452,8 @@ $string['searchuserspublic'] = 'Show users in public search';
$string['searchuserspublicdescription'] = 'Allow users\' names to appear in public search results. This needs to have \'publicsearchallowed\' set to true and be using a search plugin that allows public search, e.g. Elasticsearch. Changing this setting will require search re-indexing.';
$string['sessionlifetime'] = 'Session lifetime';
$string['sessionlifetimedescription'] = 'Time in minutes after which an inactive logged-in user will be automatically logged out.';
$string['setsiteoptionsfailed'] = 'Failed setting the %s option';
$string['setsiteoptionsfailed1'] = 'Failed setting this option';
$string['setsiteoptionsfailednotice'] = 'Failed to set %s field options';
$string['showonlineuserssideblock'] = 'Show online users';
$string['showonlineuserssideblockdescriptionmessage1'] = 'Users can see a sidebar with a list of the online users.';
$string['showselfsearchsideblock1'] = 'Portfolio search';