feat: adds tax numbers to organization and customers #269
Conversation
|
@kochie is attempting to deploy a commit to the Bigcapital Team on Vercel. A member of the Team first needs to authorize it. |
packages/server/Dockerfile
Outdated
| COPY ./packages/server/package*.json ./packages/server/ | ||
|
|
||
| # RUN curl -fsSL https://get.pnpm.io/install.sh | sh - | ||
| RUN npm install -g pnpm |
There was a problem hiding this comment.
Yup, after migrated from npm to pnpm I have to change the Dockerfile instructions too. btw this Dockerfile is not for docker-compose just for CI/CD to build and deploy the server container to the public registry.
| @@ -0,0 +1,11 @@ | |||
| exports.up = function (knex) { | |||
| return knex.schema.table('tenants_metadata', (table) => { | |||
| table.string('tax_number') | |||
There was a problem hiding this comment.
the tax_number should be under tax_rates table, btw application is multi-tenant architecture and has two different type of databases, system database has all the users information and tenants databases each organization account has seperate database has everything related to it like invoices, items, bills, etc. that migration should be on tenants level.
To create a new tenant DB migration. You can from the root directory hit node packages/server/build/commands.js tenants:migrate:make create_new_table after building pnpm run build:server
There was a problem hiding this comment.
I don't think that's the right place to put it, the tax number is a unique identifier associated with a company. So each tenant should have the ability to use a number. Also it's not actually associated with different tax rates (but depending on region the amount of tax does depend on it I'd suppose). The main reason I'm adding it is because in Australia it's a requirement to have an Australian Business Number (ABN) on any tax invoice.
You are right however that it should be added to all customers. I've actually done that already I'm just cleaning it up.
There was a problem hiding this comment.
It was my bad, you're absolutely right, I thought the tax number you want to add it under tax rates. Good work buddy.
|
Really appreciate your contribution, left some notes, your work is awesome and we can merge it after taking a look at these comments and some tweaks. again thanks for contribution |
There was a problem hiding this comment.
Good work man,
- Reverted the Kubernetes commit it's out of pull-request's context.
- Removed the organization tax number from the setup form, always we want it the setup with minimum fields if the user wanted to add it they can go to the preferences.
- Prettified code changes.
- We will display the organization tax number on pdf document once we finish pdf printing.
Let us approve it and merge it for next release. We really appreciate your contribution keep push awesome work.
|
@all-contributors please add @kochie for code. |
|
I've put up a pull request to add @kochie! 🎉 |
When creating invoices in most counties (i.e. Australia, NZ, USA, EU) a tax number (ABN, VAT) needs to be included on the invoice somewhere so that proper declarations can be made.
This PR adds support for tax numbers which are generalised as a text field.