From 492c85c2f086299b3e398188e06541d6d44d3741 Mon Sep 17 00:00:00 2001
From: Moritz Bunkus <m.bunkus@linet-services.de>
Date: Wed, 6 Apr 2016 13:59:31 +0200
Subject: [PATCH] =?utf8?q?DeliveryOrder->new=5Ffrom:=20kein=20$custom=5Fsh?=
 =?utf8?q?ipto-Objekt=20zur=C3=BCckgeben?=
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Falls das Quellobjekt eine individuelle Lieferadresse besaß, wurden bei
new_from() zwei Objekte zurückgegeben: das neue Lieferscheinobjekt und
ein Clone der individuellen Lieferadresse. Diese waren nicht verknüpft.
Der Aufrufer musste daher zuerst das Lieferscheinobjekt speichern,
dessen ID beim gecloneten Lieferadressenobjekt hinterlegen und das
anschließend speichern.

Dies ist umständlich und fehlerträchtig. So hat z.B. der einzige
bisherige Nutzer dieses Interfaces,
SL::DB::Order->convert_to_delivery_order, das bereits falsch gemacht und
vergessen, beim Lieferadressenobjekt die ID des neuen
Lieferscheinobjektes einzutragen. Somit wurden Lieferadressen erzeugt,
die keinerlei Verknüpfung hatten.

Das geänderte Interface hinterlegt das Objekt für die individuelle
Lieferadresse schlicht in $new_delivery_order->custom_shipto. Dort wird
das Objekt gespeichert, wenn der Lieferschein selber gespeichert wird.
---
 SL/DB/DeliveryOrder.pm | 25 ++++++-------------------
 SL/DB/Order.pm         | 21 ++++++---------------
 2 files changed, 12 insertions(+), 34 deletions(-)

diff --git a/SL/DB/DeliveryOrder.pm b/SL/DB/DeliveryOrder.pm
index bc518018e..b7b7a82f6 100644
--- a/SL/DB/DeliveryOrder.pm
+++ b/SL/DB/DeliveryOrder.pm
@@ -130,13 +130,10 @@ sub new_from {
 
   # Custom shipto addresses (the ones specific to the sales/purchase
   # record and not to the customer/vendor) are only linked from
-  # shipto -> delivery_orders. Meaning delivery_orders.shipto_id
-  # will not be filled in that case. Therefore we have to return the
-  # new shipto object as a separate object so that the caller can
-  # save it, too.
-  my $custom_shipto;
+  # shipto → delivery_orders. Meaning delivery_orders.shipto_id
+  # will not be filled in that case.
   if (!$source->shipto_id && $source->id) {
-    $custom_shipto = $source->custom_shipto->clone($class) if $source->can('custom_shipto') && $source->custom_shipto;
+    $args{custom_shipto} = $source->custom_shipto->clone($class) if $source->can('custom_shipto') && $source->custom_shipto;
 
   } else {
     $args{shipto_id} = $source->shipto_id;
@@ -172,7 +169,7 @@ sub new_from {
 
   $delivery_order->items(\@items);
 
-  return ($delivery_order, $custom_shipto);
+  return $delivery_order;
 }
 
 sub customervendor {
@@ -258,18 +255,8 @@ quotations and purchase orders) are supported as sources.
 The conversion copies order items into delivery order items. Dates are copied
 as appropriate, e.g. the C<transdate> field will be set to the current date.
 
-Returns one or two objects depending on the context. In list context
-the new delivery order instance and a shipto instance will be
-returned. In scalar instance only the delivery order instance is
-returned.
-
-Custom shipto addresses (the ones specific to the sales/purchase
-record and not to the customer/vendor) are only linked from C<shipto>
-to C<delivery_orders>. Meaning C<delivery_orders.shipto_id> will not
-be filled in that case. That's why a separate shipto object is created
-and returned.
-
-The objects returned are not saved.
+Returns the new delivery order instance. The object returned is not
+saved.
 
 C<%params> can include the following options:
 
diff --git a/SL/DB/Order.pm b/SL/DB/Order.pm
index b7bdae070..097d471f3 100644
--- a/SL/DB/Order.pm
+++ b/SL/DB/Order.pm
@@ -157,12 +157,11 @@ sub convert_to_invoice {
 sub convert_to_delivery_order {
   my ($self, @args) = @_;
 
-  my ($delivery_order, $custom_shipto);
+  my $delivery_order;
   if (!$self->db->with_transaction(sub {
     require SL::DB::DeliveryOrder;
-    ($delivery_order, $custom_shipto) = SL::DB::DeliveryOrder->new_from($self, @args);
+    $delivery_order = SL::DB::DeliveryOrder->new_from($self, @args);
     $delivery_order->save;
-    $custom_shipto->save if $custom_shipto;
     $self->link_to_record($delivery_order);
     # TODO extend link_to_record for items, otherwise long-term no d.r.y.
     foreach my $item (@{ $delivery_order->items }) {
@@ -183,10 +182,10 @@ sub convert_to_delivery_order {
     $self->update_attributes(delivered => 1);
     1;
   })) {
-    return wantarray ? () : undef;
+    return undef;
   }
 
-  return wantarray ? ($delivery_order, $custom_shipto) : $delivery_order;
+  return $delivery_order;
 }
 
 sub number {
@@ -261,16 +260,8 @@ C<true>, and C<$self> is saved.
 The arguments in C<%params> are passed to
 L<SL::DB::DeliveryOrder::new_from>.
 
-Returns C<undef> on failure. Otherwise the return value depends on the
-context. In list context the new delivery order and a shipto instance
-will be returned. In scalar instance only the delivery order instance
-is returned.
-
-Custom shipto addresses (the ones specific to the sales/purchase
-record and not to the customer/vendor) are only linked from C<shipto>
-to C<delivery_orders>. Meaning C<delivery_orders.shipto_id> will not
-be filled in that case. That's why a separate shipto object is created
-and returned.
+Returns C<undef> on failure. Otherwise the new delivery order will be
+returned.
 
 =head2 C<convert_to_invoice %params>
 
-- 
2.20.1