Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Tax Rates #830

Merged
merged 3 commits into from
Jan 2, 2020
Merged

[11.x] Tax Rates #830

merged 3 commits into from
Jan 2, 2020

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Dec 13, 2019

This PR will provide support for Tax Rates within Cashier. I'm picking up from the PR #818 by @pierrocknroll which already did a great job. I'm trying to be as thorough as possible with this PR.

Basically this PR allows for a customer to overwrite the taxRates method and return a list with Tax Rate ids. These will then be applied by default to any created subscription. By default an empty array is returned which is the equivalent of the currently returned 0 percent.

Closes #657

Tax Exempt and Reverse Charge

This PR also adds new isNotTaxExempt, isTaxExempt and reverseChargeApplies methods to both the Billable trait and Invoice objects to easily determine if a customer is exempted from taxes or when a reverse charge applies. The state for this still needs to be set manually on the Customer. More details here: https://stripe.com/docs/billing/taxes/tax-rates#tax-exempt-and-reverse-charge

Tax Ids

One note: this PR does not provide support for Tax Ids which is also new in Stripe. Tax Ids are handled at customer level. We could perhaps implement better Tax Id support into Cashier at some point but at the moment I don't see a direct need.

Breaking Changes

  • Instead of defining a default tax percentage on the Billable model, an array of Tax Rate ids needs to be returned.
  • The syncTaxPercentage on the Subscription model has been renamed to syncTaxRates.
  • The InvoiceItem class has been renamed to InvoiceLineItem which better represents what it actually is and is the same naming that Stripe uses. Several methods have also been renamed to better reflect this.

Todo

  • Implement tax indication on invoice line items
  • Check with Stripe invoices how taxes are displayed when tax exemption applies
  • Add tests for the new functionality

Reading Material

Stripe migration guide: https://stripe.com/docs/billing/migration/taxes
Tax Rates documentation: https://stripe.com/docs/billing/taxes/tax-rates
Tax Rates on invoices: https://stripe.com/docs/billing/invoices/tax-rates

New Invoice PDF

Here's an example of a new Invoice PDF:

Screen Shot 2019-12-20 at 17 58 56

@driesvints
Copy link
Member Author

@benabbottnz thanks. That's the guide I was following as well. I've added a small reading section to the main description.

@driesvints driesvints force-pushed the tax-rates branch 2 times, most recently from e204260 to 6a1ea60 Compare December 20, 2019 17:00
@driesvints driesvints marked this pull request as ready for review December 20, 2019 17:01
@pierrocknroll
Copy link

Great job @driesvints !

@ReckeDJ
Copy link

ReckeDJ commented Dec 23, 2019

Awesome @driesvints!

@lsmith77
Copy link

great thx!

Implementation of Tax ID on the customer level is important since this ensures the tax ID will show up on the invoice.

@driesvints
Copy link
Member Author

Implementation of Tax ID on the customer level is important since this ensures the tax ID will show up on the invoice.

When you say important: what functionality is Cashier lacking that prevents you from doing it right now?

@lsmith77
Copy link

its not prevented but it is also not made easy :)

ie. I can add my own calls to add this information on the stripe customer instance.

FYI the API is not very mature yet, ie. its not possible to fetch a TAX ID object by VAT ID and more importantly it is not possible to update a TAX ID status, which is actually quite critical, since the company status needs to be validated regularly, to for example determine if the customer is infact tax exempt or not.

@driesvints
Copy link
Member Author

@lsmith77 if you have an idea for a good api for Stripe Tax Ids in Cashier feel free to open up an issue so we can discuss how to best implement it. Please try to note how it'll be better than using the Stripe api directly. Thanks

@lsmith77
Copy link

done #841

@carlosten
Copy link

Nice work! Thank you very much, this was a very needed feature. Any estimation of when this will be available in the stable version? Again, thank you very much for the great job!

@driesvints
Copy link
Member Author

@carlosten I still have plans for some other features but I need to focus on some laravel/framework stuff first. I expect end of January, beginning February.

@taylorotwell taylorotwell merged commit f8b02e5 into master Jan 2, 2020
@driesvints driesvints deleted the tax-rates branch January 2, 2020 15:16
@JackEllis
Copy link

@lsmith77 @driesvints This is how we do it:

In the user model:

public function createAsStripeCustomer(array $options = [])
{
    return $this->billableCreateAsStripeCustomer(array_merge($options, $this->getExtraStripeOptions()));
}

and then

protected function getExtraStripeOptions()
{
    $options = [
        'email' => $this->email,
        // etc.
    ];

    if (! empty($this->vat_number)) {
        $options['invoice_settings'] = [
            'custom_fields' => [
                [
                'name'  => 'VAT Number',
                'value' => $this->vat_number,
                ],
            ],
        ];
    }

    return $options;
}

@lsmith77
Copy link

lsmith77 commented Jan 8, 2020

