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] New module l10n_it_account_balance_report #1571

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

SilvioGregorini
Copy link
Contributor

Descrizione del problema o della funzionalità: aggiunti report .pdf e .xls a sezioni contrapposte per conto economico e stato patrimoniale

Comportamento attuale prima di questa PR: /

Comportamento desiderato dopo questa PR: /

--
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

@SilvioGregorini SilvioGregorini force-pushed the 12.0-account_balance_report branch 2 times, most recently from 2c1cb4c to e82992c Compare December 23, 2019 21:45
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.

Controlla travis plis

eLBati
eLBati previously requested changes Jan 10, 2020
class AccountType(models.Model):
_inherit = 'account.account.type'

account_balance_report_section = fields.Selection(
Copy link
Member

Choose a reason for hiding this comment

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

Come mai non usare il campo esistente internal_group?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eLBati mi era stato detto così allo sprint napoletano di giugno, non essendo un esperto di contabilità mi son fidato 😅

Copy link
Member

Choose a reason for hiding this comment

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

Ok però bisogna trovare la risposta

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alessandrocamilli tu sai dirci di più?

Copy link
Contributor

Choose a reason for hiding this comment

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

Il campo serve per definire se il conto va stampato tra i costi o ricavi del conto economico, oppure se tra le attività o passività dello stato patrimoniale. Il campo internal_group definisce questo? Altrimenti lasciamo così com'è

Copy link
Member

Choose a reason for hiding this comment

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

Il campo internal_group può assumere i seguenti valori:

        ('equity', 'Equity'),
        ('asset', 'Asset'),
        ('liability', 'Liability'),
        ('income', 'Income'),
        ('expense', 'Expense'),

quindi molto simili a account_balance_report_section (a parte equity e asset che per account_balance_report_section si tratta sempre di attività).

Comunque se vedete problemi nell'utilizzare internal_group, vi chiedo solo di scriverlo in un commento sopra la definizione di account_balance_report_section

Copy link
Contributor

Choose a reason for hiding this comment

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

io non userei internal_group per lasciare la stampa indipendente da quelle che possono essere le definizioni contabili di Odoo. @SilvioGregorini puoi scrivere questo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eLBati @alessandrocamilli ho aggiornato la PR e, contestualmente, anche la PR su 11.0: #1282

Copy link
Member

Choose a reason for hiding this comment

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

ok, grazie

@eLBati
Copy link
Member

eLBati commented Jan 23, 2020

@SilvioGregorini puoi fare rebase per includere 74b8e6b ?

@SilvioGregorini
Copy link
Contributor Author

@eLBati fatto

@eLBati eLBati dismissed their stale review January 24, 2020 10:56

Abstaining for now

Copy link
Contributor

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@sergiocorato sergiocorato left a comment

Choose a reason for hiding this comment

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

L'xlsx inoltre esce in formato inglese, al contrario degli altri report

@SilvioGregorini
Copy link
Contributor Author

@eLBati maintainer aggiunto.
@sergiocorato ora dovrebbe essere tutto ok, al di fuori del bug relativo alla lingua del file .xls su cui sinceramente non capisco cosa intervenga... il fatto che non sia definito alcun file it.po può influenzare?

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 per la PR!
Giusto due piccole note.

P.S.: riesci per cortesia ad aggiungere il .pot? Rende più facile la revisione documentale/traduzioni. Grazie. :-)

'name': "Italian Localisation -"
" Stampe Stato Patrimoniale e Conto Economico",
'summary': "Report PDF e XLS per Stato Patrimoniale e Conto Economico"
" a sezioni contrapposte",
Copy link
Contributor

Choose a reason for hiding this comment

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

Come da linee guida direi ITA -, inoltre stato patrimoniale e conto economico andrebbero con la lettera minuscola.

P.S.: in questo caso, per uniformità con le attuali traduzioni di contabilità (e con il glossario) sarebbe consigliabile tradurre Report con Rendiconto.

Copy link
Contributor

Choose a reason for hiding this comment

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

Come da linee guida direi ITA -

Scusa, probabilmente mi sono espresso male.
In name bastava sostituire Italian Localization - con ITA - senza aggiungere Rendicontazione.
Se invece è una cosa voluta no problem. ;-)

P.S.: il report a cui mi riferivo è quello nel summary, per uniformità sarebbe da sostituire con Rendiconto oppure Rendiconti.

inoltre stato patrimoniale e conto economico andrebbero con la lettera minuscola.

Sono presenti anche in summary.

@SilvioGregorini SilvioGregorini force-pushed the 12.0-account_balance_report branch 2 times, most recently from e5364bb to 9bd17aa Compare February 5, 2020 15:54
@SilvioGregorini
Copy link
Contributor Author

