Skip to content

[FIX] l10n_it_edi_doi_extension: removed check on DOI tax being the only one#5135

Open
micheledic wants to merge 1 commit intoOCA:18.0from
stesi:18.0_remove_doi_tax_check
Open

[FIX] l10n_it_edi_doi_extension: removed check on DOI tax being the only one#5135
micheledic wants to merge 1 commit intoOCA:18.0from
stesi:18.0_remove_doi_tax_check

Conversation

@micheledic
Copy link
Contributor

Removed also on odoo core odoo/odoo@b88caf8

Copy link
Contributor

@matteonext matteonext left a comment

Choose a reason for hiding this comment

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

LGTM

@micheledic
Copy link
Contributor Author

@OCA/local-italy-maintainers si riesce a mergiare? non necessità di review funzionale, è stato rimosso un controllo già rimosso sul nativo

Copy link
Contributor

@Borruso Borruso left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@monen17 monen17 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!
Visto che questo caso d'uso deve essere coperto, è possibile aggiungere un test che lo implementa? Così non c'è rischio di regressioni.

@micheledic
Copy link
Contributor Author

Grazie della PR! Visto che questo caso d'uso deve essere coperto, è possibile aggiungere un test che lo implementa? Così non c'è rischio di regressioni.

Nel caso della dichiarazione d'intento che io sappia come altra aliquota può andare la ritenuta d'acconto , bollo , inarcassa ecc ma sono logiche introdotte con l10n_it_edi_witholding . Non posso mettere il test in questo modulo ne in l10n_it_edi_witholding in quanto possono vivere da soli e uno non ha conoscenza dell'altro . Si potrebbe aggiungere un aliquota a caso sulla riga oltre la doi ma non replicherebbe il caso d'uso normale

@monen17 monen17 added the missing issue PRs must be linked to an issue. label Mar 6, 2026
Copy link
Contributor

@monen17 monen17 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! Visto che questo caso d'uso deve essere coperto, è possibile aggiungere un test che lo implementa? Così non c'è rischio di regressioni.

Nel caso della dichiarazione d'intento che io sappia come altra aliquota può andare la ritenuta d'acconto , bollo , inarcassa ecc ma sono logiche introdotte con l10n_it_edi_witholding . Non posso mettere il test in questo modulo ne in l10n_it_edi_witholding in quanto possono vivere da soli e uno non ha conoscenza dell'altro . Si potrebbe aggiungere un aliquota a caso sulla riga oltre la doi ma non replicherebbe il caso d'uso normale

Capito, non era chiaro servisse un'imposta particolare dal commit

[FIX] l10n_it_edi_doi_extension: removed check on DOI tax being the only one

Potresti chiarirlo?

Comunque la ritenuta è semplicemente un'imposta che ha amount 0, secondo me ci sta aggiungere un test con un'imposta che ha amount 0 e indicare nel test che è per coprire il caso di una ritenuta.

Potresti anche aprire una issue così verifichiamo se il problema c'è nelle altre versioni?

@micheledic micheledic force-pushed the 18.0_remove_doi_tax_check branch 3 times, most recently from 4ec9ab3 to c98dbde Compare March 6, 2026 11:27
@micheledic
Copy link
Contributor Author

Grazie della PR! Visto che questo caso d'uso deve essere coperto, è possibile aggiungere un test che lo implementa? Così non c'è rischio di regressioni.

Nel caso della dichiarazione d'intento che io sappia come altra aliquota può andare la ritenuta d'acconto , bollo , inarcassa ecc ma sono logiche introdotte con l10n_it_edi_witholding . Non posso mettere il test in questo modulo ne in l10n_it_edi_witholding in quanto possono vivere da soli e uno non ha conoscenza dell'altro . Si potrebbe aggiungere un aliquota a caso sulla riga oltre la doi ma non replicherebbe il caso d'uso normale

Capito, non era chiaro servisse un'imposta particolare dal commit

[FIX] l10n_it_edi_doi_extension: removed check on DOI tax being the only one

Potresti chiarirlo?

Comunque la ritenuta è semplicemente un'imposta che ha amount 0, secondo me ci sta aggiungere un test con un'imposta che ha amount 0 e indicare nel test che è per coprire il caso di una ritenuta.

Potresti anche aprire una issue così verifichiamo se il problema c'è nelle altre versioni?

fatto! ho modificato un po' anche la funzione del test per accettare più taxes nella funzione di creazione dell'invoice

Copy link
Contributor

@monen17 monen17 left a comment

Choose a reason for hiding this comment

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

Grazie mille del test e de vari adattamenti tax/taxes!
Ho visto però che è spuntata un'altra modifica nel modulo che non mi convince molto, puoi verificare?

@micheledic micheledic force-pushed the 18.0_remove_doi_tax_check branch 4 times, most recently from 5aebc3f to bd2c8fc Compare March 6, 2026 14:27
@micheledic micheledic force-pushed the 18.0_remove_doi_tax_check branch from bd2c8fc to 7030a0a Compare March 6, 2026 14:38
Copy link
Contributor

@monen17 monen17 left a comment

Choose a reason for hiding this comment

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

Se riesci anche a riassumere un po' il titolo del commit per farlo restare nei caratteri consentiti è meglio:

please check if the commit message is cut with ellipsis.

(da https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message)
magari puoi abbreviare il nome del modulo in l10n_it_edi_doi_ext.

A livello di codice per me ora va bene, grazie!
La lascerei comunque aperta ancora qualche giorno per permettere anche ad altri di darci un'occhiata.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

…her taxes

This PR is created to accept other taxes than DOI(for example inarcassa)
removed also on odoo core odoo/odoo@b88caf8
@micheledic micheledic force-pushed the 18.0_remove_doi_tax_check branch from 7030a0a to fc55f72 Compare March 10, 2026 09:33
@micheledic
Copy link
Contributor Author

Se riesci anche a riassumere un po' il titolo del commit per farlo restare nei caratteri consentiti è meglio:

please check if the commit message is cut with ellipsis.

(da https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message) magari puoi abbreviare il nome del modulo in l10n_it_edi_doi_ext.

A livello di codice per me ora va bene, grazie! La lascerei comunque aperta ancora qualche giorno per permettere anche ad altri di darci un'occhiata.

rinominato! è possibile mergiarla a stretto giro @OCA/local-italy-maintainers ? Preferisco sempre evitare di usare il fork in ambienti di produzione ma aggiornare direttamente il branch OCA

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants