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.

Sign up for GitHub

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

Jump to bottom

[FIX] mail: fix error open starred #98347

Open
wants to merge 1 commit into
base: 15.0
Choose a base branch
from
Open

[FIX] mail: fix error open starred #98347

wants to merge 1 commit into from

Conversation

daonamutc1
Copy link
Contributor

@daonamutc1 daonamutc1 commented Aug 18, 2022 •
edited

  • When a user stars messages from a channel and messages from an unauthorized document.
  • The check_access_rule function does not filter messages on the channel. This function then checks the read permission of the document. And raise an error
  • This commit will remove the messages that have been checked above from the document_related_candidate_ids list
Screen.Recording.2022-08-18.at.13.16.40.mov

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

@robodoo
Copy link
Contributor

robodoo commented Aug 18, 2022

Pull request status dashboard.

@C3POdoo C3POdoo requested review from a team August 18, 2022 07:30
@C3POdoo C3POdoo added the Discuss Discuss, mail, chatter, livechat, attachments label Aug 18, 2022
@daonamutc1 daonamutc1 force-pushed the v15_fix_mail branch 2 times, most recently from 509b7de to 3f37a98 Compare August 18, 2022 08:18
@daonamutc1
Copy link
Contributor Author

@tde-banana-odoo May you review this PR?

@daonamutc1
[FIX] mail: fix starred opening
4f93322 
- When a user stars messages from a channel and messages from an unauthorized document.
- The check_access_rule function does not filter messages on the channel. This function then checks the read permission of the document. And raise an error
- This commit will remove the messages that have been checked above from the document_related_candidate_ids list
@daonamutc1
Copy link
Contributor Author

@seb-odoo Can you review this PR?

@seb-odoo
Copy link
Contributor

Hello, we don't really have time to check for this right now, as we're working to finish the next version.

Also it's not very clear to me what you are trying to fix, and it is not covered by tests, so it's not an obvious approval.

@daonamutc1
Copy link
Contributor Author

daonamutc1 commented Aug 31, 2022 •
edited

Hello, we don't really have time to check for this right now, as we're working to finish the next version.

Also it's not very clear to me what you are trying to fix, and it is not covered by tests, so it's not an obvious approval.

In the video you can see I've starred messages in the history and general channels.
Messages in the history come from a record that the user is not authorized to read. (1)
Then I go to Stared and an error message pops up.

@seb-odoo
Copy link
Contributor

seb-odoo commented Jul 6, 2023

Hello, could you write a test to cover the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Discuss Discuss, mail, chatter, livechat, attachments
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@daonamutc1 @robodoo @seb-odoo @C3POdoo