Skip to content

Conversation

@acsonefho
Copy link
Contributor

Forward port of #365
Use the current company to trigger the job (+ add related tests)

Use the current company to trigger the job (+ add related tests)

[14.0][FIX] queu_job: allowed_company_ids => use with_company(...)
Fill allowed_company_ids from context with the job's company instead of every allowed companies of the user.
Because most of the time, a job is related to only one company. And adding every allowed companies of the user into the context may load some unexpected records (during search for example). Because standards ir.rule use ['|',('company_id','=',False),('company_id', 'in', company_ids)] and this 'company_ids' is filled with every allowed companies from the context.
@acsonefho
Copy link
Contributor Author

@rousseldenis

@guewen
Copy link
Member

guewen commented Nov 6, 2021

Hi, same remarks as on #365. Maybe you could answer on the latter ?

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@dreispt
Copy link
Member

dreispt commented Nov 27, 2021

/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-387-by-dreispt-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 8492f68 into OCA:14.0 Nov 27, 2021
@OCA-git-bot
Copy link
Contributor

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

@sbidoul sbidoul deleted the 14.0-queue_job_current_company branch October 18, 2022 09:02
@sbidoul
Copy link
Member

sbidoul commented Oct 18, 2022

Hm, I'm not sure it was right to merge this. The proper solution is the queue job context propagation which we merged in #432. @guewen what do you think?

@guewen
Copy link
Member

guewen commented Oct 18, 2022

Indeed, it shouldn't have been merged

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.

7 participants