Merge pull request #1136 from jfefe/extrarequired

Missing checks on extrafields

htdocs/comm/propal.php

@@ -331,7 +331,7 @@ else if ($action == 'add' && $user->rights->propal->creer)
 			$object->origin		= GETPOST('origin');
 			$object->origin_id	= GETPOST('originid');
 
-			for ($i = 1 ; $i <= $conf->global->PRODUCT_SHOW_WHEN_CREATE; $i++)
+			for ($i = 1; $i <= $conf->global->PRODUCT_SHOW_WHEN_CREATE; $i++)
 			{
 				if ($_POST['idprod'.$i])
 				{
@@ -344,55 +344,61 @@ else if ($action == 'add' && $user->rights->propal->creer)
 
 			// Fill array 'array_options' with data from add form
 			$ret = $extrafields->setOptionalsFromPost($extralabels,$object);
-
+			if($ret < 0) {
-			$id = $object->create($user);
+				$error++;
+				$action = 'create';
+			}
 		}
 
-		if ($id > 0)
+		if(!$error) {
-		{
+			$id = $object->create($user);
-			// Insertion contact par defaut si defini
-			if (GETPOST('contactidp') > 0)
-			{
-				$result=$object->add_contact(GETPOST('contactidp'),'CUSTOMER','external');
-				if ($result < 0)
-				{
-					$error++;
-					setEventMessage($langs->trans("ErrorFailedToAddContact"), 'errors');
-				}
-			}
 
-			if (! $error)
+			if ($id > 0)
 			{
-				$db->commit();
+				// Insertion contact par defaut si defini
-
+				if (GETPOST('contactidp') > 0)
-				if (empty($conf->global->MAIN_DISABLE_PDF_AUTOUPDATE))
 				{
-					// Define output language
+					$result=$object->add_contact(GETPOST('contactidp'),'CUSTOMER','external');
-					$outputlangs = $langs;
+					if ($result < 0)
-					if (! empty($conf->global->MAIN_MULTILANGS))
 					{
-						$outputlangs = new Translate("",$conf);
+						$error++;
-						$newlang=(GETPOST('lang_id') ? GETPOST('lang_id') : $object->client->default_lang);
+						setEventMessage($langs->trans("ErrorFailedToAddContact"), 'errors');
-						$outputlangs->setDefaultLang($newlang);
 					}
-					$ret=$object->fetch($id);    // Reload to get new records
-					propale_pdf_create($db, $object, $object->modelpdf, $outputlangs, $hidedetails, $hidedesc, $hideref);
 				}
 
-				header('Location: '.$_SERVER["PHP_SELF"].'?id='.$id);
+				if (! $error)
-				exit;
+				{
+					$db->commit();
+
+					if (empty($conf->global->MAIN_DISABLE_PDF_AUTOUPDATE))
+					{
+						// Define output language
+						$outputlangs = $langs;
+						if (! empty($conf->global->MAIN_MULTILANGS))
+						{
+							$outputlangs = new Translate("",$conf);
+							$newlang=(GETPOST('lang_id') ? GETPOST('lang_id') : $object->client->default_lang);
+							$outputlangs->setDefaultLang($newlang);
+						}
+						$ret=$object->fetch($id);    // Reload to get new records
+						propale_pdf_create($db, $object, $object->modelpdf, $outputlangs, $hidedetails, $hidedesc, $hideref);
+					}
+
+					header('Location: '.$_SERVER["PHP_SELF"].'?id='.$id);
+					exit;
+				}
+				else
+				{
+					$db->rollback();
+				}
 			}
 			else
 			{
+				dol_print_error($db,$object->error);
 				$db->rollback();
+				exit;
 			}
 		}
-		else
-		{
-			dol_print_error($db,$object->error);
-			$db->rollback();
-			exit;
-		}
 	}
 }
 
@@ -1121,24 +1127,30 @@ else if ($action == 'update_extras')
 	$extralabels=$extrafields->fetch_name_optionals_label($object->table_element);
 	$ret = $extrafields->setOptionalsFromPost($extralabels,$object);
 
