OCA / multi-company

GNU Affero General Public License v3.0
100 stars 281 forks source link

[14.0] [FIX] account_invoice_inter_company: Remove self company from eligible companies in `_find_company_from_invoice_partner` #526

Closed renda-dev closed 10 months ago

renda-dev commented 10 months ago

In some countries (for example, Italy) they need to create invoices to themselves which should not be counted as "inter company invoice"

This PR address that issue, also because, from my prospective, it does not make a lot of sense to create an invoice to yourself and also create the related inter-company invoice as well.

francesco-ooops commented 10 months ago

@OCA/intercompany-maintainers is this ok for you?

francesco-ooops commented 10 months ago

@pedrobaeza merge on existing reviews?

pedrobaeza commented 10 months ago

/ocabot merge patch

OCA-git-bot commented 10 months ago

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

OCA-git-bot commented 10 months ago

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-526-by-pedrobaeza-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.

pedrobaeza commented 10 months ago

Ouch, this is also affected by https://github.com/OCA/oca-addons-repo-template/pull/214

renda-dev commented 10 months ago

Would it be possible to be merged as minor?

I kinda need this change in another PR (https://github.com/OCA/l10n-italy/pull/3659) 👀

pedrobaeza commented 10 months ago

No, it's not possible. If you are in a hurry, make another PR applying manually on the file the patch proposed in the template.

francesco-ooops commented 10 months ago

@pedrobaeza for your knowledge, any idea why patched versions are not updated in other repos?

pedrobaeza commented 10 months ago

I don't get your question.

francesco-ooops commented 10 months ago

@renda-dev can you explain the issue you're facing as you're the technical one?

renda-dev commented 10 months ago

@renda-dev can you explain the issue you're facing as you're the technical one?

Sure! Please correct me if I’m mistaken tho @pedrobaeza

Substantially in other OCA's repos, the dependencies for testing are being fetched in Looking in indexes: https://wheelhouse.odoo-community.org/oca-simple-and-pypi (quick ref in this PR) and in my recent experience it considerate only the latest minor (or major) version of the module, ignoring pre-releases (patches).

These small changes are pretty important for us because integrating account_invoice_inter_company in l10n-italy would break a couple of modules and tests will fail (for the same reason explained in this PR's description)

The small term solution to this problem would be to manually add the pre-release version to test-requirements.txt (like here), but that's obviously wrong.

Please, let me know if I'm clear enough and if there are any other solutions for bringing minor module versions to other OCA repos, thanks!

Quick note: We've had experience with this issue where there was no bumping at all, maybe even patches are enough to update the module in pypi :)

pedrobaeza commented 10 months ago

But why don't you simply fix this repository and then we can merge?

https://x.com/PedroMBaeza/status/1717834741943009487?s=20

aleuffre commented 10 months ago

I think what @renda-dev is pointing out is a separate problem from the pre-commit one.

If you merge without increasing the major or minor version of the module, the pypi package for the module is not updated and the change is not available for testing on other repositories, therefore blocking any other PR that depends on this one.

pedrobaeza commented 10 months ago

OK, but I think this is not a problem here. I tried to merge with patch option, which also put the same mechanism and it's not a problem. There's no need of bumping the minor version AFAIK. The only problem is if we use nobump option.

renda-dev commented 10 months ago

OK, but I think this is not a problem here. I tried to merge with patch option, which also put the same mechanism and it's not a problem. There's no need of bumping the minor version AFAIK. The only problem is if we use nobump option.

Thanks a lot for clarifying, I'm totally fine with patch then :)

pedrobaeza commented 10 months ago

/ocabot merge patch

OCA-git-bot commented 10 months ago

This PR looks fantastic, let's merge it! Prepared branch 14.0-ocabot-merge-pr-526-by-pedrobaeza-bump-patch, awaiting test results.

OCA-git-bot commented 10 months ago

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

francesco-ooops commented 10 months ago

@renda-dev can you check if it can be fw-ported?

renda-dev commented 10 months ago

Yes, there shouldn't be any problem at all @francesco-ooops