Skip to content
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

[16.0] [MIG] l10n_it_reverse_charge #3300

Closed

Conversation

are-agilebg
Copy link
Contributor

Sostituisce #3103 integrando le modifiche applicate sul ramo 14.0 successivamente alla versione 14.0.1.1.13 (vedi #3103 (comment)).

dcorio and others added 3 commits April 28, 2023 16:46
[IMP] pep8

[FIX] typos and pep8

[IMP] l10n_it_reverse_charge refactoring

[ADD] account_move_skip

[IMP] skip expr

[IMP] i18n

[IMP] tax mapping

[IMP] mandatory fields

[IMP] autohide if == 0

[REM] pdb

[IMP] no need to specify expr

[FIX] write if no credit/debit
[ADD] l10n_it_reverse_charge: add initial draft of README

[ADD] l10n_it_reverse_charge: add note about 'Integrazione IVA'

[ADD] l10n_it_reverse_charge: add check if there is default credit account defined on Self Invoice Payment Journal

[ADD] l10n_it_reverse_charge: set rc on invoice.line according to RC Type defined on fiscal position

[FIX] l10n_it_reverse_charge: rename tax_src_id and tax_dest_id fields to be more consinstent and easier to understand their purpose

[IMP] l10n_it_reverse_charge: adapt to latest OCA guidelines

[FIX] l10n_it_reverse_charge: fix 'WARNING test_reverse_charge openerp.modules.loading: The model account.rc.type has no access rules'

[ADD] l10n_it_reverse_charge
Handle invoice supplier cancellation and reopening for method 'selfinvoice':
* cancellation: both supplier invoice and selfinvoice are cancelled
* reopening: data on existing selfinvoice are copied to supplier invoice

[FIX] l10n_it_reverse_charge: fix WARNING account.invoice.line.create() with unknown fields: uom_id

[IMP] l10n_it_reverse_charge: improve README

[IMP] l10n_it_reverse_charge: add more comments for making code easier to understand

[IMP] l10n_it_reverse_charge: add help to tax_ids field

[FIX] l10n_it_reverse_charge:
* do not copy rc_self_invoice_id
* check if inv has rc_self_invoice_id before to proceed with its cancellation

[FIX] l10n_it_reverse_charge: avoid product from self invoice lines to better handle related stats

[ADD] 8.0-l10n_it_reverse_charge: copy self_invoice_text to self invoice name
[W7910(wrong-tabs-instead-of-spaces), ]  Use wrong tabs indentation instead of four spaces

FIX screenshots

FIX allow to use and compute RC field in invoice line form pop up

l10n_it_reverse_charge ADD "With additional supplier self invoice"
to enable the creation of an additional supplier self
invoice. This is tipically used for extraUE suppliers,
in order to show, in supplier register, an invoice to the
specified partner (tipically, my company), instead of the
extraUE partner

Also FIX
ValueError: "local variable 'tax_code_id' referenced before assignment" while evaluating
u'invoice_validate()'
when tax mapping not found
@are-agilebg are-agilebg force-pushed the 16.0-mig-l10n_it_reverse_charge branch 2 times, most recently from 5366e32 to 117255f Compare May 10, 2023 12:27
@are-agilebg are-agilebg marked this pull request as ready for review May 10, 2023 12:34
@tafaRU tafaRU mentioned this pull request May 10, 2023
75 tasks
Copy link
Contributor

@michelerusti michelerusti left a comment

Choose a reason for hiding this comment

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

Mi sembra sia rimasto da schiacciare i vari commit amministrativi dei bot, per il resto tutto ok!

https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

@are-agilebg are-agilebg force-pushed the 16.0-mig-l10n_it_reverse_charge branch 12 times, most recently from 2385392 to c0c5c36 Compare May 11, 2023 07:04
@are-agilebg are-agilebg force-pushed the 16.0-mig-l10n_it_reverse_charge branch from f309def to 56a8a1e Compare July 7, 2023 12:18
@tafaRU
Copy link
Member

tafaRU commented Jul 7, 2023

#3287 da includere

lo includiamo successivamente per #3286 (comment)

Copy link
Contributor

@MarcoCalcagni MarcoCalcagni left a comment

Choose a reason for hiding this comment

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

Ok test funzionali

@MarcoCalcagni
Copy link
Contributor

MarcoCalcagni commented Jul 7, 2023

