Create invoice item #436

Paul424 · 2017-10-23T14:03:24Z

What's this PR do?

Support the new "billing" feature so you can add payment method "send invoice".

Any background context you want to provide?

These changes are required to support ACH, but even if you are not in US/USD then you can still build it yourself using the newly added functions. So for instance on invoice.created you send the invoice to the customer and when billling is set to send_invoice you close the invoice. Then when payment is send; you open the invoice and set it paid (or forgive).

What ticket or issue # does this fix?

Closes #428

Definition of Done (check if considered and/or addressed):

Are all backwards incompatible changes documented in this PR?
Have all new dependencies been documented in this PR?
[n ] Has the appropriate documentation been updated (if applicable)?
[ n] Have you written tests to prove this change works (if applicable)?

…ields to model

This reverts commit c8acb98.

…levant fields to model" This reverts commit bae60ab.

This reverts commit a03b657.

… invoice items

paltman

Thanks for this feature! I just had a handful of requests, all are pretty minor.

I'm currently swamped trying to get a big release buttoned up (Samwise Milestone), but I expect there will be another minor release to come shortly afterward and I'd like to plan to include this in that release.

paltman · 2017-10-25T13:34:14Z

pinax/stripe/actions/invoices.py

@@ -63,9 +63,116 @@ def pay(invoice, send_receipt=True):
    return False


+def paid(invoice):


Would prefer this to be a verb. Can we change to mark_paid so it's clearer what the function is doing?

paltman · 2017-10-25T13:36:01Z

pinax/stripe/actions/invoices.py

+        stripe_invoice = invoice.stripe_invoice
+        stripe_invoice.paid = True
+        stripe_invoice_ = stripe_invoice.save()
+        sync_invoice_from_stripe_data(stripe_invoice_)


I don't think we have to worry about creating a new variable. Let's just:

stripe_invoice.save()

paltman · 2017-10-25T13:37:17Z

pinax/stripe/actions/invoices.py

+        sync_invoice_from_stripe_data(stripe_invoice_)
+
+
+def open(invoice):


Let's rename this to reopen to 1) not clash with the builtin open, and 2) be more clear in the function name of what it is doing.

paltman · 2017-10-25T13:38:20Z

pinax/stripe/actions/invoices.py

+    if not invoice.closed:
+        stripe_invoice = invoice.stripe_invoice
+        stripe_invoice.closed = True
+        stripe_invoice_ = stripe_invoice.save()


Again, let's just call save() and pass the original object to the sync method.

paltman · 2017-10-25T13:38:27Z

pinax/stripe/actions/invoices.py

+    if invoice.closed:
+        stripe_invoice = invoice.stripe_invoice
+        stripe_invoice.closed = False
+        stripe_invoice_ = stripe_invoice.save()


Again, let's just call save() and pass the original object to the sync method.

paltman · 2017-10-25T13:40:45Z

pinax/stripe/models.py

@@ -240,6 +240,8 @@ class Subscription(StripeObject):
    status = models.CharField(max_length=25)  # trialing, active, past_due, canceled, or unpaid
    trial_end = models.DateTimeField(blank=True, null=True)
    trial_start = models.DateTimeField(blank=True, null=True)
+    billing = models.CharField(max_length=32, default=u'charge_automatically')  # charge_automatically or send_invoice


This should be an enum/choices to avoid the hard coded string.

paltman · 2017-10-25T13:41:11Z

pinax/stripe/models.py

@@ -289,6 +292,9 @@ class Invoice(StripeObject):
    total = models.DecimalField(decimal_places=2, max_digits=9)
    date = models.DateTimeField()
    webhooks_delivered_at = models.DateTimeField(null=True)
+    billing = models.CharField(max_length=32, default=u'charge_automatically')  # charge_automatically or send_invoice


This should be an enum/choices to avoid the hard coded string.

codecov · 2017-10-30T11:59:41Z

Codecov Report

Merging #436 into master will decrease coverage by 1.82%.
The diff coverage is 43.85%.

@@            Coverage Diff             @@
##           master     #436      +/-   ##
==========================================
- Coverage    99.4%   97.57%   -1.83%     
==========================================
  Files          34       34              
  Lines        1682     1735      +53     
  Branches      144      152       +8     
