Skip to content

Conversation

@StraubCreative
Copy link
Contributor

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

@StraubCreative StraubCreative added the new content request Request for a new content label Apr 22, 2022
@StraubCreative StraubCreative requested a review from a team April 22, 2022 03:57
@StraubCreative StraubCreative self-assigned this Apr 22, 2022
@robodoo
Copy link
Collaborator

robodoo commented Apr 22, 2022

Copy link
Contributor

@jcs-odoo jcs-odoo left a 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 :)

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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!

@StraubCreative StraubCreative added content improvement and removed new content request Request for a new content labels May 3, 2022
@StraubCreative StraubCreative force-pushed the 14.0-accounting-add-avatax-integration-zst branch from c0c8dde to 6cc1d4c Compare May 5, 2022 23:18
@StraubCreative
Copy link
Contributor Author

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!

@StraubCreative StraubCreative requested a review from jcs-odoo May 6, 2022 00:07
@jcs-odoo jcs-odoo force-pushed the 14.0-accounting-add-avatax-integration-zst branch 2 times, most recently from 363fdd0 to 0508f31 Compare May 6, 2022 09:21
Copy link
Contributor

@jcs-odoo jcs-odoo left a 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!

@jcs-odoo jcs-odoo requested a review from a team May 6, 2022 09:22
Copy link
Collaborator

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

robodoo pushed a commit that referenced this pull request May 6, 2022
closes #1845

Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>
@robodoo robodoo closed this May 6, 2022
@robodoo robodoo temporarily deployed to merge May 6, 2022 15:09 Inactive
@fw-bot fw-bot deleted the 14.0-accounting-add-avatax-integration-zst branch May 20, 2022 15:47
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.

5 participants