-	// Actions on extra fields (by external module or standard code)
+	if($ret < 0) {
-	// FIXME le hook fait double emploi avec le trigger !!
+		$error++;
-	$hookmanager->initHooks(array('propaldao'));
+		$action = 'edit_extras';
-	$parameters=array('id'=>$object->id);
+	}
-	$reshook=$hookmanager->executeHooks('insertExtraFields',$parameters,$object,$action);    // Note that $action and $object may have been modified by some hooks
+
-	if (empty($reshook))
+	if(!$error) {
-	{
+		// Actions on extra fields (by external module or standard code)
-		if (empty($conf->global->MAIN_EXTRAFIELDS_DISABLED)) // For avoid conflicts if trigger used
+		// FIXME le hook fait double emploi avec le trigger !!
+		$hookmanager->initHooks(array('propaldao'));
+		$parameters=array('id'=>$object->id);
+		$reshook=$hookmanager->executeHooks('insertExtraFields',$parameters,$object,$action);    // Note that $action and $object may have been modified by some hooks
+		if (empty($reshook))
 		{
-			$result=$object->insertExtraFields();
+			if (empty($conf->global->MAIN_EXTRAFIELDS_DISABLED)) // For avoid conflicts if trigger used
-			if ($result < 0)
 			{
-				$error++;
+				$result=$object->insertExtraFields();
+				if ($result < 0)
+				{
+					$error++;
+				}
 			}
 		}
+		else if ($reshook < 0) $error++;
 	}
-	else if ($reshook < 0) $error++;
-
 }
 
 if (! empty($conf->global->MAIN_DISABLE_CONTACTS_TAB) && $user->rights->propal->creer)

htdocs/commande/fiche.php

@@ -275,117 +275,130 @@ else if ($action == 'add' && $user->rights->commande->creer)
 
 			// Fill array 'array_options' with data from add form
 			$ret = $extrafields->setOptionalsFromPost($extralabels,$object);
+			if($ret < 0)
+				$error++;
 
