-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[IMP] Taxes: improve tax configuration doc #14908
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
5a62fc3 to
068288c
Compare
068288c to
1ef8407
Compare
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 @antoine162 , thanks for this update to the taxes doc! Honestly I didn't have too much to say because you did a great job. Let's change the formatting a bit in the Tax Computation section as I explained in a comment, and let me know if you have questions on any of my other suggestions 🙂
| :guilabel:`Percentage of Price` tax computation with the :ref:`taxes/included-in-price` | ||
| option. | ||
|
|
||
| - **Python code** |
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.
Note this has changed to Custom Formula as of 18. We can handle that in a fw port :) make sure to change it in this whole section and not just the heading!
b092888 to
75a65e7
Compare
bea8a8a to
f3aab31
Compare
|
@dade-odoo I think it's ready for a final review by you! In my latest push, I just slightly modified the "Affects base of subsequent taxes" section and added a section for "Base affected by preceding taxes". |
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.
Thanks @antoine162 ! Very helpful update regarding base affected/affect base - a few notes to clean it up and then we can send it to be-doc-review :)
f3aab31 to
7f87a27
Compare
|
Thank you so much @dade-odoo ! |
7f87a27 to
3e4fd58
Compare
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 @antoine162 ! You are already getting a handle on the documentation, nice job!
I made a few suggestions - referring to the guidelines for the most part - and some of them should be applied throughout the PR.
If you have any questions, don't hesitate to ask :)
| - :ref:`taxes/computation/fixed`: a fixed amount | ||
| - :ref:`taxes/computation/percentage-of-price`: a percentage of the tax-excluded sales price | ||
| - :ref:`taxes/computation/percentage-of-price-tax-included`: a percentage of the tax-included total | ||
| - :ref:`taxes/computation/python-code`: a custom user-defined formula. |
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.
| - :ref:`taxes/computation/python-code`: a custom user-defined formula. | |
| - :ref:`taxes/computation/python-code`: a custom user-defined formula |
You should only use a period at the end of a list item if it forms a complete sentence (as per our guidelines, even if we could argue that the last item represents the end of the sentence ;)). This applies to other lists as well.
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.
Okay, good to know!
|
|
||
| .. _taxes/computation/group-of-taxes: | ||
|
|
||
| Group of Taxes |
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.
| Group of Taxes | |
| Group of taxes |
We use sentence case for headings, even though this contradicts another guideline, which is to refer to Odoo's UI elements as they appear. In any case, sentence case takes precedence, and this applies to all subsequent headings :)
You will need to adapt the references above (e.g., :ref:`Group of Taxes <taxes/computation/group-of-taxes>` to follow the second guidelines :)
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.
This makes sense, thank you very much!
| +=============+=============+==========+==========+ | ||
| | 1,000 | 1,000 | 100 | 1,100.00 | | ||
| +-------------+-------------+----------+----------+ | ||
| The exact tax computation depends on the :ref:`taxes/included-in-price` field, which determines |
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.
| The exact tax computation depends on the :ref:`taxes/included-in-price` field, which determines | |
| The exact tax computation depends on the :ref:`Included in Price <taxes/included-in-price>` field, which determines |
To use the field's label as in the UI. This applies to other refs as well :)
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.
I've done that throughout the doc.
| Note that the real tax rate in terms of the tax-excluded price is | ||
| :math:`\frac{100}{900} = 11.111\%`. | ||
|
|
||
| .. warning:: |
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.
| .. warning:: | |
| .. important:: |
If you believe this information is particularly important, you could consider moving it to the top, but I wouldn't use a warning :)
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.
I've done that - thanks for the feedback on using important vs warning.
| .. example:: | ||
| :guilabel:`Python Code`: `result = price_unit * 0.10` | ||
| :guilabel:`Applicable Code`: `result = true` | ||
| .. warning:: |
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.
| .. warning:: | |
| .. important:: |
Consider moving it to the top if it's really important :)
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.
Thanks - I've done that.
I've also rewritten this block to be more specific, as many people resorting to Python taxes do so without realizing that fixed taxes could fit their use case really well and avoid many of the limitations of Python taxes.
| .. note:: | ||
| By default, only the :guilabel:`Tax excluded` column is displayed on invoices. To display the | ||
| :guilabel:`Tax included` column, click the **dropdown toggle** button and check | ||
| :guilabel:`Tax included` column, click the **dropdown toggle** button and enable |
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.
| :guilabel:`Tax included` column, click the **dropdown toggle** button and enable | |
| :guilabel:`Tax included` column, click the :icon:`oi-settings-adjust` :dropdown:`(dropdown toggle)` button and enable |
You can add the icon here :)
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.
Looking again at this note, I think this content doesn't belong here - definitely not in the Included in price section, which is purely about a configuration option on taxes, and possibly not even in the Taxes page (as it refers to the UI on invoices, noting the same column is there on sale and purchase orders as well).
What would be the best way to handle this?
- Should we add a section describing the UI related to tax computations on invoices / sale orders / purchase orders? I feel like this might be considered to be missing content at the moment.
- Would this content belong in the Taxes page or in a different one?
- Should we add this content now or in a different task?
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.
I can't think of any other UI related to tax computations on invoices, and this field is removed from invoices from 18 forward. It remains on SOs/POs, so if the responsible would like to document it in either of those apps, they're welcome too, but I think we can remove this note entirely :)
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.
sounds good, I've removed it 👍
3e4fd58 to
effba86
Compare
task-5094417 We improve the following sections of the Taxes page: - Tax computations: At the moment, the section for the `Percentage of Price tax included` tax computation doesn't distinguish itself enough from the 'Included in Price' option which people usually want. We also improve the examples. - Included in Price - Affects base of subsequent taxes - Base affected by preceding taxes: this option was not documented, so we add a section for it.
effba86 to
a544342
Compare
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+
task-5094417 We improve the following sections of the Taxes page: - Tax computations: At the moment, the section for the `Percentage of Price tax included` tax computation doesn't distinguish itself enough from the 'Included in Price' option which people usually want. We also improve the examples. - Included in Price - Affects base of subsequent taxes - Base affected by preceding taxes: this option was not documented, so we add a section for it. closes #14908 Signed-off-by: Xavier Platteau (xpl) <[email protected]>

task-5094417
We improve the following sections of the Taxes page:
Tax computations:
At the moment, the section for the
Percentage of Price tax includedtax computation doesn't distinguish itself enough from the 'Included in Price' option which people usually want. We also improve the examples.Included in Price
Affects base of subsequent taxes
Base affected by preceding taxes: this option was not documented, so we add a section for it.