-
Notifications
You must be signed in to change notification settings - Fork 10.9k
[IMP] Accounting: Adding ISO 20022 info #15248
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
|
Hello @cas-odoo - this is ready for a review! |
|
@vpd-odoo - this is also ready for a review. I have asked for reviews from CAS and HUN, FYI. |
content/applications/finance/fiscal_localizations/united_states.rst
Outdated
Show resolved
Hide resolved
dade-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.
Some of this information should be more generally available and not just in the US localization (i.e., how to create a bank account). Since not all of them exist already (thanks for pointing them out with this PR!) I'm fine to let this go ahead as it is and then we'll get started on a general update to batch payments, ISO 20022, etc. and in that PR we'll remove any duplicate information here in the US localization page and link to the general information we create.
TLDR: Thanks! Accounting doc team will approve for now and edit later :)
content/applications/finance/fiscal_localizations/united_states.rst
Outdated
Show resolved
Hide resolved
|
Sounds good, @dade-odoo! As for the bank form not appearing correctly, I changed everything back to the US (there was one company I set to Italy, because I was working on an Expense Card doc, and it's not available in the US!). But after making the changes, it didn't show up. We can put a pin in that, though, since I don't think it will block anyone (everyone knows how to select either a checking or savings account). I agree about the 'general content', but the reason I added the bank info was that there is bank info in the Employees doc- BUT the info requested there is different. Since the forms were not the same, I added the section. I agree with not duplicating "basics" info, but I just couldn't find a place to link that was accurate. Another option is adding that info to a Contacts doc, then linking it to that. But again, that can be for the next version. Since I see you approved it, I will pass this on to our tech/final review and get this published! |
e60e532 to
2af7bbf
Compare
|
Hi @Felicious - this is ready for a final/tech review. FYI, I have ONLY added info to the new ISO section at the bottom. I did go in and remove the "center" from the images, and I also changed app names ot all be bold, but I did NOT go in and redo images from the doc (they have call outs). DADE has approved the doc, and we have discussed that it will change in the next version, but it is good to publish for now! Thank you =) |
Felicious
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.
Looks great, @larm-odoo!
The scope of this update is quite large. Even though we typically classify new documents as 5 points and updates to existing ones as 3 points, this contribution is comprehensive and complex enough that it could reasonably stand as its own document. We chose not to break it out only because of how the accounting scope is currently organized.
Given the amount of research, accuracy, coordination with PEs and Dallas, and the overall depth reflected here, could you increase the point label from 3 to 5? I think that would more accurately represent the work captured in this PR.
I only noticed a few minor RST syntax issues. I’m surprised they weren’t caught by make review—might be worth passing along as feedback for our internal tooling team (;
@robodoo delegate+
content/applications/finance/fiscal_localizations/united_states.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/united_states.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/united_states.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/united_states.rst
Outdated
Show resolved
Hide resolved
|
Hi @Felicious -I just double-checked the 'make review' tool, and it did NOT catch those RST issues you did! I will pass that along. It only caught the lines that were too long (but were links, on their own line, so it's impossible to shorten them). |
2af7bbf to
9415d60
Compare
|
@robodoo r+ |
closes #15248 Signed-off-by: Lara Martini (larm) <larm@odoo.com>

New ISO 20022 info needs to be added to the USA accounting localization info. Info provided on this doc and this video, provided by DADE.
Original task card for this doc.