-			$object_id = $object->create($user);
+			if(!$error) {
+				$object_id = $object->create($user);
 
-			if ($object_id > 0)
+				if ($object_id > 0)
-			{
-				dol_include_once('/'.$element.'/class/'.$subelement.'.class.php');
-
-				$classname = ucfirst($subelement);
-				$srcobject = new $classname($db);
-
-				dol_syslog("Try to find source object origin=".$object->origin." originid=".$object->origin_id." to add lines");
-				$result=$srcobject->fetch($object->origin_id);
-				if ($result > 0)
 				{
-					$lines = $srcobject->lines;
+					dol_include_once('/'.$element.'/class/'.$subelement.'.class.php');
-					if (empty($lines) && method_exists($srcobject,'fetch_lines'))  $lines = $srcobject->fetch_lines();
 
-					$fk_parent_line=0;
+					$classname = ucfirst($subelement);
-					$num=count($lines);
+					$srcobject = new $classname($db);
 
-					for ($i=0;$i<$num;$i++)
+					dol_syslog("Try to find source object origin=".$object->origin." originid=".$object->origin_id." to add lines");
+					$result=$srcobject->fetch($object->origin_id);
+					if ($result > 0)
 					{
-						$label=(! empty($lines[$i]->label)?$lines[$i]->label:'');
+						$lines = $srcobject->lines;
-						$desc=(! empty($lines[$i]->desc)?$lines[$i]->desc:$lines[$i]->libelle);
+						if (empty($lines) && method_exists($srcobject,'fetch_lines'))  $lines = $srcobject->fetch_lines();
-						$product_type=(! empty($lines[$i]->product_type)?$lines[$i]->product_type:0);
 
-						// Dates
+						$fk_parent_line=0;
-						// TODO mutualiser
+						$num=count($lines);
-						$date_start=$lines[$i]->date_debut_prevue;
-						if ($lines[$i]->date_debut_reel) $date_start=$lines[$i]->date_debut_reel;
-						if ($lines[$i]->date_start) $date_start=$lines[$i]->date_start;
-						$date_end=$lines[$i]->date_fin_prevue;
-						if ($lines[$i]->date_fin_reel) $date_end=$lines[$i]->date_fin_reel;
-						if ($lines[$i]->date_end) $date_end=$lines[$i]->date_end;
 
-						// Reset fk_parent_line for no child products and special product
+						for ($i=0;$i<$num;$i++)
-						if (($lines[$i]->product_type != 9 && empty($lines[$i]->fk_parent_line)) || $lines[$i]->product_type == 9) {
-							$fk_parent_line = 0;
-						}
-
-						$result = $object->addline(
-							$object_id,
-							$desc,
-							$lines[$i]->subprice,
-							$lines[$i]->qty,
-							$lines[$i]->tva_tx,
-							$lines[$i]->localtax1_tx,
-							$lines[$i]->localtax2_tx,
-							$lines[$i]->fk_product,
-							$lines[$i]->remise_percent,
-							$lines[$i]->info_bits,
-							$lines[$i]->fk_remise_except,
-							'HT',
-							0,
-							$date_start,
-							$date_end,
-							$product_type,
-							$lines[$i]->rang,
-							$lines[$i]->special_code,
-							$fk_parent_line,
-							$lines[$i]->fk_fournprice,
-							$lines[$i]->pa_ht,
-							$label
-						);
-
-						if ($result < 0)
 						{
-							$error++;
+							$label=(! empty($lines[$i]->label)?$lines[$i]->label:'');
-							break;
+							$desc=(! empty($lines[$i]->desc)?$lines[$i]->desc:$lines[$i]->libelle);
+							$product_type=(! empty($lines[$i]->product_type)?$lines[$i]->product_type:0);
+
+							// Dates
+							// TODO mutualiser
+							$date_start=$lines[$i]->date_debut_prevue;
+							if ($lines[$i]->date_debut_reel) $date_start=$lines[$i]->date_debut_reel;
+							if ($lines[$i]->date_start) $date_start=$lines[$i]->date_start;
+							$date_end=$lines[$i]->date_fin_prevue;
+							if ($lines[$i]->date_fin_reel) $date_end=$lines[$i]->date_fin_reel;
+							if ($lines[$i]->date_end) $date_end=$lines[$i]->date_end;
+
+							// Reset fk_parent_line for no child products and special product
+							if (($lines[$i]->product_type != 9 && empty($lines[$i]->fk_parent_line)) || $lines[$i]->product_type == 9) {
+								$fk_parent_line = 0;
+							}
+
+							$result = $object->addline(
+								$object_id,
+								$desc,
+								$lines[$i]->subprice,
+								$lines[$i]->qty,
+								$lines[$i]->tva_tx,
+								$lines[$i]->localtax1_tx,
+								$lines[$i]->localtax2_tx,
+								$lines[$i]->fk_product,
+								$lines[$i]->remise_percent,
+								$lines[$i]->info_bits,
+								$lines[$i]->fk_remise_except,
+								'HT',
+								0,
+								$date_start,
+								$date_end,
+								$product_type,
+								$lines[$i]->rang,
+								$lines[$i]->special_code,
+								$fk_parent_line,
+								$lines[$i]->fk_fournprice,
+								$lines[$i]->pa_ht,
+								$label
+							);
+
+							if ($result < 0)
+							{
+								$error++;
+								break;
+							}
+
+							// Defined the new fk_parent_line
+							if ($result > 0 && $lines[$i]->product_type == 9) {
+								$fk_parent_line = $result;
+							}
 						}
 
-						// Defined the new fk_parent_line
+						// Hooks
-						if ($result > 0 && $lines[$i]->product_type == 9) {
+						$parameters=array('objFrom'=>$srcobject);
-							$fk_parent_line = $result;
+						$reshook=$hookmanager->executeHooks('createFrom',$parameters,$object,$action);    // Note that $action and $object may have been modified by hook
-						}
+						if ($reshook < 0) $error++;
+					}
+					else
+					{
+						$mesg=$srcobject->error;
+						$error++;
 					}
-
-					// Hooks
-					$parameters=array('objFrom'=>$srcobject);
-					$reshook=$hookmanager->executeHooks('createFrom',$parameters,$object,$action);    // Note that $action and $object may have been modified by hook
-					if ($reshook < 0) $error++;
 				}
 				else
 				{
-					$mesg=$srcobject->error;
+					$mesg=$object->error;
 					$error++;
 				}
 			}
 			else
 			{
-				$mesg=$object->error;
+				// Required extrafield left blank, error message already defined by setOptionalsFromPost()
-				$error++;
+				$action='create';
 			}
 		}
 		else
 		{
 			// Fill array 'array_options' with data from add form
 			$ret = $extrafields->setOptionalsFromPost($extralabels,$object);
+			if($ret < 0)
+				$error++;
 
-			$object_id = $object->create($user);
+			if(!$error) {
+				$object_id = $object->create($user);
 
-			// If some invoice's lines already known
+				// If some invoice's lines already known
-			$NBLINES=8;
+				$NBLINES=8;
-			for ($i = 1 ; $i <= $NBLINES ; $i++)
+				for ($i = 1 ; $i <= $NBLINES ; $i++)
-			{
-				if ($_POST['idprod'.$i])
 				{
-					$xid = 'idprod'.$i;
+					if ($_POST['idprod'.$i])
-					$xqty = 'qty'.$i;
+					{
-					$xremise = 'remise_percent'.$i;
+						$xid = 'idprod'.$i;
-					$object->add_product($_POST[$xid],$_POST[$xqty],$_POST[$xremise]);
+						$xqty = 'qty'.$i;
+						$xremise = 'remise_percent'.$i;
+						$object->add_product($_POST[$xid],$_POST[$xqty],$_POST[$xremise]);
+					}
 				}
 			}
 		}
@@ -1130,23 +1143,32 @@ else if ($action == 'update_extras')
 	$extralabels=$extrafields->fetch_name_optionals_label($object->table_element);
 	$ret = $extrafields->setOptionalsFromPost($extralabels,$object);
 
-	// Actions on extra fields (by external module or standard code)
+	if($ret < 0)
-	// FIXME le hook fait double emploi avec le trigger !!
+		$error++;
-	$hookmanager->initHooks(array('orderdao'));
+
-	$parameters=array('id'=>$object->id);
+	if(!$error) {
-	$reshook=$hookmanager->executeHooks('insertExtraFields',$parameters,$object,$action);    // Note that $action and $object may have been modified by some hooks
+		// Actions on extra fields (by external module or standard code)
-	if (empty($reshook))
+		// FIXME le hook fait double emploi avec le trigger !!
-	{
+		$hookmanager->initHooks(array('orderdao'));
-		if (empty($conf->global->MAIN_EXTRAFIELDS_DISABLED)) // For avoid conflicts if trigger used
+		$parameters=array('id'=>$object->id);
+		$reshook=$hookmanager->executeHooks('insertExtraFields',$parameters,$object,$action);    // Note that $action and $object may have been modified by some hooks
+		if (empty($reshook))
 		{
-			$result=$object->insertExtraFields();
+			if (empty($conf->global->MAIN_EXTRAFIELDS_DISABLED)) // For avoid conflicts if trigger used
-			if ($result < 0)
 			{
-				$error++;
+				$result=$object->insertExtraFields();
+				if ($result < 0)
+				{
+					$error++;
+				}
 			}
 		}
+		else if ($reshook < 0) $error++;
+	}
+	else
+	{
+		$action = 'edit_extras';
 	}
-	else if ($reshook < 0) $error++;
 
 }
 
@@ -1335,7 +1357,7 @@ if ($action == 'send' && ! GETPOST('addfile') && ! GETPOST('removedfile') && ! G
 	}
 }
 
