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.

Sign up for GitHub

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

Jump to bottom

[16.0] [MIG] l10n_it_fatturapa_in #3000

Merged
merged 155 commits into from
Apr 14, 2023
Merged

[16.0] [MIG] l10n_it_fatturapa_in #3000

merged 155 commits into from
Apr 14, 2023

Conversation

TheMule71
Copy link
Contributor

No description provided.

@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch 5 times, most recently from dbd4932 to e94ccf9 Compare November 3, 2022 02:03
@TheMule71 TheMule71 mentioned this pull request Nov 3, 2022
Open
80 tasks
@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch 2 times, most recently from 9e5384d to 0206c5f Compare November 3, 2022 23:35
@TheMule71
Copy link
Contributor Author

Alcuni test vengono skippati, perché provocano errori DB che impediscono l'esecuzione di test successivi.

@primes2h
Copy link
Contributor

primes2h commented Nov 8, 2022

/ocabot migration l10n_it_fatturapa_in

@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch from 0206c5f to 83027a5 Compare November 10, 2022 14:20
@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch 2 times, most recently from b47b943 to 69480f3 Compare December 6, 2022 23:30
@TheMule71
Copy link
Contributor Author

TheMule71 commented Dec 8, 2022 •
edited
Loading

Promemoria: #3004

@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch 5 times, most recently from f45c7ae to e6332e7 Compare December 16, 2022 11:56
@TheMule71 TheMule71 marked this pull request as ready for review December 30, 2022 16:47
@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch 2 times, most recently from 6905f00 to f965f29 Compare December 30, 2022 19:50
@TheMule71
Copy link
Contributor Author

TheMule71 commented Dec 30, 2022 •
edited
Loading

Include #3111 per far passare i test. Ovviamente, andrebbe mergiata prima quella.

Tengo temporaneamente separato il commit di fix dei test per agevolare la review.

@TheMule71 TheMule71 mentioned this pull request Dec 30, 2022
Merged
@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch from bd88242 to 813119a Compare December 30, 2022 21:51
@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch 2 times, most recently from f5b69d9 to 23b323c Compare January 13, 2023 08:56
@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch 8 times, most recently from 4739c21 to 489e97d Compare April 7, 2023 13:29
@TheMule71
Copy link
Contributor Author

@SirTakobi dovrei aver rivisto tutto.

@SirTakobi SirTakobi mentioned this pull request Apr 13, 2023
Merged
SirTakobi
SirTakobi approved these changes Apr 13, 2023
View reviewed changes
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 mille delle modifiche,
ho solo verificato la storia dei commit e fatto revisione del codice.

Ho messo ancora qualche commento per le questioni in sospeso ma per me non c'è più nulla di bloccante per il merge.

l10n_it_fatturapa_in/__manifest__.py
@@ -0,0 +1,36 @@
# Copyright 2015 AgileBG SAGL <http://www.agilebg.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

Manca 93a6dab, non c'è però una issue per tracciarlo (la PR è #3245).
La modifica è molto semplice, vedi tu se preferisci creare una issue e segnare lì che manca, oppure includere il commit (insieme al commit dei bot che ha causato)

l10n_it_fatturapa_in/i18n/it.po
@@ -0,0 +1,1926 @@
# Translation of Odoo Server.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mancano le traduzioni aggiunte in 8fed160

l10n_it_fatturapa_in/tests/fatturapa_common.py Outdated
self.env["account.account"]
.search(
@classmethod
def setUpClass(cls):
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py Outdated
@@ -363,7 +354,7 @@ def test_14_xml_import(self):
self.assertEqual(invoice.payment_reference, "136")
self.assertEqual(invoice.partner_id.name, "SOCIETA' ALPHA SRL")
self.assertEqual(invoice.amount_untaxed, 25.00)
self.assertEqual(invoice.amount_tax, 0.0)
self.assertEqual(invoice.amount_tax, 3.75)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py Outdated
@@ -712,25 +704,24 @@ def test_36_xml_import(self):
)

