documentation icon indicating copy to clipboard operation
documentation copied to clipboard

Published 20 hours ago • verified publisher icon odoo

[IMP] l10n: update AR documentation

Open samueljlieber opened this issue 3 years ago • 20 comments

Task ID: #2908400

samueljlieber avatar Jul 08 '22 14:07 samueljlieber

Pull request status dashboard.

robodoo avatar Jul 08 '22 14:07 robodoo

Hey @StraubCreative 😄 This doc is ready for a preliminary review. Please let me know if you have any suggestions for improvement!

samueljlieber avatar Jul 11 '22 19:07 samueljlieber

Hi @StraubCreative

Thanks for all of the info in each suggestion! I made all of the requested changes except for items that need content/information from the l10n team. I left those items unresolved and I will work with the team to get the content :)

cc: @vbe-odoo

samueljlieber avatar Aug 25 '22 19:08 samueljlieber

Looks good @samueljlieber. Very quick work 🙂

Two errors I found which are contributing to the failed ci/doc check.

  1. content/applications/finance/accounting/fiscal_localizations/localizations/argentina.rst:: WARNING: Anonymous hyperlink mismatch: 1 references but 0 targets. See "backrefs" attribute for IDs.

  2. content/applications/finance/accounting/fiscal_localizations/localizations/argentina.rst:: WARNING: image file not readable: applications/finance/accounting/fiscal_localizations/localizations/argentina/bank-account-relation-error.png

One is a hyperlink issue (not properly formatted). The other is a failed image reference (e.g. image missing, file path wrong in RST, etc.)

Be sure to run make html before every push to avoid failed CI checks like this ✅

StraubCreative avatar Aug 25 '22 22:08 StraubCreative

Hi @vbe-odoo, I removed the sentence referencing the common errors section that currently doesn't exist. I also updated the Get AFIP Certificate link :)

@StraubCreative's remaining content suggestions:

  1. Sentence is unfinished
  2. Sentences don't go deep enough on context
  3. Needs more written content
  4. Missing written content
  5. Missing written content
  6. Missing written content

samueljlieber avatar Aug 30 '22 15:08 samueljlieber

Hi @gza-odoo 👋 thank you for making these suggestions! @StraubCreative I made the changes, ready for another review. Thank you! :)

samueljlieber avatar Sep 19 '22 16:09 samueljlieber

Commit 88a5326 updated document per ZST's suggestions (Thank you 🙏). Remaining suggestions:

samueljlieber avatar Sep 22 '22 18:09 samueljlieber

