Skip to content

Conversation

@sbidoul
Copy link
Member

@sbidoul sbidoul commented Oct 18, 2022

Revert #387 which should not have been merged, and update test to illustrate the proper way to pass with_company to jobs.

@OCA-git-bot
Copy link
Contributor

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

@sbidoul sbidoul requested a review from guewen October 18, 2022 11:05
@sbidoul
Copy link
Member Author

sbidoul commented Oct 18, 2022

@acsonefho @Cedric-Pigeon

Copy link
Contributor

@acsonefho acsonefho left a comment

Choose a reason for hiding this comment

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

That make sense

Copy link
Member

@guewen guewen left a comment

Choose a reason for hiding this comment

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

Oh thanks!

@pedrobaeza pedrobaeza added this to the 14.0 milestone Oct 18, 2022
@pedrobaeza
Copy link
Member

But the problem about having several allowed companies in the context still remains, isn't it?

@sbidoul
Copy link
Member Author

sbidoul commented Oct 18, 2022

But the problem about having several allowed companies in the context still remains, isn't it?

Which problem exactly?

@pedrobaeza
Copy link
Member

That the method that is executed in the job doesn't have the expected main company / allowed companies.

@sbidoul
Copy link
Member Author

sbidoul commented Oct 18, 2022

Doing env[model].with_company(company).with_delay().method(), together with #462 (which will become the default in 16), is the proper way to do it and will correctly propagate allowed_company_ids to the jobs.

@pedrobaeza
Copy link
Member

And why a separate module?

@sbidoul
Copy link
Member Author

sbidoul commented Oct 18, 2022

And why a separate module?

For backward compatibility. In 16.0 it becomes the default. See #432 (comment) and the many comments before and in linked issues and PRs for context :)

@pedrobaeza
Copy link
Member

Well, OK, let's have such module in mind in multi-company environments...

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-466-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f1d3b10 into OCA:14.0 Oct 18, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 419ada0. 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.

6 participants