def test_37_xml_import_dates(self):
self.env.user.lang = "it_IT"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py Outdated
self.assertTrue(vat_partner_exists())

# XXX changed to false, now it works as intented
self.assertFalse(vat_partner_exists())
Copy link
Contributor

Choose a reason for hiding this comment

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

Può essere che creando l'attachment tramite Form (com'era prima di questa PR) non serva leggere un campo per scatenare il tutto?
Ad ogni modo, l'importante è che non sia necessario modificare la logica del test.
👍

l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py Outdated
@@ -1039,15 +1040,23 @@ def test_e_invoice_field_compute(self):
self.assertEqual(invoices.mapped("e_invoice_validation_error"), [False, False])

def test_duplicated_vat_on_partners(self):
supplier = self.env["res.partner"].search(
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

l10n_it_fatturapa_in/views/account_view.xml
<field name="admin_ref" />
</field>
<field name="amount_tax" position="after">
<!-- field name="price_subtotal" position="after">
<field
name="efatt_rounding"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

l10n_it_fatturapa_in/wizard/wizard_import_fatturapa.py Outdated
@@ -443,29 +444,29 @@ def get_account_taxes(self, AliquotaIVA, Natura):
("type_tax_use", "=", "purchase"),
("kind_id.code", "=", Natura),
("amount", "=", 0.0),
("company_id", "=", self.env.company.id),
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, e le regole di sicurezza non vengono applicate perché i test vengono eseguiti con OdooBot che le bypassa in https://github.com/odoo/odoo/blob/f689c4f53fc4663b17c57bd1d704b9ffb9f11f8d/odoo/models.py#L4418.

Quindi questa aggiunta è necessaria solo nei test, perché normalmente non viene usato OdooBot e le regole vengono applicate.
Ti ho proposto TheMule71#37 per rimuovere questa modifica.

l10n_it_fatturapa_in/wizard/wizard_import_fatturapa.py Outdated
@@ -772,6 +776,8 @@ def _addGlobalDiscount(self, invoice_id, DatiGeneraliDocumento):
line_vals["product_id"] = global_discount_product.id
line_vals["name"] = global_discount_product.name
self.adjust_accounting_data(global_discount_product, line_vals)
elif company.arrotondamenti_tax_id:
Copy link
Contributor

Choose a reason for hiding this comment

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

Avevo capito che qui si sarebbe dovuta rimuovere l'imposta di default, ho aggiunto la modifica in TheMule71#37.

Il commit che attualmente è separato penso sia una nuova funzionalità, se vuoi mantenerla in questa PR servirebbe anche la issue di tracciamento.

@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). 🤖

@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch from f5f9646 to 3922943 Compare April 14, 2023 08:18
@TheMule71 @andreampiovesana @SirTakobi
[MIG] l10n_it_fatturapa_in: Migration to 16.0
615c6dd 
Co-authored-by: andreapiovesana <[email protected]>
Co-authored-by: Simone Rubino <[email protected]>
@TheMule71 TheMule71 force-pushed the 16.0-mig-l10n_it_fatturapa_in branch from 3922943 to 615c6dd Compare April 14, 2023 08:24
@tafaRU
Copy link
Member

tafaRU commented Apr 14, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-3000-by-tafaRU-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 04f6b5c into OCA:16.0 Apr 14, 2023
@OCA-git-bot
Copy link
Contributor

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

@francesco-ooops francesco-ooops mentioned this pull request Dec 27, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francesco-ooops francesco-ooops francesco-ooops left review comments

@andreampiovesana andreampiovesana andreampiovesana approved these changes

@francescapenso francescapenso francescapenso approved these changes

@marcelofrare marcelofrare marcelofrare approved these changes

@marco-marchiori marco-marchiori marco-marchiori approved these changes

@TonyMasciI TonyMasciI TonyMasciI approved these changes

@SirTakobi SirTakobi SirTakobi approved these changes

@primes2h primes2h Awaiting requested review from primes2h

Assignees
No one assigned
Labels
approved merged 🎉 migration ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.