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

[FIX] mail: check access rights when automatically adding followers #31292

Draft
wants to merge 2 commits into
base: saas-12.1
from

Conversation

Projects
None yet
4 participants
@tbe-odoo
Copy link
Contributor

tbe-odoo commented Feb 20, 2019

  • If a partner doesn't have read access to the records we are trying to
    automatically add him as a follower, we don't.

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[FIX] mail: check access rights when automatically adding followers
- If a partner doesn't have read access to the records we are trying to
  automatically add him as a follower, we don't.

@robodoo robodoo added the seen 🙂 label Feb 20, 2019

@tbe-odoo tbe-odoo requested review from tde-banana-odoo and odony Feb 20, 2019

@C3POdoo C3POdoo added the RD label Feb 20, 2019

sudoed_records.check_access_rule('read')
except exceptions.AccessError:
return False
return True

This comment has been minimized.

@odony

odony Feb 20, 2019

Contributor

Shouldn't we reverse the return logic?

  • return True right after checking access, because as soon as there is 1 user with access it's enough
  • just pass in the except: block, to allow testing next users for that partner
  • return False at the end of the method, if no user worked

This comment has been minimized.

@tbe-odoo

tbe-odoo Feb 21, 2019

Author Contributor

Yup, the code was way too strict this way.

@tbe-odoo

This comment has been minimized.

Copy link
Contributor Author

tbe-odoo commented Feb 22, 2019

Hello @tde-banana-odoo , could you take a look at this PR and tell us what you think about the change it will bring?

Do you agree about it?

Thanks!

@tbe-odoo

This comment has been minimized.

Copy link
Contributor Author

tbe-odoo commented Mar 12, 2019

Hello @tde-banana-odoo , could you take a look at this PR and tell us what you think about the change it will bring?

Do you agree about it?

Thanks!

@tde-banana-odoo , could you take a look at this PR please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.