We need just two quick things here @gza-odoo @vbe-odoo Please see @samueljlieber comment above (click the blue links in the list which will take you to each comment describing what's needed).

After those I think we can open the PR to Accounting team 🚀

StraubCreative avatar Sep 23 '22 20:09 StraubCreative

Hi @StraubCreative, I updated the two points in my previous comment with the comments the team provided, thank you @vbe-odoo and @gza-odoo!

Regarding the typo that says assignes - that is no longer present 👍

Also regarding @gza-odoo's comment I made a small change for context clarity: type of document that does not detail the taxes. They are included in the total amount. to type of document that does not detail the taxes, since the taxes are included in the total amount. Please let me know if this is incorrect :)

samueljlieber avatar Sep 27 '22 18:09 samueljlieber

I'm ok with what you suggest @samueljlieber :)

gza-odoo avatar Sep 27 '22 20:09 gza-odoo

Hi @StraubCreative, your edits have been made 👍

samueljlieber avatar Sep 27 '22 20:09 samueljlieber

Thanks @samueljlieber for the quick changes 🙏

I'm going to open this up to Accounting for review now.

Note: On the next batch of change requests, let's fix the commit message to [IMP] l10n: AR update documentation so it's country-specific and reflects the original commit for the PR.

StraubCreative avatar Sep 27 '22 21:09 StraubCreative

Hi @toaa-odoo, thank you so much for your detailed corrections! 😃

In commit d070461 I updated the commit message to @StraubCreative suggestion and updated the document with most of @toaa-odoo's corrections, but I do need some assistance (@vbe-odoo & @gza-odoo) regarding the points below:

  1. Can you help me identify where and when document type is visible in Odoo in this doc? I changed document type to be lowercase throughout the document, per @toaa-odoo's suggestion, but I am unsure of where it should be labeled as visible (:guilabel:Document Type)
  2. Line 109: I think a preposition is missing after "used”
  3. Line 216: can you clarify which (forms or documents) is correct in this sentence?
  4. Line 248 and 249: can you confirm “regimen” or “regime”?
  5. Line 395: following responsibility → unclear, is the responsibility of Responsable Inscripto?
  6. Line 528 and 529: unclear, what is originator document, and passed to the note?
  7. Line 539: line is unclear, scenarios related to what workflow?
  8. Line 543: what is “annul” ?
  9. Line 732: Affidavit ?
  10. Line 742: Affidavit ?


Thank you everyone ☀️

samueljlieber avatar Sep 29 '22 20:09 samueljlieber

Hi @samueljlieber, Below are my comments:

  1. What do you mean by "labeled as visible"?
  2. Yes, after "used" should go "in" (I don't know why that preposition is not here since in the original Google doc that word is there).
  3. We can replace the whole sentence with what I have in the Google doc: The information required for the document types is included by default so the user does not need to fill anything on this view: If that's not an option I'd pick "forms".
  4. Regime
  5. We mean the AFIP responsibility type, which in this case is "Responsable Inscripto".
  6. I would replace the sentence with: Use the Credit and Debit Note buttons so all the information from the invoice is transferred to the new Credit/Debit Note.
  7. I would replace the sentence with: When creating a Credit Note we can have two scenarios:
  8. Please replace it with: to annulate 9/10) Affidavit is a written statement confirmed by oath or affirmation, for use as evidence in court. From my point of view, that word fits better what we want to say. The other option is the literal translation from Spanish: sworn declaration (declaracion jurada). We're open to any other option you think makes more sense. Please @vbe-odoo let us know your thoughts regarding these suggestions. Best.

gza-odoo avatar Sep 30 '22 23:09 gza-odoo

Hi @gza-odoo thank you for the additional information 🙂 In commit 19e7ed3 I updated the doc for points with a ✅ with the outstanding points are listed below:

  1. What I mean by this (link!) is I need to know where the words document type are visible within the Odoo interface (as a title?, a button?, a field name?, etc...). We specify in the documentation with special bold formatting when something is visible in the Odoo interface.
  2. ✅
  3. ✅
  4. ✅
  5. @toaa-odoo does @gza-odoo's comment clarify?
  6. ✅
  7. ✅
  8. ✅
  9. @toaa-odoo does @gza-odoo's comment clarify?
  10. @toaa-odoo does @gza-odoo's comment clarify?

samueljlieber avatar Oct 03 '22 20:10 samueljlieber

Hi @gza-odoo thank you for the additional information slightly_smiling_face In commit 19e7ed3 I updated the doc for points with a white_check_mark with the outstanding points are listed below:

  1. What I mean by this (link!) is I need to know where the words document type are visible within the Odoo interface (as a title?, a button?, a field name?, etc...). We specify in the documentation with special bold formatting when something is visible in the Odoo interface.
  2. white_check_mark
  3. white_check_mark
  4. white_check_mark
  5. @toaa-odoo does @gza-odoo's comment clarify?
  6. white_check_mark
  7. white_check_mark
  8. white_check_mark
  9. @toaa-odoo does @gza-odoo's comment clarify?
  10. @toaa-odoo does @gza-odoo's comment clarify?

Hello @samueljlieber, yes it's much clearer now. Thank you!

toaa-odoo avatar Oct 04 '22 06:10 toaa-odoo

Hi @samueljlieber , Hope you're doing great. Below are my comments:

  1. Yes, you can find "Document Types" as a menu item in the configuration menu, accounting app; also, it's a title when you click on that menu. You can find it as a field name as well. Best.

gza-odoo avatar Oct 05 '22 22:10 gza-odoo

Hi @gza-odoo 👋 Thank you for confirming where Document Types is generally found within Odoo.

Hi @toaa-odoo 🙂 I went through and set up a test database and followed along with the AR l10n doc to find specifically where in the doc we should label the text as:guilabel:Document Type(s). You can see the changes in commit 8110391.

While following along I found a small change to line 41: Searching for modules with the text Argentina did not show the modules in the picture and the search text in the picture is l10n_ar - that had the right search results.

There is an instance where the Identification Types are listed in the image and referenced on the interface, so I gave that a :guilabel: - see lines 146, & 153-154.

There is an instance where the document type's letter is referenced in the interface, so I also gave that a :guilabel: - see lines 232-234.

There is an instance where the field Document Number is referenced, I gave that a :guilabel: - see line 599 - and capitalized it on line 609.

I did my best to change only where referenced/visible on the interface, I hope these changes are correct! Thank you!

samueljlieber avatar Oct 11 '22 18:10 samueljlieber

Thank you @toaa-odoo 😀 your suggestions have been implemented in 15b7bab.

Hi @vbe-odoo and @gza-odoo can you help me determine how we should label Argentinian/Argentinean throughout the document per @toaa-odoo's point:

I noticed in screenshot content/applications/finance/accounting/fiscal_localizations/localizations/argentina/select-fiscal-package.png that Argentinian is spelled 'Argentinean localization'. Both are correct ways to spell the adjective, but if you were multiple working on this, you might have spelled it differently within the module. Check among yourselves that you indeed used the same spelling throughout.

Thank you! ⭐️

samueljlieber avatar Oct 17 '22 16:10 samueljlieber

Hi @vbe-odoo and @gza-odoo can you help me determine how we should label Argentinian/Argentinean throughout the document per @toaa-odoo's point:

I noticed in screenshot content/applications/finance/accounting/fiscal_localizations/localizations/argentina/select-fiscal-package.png that Argentinian is spelled 'Argentinean localization'. Both are correct ways to spell the adjective, but if you were multiple working on this, you might have spelled it differently within the module. Check among yourselves that you indeed used the same spelling throughout.

Hi @samueljlieber I think we should use this one "Argentinean localization" as is the one that we have in the database and on the screenshots, as both are OK, let's keep the one we have been using when installing l10n_ar in a database, as it will more difficult to change the name there (and on the screenshots) than in this DOC.

Let me know what are your thoughts on this. Thanks! cc' @gza-odoo @fvz-odoo @ren-odoo @toaa-odoo

vbe-odoo avatar Oct 17 '22 18:10 vbe-odoo

@vbe-odoo I think your suggestion is the way to go - I will update the document to use "Argentinean localization". Thanks!

samueljlieber avatar Oct 18 '22 18:10 samueljlieber

Commit a02776e to update Argentinian to Argentinean throughout the document 🙂

CC: @toaa-odoo

samueljlieber avatar Oct 18 '22 18:10 samueljlieber

@jcs-odoo wanted to review it

xpl-odoo avatar Nov 28 '22 13:11 xpl-odoo

53b451c addressed merge conflict on prior commit a02776e after #2983 went into effect.

Ready for your review with all checks passing @jcs-odoo

StraubCreative avatar Dec 02 '22 21:12 StraubCreative

Pinging for merge already so we can move forward. We can still improve afterwards if needed :)

jcs-odoo avatar Dec 05 '22 14:12 jcs-odoo

Thanks for the review @jcs-odoo It seems like some of your suggestions would benefit from the new model/structure introduced in #2659. So we'll push that once next and then take all of your feedback into a new PR once both are merged.

cc: @samueljlieber

StraubCreative avatar Dec 05 '22 18:12 StraubCreative