@primes2h ho aggiunto .po e .pot, modificato il nome del modulo.
Altro che dovrei fare?

@sergiocorato
Copy link
Contributor

sergiocorato commented Feb 5, 2020

@eLBati maintainer aggiunto.
@sergiocorato ora dovrebbe essere tutto ok, al di fuori del bug relativo alla lingua del file .xls su cui sinceramente non capisco cosa intervenga... il fatto che non sia definito alcun file it.po può influenzare?

@SilvioGregorini non vedo usato il metodo write_number che viene usato negli altri moduli che esportano xlsx, ma solo il metodo write_string, a naso senza fare test direi potrebbe dipendere da questo

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.

Dopo queste per me 👍
Grazie mille.

'name': "Italian Localisation -"
" Stampe Stato Patrimoniale e Conto Economico",
'summary': "Report PDF e XLS per Stato Patrimoniale e Conto Economico"
" a sezioni contrapposte",
Copy link
Contributor

Choose a reason for hiding this comment

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

Come da linee guida direi ITA -

Scusa, probabilmente mi sono espresso male.
In name bastava sostituire Italian Localization - con ITA - senza aggiungere Rendicontazione.
Se invece è una cosa voluta no problem. ;-)

P.S.: il report a cui mi riferivo è quello nel summary, per uniformità sarebbe da sostituire con Rendiconto oppure Rendiconti.

inoltre stato patrimoniale e conto economico andrebbero con la lettera minuscola.

Sono presenti anche in summary.

@SilvioGregorini
Copy link
Contributor Author

@sergiocorato non penso sia lì il problema, quei due metodi non toccano le traduzioni, semplicemente ottimizzano la scrittura dei dati sul file .xls, quindi già lì dovrebbe ricevere la stringa tradotta

@SilvioGregorini
Copy link
Contributor Author

@primes2h sistemato il manifest e il typo nel template

@@ -0,0 +1,7 @@
**Italiano**

Report PDF e XLS per stato patrimoniale e conto economico a sezioni contrapposte.
Copy link
Contributor

Choose a reason for hiding this comment

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

Scusami, mi era sfuggito. 🙏
Anche questo andrebbe uniformato.

P.s: per quanto riguarda il problema dell'xls non tradotto se riesco provo a darci un'occhiata.

Copy link
Contributor

Choose a reason for hiding this comment

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

P.S.2: dimenticavo, il file it.po non l'ho revisionato, meglio farlo direttamente su weblate dopo il merge del modulo.

@sergiocorato
Copy link
Contributor

sergiocorato commented Feb 6, 2020

@sergiocorato non penso sia lì il problema, quei due metodi non toccano le traduzioni, semplicemente ottimizzano la scrittura dei dati sul file .xls, quindi già lì dovrebbe ricevere la stringa tradotta

@SilvioGregorini non riesco ad approfondire a breve, però se scrivi 10.05 come stringa tale esce, se la scrivi come numero poi viene espressa in base al locale no? beh, era un'idea
p.s. parliamo della stessa cosa no? i valori numerici in singole caselle, non valori numerici concatenati con altri di testo

@primes2h
Copy link
Contributor

primes2h commented Feb 6, 2020

@SilvioGregorini
Copy link
Contributor Author

@primes2h modificato anche DESCRIPTION.rst
@sergiocorato trovato il bug, ora dovrebbe essere ok... puoi fare qualche test rapido?

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.

@SilvioGregorini 👍

Ho fatto una prova di esportazione, ma l'xlsx è ancora tutto in inglese.

@SilvioGregorini SilvioGregorini force-pushed the 12.0-account_balance_report branch 2 times, most recently from aa9c3b4 to 2a47c11 Compare February 7, 2020 17:15
@sergiocorato
Copy link
Contributor

@SilvioGregorini ottimo, se puoi solo correggere queste (poca roba) https://travis-ci.org/OCA/l10n-italy/jobs/647430918#L336-L338

@sergiocorato
Copy link
Contributor

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-1571-by-sergiocorato-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 10, 2020
Signed-off-by sergiocorato
@primes2h
Copy link
Contributor

primes2h commented Feb 10, 2020

@ser

@SilvioGregorini 👍

Ho fatto una prova di esportazione, ma l'xlsx è ancora tutto in inglese.

@sergiocorato c'era ancora questa cosa da verificare, per questo la mia approvazione era ancora in sospeso.
Nel runbot l'xlsx esce ancora in inglese, anche se in locale (appena provato) risulta correttamente in italiano. Idee?

@sergiocorato
Copy link
Contributor

sergiocorato commented Feb 10, 2020 via email

@OCA-git-bot OCA-git-bot merged commit e401162 into OCA:12.0 Feb 10, 2020
@OCA-git-bot
Copy link
Contributor

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

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