Skip to content

[18.0] [MIG] l10n_it_vat_registries#4664

Merged
OCA-git-bot merged 48 commits into
OCA:18.0from
odooNextev:18.0-mig-l10n_it_vat_registries
May 9, 2025
Merged

[18.0] [MIG] l10n_it_vat_registries#4664
OCA-git-bot merged 48 commits into
OCA:18.0from
odooNextev:18.0-mig-l10n_it_vat_registries

Conversation

@odooNextev

Copy link
Copy Markdown
Contributor

No description provided.

@stenext stenext force-pushed the 18.0-mig-l10n_it_vat_registries branch 4 times, most recently from d244386 to b5f6f21 Compare March 20, 2025 20:49
@eLBati

eLBati commented Mar 21, 2025

Copy link
Copy Markdown
Member

/ocabot migration l10n_it_vat_registries

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Mar 21, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 21, 2025
46 tasks
jado95 added a commit to DinamicheAziendali/l10n-italy that referenced this pull request Mar 28, 2025
l10n_it_appointment_code: OCA#4665
l10n_it_vat_registries: OCA#4664
l10n_it_vat_statement_communication: OCA#4676
@eLBati

eLBati commented Apr 3, 2025

Copy link
Copy Markdown
Member

/ocabot rebase

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 18.0.

@OCA-git-bot OCA-git-bot force-pushed the 18.0-mig-l10n_it_vat_registries branch from b5f6f21 to 3192df1 Compare April 3, 2025 09:27
@eLBati

eLBati commented Apr 10, 2025

Copy link
Copy Markdown
Member

/ocabot rebase

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 18.0.

@OCA-git-bot OCA-git-bot force-pushed the 18.0-mig-l10n_it_vat_registries branch from 3192df1 to 809dbad Compare April 10, 2025 13:30
Comment thread test-requirements.txt Outdated
@@ -0,0 +1 @@
odoo-addon-l10n_it_account @ git+https://github.com/OCA/l10n-italy.git@refs/pull/4555/head#subdirectory=l10n_it_account

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

merged, si può togliere

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fatto

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Test passati dopo aver fatto il FW del commit per rimuovere cee_type

@stenext stenext force-pushed the 18.0-mig-l10n_it_vat_registries branch from 809dbad to b13a712 Compare April 10, 2025 13:39
@odooNextev odooNextev requested a review from eLBati April 10, 2025 13:39
@eLBati

eLBati commented Apr 10, 2025

Copy link
Copy Markdown
Member

/ocabot rebase

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 18.0.

@OCA-git-bot OCA-git-bot force-pushed the 18.0-mig-l10n_it_vat_registries branch from b13a712 to b7fd18c Compare April 10, 2025 13:42
@stenext stenext force-pushed the 18.0-mig-l10n_it_vat_registries branch from b7fd18c to 1e6f42a Compare April 10, 2025 13:51
@eLBati

eLBati commented Apr 10, 2025

Copy link
Copy Markdown
Member

/ocabot rebase

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 18.0.

@OCA-git-bot OCA-git-bot force-pushed the 18.0-mig-l10n_it_vat_registries branch from 1e6f42a to a0ee9b4 Compare April 10, 2025 14:08
Borruso added a commit to DinamicheAziendali/l10n-italy that referenced this pull request Apr 18, 2025
@eLBati eLBati force-pushed the 18.0-mig-l10n_it_vat_registries branch 2 times, most recently from 580b962 to 7482d26 Compare April 18, 2025 14:47

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test funzionale: OK

@monen17 monen17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oltre alle cose scritte sotto, i messaggi di alcuni commit andrebbero tradotti in inglese per https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message:

Commit messages are in English

Ci sono ancora dei commit in italiano, sono da scrivere in inglese:
image

