OCA / l10n-italy

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

[16.0] l10n_it_fatturapa: tolto staticmethod a ftpa_preview_xml #4132

Closed TennyMkt closed 2 months ago

TennyMkt commented 4 months ago

Risolve https://github.com/OCA/l10n-italy/issues/4087 per la 16.0

def ftpa_preview(self): era definita come @staticmethod quindi il parametro self veniva considerato come un parametro di nome self e non era possibile fare attachment.ftpa_preview()

TennyMkt commented 3 months ago

In generale sarebbe meglio impattare solo un modulo in ogni commit, ma in questo caso penso che mettere tutto in un commit sia l'unico modo per non produrre commit intermedi rotti. Potresti però aggiungere il nome dei moduli modificati nel messaggio del commit? Anche solo con una wildcard tipo

[IMP] l10n_itfatturapa*: ...

Aggiornato messaggio della commit

Le righe che hai modificato in l10n_it_fatturapa_in e l10n_it_fatturapa_out non sono coperte dai test, hai modo di aggiungere un semplice test per verificare che funzionino come ti aspetti?

Non saprei come fare il test, la ftpa_preview ritorna un dizionario composto da costanti e 1 valore di un ir.attachment

SirAionTech commented 3 months ago

Le righe che hai modificato in l10n_it_fatturapa_in e l10n_it_fatturapa_out non sono coperte dai test, hai modo di aggiungere un semplice test per verificare che funzionino come ti aspetti?

Non saprei come fare il test, la ftpa_preview ritorna un dizionario composto da costanti e 1 valore di un ir.attachment

Basterebbe qualcosa tipo

def test_ftpa_preview(self)
    preview_action = e_invoice.ftpa_preview()
    self.assertEqual(preview_action["url"], e_invoice.ftpa_preview_link)
TennyMkt commented 2 months ago

Le righe che hai modificato in l10n_it_fatturapa_in e l10n_it_fatturapa_out non sono coperte dai test, hai modo di aggiungere un semplice test per verificare che funzionino come ti aspetti?

Non saprei come fare il test, la ftpa_preview ritorna un dizionario composto da costanti e 1 valore di un ir.attachment

Basterebbe qualcosa tipo

def test_ftpa_preview(self)
    preview_action = e_invoice.ftpa_preview()
    self.assertEqual(preview_action["url"], e_invoice.ftpa_preview_link)

Aggiornata PR con i test

OCA-git-bot commented 2 months ago

This PR looks fantastic, let's merge it! Prepared branch 16.0-ocabot-merge-pr-4132-by-SirAionTech-bump-patch, awaiting test results.

OCA-git-bot commented 2 months ago

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

OCA-git-bot commented 2 months ago

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

francesco-ooops commented 2 months ago

Le righe che hai modificato in l10n_it_fatturapa_in e l10n_it_fatturapa_out non sono coperte dai test, hai modo di aggiungere un semplice test per verificare che funzionino come ti aspetti?

Non saprei come fare il test, la ftpa_preview ritorna un dizionario composto da costanti e 1 valore di un ir.attachment

Basterebbe qualcosa tipo

def test_ftpa_preview(self)
    preview_action = e_invoice.ftpa_preview()
    self.assertEqual(preview_action["url"], e_invoice.ftpa_preview_link)

Aggiornata PR con i test

@TennyMkt puoi allineare la PR per la 14? Grazie!

TennyMkt commented 2 months ago

Le righe che hai modificato in l10n_it_fatturapa_in e l10n_it_fatturapa_out non sono coperte dai test, hai modo di aggiungere un semplice test per verificare che funzionino come ti aspetti?

Non saprei come fare il test, la ftpa_preview ritorna un dizionario composto da costanti e 1 valore di un ir.attachment

Basterebbe qualcosa tipo

def test_ftpa_preview(self)
    preview_action = e_invoice.ftpa_preview()
    self.assertEqual(preview_action["url"], e_invoice.ftpa_preview_link)

Aggiornata PR con i test

@TennyMkt puoi allineare la PR per la 14? Grazie!

Fatto ( https://github.com/OCA/l10n-italy/pull/4133 )