If the user does not tick the 'I agree with...' checkbox then they receive an error message of 'field is required.'
This is not very informative.

Should it not say something like 'You must agree with the Terms and Conditions'

Comments

soglesby’s picture

soglesby commented

uc_termsofservice.module function uc_termsofservice_get_item (line 222).

The 'title' key in the array looks like it should really be '#title' and should be set to be $title (that is passed into function) rather than $node->title.

i.e. change from:

function uc_termsofservice_get_item($type = NULL, $title, $path, $size = NULL) {
  $options = array('attributes' => array('class' => 'uc_termsofservice-child'. (!empty($size) ? ' uc_termsofservice-size['. $size .']' : '')));
  $required = variable_get('uc_termsofservice_'. $type .'_required', 0);
  $form['tos_agree_popup'] = array(
    '#type' => 'checkboxes',
    'title' => t('@title', array('@title' => $node->title)),
    '#options' => array('agreed' => t('I agree with the !tos', array('!tos' => l($title, $path .'/'. $type, $options)))),
    '#required' => $required,
  );
  return $form;
}

to:

function uc_termsofservice_get_item($type = NULL, $title, $path, $size = NULL) {
  $options = array('attributes' => array('class' => 'uc_termsofservice-child'. (!empty($size) ? ' uc_termsofservice-size['. $size .']' : '')));
  $required = variable_get('uc_termsofservice_'. $type .'_required', 0);
  $form['tos_agree_popup'] = array(
    '#type' => 'checkboxes',
    '#title' => t('@title', array('@title' => $title)),
    '#options' => array('agreed' => t('I agree with the !tos', array('!tos' => l($title, $path .'/'. $type, $options)))),
    '#required' => $required,
  );
  return $form;
}

This also means that the title is displayed before the checkbox.
To display a more informative message the validation and error message would need to be passed into Forms API rather than using the required flag.

Regards,
Steven

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Assigned: Unassigned » pcambra

You're right with the fapi error, I will correct this in the -dev version, and include other changes.

I agree there should be a more sensitive error when validating the required "Terms of Service", I will place a validate function instead of fapi's #required.

Thanks for reporting back.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Version: 6.x-1.1 » 6.x-1.x-dev
Status: Active » Fixed

Hi

I've removed the title for the checkout, I think it is better without it, I've also removed the required from the fapi configuration and the message now is by default:
In order to continue with the checkout process, you should accept first the !tos

When !tos is the node title of the tos node, of course this can be changed in translation basis.

Please test it from -dev release it will be refreshed soon, meanwhile you can checkout it from CVS.

Thanks for reporting!

Status: Fixed » Closed (fixed)

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