-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[ADD] finance/accounting: add avatax integration #1845
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
Conversation
jcs-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @StraubCreative !
Thanks for this Accounting Pull Request :)
For apps handled by the BE team, you can just add a task in the documentation project with some indications if you have them, and ping the content writer or me. But thank you for having done this one then ;)
I wrote a few comments, but some need to be applied to the rest of the document (images, guilabel, sentence case etc).
- I suggested something slightly different for the configuration section.
- The credentials part could use a brief indication on how to retrieve the credentials.
- The configuration section only mentions that you need to add credentials, it doesn't explain anything about the environment, other options and buttons.
- Your text should suffice without images, and images come to help understand. It is sometimes the opposite (e.g., the part about overriding the avatax category on a product form). It's not a video made into a static page ^^
:guilabel:should now be used wherever it's an UI element, be it a button, the name of a view or field etc.- see comments about images filenames + "HD resolution" instead of "4K resolution"
- tip to also take smaller images regardless of the resolution: rescale the window so what you need fits in less width. I personally open Chrome's inspector to play with the width of the screen. + crop more! Let's just show what's relevant and that helps understanding the text.
- the commit message doesn't need the "finance" category. However, you should add the number of the related odoo task (the number of the task in the url) as such:
[ADD] accounting: avatax integration task-id 1234567 - you can also copy-paste the url of the odoo task in the first message on this Pull Request. This way, reviewers and mergers can easily update the status of the task.
- the label "new content request" is rather for issues. No need for such label anymore, but the most appropriate one would be "content improvement". But since close to 100% of the PR on this repo are content improvement, it's not really necessary.
Thanks and have a good one :)
content/applications/finance/accounting/taxation/taxes/avatax.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/taxation/taxes/avatax.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/taxation/taxes/avatax.rst
Outdated
Show resolved
Hide resolved
| The Avatax integration is available on Sale Orders and Invoices with the included Avatax fiscal | ||
| position. | ||
|
|
||
| Before using the integration, specify an Avatax Category on the product categories you use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any doc about product categories to link here and in a seealso section, maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't find a "product categories" doc that was relevant here.
The seealso doc I included at the bottom was fiscal positions just like you see on TaxCloud doc. Open to suggestions if you have any!
content/applications/finance/accounting/taxation/taxes/avatax.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/taxation/taxes/avatax.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/taxation/taxes/avatax.rst
Outdated
Show resolved
Hide resolved
c0c8dde to
6cc1d4c
Compare
|
Hi @jcs-odoo 👋 Please see newest revisions on 6cc1d4c. I made a number of the changes you asked for, sticking closely to the technical stuff and not so much the content like we talked about briefly. One change I took the liberty of making was to streamline the spelling of Avatax (instead of AvaTax) just to make it more consistent and that's the spelling the R&D team decided to favor anyway in the UI. Please let me know there are any other changes you'd like me to make before we try to expand the content next. Can knock out quickly if you see anything else. Thanks! |
363fdd0 to
0508f31
Compare
jcs-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @StraubCreative :)
I took the liberty to push some changes
- some typos
- line break error
- a :guilabel: instead of bold
- rescale and crop the images
Feel free to check the diff between our versions if you'd like.
Let's merge this as-is for now. We'll add more details later :)
Thanks again!
AntoineVDV
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robodoo r+
closes #1845 Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>
Overview
New v14 module doc requested by @jorenvo
Summary of changes
This PR adds new Avatax documentation inside **Accounting / Taxation / Taxes **
This can be forward-ported beyond 14.0 to later versions