Adds database support for tax identifiers #2832
Conversation
… tax fields to B2BOrder
| tax_country_code = models.CharField(max_length=2, blank=True, null=True) | ||
| tax_rate = models.DecimalField( | ||
| max_digits=6, decimal_places=4, null=True, blank=True, default=0 | ||
| ) | ||
| tax_rate_name = models.CharField(max_length=100, null=True, default="VAT") | ||
|
|
||
| # These represent the customer's tax identifier for the order | ||
| customer_tax_identifier = models.TextField(default="") | ||
| customer_tax_identifier_name = models.CharField( | ||
| max_length=100, null=True, default="" | ||
| ) | ||
|
|
||
| # This is the FK to the tax rate for MIT's tax identifier | ||
| mit_tax_identifier = models.ForeignKey( | ||
| TaxRate, on_delete=models.DO_NOTHING, blank=True, null=True | ||
| ) | ||
|
|
There was a problem hiding this comment.
Although we know we used separate fields to keep the data consistent w.r.t to the time of order, now that we are moving onwards, and added a foreign key to the same table as well. Do you think this might be the time we start using table versioning? Just like how we use product versions?
There was a problem hiding this comment.
Yeah, this is probably the better route, especially at the moment since we're not (AFAIK) using this actively quite yet. Doing this will be a bigger change so it will be a bit before it's done.
There was a problem hiding this comment.
I agree. Making a decision about doing this would probably depend upon:
- What's the urgency of this?
- Are we using the Tax feat in production yet? If yes, We'll also need to migrate the data.
FYI, @cachob
There was a problem hiding this comment.
I can't speak to urgency but we should not be collecting tax currently - the code is there but all the defined tax rates are turned off, which effectively disables the tax collection code.
There was a problem hiding this comment.
@jkachel Thanks for your response. My bad, My intention was actually to ask this from @cachob.
@cachob Could you respond on #2832 (comment)?
|
I think I’m missing some context here. What data are you considering versioning?
I'm cautious about versioning because it adds a lot of complexity which flows downstream through the data platform and reporting.
What would be the downside if we don't version?
… On Nov 28, 2023, at 2:46 AM, Arslan Ashraf ***@***.***> wrote:
Do you think this might be the time we start using table versioning? Just like how we use product versions?
|
Actually, the TaxRate table entries might be edited over time because the Tax rate itself might change in the future. However, we wanted to retain the older tax rate for orders that were placed at a specific time so that each order price remains consistent over time. e.g. If an order was placed with a GST of 18% in June 2023, It should remain even when that GST tax rate changes to 20% after some time. To solve this, We initially introduced independent fields in the Order model and we'd just copy the Tax Rate name, %, etc in the Order table at the time of order so the Order doesn't depend upon the Tax Rate table.
I agree this would add complexity.
Just that we would have extra fields in Order and B2B orders, Which we can definitely live with if we don't version the Tax Rate table. |
|
After looking back at this, I think the best/easiest solution to this would be to capture MIT's GSTIN in the @arslanashraf7 thoughts? |
Thanks for bringing this back to my attention. I agree, if we don't want to keep a versioning table we will need to capture them in independent fields in order and tax rate tables. Just putting it out here so you know how I'm understanding it:
|
Pre-Flight checklist
What are the relevant tickets?
Closes mitodl/hq#2794
What's this PR do?
This PR contains only model additions.
To the
TaxRatetable:To the
Orderstable:TaxRatetable, solely for linking MIT's tax identifier to the order. This defaults to null.To the
B2BOrderstable:TaxRatetable, solely for linking MIT's tax identifier to the order. This defaults to null.These should facilitate the process of adding the GSTIN and collecting tax IDs for B2B orders in the frontend. They should appear in the serializers as expected. This PR doesn't contain any frontend changes - those will be taken care of in a separate issue and PR (since I believe we're waiting on that info at the moment).
How should this be manually tested?
Automated tests should pass. This is just a set of model changes so you should see the new fields in the database.
What GIF best describes this PR or how it makes you feel?
💸