Skip to content

[14.0] revert #387 - #466

Merged
OCA-git-bot merged 2 commits into
OCA:14.0from
acsone:14.0-revert-387
Oct 18, 2022
Merged

[14.0] revert #387#466
OCA-git-bot merged 2 commits into
OCA:14.0from
acsone:14.0-revert-387

Conversation

@sbidoul

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor

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

@sbidoul
sbidoul requested a review from guewenOctober 18, 2022 11:05
@sbidoul

Copy link
Copy Markdown
MemberAuthor

@acsonefhoacsonefho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That make sense

@guewenguewen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh thanks!

@pedrobaezapedrobaeza added this to the 14.0 milestone Oct 18, 2022
@pedrobaeza

Copy link
Copy Markdown
Member

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

@sbidoul

Copy link
Copy Markdown
MemberAuthor

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

Which problem exactly?

@pedrobaeza

Copy link
Copy Markdown
Member

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

@sbidoul

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

And why a separate module?

@sbidoul

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
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

Copy link
Copy Markdown
Contributor

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

Sign up for freeto 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

@sbidoul@OCA-git-bot@pedrobaeza@guewen@rousseldenis@acsonefho