-if (! empty($conf->global->MAIN_DISABLE_CONTACTS_TAB) && $user->rights->commande->creer)
+if (! $error && ! empty($conf->global->MAIN_DISABLE_CONTACTS_TAB) && $user->rights->commande->creer)
 {
 	if ($action == 'addcontact')
 	{

htdocs/compta/facture.php

@@ -659,7 +659,8 @@ else if ($action == 'add' && $user->rights->facture->creer)
 	// Fill array 'array_options' with data from add form
 	$extralabels=$extrafields->fetch_name_optionals_label($object->table_element);
 	$ret = $extrafields->setOptionalsFromPost($extralabels,$object);
-
+	if($ret < 0)
+		$error++;
 
 	// Replacement invoice
 	if ($_POST['type'] == 1)
@@ -1858,24 +1859,32 @@ if ($action == 'update_extras')
 	// Fill array 'array_options' with data from add form
 	$extralabels=$extrafields->fetch_name_optionals_label($object->table_element);
 	$ret = $extrafields->setOptionalsFromPost($extralabels,$object);
+	if($ret < 0)
+		$error++;
 
-	// Actions on extra fields (by external module or standard code)
+	if(!$error) {
-	// FIXME le hook fait double emploi avec le trigger !!
+		// Actions on extra fields (by external module or standard code)
-	$hookmanager->initHooks(array('invoicedao'));
+		// FIXME le hook fait double emploi avec le trigger !!
-	$parameters=array('id'=>$object->id);
+		$hookmanager->initHooks(array('invoicedao'));
-	$reshook=$hookmanager->executeHooks('insertExtraFields',$parameters,$object,$action); // Note that $action and $object may have been modified by some hooks
+		$parameters=array('id'=>$object->id);
-	if (empty($reshook))
+		$reshook=$hookmanager->executeHooks('insertExtraFields',$parameters,$object,$action); // Note that $action and $object may have been modified by some hooks
-	{
+		if (empty($reshook))
-		if (empty($conf->global->MAIN_EXTRAFIELDS_DISABLED)) // For avoid conflicts if trigger used
 		{
-			$result=$object->insertExtraFields();
+			if (empty($conf->global->MAIN_EXTRAFIELDS_DISABLED)) // For avoid conflicts if trigger used
-			if ($result < 0)
 			{
-				$error++;
+				$result=$object->insertExtraFields();
+				if ($result < 0)
+				{
+					$error++;
+				}
 			}
 		}
+		else if ($reshook < 0) $error++;
+	}
+	else
+	{
+		$action = 'edit_extras';
 	}
-	else if ($reshook < 0) $error++;
 
 }
 

htdocs/core/class/extrafields.class.php

@@ -894,7 +894,9 @@ class ExtraFields
 	 */
 	function setOptionalsFromPost($extralabels,&$object)
 	{
-		global $_POST;
+		global $_POST, $langs;
+		$nofillrequired='';// For error when required field left blank
+		$error_field_required = array();
 
 		if (is_array($extralabels))
 		{
@@ -902,6 +904,11 @@ class ExtraFields
 			foreach ($extralabels as $key => $value)
 			{
 				$key_type = $this->attribute_type[$key];
+				if($this->attribute_required[$key] && !GETPOST("options_$key",2))
+				{
+					$nofillrequired++;
+					$error_field_required[] = $value;
+				}
 
 				if (in_array($key_type,array('date','datetime')))
 				{
@@ -929,7 +936,14 @@ class ExtraFields
 				$object->array_options["options_".$key]=$value_key;
 			}
 
-			return 1;
+			if($nofillrequired) {
+				$langs->load('errors');
+				setEventMessage($langs->trans('ErrorFieldsRequired').' : '.implode(', ',$error_field_required),'errors');
+				return -1;
+			}
+			else {
+				return 1;
+			}
 		}
 		else {
 			return 0;