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

[12.0][l10n_it_fatturapa_in] Supporto per la rilevazione degli arroto… #1395

Conversation

robyf70
Copy link
Contributor

@robyf70 robyf70 commented Aug 1, 2019

…ndamenti in fatture elettroniche d'acquisto

Descrizione del problema o della funzionalità:

#1375

La procedura di importazione delle fatture elettroniche di acquisto non supporta la gestione degli arrotondamenti, pertanto la fattura che viene generata risulta errata e quindi da "sistemare" a mano

Comportamento attuale prima di questa PR:

Non c'è supporto degli arrotondamenti in fatture elettroniche d'acquisto

Comportamento desiderato dopo questa PR:

Supporto completo per la rilevazione degli arrotondamenti attivi e passivi

--
Confermo di aver firmato il CLA https://odoo-community.org/page/cla e di aver letto le linee guida su https://odoo-community.org/page/contributing

@eLBati
Copy link
Member

eLBati commented Sep 27, 2019

Ci sono conflitti su l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py

@robyf70 robyf70 force-pushed the 12.0-l10n_it_fatturapa_in_fix_add_arrotondamenti_when_generating_the_bill branch from 9f4c7f9 to 71d16a5 Compare September 28, 2019 08:07
@robyf70
Copy link
Contributor Author

robyf70 commented Sep 28, 2019

@eLBati Ho risolto i conflitti

Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

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

Mancano le modifiche al ROADMAP.rst come in #1391

Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

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

Mi pare che manchi anche il test aggiunto da #1374

Puoi verificare che le modifiche siano allineate?

@robyf70
Copy link
Contributor Author

robyf70 commented Oct 1, 2019

Mi pare che manchi anche il test aggiunto da #1374

Puoi verificare che le modifiche siano allineate?

Forse parli della #1391 che dovrebbe essere mergiata per gestire tutte le casistiche. Comunque si, manca il test aggiuntivo che devo inserire in questa PR.

@robyf70 robyf70 force-pushed the 12.0-l10n_it_fatturapa_in_fix_add_arrotondamenti_when_generating_the_bill branch 2 times, most recently from 17a2589 to 36ea660 Compare October 1, 2019 21:42
@robyf70
Copy link
Contributor Author

robyf70 commented Oct 3, 2019

@eLBati Ora è ok

Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

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

Ultima domanda: gli XML li hai anche testati su ADE, o comunque ti aspetti che vadano bene perchè hai modificato solo una riga?

@robyf70 robyf70 force-pushed the 12.0-l10n_it_fatturapa_in_fix_add_arrotondamenti_when_generating_the_bill branch from 36ea660 to 6cd0225 Compare October 8, 2019 21:36
@robyf70
Copy link
Contributor Author

robyf70 commented Oct 9, 2019

@eLBati I file sono arrivati da alcuni clienti che hanno avuto 2 casistiche diverse

Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

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

Grazie

Copy link
Contributor

@primes2h primes2h left a comment

Choose a reason for hiding this comment

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

Ciao, se ho inteso bene il significato, il termine più corretto per arrontondamenti attivi/passivi dovrebbe essere "round(ing) up" e "round(ing) down".

@robyf70
Copy link
Contributor Author

robyf70 commented Oct 9, 2019

Ciao, se ho inteso bene il significato, il termine più corretto per arrontondamenti attivi/passivi dovrebbe essere "round(ing) up" e "round(ing) down".

In questo caso si tratta di arrotondamenti attivi e passivi

@primes2h
Copy link
Contributor

primes2h commented Oct 9, 2019

Ciao, se ho inteso bene il significato, il termine più corretto per arrontondamenti attivi/passivi dovrebbe essere "round(ing) up" e "round(ing) down".

In questo caso si tratta di arrotondamenti attivi e passivi

Si, ho scritto male io, infatti avevo già corretto il commento.
Non penso cambi molto comunque, "Active/Passive rounding" non lo trovo in ambito contabile, ma ho trovato questo:

https://www.proz.com/kudoz/italian-to-english/accounting/1534252-arrotondamenti-passivi.html

Copy link
Member

@SimoRubi SimoRubi left a comment

Choose a reason for hiding this comment

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

Per me ok (code review)

@robyf70
Copy link
Contributor Author

robyf70 commented Oct 9, 2019

Ciao, se ho inteso bene il significato, il termine più corretto per arrontondamenti attivi/passivi dovrebbe essere "round(ing) up" e "round(ing) down".