@JackEllis frm my understanding this will not result in stripe setting the VAT field on the customer. I never used this feature, so this might be a quick way to get the VAT to show up on the invoice indeed. But I think going forward it makes more sense to use the dedicated VAT field (also to track the status of the VAT ID).

@driesvints
Copy link
Member Author

This PR has nothing to do with Tax Ids. Only Tax Rates.

Any Tax Id action is a simple CRUD action on the Stripe API and you can use the Stripe SDK directly for that.

@JackEllis
Copy link

@driesvints Gotcha, very cool!

@carlosten
Copy link

Hello,

I was testing the code and found one issue. I am not sure if I was doing it right. I created a subscription with the new taxRates() and the invoice was not showing any tax. (In Stripe the subscription was correctly generated with the correct taxes.)

To make the invoice show the taxes, I changed the line 135 of cashier/src/InvoiceLineItem.php to:
return ! empty($this->item->tax_amounts);

$this->item->tax_rates was empty and I found that the information needed was in tax_amounts.

Is this intended or is a bug? Thanks in advance for any clarification!

@lsmith77
Copy link

did you configure tax rates on stripe?

@carlosten
Copy link

Hello,
yes, tax rates are configured in Stripe and the subscription is correctly generated (with the correct taxes). The problem is only while printing the invoice to pdf, tax_rates is empty and the tax info is inside tax_amounts.

@driesvints
Copy link
Member Author

@carlosten can you open up an issue with the exact steps you've taken to reproduce the problem?

@carlosten
Copy link

@driesvints Done! #854 (comment)

@ostiepok
Copy link

Hi,

Excuse me if iam asking something obvious but iam yet not that much familiar with development on github. I would like to use the funcionality provided by this pull request. But its still only in master branch and in none of the release branches. Do we have an estimate where it will be available in release ? Or are we supposed to change source codes directly in our application ? Thanks.

@driesvints
Copy link
Member Author

@ostiepok #830 (comment)

@ostiepok
Copy link

ostiepok commented Apr 7, 2020

Hi guys, still watiting for this to be pushed to production branch. Do you have any estimate please ?

@driesvints
Copy link
Member Author

@ostiepok this got released today. v11 is tagged.

@ostiepok
Copy link

ostiepok commented Apr 7, 2020

@ostiepok this got released today. v11 is tagged.

Yay thanks ! I was checking this so many times, and even yesterday. I forgot to check today before i wrote that comment so sorry for that and thanks for great work :)

@AnalyzeMe
Copy link

AnalyzeMe commented May 22, 2020

Hello,

I have configured tax rates in Stripe and the subscription is correctly generated (with the reverse charge) when i download the invoice from the stripe dashboard i can se on the invoice "Tax to be paid on reverse charge basis".

The problem is when i download the invoice in laravel cashier 11 the "Tax to be paid on reverse charge basis" is not shown, i also think it's a bit strange that the first check is about, if the invoice has tax, if it's set to reverse it whould never have tax on it.

What am i missing here?

Is this intended or is a bug? Thanks in advance for any clarification!

@if ($invoice->hasTax())
                        @unless ($invoice->isNotTaxExempt())
                            <tr>
                                <td colspan="{{ $invoice->hasTax() ? 3 : 2 }}" style="text-align: right;">
                                    @if ($invoice->isTaxExempt())
                                        Tax is exempted
                                    @else
                                        Tax to be paid on reverse charge basis
                                    @endif
                                </td>
                                <td></td>
                            </tr>
                        @else
                            @foreach ($invoice->taxes() as $tax)
                                <tr>
                                    <td colspan="3" style="text-align: right;">
                                        {{ $tax->display_name }} {{ $tax->jurisdiction ? ' - '.$tax->jurisdiction : '' }}
                                        ({{ $tax->percentage }}%{{ $tax->isInclusive() ? ' incl.' : '' }})
                                    </td>
                                    <td>{{ $tax->amount() }}</td>
                                </tr>
                            @endforeach
                        @endunless
                    @endif

@driesvints
Copy link
Member Author

Hey @AnalyzeMe. The receipt should match the one from Stripe. Can you check if the Stripe receipt has the same information? It's probably best to open an issue for this. Can you open up one and post both a screenshot of the cashier receipt and the stripe receipt?

@AnalyzeMe
Copy link

Hey @driesvints

Thats the issue, the receipt from stripe is just perfect and contains the "Tax to be paid on reverse charge basis" but not the one i download from cashier.

I think the issue is this check: "@if ($invoice->hasTax())" this whould not be true, when i add "reverse" to the customer then i dont add any tax.?

Am i wrong or is this a bug and i need to open an issue.

Kind regards.

@driesvints
Copy link
Member Author

@AnalyzeMe it's best that you open an issue and post the screenshots.

@AnalyzeMe
Copy link

@driesvints I do that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants