Fix: [ bug #1352 ] Removing a shipping does not remove the delivery.

ChangeLog

@@ -5,6 +5,8 @@ English Dolibarr ChangeLog
 ***** ChangeLog for 3.5.3 compared to 3.5.2 *****
 Fix: Error on field accountancy code for export profile of invoices.
 Fix: [ bug #1351 ] VIES verification link broken.
+Fix: [ bug #1352 ] Removing a shipping does not remove the delivery.
+
 
 ***** ChangeLog for 3.5.2 compared to 3.5.1 *****
 Fix: Can't add user for a task.

htdocs/core/class/commonobject.class.php

@@ -47,8 +47,8 @@ abstract class CommonObject
 
     public $array_options=array();
 
-    public $linkedObjectsIds;
-    public $linkedObjects;
+    public $linkedObjectsIds;	// Loaded by ->fetchObjectLinked
+    public $linkedObjects;		// Loaded by ->fetchObjectLinked
 
     // No constructor as it is an abstract class
 
@@ -1691,7 +1691,7 @@ abstract class CommonObject
      *	@param  string	$sourcetype		Object source type
      *	@param  int		$targetid		Object target id
      *	@param  string	$targettype		Object target type
-     *	@param  string	$clause			OR, AND clause
+     *	@param  string	$clause			'OR' or 'AND' clause used when both source id and target id are provided
      *	@return	void
      */
     function fetchObjectLinked($sourceid='',$sourcetype='',$targetid='',$targettype='',$clause='OR')
@@ -2359,7 +2359,8 @@ abstract class CommonObject
 
 
     /**
-     *  Function to check if an object is used by others
+     *  Function to check if an object is used by others.
+     *  Check is done into this->childtables. There is no check into llx_element_element.
      *
      *  @param	int		$id			Id of object
      *  @return	int					<0 if KO, 0 if not used, >0 if already used

htdocs/expedition/class/expedition.class.php

@@ -785,9 +785,10 @@ class Expedition extends CommonObject
     }
 
     /**
-	 * 	Delete shipment
+	 * 	Delete shipment.
+	 *  Warning, do not delete a shipment if a delivery is linked to (with table llx_element_element)
 	 *
-	 * 	@return	int		>0 if OK otherwise if KO
+	 * 	@return	int		>0 if OK, 0 if deletion done but failed to delete files, <0 if KO
 	 */
 	function delete()
 	{
@@ -796,6 +797,14 @@ class Expedition extends CommonObject
 
 		$error=0;
 
+		// Add a protection to refuse deleting if shipment has at least one delivery
+		$this->fetchObjectLinked($this->id, 'shipping', 0, 'delivery');	// Get deliveries linked to this shipment
+		if (count($this->linkedObjectsIds) > 0)
+		{
+			$this->error='ErrorThereIsSomeDeliveries';
+			return -1;
+		}
+
 		$this->db->begin();
 
 		// Stock control
@@ -840,7 +849,7 @@ class Expedition extends CommonObject
 			}
 		}
 
-		if(! $error)
+		if (! $error)
 		{
 			$sql = "DELETE FROM ".MAIN_DB_PREFIX."expeditiondet";
 			$sql.= " WHERE fk_expedition = ".$this->id;
@@ -858,31 +867,6 @@ class Expedition extends CommonObject
 
 					if ($this->db->query($sql))
 					{
-						$this->db->commit();
-
-						// On efface le repertoire de pdf provisoire
-						$ref = dol_sanitizeFileName($this->ref);
-						if (! empty($conf->expedition->dir_output))
-						{
-							$dir = $conf->expedition->dir_output . '/sending/' . $ref ;
-							$file = $dir . '/' . $ref . '.pdf';
-							if (file_exists($file))
-							{
-								if (! dol_delete_file($file))
-								{
-									return 0;
-								}
-							}
-							if (file_exists($dir))
-							{
-								if (!dol_delete_dir($dir))
-								{
-									$this->error=$langs->trans("ErrorCanNotDeleteDir",$dir);
-									return 0;
-								}
-							}
-						}
-
 						// Call triggers
 			            include_once DOL_DOCUMENT_ROOT.'/core/class/interfaces.class.php';
 			            $interface=new Interfaces($this->db);
@@ -890,7 +874,40 @@ class Expedition extends CommonObject
 			            if ($result < 0) { $error++; $this->errors=$interface->errors; }
 			            // End call triggers
 
-						return 1;
+			            if (! $error)
+			            {
+							$this->db->commit();
+
+							// We delete PDFs
+							$ref = dol_sanitizeFileName($this->ref);
+							if (! empty($conf->expedition->dir_output))
+							{
+								$dir = $conf->expedition->dir_output . '/sending/' . $ref ;
+								$file = $dir . '/' . $ref . '.pdf';
+								if (file_exists($file))
+								{
+									if (! dol_delete_file($file))
+									{
+										return 0;
+									}
+								}
+								if (file_exists($dir))
+								{
+									if (!dol_delete_dir($dir))
+									{
+										$this->error=$langs->trans("ErrorCanNotDeleteDir",$dir);
+										return 0;
+									}
+								}
+							}
+
+							return 1;
+			            }
+			            else
+						{
+							$this->db->rollback();
+							return -1;
+						}
 					}
 					else
 					{

htdocs/expedition/fiche.php

@@ -183,7 +183,7 @@ if ($action == 'add')
 
 /*
  * Build a receiving receipt
-*/
+ */
 else if ($action == 'create_delivery' && $conf->livraison_bon->enabled && $user->rights->expedition->livraison->creer)
 {
     $result = $object->create_delivery($user);
@@ -235,8 +235,9 @@ else if ($action == 'confirm_delete' && $confirm == 'yes' && $user->rights->expe
         exit;
     }
     else
-    {
-        $mesg = $object->error;
+	{
+		$langs->load("errors");
+        setEventMessage($langs->trans($object->error),'errors');
     }
 }
 

htdocs/langs/en_US/errors.lang

@@ -128,6 +128,7 @@ ErrorPaymentModeDefinedToWithoutSetup=A payment mode was set to type %s but setu
 ErrorPHPNeedModule=Error, your PHP must have module <b>%s</b> installed to use this feature.
 ErrorOpenIDSetupNotComplete=You setup Dolibarr config file to allow OpenID authentication, but URL of OpenID service is not defined into constant %s
 ErrorWarehouseMustDiffers=Source and target warehouses must differs
+ErrorThereIsSomeDeliveries=Error, there is some deliveries linked to this shipment. Deletion refused.
 
 # Warnings
 WarningMandatorySetupNotComplete=Mandatory setup parameters are not yet defined