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

[IMP] mail: avoid broadcasting partner lists and tracking values #162065

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

Conversation

phenix-factory
Copy link
Contributor

@phenix-factory phenix-factory commented Apr 16, 2024

Before this PR, _message_format function always included data that were only
relevant to the logged user. It also leak partner_ids.

This PR condition the needaction_partner_ids, history_partner_ids,
starredPersonas and trackingValues values to a new param.
This allows us to control when those data should be sent to the user.

PR enterprise: https://github.com/odoo/enterprise/pull/61353

@robodoo
Copy link
Contributor

robodoo commented Apr 16, 2024

@C3POdoo C3POdoo added the RD research & development, internal work label Apr 16, 2024
@phenix-factory phenix-factory force-pushed the master-remove-need-action-from-message-format-did branch 12 times, most recently from afb71f5 to 9b4818c Compare April 17, 2024 11:05
@phenix-factory phenix-factory marked this pull request as ready for review April 17, 2024 12:50
@C3POdoo C3POdoo requested review from a team April 17, 2024 12:55
@tde-banana-odoo
Copy link
Contributor

No task, no explanation ? Don't mark as ready when we cannot review.

@phenix-factory phenix-factory force-pushed the master-remove-need-action-from-message-format-did branch from 9b4818c to b9e7b61 Compare April 17, 2024 13:21
@phenix-factory phenix-factory changed the title [IMP] mail: avoid broadcasting partner lists [IMP] mail: avoid broadcasting partner lists and tracking values Apr 17, 2024
@phenix-factory
Copy link
Contributor Author

@tde-banana-odoo I update the commit and the PR description to provide some context.

Copy link
Contributor

@tde-banana-odoo tde-banana-odoo left a comment

Choose a reason for hiding this comment

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

Thanks ! Zboing !

addons/mail/models/mail_message.py Outdated Show resolved Hide resolved
addons/test_mail/tests/test_mail_message.py Outdated Show resolved Hide resolved
@phenix-factory phenix-factory force-pushed the master-remove-need-action-from-message-format-did branch 5 times, most recently from bc5f485 to 41db430 Compare April 18, 2024 08:13
@phenix-factory phenix-factory force-pushed the master-remove-need-action-from-message-format-did branch from 41db430 to 893f6a3 Compare April 18, 2024 08:49
@phenix-factory phenix-factory force-pushed the master-remove-need-action-from-message-format-did branch 6 times, most recently from a714fcc to 81cad5d Compare April 23, 2024 08:36
Copy link
Contributor

@alexkuhn alexkuhn left a comment

Choose a reason for hiding this comment

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

PR looks more code cleaning than anything.
I don't mind it, but as it is it introduces some disparity between _message_format_personalize and _message_format(for_current_user).

It'd be nice to match the wording, and also they have similar concerns so would be also nice to improve locality by using similar LOCs. For example, _message_format_personalize code could be moved into _message_format and there's a parameter to personalise for given partners.

phenix-factory added a commit to odoo-dev/odoo that referenced this pull request Apr 23, 2024
Before this PR, _message_format send all of the user that starred a message to
the JS code.
Since odoo#162065, this behaviour is not necessary
anymore and a boolean can be used.
phenix-factory added a commit to odoo-dev/odoo that referenced this pull request Apr 23, 2024
Before this PR, _message_format sent all of the users that starred a message to
the JS code.
Since odoo#162065, this behavior is not necessary
anymore and a boolean can be used.
phenix-factory added a commit to odoo-dev/odoo that referenced this pull request Apr 23, 2024
Before this PR, _message_format send all of the user that had a "need action" on a message to the JS code.
Since odoo#162065, this behaviour is not necessary
anymore and a boolean can be used.
phenix-factory added a commit to odoo-dev/odoo that referenced this pull request Apr 23, 2024
Before this PR, `_message_format` sends all of the users that had a "need action" on a message to the JS code.
Since odoo#162065, this behavior is not necessary
anymore and a boolean can be used.
phenix-factory added a commit to odoo-dev/odoo that referenced this pull request Apr 23, 2024
Before this PR, `_message_format` sent all of the users that starred a message to
the JS code.
Since odoo#162065, this behavior is not necessary
anymore and a boolean can be used.
@phenix-factory phenix-factory force-pushed the master-remove-need-action-from-message-format-did branch 6 times, most recently from 432d86c to 7949d3e Compare April 24, 2024 09:47
Before this PR, `_message_format` function always included data that were only
relevant to the logged user. It also `leak partner_ids`.

This PR condition the `needaction_partner_ids`, `history_partner_ids`,
`starredPersonas` and `trackingValues` values to a new param.
This allows us to control when those data should be sent to the user.

PR enterprise: odoo/enterprise#61353
@phenix-factory phenix-factory force-pushed the master-remove-need-action-from-message-format-did branch from 7949d3e to 44f86cf Compare April 29, 2024 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants