[16.0][FIX] accounting menu not translatable

[primes2h]

Risolve #2885 per la v.16.0

[andreampiovesana]

LGTM

[MaurizioPellegrinet]

LGTM

[francesco-ooops]

/ocabot rebase

[OCA-git-bot]

Congratulations, PR rebased to 16.0.

[OCA-git-bot]

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

[andreampiovesana]

LGTM
Merge?

[SirAionTech]

Grazie della PR!
Come scrissi in #2675 (review):

Secondo me il problema è più generico di account vs l10n_it_account vs account_accountant quindi si potrebbe aprire una issue.

e andrebbe quindi risolto direttamente in Odoo.

Rispetto ad allora però ho trovato dei possibili problemi con questo approccio, vedi il commento qui sotto.

Se vogliamo continuare ad usare dei work-around, si potrebbe usare un approccio più dinamico, tipo

l10n-italy/l10n_it_riba/models/ir_ui_menu.py

Lines 18 to 26 in f3bbdcd

	# Move the RiBa menu if any of
	# its siblings (menu having same parent before write)
	# is moved (parent changes).
	# This happens when account_accountant (enterprise)
	# is installed or uninstalled.
	root_riba_menu = self.env.ref("l10n_it_riba.menu_riba")
	parent_riba_menu = root_riba_menu.parent_id
	if old_parent == parent_riba_menu and new_parent_id != old_parent.id:
	root_riba_menu.parent_id = new_parent_id

.

[kamzata]

Please, can anyone merge this? LGTM?

[primes2h]

Grazie della PR! Come scrissi in #2675 (review):

Secondo me il problema è più generico di account vs l10n_it_account vs account_accountant quindi si potrebbe aprire una issue.

e andrebbe quindi risolto direttamente in Odoo.

Ho approfondito la questione e ho scoperto che tecnicamente questo problema non è un vero e proprio bug.
Da quello che ho capito, il problema di fondo è che in Odoo non è ancora stato implementato un meccanismo di inherit per i menu e finché non verrà fatto non sarà possibile risolvere correttamente la cosa.

Questo comporta che quando ad esempio vado a modificare una voce di menù in questo modo, quello che succede è che per assurdo viene modificata la stringa sorgente del modulo account.
Infatti esportando il file it.po di account dentro troviamo

msgid "Accounting"
msgstr "Fatturazione"

Questo spiega perfettamente il comportamento attuale di Odoo quando viene installato il modulo l10n_it_account. In inglese compare il menù Accounting e in italiano compare Fatturazione.

Rispetto ad allora però ho trovato dei possibili problemi con questo approccio, vedi il commento qui sotto.

Concordo con te che questo approccio apre le porte a diversi potenziali (e reali) problemi.

Se vogliamo continuare ad usare dei work-around, si potrebbe usare un approccio più dinamico, tipo

l10n-italy/l10n_it_riba/models/ir_ui_menu.py

Lines 18 to 26 in f3bbdcd

	# Move the RiBa menu if any of
	# its siblings (menu having same parent before write)
	# is moved (parent changes).
	# This happens when account_accountant (enterprise)
	# is installed or uninstalled.
	root_riba_menu = self.env.ref("l10n_it_riba.menu_riba")
	parent_riba_menu = root_riba_menu.parent_id
	if old_parent == parent_riba_menu and new_parent_id != old_parent.id:
	root_riba_menu.parent_id = new_parent_id

.

Un approccio del genere applicato a questo caso porterebbe a mio parere a complicare ancora di più le cose con il forte rischio di effetti collaterali futuri non prevedibili (es. interazioni con metodi per far funzionare correttamente i menù con il modulo account_accountant, come ad es. il modulo riba o altri in OCA).

Per risolvere alla radice la problematica propongo un nuovo approccio, è un workaround che va a modificare direttamente la stringa tradotta in account.
Agli effetti pratici il risultato che si ottiene equivale a quello che fa il sistema con la stringa sorgente.
In questo modo il menù resta quello originale e funziona correttamente con qualsiasi modulo extra che va ad aggiungere sottomenù.

Cosa ne pensi?

[SirAionTech]

Per risolvere alla radice la problematica propongo un nuovo approccio, è un workaround che va a modificare direttamente la stringa tradotta in account.
Agli effetti pratici il risultato che si ottiene equivale a quello che fa il sistema con la stringa sorgente.
In questo modo il menù resta quello originale e funziona correttamente con qualsiasi modulo extra che va ad aggiungere sottomenù.

Cosa ne pensi?

Molto meglio grazie, e ho anche imparato qualche trucchetto su come funzionano le traduzioni, tipo _get_stored_translations e update_raw 😉.

Più in generale, dici che è fattibile aggiungere un test?

[primes2h]

Per risolvere alla radice la problematica propongo un nuovo approccio, è un workaround che va a modificare direttamente la stringa tradotta in account.
Agli effetti pratici il risultato che si ottiene equivale a quello che fa il sistema con la stringa sorgente.
In questo modo il menù resta quello originale e funziona correttamente con qualsiasi modulo extra che va ad aggiungere sottomenù.
Cosa ne pensi?

Molto meglio grazie, e ho anche imparato qualche trucchetto su come funzionano le traduzioni, tipo _get_stored_translations e update_raw 😉.

Più in generale, dici che è fattibile aggiungere un test?

Adesso non riesco, se possibile sarebbe meglio procedere senza. Si può sempre aggiungere successivamente con un'altra PR.

[MaurizioPellegrinet]

Test funzionale: OK

[mymage]

LGTM
Nel runboat ho provato a mettere Francese e ovviamente mostra la stringa errata.
Penso sarebbe carino in termini community segnalare, non so dove e come, ai cugini transalpini e agli amici iberici questa modifica.

[SirAionTech]

/ocabot rebase

[OCA-git-bot]

Congratulations, PR rebased to 16.0.

[SirAionTech]

Ho fatto qualche prova e ora funziona come dovrebbero fare le traduzioni, grazie!
Ho anche fatto revisione del codice e per me è ok.
Oltre ai commenti qui sotto, rimane solo l'onnipresente

è fattibile aggiungere un test?

Ma secondo me non c'è più nulla di bloccante per il merge.

Sono poi d'accordo con #4298 (review):

sarebbe carino in termini community segnalare, non so dove e come, ai cugini transalpini e agli amici iberici questa modifica.

[SirAionTech]

Forse non deve essere in una discussione?

[SirAionTech]

/ocabot merge patch

cc @primes2h

[OCA-git-bot]

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-4298-by-SirAionTech-bump-patch, awaiting test results.

[OCA-git-bot]

@SirAionTech your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-4298-by-SirAionTech-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

[SirAionTech]

@SirAionTech your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-4298-by-SirAionTech-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

I test falliscono per #3801.

[SirAionTech]

Riproviamo
/ocabot merge patch

[OCA-git-bot]

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

[OCA-git-bot]

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