Introduce DRAFT Invoice (required for Hierarchical Account design)

[sbrossie]

Invoice will now have a enum status (DRAFT, COMMITTED).

Invoice DDL/DAO will need to be modified to reflect that new status.
By default all invoices will be created in COMMITTED (current use case), with the exception of apis that:

• insertExternalCharges
• insertCredit

We need 2 new invoice apis to transition from DRAFT to COMMITTED. The invoice bus event will only be sent for invoices that are in COMMITTED status. When the transition from DRAFT to COMMITTED occurs, this will send an invoice bus event.

For safety payment module should be modified to disregard non COMMITTED invoices.

Adding a user credit or a user external charge on a COMMITTED invoice should fail.

Clients libraries need to be updated as well (java + ruby)

Tests:

Invoice unit tests
Beatrix test to verify that a DRAFT invoice with a positive balance does not get handled by payment system, and then make the transition and check payment occurs.

[matias-aguero-hs]

We need to new invoice api to transition from DRAFT to COMMITTED

Do you want to receive the new invoice state by parameter or just assume that calling the new method the invoice will move from DRAFT to COMMITTED?

How the new api method will be called? Will be called automatically by a schedule job or manually by InvoiceResource?

[pierre]

How the new api method will be called? Will be called automatically by a schedule job or manually by InvoiceResource?

There are two scenarios:

• Hierarchical accounts: DRAFT to COMMITTED happens at the end of the day (in the account timezone) using the notification queue (when first creating the DRAFT invoice, the parent inserts an entry for that day)
• Because now credits and external charges are created in DRAFT mode, we need an API in InvoiceResource to manually transition the associated invoice (when the user wants to trigger a payment for example)

[matias-aguero-hs]

Thanks, I added a new API in InvoiceResource. For first scenario I'll work on that when I work in #459.

[matias-aguero-hs]

Adding a user credit or a user external charge on a COMMITTED invoice should fail.

Adding this validation here prevents add a credit if the invoice status is COMMITTED, but there are two tests that fail:

• testAdjustFullInvoice
• testAdustPartialInvoice

Is this the expected behaviour? Should I modify those tests and expect the exceptions?

[pierre]

Is this the expected behaviour?

Interesting -- I'm actually unclear about it, I'll have to check with @sbrossie if we want to change this behavior when he is back.

For now, let's keep the existing tests as-is. I'll keep this bug open so we can add validation later on if needed.

[matias-aguero-hs]

Sure, thank you.

[matias-aguero-hs]

For safety payment module should be modified to disregard non COMMITTED invoices.

Can you please provide us some guidance on this point?

[pierre]

Can you please provide us some guidance on this point?

When receiving events from the invoice module, the PaymentBusEventHandler should disregard non COMMITTED invoices (i.e. retrieve the invoice and not trigger the purchase if the status isn't COMMITTED).

Does that make sense?

[matias-aguero-hs]

Excellent! That's what I was trying to find. I'll also run all existing payment unit tests and add some new ones.

[pierre]

Sounds good! 🏆

[matias-aguero-hs]

PRs are created.

• killbill-api: Introduce DRAFT Invoice #457 killbill-api#11
• killbill-client-java: Introduce DRAFT Invoice #457 killbill-client-java#25
• killbill : Introduce DRAFT Invoice #457 #473

I also included some comments where I'm not sure if I implemented it properly.

Pending:

• Adding a user credit or a user external charge on a COMMITTED invoice should fail.
• Ruby client changes

[matias-aguero-hs]

I pushed more changes about PR review.

[sbrossie]

@maguero I am bit confused on what is the current semantics with regard to DRAFT invoice. Could you clarify it for me (probably write some test cases to make sure this is covered and we understand well current behavior):

1. Are DRAFT invoices taken into account when computing amount charged (regardless of HA feature)?
2. For HA specifically, if a child invoice was created (and COMMITTED) what happens to the parent during the time period until it its SUMMARY invoice gets closed? In particular, if i retrieve the parent balance, do i see the not yet committed amount on the DRAFT SUMMARY parent invoice, or not?

[matias-aguero-hs]

When we added Invoice Status concept we agreed on these options:

• Any child or regular (non child-parent relationship) invoices are created in COMMITTED status.
• Parent invoices are created in DRAFT status, and it keep this status during the day. When day ends, DRAFT invoice is closed and moved to COMMITTED state.

1. Are DRAFT invoices taken into account when computing amount charged (regardless of HA feature)?

Not sure what do you mean.

1. For HA specifically, if a child invoice was created (and COMMITTED) what happens to the parent during the time period until it its SUMMARY invoice gets closed? In particular, if i retrieve the parent balance, do i see the not yet committed amount on the DRAFT SUMMARY parent invoice, or not?

Yes, If you retrieve the parent balance you will see the full amount. The parent invoice status does not matter. TestCase

[sbrossie]

Are DRAFT invoices taken into account when computing amount charged (regardless of HA feature)?
Not sure what do you mean.

Not sure what do you mean.

My question is about the invoice balance and account balance in case we have DRAFT invoices. Are those taken into account to compute such balance (not for parent account, but for children or regular account) ? I would expect they are not taken into account. Could you verify this is the case (we maybe already have some tests)

For HA specifically, if a child invoice was created (and COMMITTED) what happens to the parent during the time period until it its SUMMARY invoice gets closed? In particular, if i retrieve the parent balance, do i see the not yet committed amount on the DRAFT SUMMARY parent invoice, or not?

Yes, If you retrieve the parent balance you will see the full amount. The parent invoice status does not matter. TestCase

Ok, assuming my understand from previous point (above) is correct, it means behavior is not symmetrical between children and parent, but probably this is OK.

[sbrossie]

@maguero Was rerunning my HA integration suite and i am still seeing the issue i described above with regards to account balance: It seems like parent DRAFT invoices are taken into account for balance computation (i would expect them to not be so this is symmetrical with what happens on the children or non HA case.

[matias-aguero-hs]

@sbrossie You're right. The invoice balance is computed for all invoices status. Tests confirm that.
Based on your description we have to change it and only compute balance for COMMITTED invoices, right? In that case we'll also change test asserts.

[matias-aguero-hs]

@sbrossie issue fixed and PR #582 created.