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] [FIX] l10n_it_fiscalcode_sale: change module dependency #3106

Merged

Conversation

primes2h
Copy link
Contributor

Il modulo l10n_it_fiscalcode_sale permette di mostrare il codice fiscale del cliente nella stampa del preventivo / ordine di vendita.
Dato che i preventivi/ordini di vendita vengono aggiunti installando sale_management (applicazione "Vendite"), il modulo dovrebbe di conseguenza dipendendere da quest'ultimo e non da sale.

La situazione attuale porta al seguente problema.
In una installazione Odoo con ad es. fatturazione elettronica, imposta di bollo e dichiarazione di intento (per obbligo dei clienti), il modulo l10n_it_fiscalcode_sale viene installato in modo automatico anche se non necessario.

Questa PR corregge il problema.

Vedi anche #3105

@OCA-git-bot
Copy link
Contributor

Hi @eLBati,
some modules you are maintaining are being modified, check this out!

@matteoopenf
Copy link
Contributor

Ma il report che eredita nelle View è del modulo sale non l'altro. Non mi sembra corretta quindi la pr .

l10n-italy/l10n_it_fiscalcode_sale/views/sale_order_report.xml

Line 3 in abdc072

@primes2h
Copy link
Contributor Author

primes2h commented Dec 29, 2022

Ma il report che eredita nelle View è del modulo sale non l'altro. Non mi sembra corretta quindi la pr .

l10n-italy/l10n_it_fiscalcode_sale/views/sale_order_report.xml

Non vedo il problema, visto chesale è il modulo base e quindi dipendenza dell'applicazione Vendite (sale_management).

È il comportamento attuale che non è corretto.
Installando l10n_it_fiscalcode_sale o l10n_it_account_stamp_sale mi aspetto che nell'interfaccia siano poi presenti i preventivi/ordini di vendita, visto che sono indicati nel README dei due moduli.

Line 3 in abdc072

Questo commit è relativo al file it.po del modulo cespiti.

@matteoopenf
Copy link
Contributor

Ma il report che eredita nelle View è del modulo sale non l'altro. Non mi sembra corretta quindi la pr .
l10n-italy/l10n_it_fiscalcode_sale/views/sale_order_report.xml

Non vedo il problema, visto chesale è il modulo base e quindi dipendenza dell'applicazione Vendite (sale_management).

È il comportamento attuale che non è corretto. Installando l10n_it_fiscalcode_sale o l10n_it_account_stamp_sale mi aspetto che nell'interfaccia siano poi presenti i preventivi/ordini di vendita, visto che sono indicati nel README dei due moduli.

Line 3 in abdc072

Questo commit è relativo al file it.po del modulo cespiti.

devo aver sbagliato a linkare il commit chiedo venia.

In ogni caso solitamente se si eredita qualcosa da un modulo esso si mette come dipendenza, perchè così si è certi dalla sua installazione, in alcuni casi non mettere la dipendenza causa errori strani o comportamenti inattesi.

@primes2h
Copy link
Contributor Author

Ma il report che eredita nelle View è del modulo sale non l'altro. Non mi sembra corretta quindi la pr .
l10n-italy/l10n_it_fiscalcode_sale/views/sale_order_report.xml

Non vedo il problema, visto chesale è il modulo base e quindi dipendenza dell'applicazione Vendite (sale_management).
È il comportamento attuale che non è corretto. Installando l10n_it_fiscalcode_sale o l10n_it_account_stamp_sale mi aspetto che nell'interfaccia siano poi presenti i preventivi/ordini di vendita, visto che sono indicati nel README dei due moduli.

Line 3 in abdc072

Questo commit è relativo al file it.po del modulo cespiti.

devo aver sbagliato a linkare il commit chiedo venia.

In ogni caso solitamente se si eredita qualcosa da un modulo esso si mette come dipendenza, perchè così si è certi dalla sua installazione, in alcuni casi non mettere la dipendenza causa errori strani o comportamenti inattesi.

In generale sono perfettamente d'accordo con te.
In questo caso caso la certezza dell'installazione è abbastanza palese visto che senza sale non funzionerebbe nemmeno sale_management.

@matteoopenf
Copy link
Contributor

Ma il report che eredita nelle View è del modulo sale non l'altro. Non mi sembra corretta quindi la pr .
l10n-italy/l10n_it_fiscalcode_sale/views/sale_order_report.xml

Non vedo il problema, visto chesale è il modulo base e quindi dipendenza dell'applicazione Vendite (sale_management).
È il comportamento attuale che non è corretto. Installando l10n_it_fiscalcode_sale o l10n_it_account_stamp_sale mi aspetto che nell'interfaccia siano poi presenti i preventivi/ordini di vendita, visto che sono indicati nel README dei due moduli.

Line 3 in abdc072

Questo commit è relativo al file it.po del modulo cespiti.

devo aver sbagliato a linkare il commit chiedo venia.
In ogni caso solitamente se si eredita qualcosa da un modulo esso si mette come dipendenza, perchè così si è certi dalla sua installazione, in alcuni casi non mettere la dipendenza causa errori strani o comportamenti inattesi.

In generale sono perfettamente d'accordo con te. In questo caso caso la certezza dell'installazione è abbastanza palese visto che senza sale non funzionerebbe nemmeno sale_management.

Ok che l'altro modulo la garantisce.

Non capisco tuttavia il vantaggio di togliere una dipendenza del genere.

@matteoopenf
Copy link
Contributor

anche perchè con il cambio di pricing di Odoo non si presenta più il problema di upsell per app in più installate e cose del genere

@primes2h
Copy link
Contributor Author

primes2h commented Dec 29, 2022

In ogni caso solitamente se si eredita qualcosa da un modulo esso si mette come dipendenza, perchè così si è certi dalla sua installazione, in alcuni casi non mettere la dipendenza causa errori strani o comportamenti inattesi.

In generale sono perfettamente d'accordo con te. In questo caso caso la certezza dell'installazione è abbastanza palese visto che senza sale non funzionerebbe nemmeno sale_management.

Ok che l'altro modulo la garantisce.

Anche il modulo Odoo sale_margin fa la stessa cosa.
Dipende da sale_management ma mi pare che erediti solo le viste di sale.

https://github.com/OCA/OCB/blob/7790dc120d2b4cdd5309a4a9a576121fcc0fe400/addons/sale_margin/__manifest__.py#L15

https://github.com/OCA/OCB/blob/16.0/addons/sale_margin/views/sale_order_views.xml

Non capisco tuttavia il vantaggio di togliere una dipendenza del genere.

Non è questione di vantaggi, è che l'installazione del modulo non porta a quello che è indicato nel suo README. Come ho scritto sopra:

Installando l10n_it_fiscalcode_sale o l10n_it_account_stamp_sale mi aspetto che nell'interfaccia siano poi presenti i preventivi/ordini di vendita, visto che sono indicati nel README dei due moduli.

Inoltre come effetto collaterale viene installato anche nei casi in cui non è previsto che lo sia (vedi descrizione della PR).

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.

LGTM

@TheMule71
Copy link
Contributor

/ocabot merge patch

@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-3106-by-TheMule71-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit dd87012 into OCA:12.0 Jan 12, 2023
@OCA-git-bot
Copy link
Contributor

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

@primes2h primes2h deleted the 12.0-fix-l10n_it_fiscalcode_sale_dependency branch January 12, 2023 19:38
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.

6 participants