questo commento è per una issue futura non impatta sulla migrazione.
nel caso di tipo partner autofattura "Altro" e Con autofattura passiva aggiuntiva "si" quando metto in bozza una fattura passiva precedentemente confermata c'è un messaggio utente "You cannot delete a tax line as it would impact the tax report"
per proseguire è necessario cancellare le fatture collegate per poi ricrearle.

per migliorie future se cancello la fattura passiva legata al RC non cancella le eventuali fatture in bozza collegate.

@eLBati
Copy link
Member

eLBati commented Jul 7, 2023

#3287 da includere

lo includiamo successivamente per #3286 (comment)

Rilasciare il modulo senza ba0be69 significa permettergli di generare registrazioni contabili sbagliate, che quindi saranno poi da correggere

Copy link
Contributor

@SirTakobi SirTakobi left a comment

Choose a reason for hiding this comment

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

Grazie della PR!
Non ho guardato tutte le modifiche ma so solo che manca #3287 e a quanto ne so è un errore abbastanza grave, è possibile includerlo nella migrazione?

@MarcoCalcagni
Copy link
Contributor

@SirTakobi
nella issue 3286 ti ho aggiunto il nostro setup che funziona come scritture contabili.

@MarcoCalcagni
Copy link
Contributor

anche senza la Pr #3287

ottengo questo
image

partendo da questa fattura
image

posizione fiscale
image

inversione contabile
image

registro rc
image

image

image

@MarcoCalcagni
Copy link
Contributor

appena mergiata preparo il read me con tutta la configurazione per la 16 . eventualmente anche con la pr di Simone

@aleuffre
Copy link
Contributor

aleuffre commented Jul 7, 2023

A questo punto non so se sarà più rilevante, ma visto che ci ho passato un bel po' di tempo qualche dettaglio tecnico sul test che fallisce nell'integrazione di #3287 , nel caso qualcuno lo debba riprendere e non vuole iniziare da capo:

Il test è test_intra_EU_exempt

Fallisce perchè è l'unico caso di fattura con tassazione 0%. A quel punto, il problema si ha quando si creano le move_lines all'interno della funzione _prepare_rc_invoice_payment, in particolare queste righe:

        # Lines to be reconciled with the original supplier Invoice (self)
        rc_tax_amount = self.compute_rc_amount_tax_main_currency()
        payment_credit_line_data = self.rc_credit_line_vals(
            rc_type.transitory_account_id,
            rc_tax_amount,
        )
        line_to_reconcile = self._rc_get_move_line_to_reconcile()
        payment_debit_line_data = self.rc_debit_line_vals(
            line_to_reconcile.account_id,
            rc_tax_amount,
        )

rc_tax_amount è 0, quindi payment_debit_line_data diventa una move_line con credit e debit = 0; a questo punto, appena la move_line viene creata, Odoo la marca immediatamente come riconciliata.

Quando viene chiamata rc_lines_to_rec.reconcile() qualche riga più in basso, viene restituito un errore perché la riga è già stata riconciliata.

@TheMule71
Copy link
Contributor

TheMule71 commented Jul 7, 2023

A questo punto la _rc_get_move_line_to_reconcile() potrebbe filtrare per line.reconciled == False ?

@aleuffre
Copy link
Contributor

aleuffre commented Jul 10, 2023

A questo punto la _rc_get_move_line_to_reconcile() potrebbe filtrare per line.reconciled == False ?

Confermo che con la seguente modifica al file account_move.py righe 334 e 335:

rc_lines_to_rec = line_to_reconcile | payment_line_to_reconcile
rc_lines_to_rec.filtered(lambda x: not x.reconciled).reconcile()

(aggiunto solo il filtered())

i test passano. Se sia anche corretto il risultato prodotto non lo saprei dire.

@tafaRU
Copy link
Member

tafaRU commented Jul 10, 2023

Ok grazie @aleuffre! A questo punto ti direi di creare una PR verso di questa contente le seguente modifiche:

@aleuffre
Copy link
Contributor

aleuffre commented Jul 10, 2023

Ok grazie @aleuffre! A questo punto ti direi di creare una PR verso di questa contente le seguente modifiche:

