Changes from PR dj-stripe#320 review

Adds a new _attach_objects_post_save_hook, for attaching Stripe objects
to an instance only after it is saved.

djstripe/models.py

@@ -542,35 +542,29 @@ def _attach_objects_hook(self, cls, data):
         if subscription:
             self.subscription = subscription
 
-        self._attach_invoice_items(cls, data)
-
-    def _attach_invoice_items(self, cls, data):
-        if not self.pk:
-            # InvoiceItems need a saved invoice because they're associated via
-            # a RelatedManager.  It might be better to make this pattern more
-            # generic with some sort-of _attach_objects_post_save_hook().
-            self.save()
-
+    def _attach_objects_post_save_hook(self, cls, data):
+        # InvoiceItems need a saved invoice because they're associated via a
+        # RelatedManager, so this must be done as part of the post save hook.
         cls._stripe_object_to_invoice_items(InvoiceItem, data, self)
 
     @property
     def plan(self):
         """ Gets the associated plan for this invoice.
 
         In order to provide a consistent view of invoices, it is necessary to
-        examine the invoice items for the associated subscription plan rather
-        than the top-level link.  The reason for this is that the subscription
-        plan when requested by the customer, but the invoice item plan will
-        remain the same.  This makes it difficult to work with an invoice
-        history if the plan is expected to remain consistent (e.g. for creating
-        downloadable invoices).
+        extract it from the first invoice item with a plan, rather than getting
+        it from the subscription by foreign key.  The reason for this is that
+        the subscription plan when requested by the customer, but the invoice
+        item plan will remain the same. This makes it difficult to work with an
+        invoice history if the plan is expected to remain consistent (e.g. for
+        creating downloadable invoices).
 
         :returns: The associated plan for the invoice.
         :rtype: ``djstripe.models.Plan``
         """
-        for item in self.invoiceitems.all():
-            if item.plan:
-                return item.plan
+        for invoiceitem in self.invoiceitems.all():
+            if invoiceitem.plan:
+                return invoiceitem.plan
 
         if self.subscription:
             return self.subscription.plan
@@ -584,15 +578,16 @@ def __init__(self, *args, **kwargs):
         super(UpcomingInvoice, self).__init__(*args, **kwargs)
         self._items = []
 
-    def _attach_invoice_items(self, cls, data):
+    def _attach_objects_hook(self, cls, data):
+        super(UpcomingInvoice, self)._attach_objects_hook(cls, data)
         self._items = cls._stripe_object_to_invoice_items(InvoiceItem, data, self)
 
     @property
     def invoiceitems(self):
         """ Gets the invoice items associated with this upcoming invoice.
 
         This differs from normal (non-upcoming) invoices, in that upcoming
-        invoices are in-memory and do not persist to the database.  Therefore,
+        invoices are in-memory and do not persist to the database. Therefore,
         all of the data comes from the Stripe API itself.
 
         Instead of returning a normal queryset for the invoiceitems, this will

djstripe/stripe_objects.py

@@ -176,6 +176,22 @@ def _attach_objects_hook(self, cls, data):
         """
         Gets called by this object's create and sync methods just before save.
         Use this to populate fields before the model is saved.
+
+        :param cls: The target class for the instantiated object.
+        :param data: The data dictionary received from the Stripe API.
+        :type data: dict
+        """
+
+        pass
+
+    def _attach_objects_post_save_hook(self, cls, data):
+        """
+        Gets called by this object's create and sync methods just after save.
+        Use this to populate fields after the model is saved.
+
+        :param cls: The target class for the instantiated object.
+        :param data: The data dictionary received from the Stripe API.
+        :type data: dict
         """
 
         pass
@@ -195,6 +211,7 @@ def _create_from_stripe_object(cls, data, save=True):
         instance._attach_objects_hook(cls, data)
         if save:
             instance.save()
+        instance._attach_objects_post_save_hook(cls, data)
         return instance
 
     @classmethod
@@ -311,7 +328,7 @@ def _stripe_object_to_invoice_items(cls, target_cls, data, invoice):
                 if line.get("type") == "subscription":
                     # Lines for subscriptions need to be keyed based on invoice and
                     # subscription, because their id is *just* the subscription
-                    # when received from Stripe.  This means that future updates to
+                    # when received from Stripe. This means that future updates to
                     # a subscription will change previously saved invoices - Doing
                     # the composite key avoids this.
                     if not line["id"].startswith(invoice.stripe_id):
@@ -364,6 +381,7 @@ def sync_from_stripe_data(cls, data):
             instance._sync(cls._stripe_object_to_record(data))
             instance._attach_objects_hook(cls, data)
             instance.save()
+            instance._attach_objects_post_save_hook(cls, data)
 
         return instance