In questo caso si tratta di arrotondamenti attivi e passivi

Si, ho scritto male io, infatti avevo già corretto il commento.
Non penso cambi molto comunque, "Active/Passive rounding" non lo trovo in ambito contabile, ma ho trovato questo:

https://www.proz.com/kudoz/italian-to-english/accounting/1534252-arrotondamenti-passivi.html

@primes2h @eLBati Quindi cambio la traduzione?

@eLBati
Copy link
Member

eLBati commented Oct 9, 2019

Per me OK

@robyf70 robyf70 force-pushed the 12.0-l10n_it_fatturapa_in_fix_add_arrotondamenti_when_generating_the_bill branch from 6cd0225 to b5315c3 Compare October 10, 2019 09:04
@robyf70
Copy link
Contributor Author

robyf70 commented Oct 10, 2019

@eLBati @primes2h Ho appena pushato i cambiamenti alla traduzione

Comment on lines 19 to 26
'account.account', 'Rounding Up Account',
domain=[('deprecated', '=', False)],
help="Account used to rounding up amount on bills."
)
arrotondamenti_passivi_account_id = fields.Many2one(
'account.account', 'Rounding Down Account',
domain=[('deprecated', '=', False)],
help="Account used to rounding down amount on bills."
Copy link
Contributor

Choose a reason for hiding this comment

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

In questo caso dovrebbe essere usata la forma Round up account, Round down account. Vedi ad es.:

https://kb.sambapos.com/en/2-3-10-a-how-to-create-bill-rounding/

Stessa cosa direi per la forma verbale:
...used to round up(down) amount on bills.

arrotondamenti_tax_id = fields.Many2one(
'account.tax', 'Rounding Tax',
domain=[('type_tax_use', '=', 'purchase'), ('amount', '=', 0.0)],
help="Tax used to both up and down roundings on bills."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
help="Tax used to both up and down roundings on bills."
help="Tax used to both round up and down bills amount."

oppure amount on bills, l'importante è che ci sia uniformità con le stringhe simili alle righe 21 e 26

arrotondamenti_tax_id
if not arrotondamenti_tax_id:
self.log_inconsistency(
_('Tax for active and passive rounding is not set')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_('Tax for active and passive rounding is not set')
_('Rounding up and down tax is not set')

if rounding > 0.0:
line_vals = {
'invoice_id': invoice.id,
'name': _("Passive Rounding"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'name': _("Passive Rounding"),
'name': _("Rounding down"),

elif rounding < 0.0:
line_vals = {
'invoice_id': invoice.id,
'name': _("Active Rounding"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'name': _("Active Rounding"),
'name': _("Rounding up"),

arrotondamenti_attivi_account_id = self.env.user.company_id.\
arrotondamenti_attivi_account_id
if not arrotondamenti_attivi_account_id:
raise UserError(_("Active rounding account is not set "
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise UserError(_("Active rounding account is not set "
raise UserError(_("Round up account is not set "

arrotondamenti_passivi_account_id = self.env.user.company_id.\
arrotondamenti_passivi_account_id
if not arrotondamenti_passivi_account_id:
raise UserError(_("Passive rounding account is not set "
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise UserError(_("Passive rounding account is not set "
raise UserError(_("Round down account is not set "

@robyf70 robyf70 force-pushed the 12.0-l10n_it_fatturapa_in_fix_add_arrotondamenti_when_generating_the_bill branch from b5315c3 to e5295a2 Compare October 10, 2019 14:55
@robyf70
Copy link
Contributor Author

robyf70 commented Oct 10, 2019

@primes2h Ok! Fatto!

@sergiocorato
Copy link
Contributor

@robyf70 se vuoi verificare cosa è applicabile di questa PR

@robyf70
Copy link
Contributor Author

robyf70 commented Oct 10, 2019

@sergiocorato La controllo subito

@robyf70
Copy link
Contributor Author

robyf70 commented Oct 10, 2019

@sergiocorato Veramente strano l'XML della PR! Sinceramente non capisco cosa debbano arrotondare se il calcolo è già corretto senza arrotondamento. Che ne pensi?

Comment on lines 1062 to 1069
raise UserError(_("Rounding up account is not set "
"in Accounting Settings"))

arrotondamenti_passivi_account_id = self.env.user.company_id.\
arrotondamenti_passivi_account_id
if not arrotondamenti_passivi_account_id:
raise UserError(_("Rounding down account is not set "
"in Accounting Settings"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Come scritto qui #1395 (comment) e indicato in #1395 (comment) e in #1395 (comment)
la forma corretta qui dovrebbe essere round up account e round down account.

arrotondamenti_tax_id = fields.Many2one(
'account.tax', 'Rounding Tax',
domain=[('type_tax_use', '=', 'purchase'), ('amount', '=', 0.0)],
help="Tax used to round up and down on bills."
Copy link
Contributor

Choose a reason for hiding this comment

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

Il both andava bene, non so perché l'hai tolto. Comunque manca ancora una parte, vedi #1395 (comment) controllando l'uniformità con le righe 21 e 26.

@robyf70 robyf70 force-pushed the 12.0-l10n_it_fatturapa_in_fix_add_arrotondamenti_when_generating_the_bill branch from e5295a2 to 3b6ebc7 Compare October 12, 2019 09:16
@robyf70
Copy link
Contributor Author

robyf70 commented Oct 12, 2019

@primes2h Abbi pazienza di fare un'altra review, ho appena pushato
@eLBati @sergiocorato Ora faccio un pò di verifiche sugli arrotondamenti dell'XML di Sergio

@robyf70
Copy link
Contributor Author

robyf70 commented Oct 12, 2019

@eLBati @sergiocorato Secondo me la fattura di Amazon, che hai aggiunto nella PR, è sbagliata e lo SDI non fà i controlli corretti sull'arrotondamento, almeno questa sembra la spiegazione più probabile. Vedi anche https://www.fatturapa.gov.it/export/fatturazione/sdi/Elenco_Controlli_V1.5.pdf a pag.23

Copy link
Contributor

@primes2h primes2h left a comment

Choose a reason for hiding this comment

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

Perdonami tu. 😅

)

arrotondamenti_attivi_account_id = fields.Many2one(
'account.account', 'Rounding Up Account',
Copy link
Contributor

Choose a reason for hiding this comment

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

Anche qui andrebbe
Round Up Account (vedi riga 1062 di https://github.com/OCA/l10n-italy/pull/1395/files#diff-a1b600a217458c87c2b54a559199629a)

help="Account used to round up amount bills amount."
)
arrotondamenti_passivi_account_id = fields.Many2one(
'account.account', 'Rounding Down Account',
Copy link
Contributor

Choose a reason for hiding this comment

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

arrotondamenti_attivi_account_id = fields.Many2one(
'account.account', 'Rounding Up Account',
domain=[('deprecated', '=', False)],
help="Account used to round up amount bills amount."
Copy link
Contributor

Choose a reason for hiding this comment

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

Adesso c'è un amount di troppo. :-)

O metti amount on bills (e uniformi allo stesso modo anche la riga 31) oppure usi bills amount.

arrotondamenti_passivi_account_id = fields.Many2one(
'account.account', 'Rounding Down Account',
domain=[('deprecated', '=', False)],
help="Account used to round down amount bills amount."
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 amount di troppo anche qui.
Vedi commento sopra.

@robyf70 robyf70 force-pushed the 12.0-l10n_it_fatturapa_in_fix_add_arrotondamenti_when_generating_the_bill branch from 3b6ebc7 to 844c887 Compare October 14, 2019 13:21
Copy link
Contributor

@primes2h primes2h left a comment

Choose a reason for hiding this comment

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

👌

Grazie!

@robyf70
Copy link
Contributor Author

robyf70 commented Oct 14, 2019

Perdonami tu.

... sono le gioie ed i dolori delle review 😅

@eLBati
Copy link
Member

eLBati commented Oct 14, 2019

Visto che il discorso su LevelPrime#6 è ancora aperto, faccio intanto merge di questa

@eLBati
Copy link
Member

eLBati commented Oct 14, 2019

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 12.0-ocabot-merge-pr-1395-by-eLBati-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 14, 2019
Signed-off-by eLBati
@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). 🤖

@OCA-git-bot OCA-git-bot merged commit 844c887 into OCA:12.0 Oct 14, 2019
@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot changed the title [12.0][l10n_it_fatturapa_in] Supporto per la rilevazione degli arroto… [12.0][l10n_it_fatturapa_in] Supporto per la rilevazione degli arroto… Oct 14, 2019
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

7 participants