* cherry-pick di [745c9aa7701ca8e217b4f5f5ba8b92b13dd1f88a](https://github.com/OCA/l10n-italy/pull/3287/commits/745c9aa7701ca8e217b4f5f5ba8b92b13dd1f88a)

* le modifiche che scrivevi su [[16.0] [MIG] l10n_it_reverse_charge #3300 (comment)](https://github.com/OCA/l10n-italy/pull/3300#issuecomment-1628391126)

Mmmh, non credo di poterlo fare...

image

La fix si trova qui https://github.com/PyTech-SRL/l10n-italy/tree/16.0-reverse-charge-test-fix

il mio cambiamento è letteralmente meno di una riga, se volete copiarlo e incollarlo direttamente sul vostro branch non mi offendo haha

@tafaRU
Copy link
Member

tafaRU commented Jul 10, 2023

Anch'io avevo avuto qualche difficoltà a creare are-agilebg#1 @are-agilebg ricordi mica come c'ero riuscito? 😅

@SirTakobi
Copy link
Contributor

Quando viene chiamata rc_lines_to_rec.reconcile() qualche riga più in basso, viene restituito un errore perché la riga è già stata riconciliata.

Grazie @aleuffre dell'analisi, sai mica come mai nella 14.0 questo non succeda?
I test non mi sembrano essere cambiati nella migrazione

@aleuffre
Copy link
Contributor

Quando viene chiamata rc_lines_to_rec.reconcile() qualche riga più in basso, viene restituito un errore perché la riga è già stata riconciliata.

Grazie @aleuffre dell'analisi, sai mica come mai nella 14.0 questo non succeda? I test non mi sembrano essere cambiati nella migrazione

Non ne sono sicuro, e adesso purtroppo non riesco a fare test più approfonditi. Ma è cambiata di molto la funzione _compute_amount_residual che si occupa di settare il campo reconciled sulle move.line, quindi sospetterei in prima battuta che possa essere quello. Magari sbaglio.

@are-agilebg
Copy link
Contributor Author

Anch'io avevo avuto qualche difficoltà a creare are-agilebg#1 @are-agilebg ricordi mica come c'ero riuscito? sweat_smile

forse avevi settato a mano il remote nell'url di github nella schermata della creazione PR, e poi in quel modo era possibile selezionare il remote/branch di interesse.. ma non ricordo con precisione

@aleuffre
Copy link
Contributor

aleuffre commented Jul 10, 2023

Anch'io avevo avuto qualche difficoltà a creare are-agilebg#1 @are-agilebg ricordi mica come c'ero riuscito? sweat_smile

forse avevi settato a mano il remote nell'url di github nella schermata della creazione PR, e poi in quel modo era possibile selezionare il remote/branch di interesse.. ma non ricordo con precisione

Grazie, sembra aver funzionato.

are-agilebg#2

Copy link
Contributor

@scigghia scigghia left a comment

Choose a reason for hiding this comment

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

👍

Co-authored-by: Simone Rubino <sir@takobi.online>
groups="base.group_multi_company"
/>
</group>
<group string="Self Invoicing">
Copy link
Contributor

Choose a reason for hiding this comment

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

puoi aggiungere colspan="4" per risolvere questo comportamento

rc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Non riesco a riprodurre la problematica, visualizzo quella schermata senza problemi anche modificando la dimensione della finestra (Chrome). Tu come ottieni quel comportamento?

Copy link
Contributor

Choose a reason for hiding this comment

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

scusami hai ragione non ho scritto come riprodurre, basta che inserisci nella tabella all'interno della form una qualsiasi riga che abbia del testo molto lungo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ho fatto alcune prove e non riesco a risolvere applicando colspan="4" come mi suggerisci. Probabilmente sto sbagliando io qualche cosa, purtroppo però non posso spendere altro tempo su questa attività. Valuta se riesci a farmi una PR con la fix direttamente.

Copy link
Contributor

Choose a reason for hiding this comment

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

posso provare a indagare meglio nel caso ti creo una PR
grazie mille

Copy link
Contributor

@SirTakobi SirTakobi left a comment

Choose a reason for hiding this comment

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

Grazie della PR!
Non è possibile portare a termine la configurazione solo seguendo le istruzioni del modulo, ma questo andrà risolto in una issue dedicata, forse #2670.

Oltre ai commenti qui sotto, ho trovato un problema eseguendo questi passi:

  1. Creare il tipo di inversione contabile
    image
  2. Impostare il tipo di inversione contabile nella posizione fiscale
    image
  3. Impostare la posizione fiscale in fornitore
  4. Creare una fattura fornitore per fornitore, con un servizio (la riga fattura sarà RC)
  5. Confermare la fattura
  6. Riportare in bozza la fattura
  7. Confermare di nuovo la fattura

Osservato:
Errore di validazione

You cannot delete a tax line as it would impact the tax report

Atteso:
Poter confermare la fattura

Copy link
Contributor

Choose a reason for hiding this comment

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

Questi commit sono da schiacciare:

Mancano alcuni commit presenti in 14.0, tipo:

È possibile riscrivere il messaggio del commit 56a8a1e in modo da seguire le linee guida https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message?
La prima riga è lunga quasi due volte il numero massimo di caratteri

Copy link
Contributor

Choose a reason for hiding this comment

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

Ho provato a eseguire pre-commit e questo file non viene generato, da dove viene?

@@ -1,38 +1,36 @@
<?xml version="1.0" encoding="utf-8" ?>
<odoo>
<data noupdate="1">
Copy link
Contributor

Choose a reason for hiding this comment

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

Come mai questi dati non hanno più noupdate?

@@ -1,25 +1,23 @@
<?xml version="1.0" encoding="utf-8" ?>
<odoo>
<data noupdate="1">
Copy link
Contributor

Choose a reason for hiding this comment

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

Come mai questi dati non hanno più noupdate?

@@ -93,7 +91,9 @@ def create_invoice(cls, partner, amounts, taxes=None, post=True):
invoice = cls.init_invoice(
"in_invoice", partner=partner, post=post, amounts=amounts, taxes=taxes
)
for line in invoice.invoice_line_ids:
for line in invoice.invoice_line_ids.filtered(
lambda l: l.display_type == "product"
Copy link
Contributor

Choose a reason for hiding this comment

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

Come mai questa modifica è necessaria per la migrazione?
Ho provato senza e non ho trovato problemi nei test

Comment on lines +128 to +145

(
cls.tax_22ai.invoice_repartition_line_ids
+ cls.tax_22ai.refund_repartition_line_ids
).filtered(
lambda repartition_line_id: repartition_line_id.repartition_type == "tax"
).write(
{
"account_id": cls.env["account.account"].search(
[
("company_id", "=", cls.env.company.id),
("account_type", "=", "asset_current"),
],
limit=1,
)
}
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Come mai questa modifica è necessaria per la migrazione?
Ho provato senza e non ho trovato problemi nei test

@@ -20,13 +20,13 @@
<field name="rc_purchase_invoice_id" readonly="True" />
</group>
<xpath
expr="//field[@name='invoice_line_ids']/tree/field[@name='price_subtotal']"
expr="//field[@name='invoice_line_ids']/tree/field[@name='tax_ids']"
Copy link
Contributor

Choose a reason for hiding this comment

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

C'è un motivo particolare per cui è necessario spostare il campo?
Se non è una modifica necessaria per la migrazione, sarebbe meglio spostarla in un altro commit in modo da poterla portare nelle altre versioni più facilmente

position="after"
>
<field name="rc" />
<field name="rc" optional="hide" />
Copy link
Contributor

Choose a reason for hiding this comment

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

In 14.0 il campo era visibile di default, come mai invece in 16.0 deve essere nascosto di default?
Se non è una modifica necessaria per la migrazione, sarebbe meglio spostarla in un altro commit in modo da poterla portare nelle altre versioni più facilmente

@@ -85,7 +83,7 @@ def test_extra_EU(self):
invoice.button_draft()
# see what done with "with invoice.env.do_in_draft()" in
# button_draft
invoice.refresh()
invoice.invalidate_model()
Copy link
Contributor

Choose a reason for hiding this comment

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

Dimenticavo: nella 16.0 apparentemente la cache è gestita meglio quindi questa riga (e le successive che erano dei refresh) potrebbero non servire più, puoi verificare?

@SirTakobi
Copy link
Contributor

Prendo in carico la migrazione per risolvere i punti aperti in #3300 (review) e #3300 (review)

@tafaRU
Copy link
Member

tafaRU commented Sep 7, 2023

Chiudo in favore di #3514

@tafaRU tafaRU closed this Sep 7, 2023
@eLBati
Copy link
Member

eLBati commented Sep 7, 2023

chiudo in favore di #3514

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.

None yet