Comment thread l10n_it_account/models/account_tax.py
Comment thread l10n_it_vat_registries/i18n/de.po Outdated
Comment thread l10n_it_account/models/account_tax.py Outdated
Comment thread l10n_it_account/models/account_tax.py Outdated
Comment thread l10n_it_vat_registries/wizard/print_registro_iva.py
Comment thread l10n_it_account/models/account_tax.py Outdated
Comment thread l10n_it_account/models/account_tax.py
Comment thread l10n_it_account/models/account_tax.py Outdated
for tax in self:
accounts = tax._get_accounts_tax()
accounts = accounts.filtered(
lambda a: a.account_type.startswith("liability")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Qui e nell'altro startswith, penso sia meglio indicare esplicitamente quali sono i tipi di conto da considerare come debito/credito, come viene fatto ad esempio per i tipi di registrazioni contabili in https://github.com/odoo/odoo/blob/44e61c6b429aab94e4656ade5e0f0275b905210a/addons/account/models/account_move.py#L5524-L5533.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

modificato

Comment thread l10n_it_account/models/account_tax.py Outdated
debit_balance = -debit_balance
credit_balance = -credit_balance
if registry_type == "customer" and tax.type_tax_use == "purchase":
# caso reverse charge in regsitro IVA vendite

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I commenti, come i messaggi dei commit, devono essere in inglese, vedi https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#33idioms:

Use English variable names and write comments in English.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

modificato

Comment on lines +249 to +238
debit_balance,
credit_balance,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Allora queste modifiche dovrebbero essere nel commit di migrazione.

@eLBati eLBati force-pushed the 18.0-mig-l10n_it_vat_registries branch from ca38b76 to d52a13f Compare May 9, 2025 07:14
@eLBati

eLBati commented May 9, 2025

Copy link
Copy Markdown
Member

i messaggi di alcuni commit andrebbero tradotti in inglese

fatto

@eLBati eLBati force-pushed the 18.0-mig-l10n_it_vat_registries branch 6 times, most recently from 30910a1 to 94309fb Compare May 9, 2025 09:15
@eLBati eLBati force-pushed the 18.0-mig-l10n_it_vat_registries branch from 94309fb to 6678fe8 Compare May 9, 2025 09:27
@monen17 monen17 self-requested a review May 9, 2025 10:49

@monen17 monen17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Grazie delle modifiche!
C'è solo una modifica al codice che non mi torna: quella delle children_tax_ids, per il resto sono problemi di commit.

Per i problemi di commit, la struttura ideale:

  • Modifiche a l10n_it_account in una PR separata
  • In questa PR:
    1. Tutti i commit della 16.0 + pre-commit per la migrazione
    2. Commit dovuti alla migrazione di l10n_it_vat_registries

Visto però che questa struttura non si riesce ad applicare perché ci sono commit applicati prima/dopo, autori diversi ecc. possiamo anche lasciare tutto in questa PR con questa struttura:

  1. Modifiche a l10n_it_account
  2. Tutti i commit della 16.0 + pre-commit per la migrazione
  3. Commit dovuti alla migrazione di l10n_it_vat_registries
  4. Commit di 16.0 che sono stati applicati dopo la migrazione, e che non si riescono a mettere nel punto giusto della storia
  5. Eventuali altri commit dovuti alla migrazione di l10n_it_vat_registries se non si riescono a schiacciare nel commit di migrazione, che però almeno inizino con [MIG].

Se devi unire commit di autori diversi, mettiti come co-autore (vedi https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors).


tax = self.env["account.tax"].with_context(**context).browse(self.id)
tax_name = tax._get_tax_name()
if not tax.children_tax_ids:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non lo stiamo già facendo in questa PR il porting del modulo?
Se è un miglioramento, andrebbe riportato anche per le altre versioni?

Comment thread l10n_it_account/models/account_tax.py
Comment on lines +36 to +38
include_rc_moves = fields.Boolean(
string="Include reverse charge moves",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Allora andrebbe nel commit di migrazione

@monen17 però il commit di migrazione ha un altro autore

O ti metti come co-autore e teniamo un solo commit di migrazione (secondo me meglio), oppure lo lasci in un commit separato, ma almeno che inizi con [MIG].

Comment thread l10n_it_vat_registries/report/report_registro_iva.xml
Comment on lines +484 to +491
<div class="section" id="other-credits">
<h2><a class="toc-backref" href="#toc-entry-7">Other credits</a></h2>
<p>The development of this module has been financially supported by:</p>
<ul class="simple">
<li>Odoo Italia Network</li>
<li>APS Odoo Italia</li>
</ul>
</div>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Questo andrebbe nel commit in cui ha ripristinato il file CREDITS.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

e vabè

Comment on lines +249 to +238
debit_balance,
credit_balance,

@monen17 monen17 May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non avevo notato che queste sono modifiche a l10n_it_account, quindi va bene lasciarle come IMP del modulo l10n_it_account.

A quanto ho capito questa nuova feature di l10n_it_account serve a far funzionare correttamente i registri IVA nella 18.0, quindi andrebbe in una PR a parte da mergiare prima della migrazione.
In questo modo si eviterebbero problemi di dipendenza tra varie PR di migrazione (mi scrivevi che la #4675 dipende da questa PR proprio a causa di queste fix).

Mi scrivevi però che ti complica la fase di sviluppo, quindi va bene lasciarlo qui, alla fine per quanto riguarda la storia dei commit è solo importante che sia in un commit separato.
Riesci però a spostarlo prima di tutti i commit di migrazione? Così dovresti anche riuscire a unire 6678fe8 e 5ac3c02 come suggerivo in #4664 (comment).

Comment on lines -1 to -3
The development of this module has been financially supported by:

- Odoo Italia Network

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Grazie, però hai ripristinato il file in un commit e rigenerato il README in un altro, puoi farlo nello stesso commit?

 - show debit/credit tax totals for reverse charge operations
 - include reverse charge documents in sales VAT registry
@eLBati eLBati force-pushed the 18.0-mig-l10n_it_vat_registries branch from 6678fe8 to 61d64fc Compare May 9, 2025 12:32
@eLBati

eLBati commented May 9, 2025

Copy link
Copy Markdown
Member

#4664 (comment) forse sì ma meglio fare così ora, lasciare indietro le altre versioni e andare avanti

@eLBati

eLBati commented May 9, 2025

Copy link
Copy Markdown
Member

/ocabot merge nobump

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-4664-by-eLBati-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 38ff9d6 into OCA:18.0 May 9, 2025
6 of 7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at b47e0c9. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.