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

[14.0][FIX] l10n_it_fatturapa_in multicompany wrong company #2352

Merged

Conversation

TheMule71
Copy link
Contributor

Descrizione del problema o della funzionalità:
Nell'importare una fattura il sistema tenta di usare la company di default dell'utente corrente e non la company selezionata a livello di interfaccia.
Comportamento attuale prima di questa PR:
L'import fallisce perché non trova un registro di tipo 'sale' per la company di default dell'utente.
Comportamento desiderato dopo questa PR:
La fattura viene importata correttamente.

N.B. Idealmente sarebbe da scrivere un test.

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

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.

@TheMule71
Copy link
Contributor Author

Revisione del codice.
Puoi correggere anche le occorrenze nei test? Ad esempio https://github.com/TheMule71/l10n-italy/blob/cba3db8e157fdbf4d977bbdff9f95c666a56c977/l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py#L210

Ci avevo guardato, ho deciso di non farlo perché lì prendiamo una company che funziona, nei test non è che dipendiamo da una scelta fatta dall'utente...

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 2, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 2, 2021
@SimoRubi
Copy link
Member

SimoRubi commented Jul 9, 2021

Revisione del codice.
Puoi correggere anche le occorrenze nei test? Ad esempio https://github.com/TheMule71/l10n-italy/blob/cba3db8e157fdbf4d977bbdff9f95c666a56c977/l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py#L210

Ci avevo guardato, ho deciso di non farlo perché lì prendiamo una company che funziona, nei test non è che dipendiamo da una scelta fatta dall'utente...

Se come scrivevi in #2043 (comment) avere i valori nel contesto è la norma, allora penso sia da usare self.env.company anche nei test così da farli avvicinare il più possibile al comportamento del codice nella realtà.

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 9, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 9, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 9, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 9, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 9, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 17, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 20, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 30, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jul 30, 2021
@TheMule71 TheMule71 force-pushed the 14.0-fix_l10n_it_fatturapa_in_multicompany branch from cba3db8 to 3c31d03 Compare July 30, 2021 16:18
@TheMule71 TheMule71 requested a review from SimoRubi July 30, 2021 17:01
@TheMule71
Copy link
Contributor Author

Revisione del codice.
Puoi correggere anche le occorrenze nei test? Ad esempio https://github.com/TheMule71/l10n-italy/blob/cba3db8e157fdbf4d977bbdff9f95c666a56c977/l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py#L210

Ci avevo guardato, ho deciso di non farlo perché lì prendiamo una company che funziona, nei test non è che dipendiamo da una scelta fatta dall'utente...

Se come scrivevi in #2043 (comment) avere i valori nel contesto è la norma, allora penso sia da usare self.env.company anche nei test così da farli avvicinare il più possibile al comportamento del codice nella realtà.

Cambiati anche i test.

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Aug 6, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Aug 20, 2021
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.

Revisione del codice, per me è ok.

@CiroBoxHub
Copy link
Contributor

👍🏼

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Sep 3, 2021
@TheMule71
Copy link
Contributor Author

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-2352-by-TheMule71-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 17, 2021
Signed-off-by TheMule71
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 14.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 14.0-ocabot-merge-pr-2352-by-TheMule71-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 17, 2021
Signed-off-by TheMule71
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 14.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 14.0-ocabot-merge-pr-2352-by-TheMule71-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 17, 2021
Signed-off-by TheMule71
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 14.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 14.0-ocabot-merge-pr-2352-by-TheMule71-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f1713b3 into OCA:14.0 Sep 17, 2021
@OCA-git-bot
Copy link
Contributor

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

@TheMule71 TheMule71 deleted the 14.0-fix_l10n_it_fatturapa_in_multicompany branch September 17, 2021 13:50
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Sep 17, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Oct 1, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Oct 8, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Oct 15, 2021
TheMule71 added a commit to TheMule71/l10n-italy that referenced this pull request Jan 21, 2022
TheMule71 added a commit to TheMule71/l10n-italy that referenced this pull request Jan 21, 2022
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jun 29, 2022
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.

4 participants