==========================================
+ Hits         1672     1693      +21     
- Misses          5       33      +28     
- Partials        5        9       +4

Flag	Coverage Δ
#py27dj110	`97.57% <43.85%> (-1.83%)`	⬇️
#py27dj111	`97.57% <43.85%> (-1.83%)`	⬇️
#py27dj18	`97.57% <43.85%> (-1.83%)`	⬇️
#py34dj110	`97.57% <43.85%> (-1.83%)`	⬇️
#py34dj111	`97.57% <43.85%> (-1.83%)`	⬇️
#py34dj18	`97.57% <43.85%> (-1.83%)`	⬇️
#py34dj20	`97.57% <43.85%> (-1.83%)`	⬇️
#py35dj110	`97.57% <43.85%> (-1.83%)`	⬇️
#py35dj111	`97.57% <43.85%> (-1.83%)`	⬇️
#py35dj18	`97.57% <43.85%> (-1.83%)`	⬇️
#py35dj20	`97.57% <43.85%> (-1.83%)`	⬇️
#py36dj111	`97.57% <43.85%> (-1.83%)`	⬇️
#py36dj20	`97.57% <43.85%> (-1.83%)`	⬇️
#py36dj20psql	`97.57% <43.85%> (-1.83%)`	⬇️

Impacted Files	Coverage Δ
pinax/stripe/webhooks.py	`100% <100%> (ø)`	⬆️
pinax/stripe/models.py	`100% <100%> (ø)`	⬆️
pinax/stripe/actions/invoices.py	`72.91% <18.75%> (-27.09%)`	⬇️
pinax/stripe/actions/subscriptions.py	`91.17% <33.33%> (-8.83%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b67b87...04a6d1c. Read the comment docs.

codecov · 2017-10-30T11:59:50Z

Codecov Report

Merging #436 into master will decrease coverage by 0.67%.
The diff coverage is 76.78%.

@@            Coverage Diff             @@
##           master     #436      +/-   ##
==========================================
- Coverage    99.4%   98.73%   -0.68%     
==========================================
  Files          33       33              
  Lines        1838     1891      +53     
  Branches      168      176       +8     
==========================================
+ Hits         1827     1867      +40     
- Misses          5       10       +5     
- Partials        6       14       +8

Flag	Coverage Δ
#py27dj110	`98.41% <76.78%> (-0.67%)`	⬇️
#py27dj111	`98.41% <76.78%> (-0.67%)`	⬇️
#py27dj18	`98.67% <76.78%> (-0.67%)`	⬇️
#py34dj110	`98.41% <76.78%> (-0.67%)`	⬇️
#py34dj111	`98.41% <76.78%> (-0.67%)`	⬇️
#py34dj18	`98.67% <76.78%> (-0.67%)`	⬇️
#py34dj20	`98.41% <76.78%> (-0.67%)`	⬇️
#py35dj110	`98.41% <76.78%> (-0.67%)`	⬇️
#py35dj111	`98.41% <76.78%> (-0.67%)`	⬇️
#py35dj18	`98.67% <76.78%> (-0.67%)`	⬇️
#py35dj20	`98.41% <76.78%> (-0.67%)`	⬇️
#py36dj111	`98.41% <76.78%> (-0.67%)`	⬇️
#py36dj20	`98.41% <76.78%> (-0.67%)`	⬇️
#py36dj20psql	`98.41% <76.78%> (-0.67%)`	⬇️

Impacted Files	Coverage Δ
pinax/stripe/webhooks.py	`99.08% <100%> (ø)`	⬆️
pinax/stripe/models.py	`100% <100%> (ø)`	⬆️
pinax/stripe/actions/subscriptions.py	`91.3% <33.33%> (-8.7%)`	⬇️
pinax/stripe/actions/invoices.py	`92.7% <78.12%> (-7.3%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4013376...2e9b3ef. Read the comment docs.

blueyed · 2017-10-30T12:13:16Z

pinax/stripe/actions/invoices.py

+    Args:
+        invoice: the invoice object to close
+    """
+    if not invoice.paid:


s/paid/forgiven/ ?

paltman · 2017-10-30T14:25:46Z

Thanks for the updates.

Do you think you can add some tests for all the new actions?

Paul424 · 2017-10-30T14:44:42Z

Yes, i know tests are lacking; i noticed coverage was slightly lower. Let me try to work on this... Paul

…

On 30-10-17 15:25, Patrick Altman wrote: Thanks for the updates. Do you think you can add some tests for all the new actions? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#436 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABVaNtwJDa1s8rGuHG2pL1mPjpJOE29Yks5sxdxqgaJpZM4QC5Wm>.

…pe dashboard

blueyed · 2017-11-06T12:51:59Z

pinax/stripe/webhooks.py

@@ -96,7 +96,8 @@ def validate(self):
                cls=stripe.StripeObjectEncoder
            )
        )
-        self.event.valid = self.event.webhook_message["data"] == self.event.validated_message["data"]
+        # Notice "data" may contain a "previous_attributes" section
+        self.event.valid = "object" in self.event.webhook_message["data"] and "object" in self.event.validated_message["data"] and self.event.webhook_message["data"]["object"] == self.event.validated_message["data"]["object"]


Looks wrong: it should also be in the validate (i.e. fetched) event then..?!

Yes, odd isn't it; but i actually found this when making a change on the subscription using the Stripe dashboard. The initial event contains info about what has changed while the fetched event does not...

That would be worth a separate test then, using the actual data/fixtures.

Regarding your last comment on the PR: please extract it into a separate PR.

Paul424 · 2017-11-06T21:39:35Z

Please give me an update when this pull is scheduled to be merged to master; i rather fix/merge a branch once ;-)

… was updated AFTER delivery to master) Conflicts: pinax/stripe/models.py pinax/stripe/webhooks.py

Paul424 · 2017-12-06T21:35:18Z

Please don't merge yet, i still need to deploy first to process some "fix" migrations, then i need to undo these to revert back to the original migrations from master....

I have had/and still have a considerable amount of work on fixing migrations; i like to propose to not change existing migrations on master any more, just leave them as they are after they are delivered and use the build in squash function to simplify the migration path when required.

Paul424 · 2017-12-07T22:23:05Z

Ok, i finished the migrations; should now be identical to the master branch + my additions. I the current status sufficient for merge?

blueyed · 2017-12-07T23:50:55Z

76.78% of diff hit

You should add more tests probably.
(Haven't looked at the diff again)

fredpalmer · 2018-02-19T20:43:21Z

FYI - we need this functionality as well. I noticed that there is possibly something missing as well from this implementation: pinax.stripe.actions.sources.sync_payment_source_from_stripe_data doesn't currently handle the data in the sources section of a payload, e.g.:

  "sources": {
    "object": "list",
    "data": [
      {
        "id": "src_xxxxxxxxxxxxxxx",
        "object": "source",
        "amount": null,
        "client_secret": "src_client_secretxxxxxxxxxxxxxxxx",
        "created": 1519071007,
        "currency": "usd",
        "customer": "cus_xxxxxxxxxxxxxxxxx",
        "flow": "receiver",
        "livemode": false,
        "metadata": {},
        "owner": {
          "address": null,
          "email": "[email protected]",
          "name": null,
          "phone": null,
          "verified_address": null,
          "verified_email": null,
          "verified_name": null,
          "verified_phone": null
        },
        "receiver": {
          "address": "110000000-test_d07ad7f50925",
          "amount_charged": 0,
          "amount_received": 0,
          "amount_returned": 0,
          "refund_attributes_method": "email",
          "refund_attributes_status": "missing"
        },
        "statement_descriptor": null,
        "status": "pending",
        "type": "ach_credit_transfer",
        "usage": "reusable",
        "ach_credit_transfer": {
          "account_number": "test_d07ad7f50925",
          "bank_name": "TEST BANK",
          "fingerprint": "IHEifPhUMGT3wIdZ",
          "routing_number": "110000000",
          "swift_code": "TSTEZ122"
        }
      }
    ],
    "has_more": false,
    "total_count": 1,
    "url": "/v1/customers/cus_xxxxxxxxxxxxxxx/sources"
  },

Paul424 added 12 commits April 19, 2017 20:21

ideal support; sry no tests yet!

a03b657

Extend webhooks required for ideal (source status) and add relevant f…

bae60ab

…ields to model

Allow for source in update subscription

c8acb98

Merge branch 'master' of github.com:Paul424/pinax-stripe

c0acca1

Merge branch 'master' of github.com:pinax/pinax-stripe

3226b70

Revert "Allow for source in update subscription"

8011951

This reverts commit c8acb98.

Revert "Extend webhooks required for ideal (source status) and add re…

b9e56ed

…levant fields to model" This reverts commit bae60ab.

Revert "ideal support; sry no tests yet!"

810a066

This reverts commit a03b657.

Add action to create invoice item

4501b0d

Merge branch 'master' of github.com:pinax/pinax-stripe

54bf5c1

Merge branch 'master' into create-invoice-item

6c721ea

Support billing/due_date on subscription and invoice + support adding…

95145cd

… invoice items

paltman requested changes Oct 25, 2017

View reviewed changes

paltman added this to the Rosie milestone Oct 25, 2017

Paul424 and others added 3 commits October 26, 2017 17:05

Update after review

9e472e9

Merge branch 'master' into create-invoice-item

9c1660c

Merge migrations

04a6d1c

blueyed reviewed Oct 30, 2017

View reviewed changes

Paul424 added 2 commits October 31, 2017 23:32

Add tests

3211821

Fix issue that subscription is not sync'ed when change made from Stri…

975bb9b

…pe dashboard

blueyed reviewed Nov 6, 2017

View reviewed changes

Merge branch 'master' of github.com:pinax/pinax-stripe

e111401

paltman modified the milestones: Rosie, December 2017 Sprint Dec 1, 2017

Merge branch 'master' of github.com:pinax/pinax-stripe

b586d3b

Merge branch 'master' into create-invoice-item + fix migrations (0010…

027d1e1

… was updated AFTER delivery to master) Conflicts: pinax/stripe/models.py pinax/stripe/webhooks.py

Paul424 added 4 commits December 6, 2017 22:56

Fix default in migration

27bd049

Fix new tests

01c88b6

Fix tests

0c2ced3

Revert fix migrations to enable delivery to master

2e9b3ef

paltman removed this from the December 2017 Sprint milestone Nov 25, 2021

paltman changed the base branch from master to 4.x November 26, 2021 16:17

paltman added the Original The original full pinax-stripe version with cached models and actions service layer label Nov 26, 2021

paltman requested review from paltman and removed request for paltman November 26, 2021 16:17

paltman changed the base branch from 4.x to original November 26, 2021 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create invoice item #436

Create invoice item #436

Paul424 commented Oct 23, 2017

paltman left a comment

paltman Oct 25, 2017

paltman Oct 25, 2017

paltman Oct 25, 2017

paltman Oct 25, 2017

paltman Oct 25, 2017

paltman Oct 25, 2017

paltman Oct 25, 2017

codecov bot commented Oct 30, 2017 •

edited

Loading

codecov bot commented Oct 30, 2017 •

edited

Loading

blueyed Oct 30, 2017

blueyed Oct 30, 2017

paltman commented Oct 30, 2017

Paul424 commented Oct 30, 2017 via email

blueyed Nov 6, 2017

Paul424 Nov 6, 2017

blueyed Nov 6, 2017

blueyed Nov 6, 2017

Paul424 commented Nov 6, 2017

Paul424 commented Dec 6, 2017

Paul424 commented Dec 7, 2017

blueyed commented Dec 7, 2017

fredpalmer commented Feb 19, 2018

		@@ -63,9 +63,116 @@ def pay(invoice, send_receipt=True):
		return False


		def paid(invoice):

		sync_invoice_from_stripe_data(stripe_invoice_)


		def open(invoice):

Create invoice item #436

Are you sure you want to change the base?

Create invoice item #436

Conversation

Paul424 commented Oct 23, 2017

What's this PR do?

Any background context you want to provide?

What ticket or issue # does this fix?

Definition of Done (check if considered and/or addressed):

paltman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 30, 2017 • edited Loading

Codecov Report

codecov bot commented Oct 30, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paltman commented Oct 30, 2017

Paul424 commented Oct 30, 2017 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Paul424 commented Nov 6, 2017

Paul424 commented Dec 6, 2017

Paul424 commented Dec 7, 2017

blueyed commented Dec 7, 2017

fredpalmer commented Feb 19, 2018

codecov bot commented Oct 30, 2017 •

edited

Loading

codecov bot commented Oct 30, 2017 •

edited

Loading