OCA / l10n-italy

Odoo Italian localization
https://www.odoo-italia.org
GNU Affero General Public License v3.0
154 stars 306 forks source link

[16.0][ADD] l10n_it_vat_registries_xlsx: new module #4433

Open TheMule71 opened 3 weeks ago

TheMule71 commented 3 weeks ago

Estende l10n_it_vat_registries e aggiunge un bottone a fianco di Print per creare un file formato excel.

Le righe sono state appiattite, il PDF originariamente è multiriga, ma ha zero senso in un foglio excel.

TheMule71 commented 3 weeks ago

Nota tecnica.

È in draft perché mi piacerebbe discutere di l10n_it_vat_settlement_date.

Ho evitato la dipendenza tecnica dal modulo, ma il report si comporta correttamente se trova la data competenza fattura (simula il comportamente del PDF).

La parte di estrazione dati è già di per sé dinamica (viene da report_xlsx_helper) percui era abbastanza naturale farlo in quel modo.

In teoria, andrebbe scritto un modulo ponte tra l10n_it_vat_settlement_date e questo, tipo l10n_it_vat_settlement_date_xlsx (magari autoinstall), solo per aggiungere la colonna. Oppure si fa dipendere l10n_it_vat_settlement_date anche da questo modulo.

Tuttavia, sembra un'inutile proliferazione dei moduli, e tanto non si elimina la parte dinamica del codice, che ci sarebbe comunque (non è stata introdotta apposta per supportare l10n_it_vat_settlement_date).

MaurizioPellegrinet commented 3 weeks ago

Ottimo il modulo ma secondo me c'è qualcosa da migliorare:

PR4433-1 PR4433-2

SirAionTech commented 2 weeks ago

Ho evitato la dipendenza tecnica dal modulo, ma il report si comporta correttamente se trova la data competenza fattura (simula il comportamente del PDF).

Così facendo però quella parte di codice non è testabile in alcun modulo.

In teoria, andrebbe scritto un modulo ponte tra l10n_it_vat_settlement_date e questo, tipo l10n_it_vat_settlement_date_xlsx (magari autoinstall), solo per aggiungere la colonna. Oppure si fa dipendere l10n_it_vat_settlement_date anche da questo modulo.

Tuttavia, sembra un'inutile proliferazione dei moduli, e tanto non si elimina la parte dinamica del codice, che ci sarebbe comunque (non è stata introdotta apposta per supportare l10n_it_vat_settlement_date).

Aggiungerei il pulsante di questa PR direttamente nel modulo l10n_it_vat_registries (così magari si riesce anche ad evitare il codice duplicato https://github.com/OCA/l10n-italy/pull/4433/files#diff-5889da9e16ed724c7dfae4c3c187f0f7a0b35f6f50ef5a06e3a93a7c55c802dfR24-R57), e la parte che qui è dedicata alla gestione del campo l10n_it_vat_settlement_date andrebbe nel modulo omonimo che già dipende da l10n_it_vat_registries.

L'unica particolarità sarebbe che dobbiamo aggiungere a l10n_it_vat_registries la dipendenza da report_xlsx_helper, ma non la trovo una dipendenza così strana per un modulo che produce dei report.