Previously to this commit needaction used by JS was not taking into account
read notifications. It means notably Inbox counter could be higher than
expected as taking into account read messages while Inbox displays only
unread messages. This commit fixes that by checking notification state when
giving needaction_partner_ids to message_format result.
This fix has a small impact on performances as message_format is used in
notification process for chat or push. A better implementation of message
format and its postprocess could probably lessen this fix impact. As we
target stable version we avoid rewriting other part of the code. Optimization
will be done in development version.
Related to task ID 1841243. This is a manual forward port of commit 69714fbeee
done in saas-11.2.
Previously to this commit needaction used by JS was not taking into account
read notifications. It means notably Inbox counter could be higher than
expected as taking into account read messages while Inbox displays only
unread messages. This commit fixes that by checking notification state when
giving needaction_partner_ids to message_format result.
This fix has a small impact on performances as message_format is used in
notification process for chat or push. A better implementation of message
format and its postprocess could probably lessen this fix impact. As we
target stable version we avoid rewriting other part of the code. Optimization
will be done in development version.
Related to task ID 1841243. This is a manual forward-port of commit 69714fbeee
done in saas-11.2.
mail.thread mixing class defines several methods used in notification process
that are not specific to mail.thread enabled models. It means some models
could override it while some other models do not even have a direct access
to that method.
When calling those methods we have to manually check for method being
available on a record or call a default implementation from mail.thread
even if the model does not inherit from the mixin.
In this commit we propose to hide this check in generic wrappers located
in mail.thread class. It checks for method existence on record and call
the model-specific implementation or falls back on standard behavior. It
allows to be sure the right method is called.
Functionally behavior should stay globally the same. However when updating
some addons code has been updated. Previous code could lead to records not
having correct alias which should not be the case anymore. This commit is
linked to task ID 47934.
Purpose is to lessen query count by avoiding to browse and/or fetch data when
not necessary. This commit does two main things
* first one is to avoid checking for existing statistics in mass_mailing
where only checking existing of a mass_mailing on a mail to send is
sufficient. It allows to avoid fetching data from statistics table;
* second one is to rewrite part of the post_process code called after sending
emails. Using list comprehension and mailing_id existence on mail_mail
allows to save several queries on standard post flows;
Most tests benefit from this code update. On overall community runbot about
3K queries are saved with those small optimizations.
mail_push module in enterprise was adding too much extra queries. mail_push
being auto-install in enterprise mail performance tests were done with its
overhead taken into account. Commit done in enterprise cleaned a bit the code
to lessen mail_push impact allowing to lessen some top counters. Community
runbot counters are now nearer enterprise-based counters.
This commit is related to enterprise commit https://github.com/odoo/enterprise/commit/0a3ba658f18e03fd76caba86cba7be32d047f0a9 .
Well, it seems we gain a lot of query when avoiding to format a message we
actually don't have to send on any bus. Cheers.
Formatting message for bus notification is quite costly as it has to fetch
data from message, partner, attachments, subtypes, ... meaning a lot of
queries. With this commit we avoid calling that method when not necessary.
It allows a gain of queries in simple cases when no-one has to be notified
as shown in the performance counters. On overall community runbot about 9K
queries are saved which is not bad for a 2-lines commit.
In some cases when subscribing followers code may ask to force subtypes
of the subscription. It means existing followers should have only asked
subtypes independently from existing subtypes. Previously it was done
by unlinking and creating new followers for same partner or channel.
This commit proposes to instead update missing subtypes and remove extra
subtypes.
It has a small impact in real life as forcing subtype is not done often.
Some tests are improved thanks to this commit. This commit is linked to
task ID 1839043. Closes#24417.
Commit 07ae0ccc4e set activities as not followed by default on records.
However this has an impact on performance test that should not happen.
Indeed performance tests should be based on technical decisions, not business
decisions. We therefore set activities as followed by default in some advanced
performance tests to always have results that can be compared.
This commit proposes to limit cache invalidation at some cases that will
trigger some behavior change when dealing with mail messages :
* creating messages linked to a document;
* update model or res_id of a message;
* updating notifications, as notified people could change some computed
fields on the record;
This commit also invalidates only mail-related fields as updating messages
should not invalidate other things than some computed fields linked to
mail.
Cache invalidation when subscribing people has been added at e6f038a821.
Indeed subscribing partners to a record may lead to an access right update
as some of them are based on followers. This is why a cache invalidation
is necessary to avoid access rights issues.
However subscribing people to a record should change their rights only on
the records involved in the subscription mechanism. We can therefore give
ids to the cache invalidation to limit to updated records.
Cache invalidation is also limited when writing on followers if writing
on model, res_id or partner_id fields. Indeed changing subtypes or
channel of a subscription should have no impact on cache and access rights.
Cache invalidation done manually in _message_subscribe is not necessary
as subscription create or update mail.followers records since f9c210923d.
Create and write of mail.followers records already ask for cache invalidation.
It is therefore not necessary to invalidate cache twice.
This commit allows to save a few queries on some tests, notably about
activities that deal with subscription and messages. As cache is now kept
it is not necessary to refetch some data, leading to a few query gain. We
gain about 1K queries on com runbot.
A side effect of limiting cache invalidation is that some unit tests require
a manual cache invalidation to have up to date results. Indeed record not
being up to date in cache was hidden by the invalidation we just removed.
When notifying partners or channels of a message, a cache invalidation is
currently done if the message has a parent. This invalidation has been done
when migrating the mail module at the new API at 4b122ad41d.
In that time notifying people of a message lead to the creation of
notifications of the parent message, if any. It was due to the chatter
being threaded and therefore displaying message with their header message.
Adding notifications for the parent was necessary to avoid access rights
issues when fetching parent message data. Indeed as being notified is one
of the rule to see a message record adding notifications was done. A cache
invalidation has therefore been added to clean the message cache and ensure
everything was fine.
Commit 88b8cd0587 changed the way notifications are modeled in Odoo.
Notification on parent message was removed and access rights changed. Threaded
mode for Chatter has also been removed. However cache invalidation has been
kept probably by fear of removing it.
It does not seem to have any viable reason to invalidate cache when a message
has a parent. Posting a message does not push other messages in users's Inbox
meaning there should not be any issue with the cache preventing to see
messages.
Removing this cache invalidation allow to gain queries in performance tests.
It has an impact on each process involving message creation which is quite
common in Odoo.
Query count for some mail tests is not always the same for a given test.
This leads notably to runbot being randomly red although there is no real
error. This commit lessen the issue of having red runbot or wrongly
incremented tests by using the recently-introduced margin parameter on
performance tests.
Some work is still ongoing to find why some tests are not completely
reproducible. In the meantime let us have some warnings instead of errors.
Manual forward-port and improvement of 81a0852077.
This commit refactors mail.thread message_auto_subscribe method. Purpose
of this method is to automatically subscribe people to documents in several
scenarios
* followers from an umbrella record to sub-records, like followers of project
added to tasks;
* responsible of documents, in addition to sending notifications to newly
added responsibles;
* some specific use case depending on some applications like managers
for expense reports;
New auto subscription now uses the recently introduced method optimized to
compute new or to-update followers. It is also rewritten so that overriding
it is easier. Indeed simply overriding _message_auto_subscribe_followers is
now sufficient to add new followers to a record, depending on some updated
values. It is also now possible to specify a template to use for some
specific notification, if requested.
Fetching auto-subscription data in subtype is also optimized. It is now done
in a single SQL query. All data related to parent (like project) and child
(like task) as well as default and internal-only data is prepared in a single
query and given back to auto subscription process. This allows to gain
queries in auto subscription based on subtype like project / task use case.
This commit has a great impact on performance counters. Complex use cases
involving tracking and responsible notifications gain about 30 queries which
is about 15% of their total query count.
This commit refactors mail.thread message_subscribe method. It is done for two
purposes. First one is to optimize performance by using the new followers
computation methods introduced recently. Second purpose is to clean the API
of message_subscribe to make it simpler to use.
This commit splits message_subscribe in two main parts :
* _message_subscribe is a private method calling the follower new
subscription methods and updating the record set;
* message_subscribe is a public wrapper on _message_subscribe that adds
access rights checks;
Simplification comes by removing message_subscribe_users that was a
shortcut to message_subscribe. Having a method to subscribe partners and
channels is sufficient as we would like to avoid bloating the public API
of mail.thread. A force parameter is also removed from message_subscribe
as this implementation detail can be induced in the computation.
Various addons using the removed methods are updated in order to use the new
subscription API. They have the same functional behavior.
This commit has a great impact when subscribing several followers. In a more
general way all code using message_post is also optimized as posting a message
generally implies subscribing followers. It also improves activity use as
posting a message and subscribing new followers are common process in
activities.
Override of create in mail.thread now uses the recently introduced
_add_default_followers to add the record's creator in the followers.
It allows to save one query when creating new records. It means for simple
models we approach the bottom limit in term of generated queries which is
quite a good news.
As explained in previous commit we can use xmlid_to_res_id instead of ref()
which leads to some query gain. This commit update some other less important
places: when setting activities as done and in the composer. This lead to
a gain in tests involving activities and composer, including tracking
triggering a template. On community runbot this leads to a gain of about 1K
queries.
As explained in previous commit we can use xmlid_to_res_id instead of ref()
which leads to some query gain. We gain 1 query on most post due notably to
notify and logging is still improved. This leads to a gain of about 4K queries
on community runbot.
Deleting comment / note / activity subtypes make the whole system quite
impossible to use. Indeed the whole communication mechanism is based on
those subtypes that are hardcoded.
There are some use of env.ref() about subtypes to try to avoid having those
removed. As there is some work to make some xml ids impossible to delete we
can already update some code to use xmlid_to_res_id instead of env.ref(). This
leads to some gain in query count as there is no exists() that is performed.
Gain: 1 query on most tests implying post, because message_format is called
by bus notification. About 1.5K queries on community runbot
Still this "sometimes one query more" in activity-related tests. Let us avoid
false red occurrences of runbot by adding one query on sometimes-failing tests.
Purpose of performance test is to have an idea of query count. Adding one query
is not harmful.
This commit adds tools methods related to activities in the mail.activity
mixin. It gives to models inheriting from the activity mixin an easy-to-use
API to schedule, unlink or mark activities as done. Purpose of those methods
is to avoid having people manually managing activities in the code to hide
the technical details of the activities model, notably access rights
or activity types.
A field is added on activity model to indicate they have been generated
automatically. This way when rescheduling or unlinking based on some
specific activity types we do not change user-created activities.
Those methods include scheduling activities, changing their dates, marking
them as done or unlinking them. Future commits will use those methods
in various addons to automatically generate activities based on workflow
we want to implement.
This commit also adds tests for the newly added code. Future commits should
probably have a look at activity security and add some tests cases to check
it is correctly taken into account. It is considered a bit out of scope for
this task.
Thanks to @jem-odoo for its in-depth review of this commit. Well thanks for
other commits also.
Use option recently introduced to use a minimal context when rendering
some mail templates such as
* notification template used to decorate messages sent to recipient
when using email-based notifications;
* user assignation template used when people are assigned on a record
and a notification is sent to inbox or email;
* message_post_with_template now uses minimal context meaning calls to
that method should be sure evaluation context is correctly set in
the given values;
This optimization allows to save some queries every time one of those
template is used, meaning all tests gain between 1 to 4 queries depending
on their complexity.
This commit is related to task ID 1817951. Closes#23291 .
Due to unknown reasons sometimes a performance test takes one query more to
run than expected. Let us therefore avoid red runbots by increasing the
query count by 1.
Commit 42659dc70c recently done in saas-11.2 added performance test cases
in test_mail module allowing to cover more use cases. Since release of
saas-11.2 some optimizations already landed in master pre-saas-11.3 and
some features have been refactored.
This implies updating the performance counters. This is done in a separate
commit to distinguish previous-counters from current counters instead of
loosing it in a diff.
Most message-post related tests have been improved by some queries as
message creation has been speeded up at f727e9d9b6. Some other commits
like 9de5d41754 also slightly improved performances. Activities-related
performance have been improved at e899812315. Tests case involving
assignation notification perform worst than in saas-11.1 . It is linked to
the feature using more advanced email and templates as well as working for
Inbox too which is not the case in saas-11.2. This improvement has been done
at e9734f4975 .
This commit is related to task 1824965. Thanks to @KangOl for being
nice with people trying to make its forward-port more difficult.
This commit is a manual forward-port of saas-11.2's 42659dc70c .
In test_mail there are performance tests involving several followers and give
counters for some heavy real-life-like use cases. Purpose of this commit is to
have more basic performance tests for main mail features, like simple post,
simple subscription of one follower. It allows to have an idea of the basic
cost of various features.
Adding use cases for message_post includes
* posting without followers (aka, no notification)
* posting with a ping (by email or by inbox)
* logging a note with optimized method _message_log or with message_post
Adding use cases for activities includes setting an activity as done. This
action posts a message and is therefore interesting to evaluate.
Adding use cases for subscription includes
* adding and re-adding one follower, with default or specified subtypes
* updating responsible field triggering a simple tracking and an assignation
email or notification (not completely supported in saas 11.2 meaning this
counter will increase when forward-ported)
* note that some subscription tests have already been added at a01933c9b4
Finally some heavier tests are added for assignation and tracking based
on QWeb view.
This commit is related to task ID 1824965 . This one is an ongoing task
and several commits may be linked to that task.
This commit is a manual forward- port of saas-11.2's a01933c9b4 .
Purpose of this commit is to test existing behavior of followers subscription
as well as adding some query count tests about message_subscribe method. As
followers subscription technical implementation may change in a near future
in order to be optimized it is a good idea to have current behavior and query
count validated.
This commit adds
* test for followers update using message_subscribe API. Tests are added
with forced and unforced subtypes to ensure behavior will not change or
will be changed while knowing it;
* performance test about message_subscribe, about adding new followers,
and subscribing already-existing followers in force mode or not;
Query count performance test shows that message_subscribe could probably
be improved. It does several things that require queries (check of access
rights, check for existing subscriptions, ensure customers have no access
to internal subtypes, ...). However we feel it could be improved in order
to speed-up the subscription of followers.
This is linked to task 49265; it is only the first steps of it.
In test_mail there are performance tests involving several followers and give
counters for some heavy real-life-like use cases. Purpose of this commit is to
have more basic performance tests for main mail features, like simple post,
simple subscription of one follower. It allows to have an idea of the basic
cost of various features.
Adding use cases for message_post includes
* posting without followers (aka, no notification)
* posting with a ping (by email or by inbox)
* logging a note with optimized method _message_log or with message_post
Adding use cases for activities includes setting an activity as done. This
action posts a message and is therefore interesting to evaluate.
Adding use cases for subscription includes
* adding and re-adding one follower, with default or specified subtypes
* updating responsible field triggering a simple tracking and an assignation
email or notification (not completely supported in saas 11.2 meaning this
counter will increase when forward-ported)
* note that some subscription tests have already been added at a01933c9b4
Finally some heavier tests are added for assignation and tracking based
on QWeb view.
This commit is related to task ID 1824965 . This one is an ongoing task
and several commits may be linked to that task.
Purpose of this commit is to test existing behavior of followers subscription
as well as adding some query count tests about message_subscribe method. As
followers subscription technical implementation may change in a near future
in order to be optimized it is a good idea to have current behavior and query
count validated.
This commit adds
* test for followers update using message_subscribe API. Tests are added
with forced and unforced subtypes to ensure behavior will not change or
will be changed while knowing it;
* performance test about message_subscribe, about adding new followers,
and subscribing already-existing followers in force mode or not;
Query count performance test shows that message_subscribe could probably
be improved. It does several things that require queries (check of access
rights, check for existing subscriptions, ensure customers have no access
to internal subtypes, ...). However we feel it could be improved in order
to speed-up the subscription of followers.
This is linked to task 49265; it is only the first steps of it.
Currently the rating value coming from the rating application is stored
on the message it belongs to. However storing it is not necessary as
there is no direct search using it. Having a computed field is sufficient
for all use cases we currently have in Odoo.
Removing the store allow to gain queries. Indeed the field is not
computed anymore after each message creation meaning we save queries
by not having to check existing ratings. It allows to gain a lot of
computation as mail.message is a critical model.
On the whole community runbot when installing all modules this leads to
a gain of more than 14K queries on 585K which means 2.4% of performances
increase. Considering the code size of this optimization this is quite an
interesting result.
Looking at test_mail performance tests we gain several queries (2/3) for
each new message which is coherent with the model change.
Finally it allows to lessen the performance difference between tests done
with test mail only and tests done with other modules already installed.
This is especially simple mail thread-enable records.
Related to task ID 51523. #Closes #23294. Done with blessing of @jem-odoo .
Send methods of mail.mail model are set private and simplified. Those
methods have been written when a lot of overrides were necessary to
generate notification emails. Since then the flow has changed but
methods have not been updated completely. This commit therefore simplified
a bit the call chain and parameters.
This commit is purely technical and does not hold any functional change.
Purpose is to clean code in order to prepare future changes in mail
application.
Commit 0d3d0e9563 added a key in context when sending emails to know
the target model name. It was used at that time to forge for footer of
notification emails.
Since that time notification emails have evolved a lot. The code based on
that context key has been removed. Context key has been transformed into a
context switch. However it is not used anymore and can be removed. It allows
to clean a bit the code and save a context switch.
This commit is purely technical and does not hold any functional change.
Purpose is to clean code in order to prepare future changes in mail
application.
This commit allows to gain some queries in mail sending process, meaning
removing context switch is interesting. One query is gained when using
the send process. Complex performance test involving several message post
gain 3 queries each.
1d57a09112 added a two-steps parent fetch when posting a message
on a discussion thread. At the time this commit was done responsible were
notified of the first thread message. It was helpful to notify them of the
original email that created the thread instead of the "document created"
generic message. Indeed for documents like leads or issues having the
original email containing the customer question is interesting when being
notified. This is why this commit was done.
However since quite a long time responsible notifications have changed.
They now receive an email notification telling they have been assigned on
a document. This commit is not necessary anymore and we therefore remove
it in order to clean a bit the message_post process.
This commit globally reverts 1d57a09112. Please note that @beledouxdenis
agreed after a long thinking.
This commit has a slight impact on mail performance tests, allowing to save
one query in some more complex test case.
Current behavior when assigning someone as responsible of a document is
to send him an email if he works using emails. Technically it is done
when writing on user_id field and based on notification_type field value
of the user.
This commit changes that behavior to push a notification to the new
responsible. It uses the recently-introduced message_notify method
allowing to push notification to recipients without being linked to
a document. It means people working in Odoo receive a message in their
Inbox while people working by email receive an email.
Assignation message contains a link to the document meaning it is always
possible to go to the document. Notification email uses the new template
defined recently. It allow to have a notification email a bit classy.
This commit implies that more notifications are created. Indeed previously
to this commit no notification were created for partners without user
or for users working in Odoo. There is therefore a performance test whose
counter increases and a slight increase in query count when installing
all modules, about 1K queries on 581K.
Activity mixin adds an activity_date_deadline field that is a related on
activity_ids. This one is a o2m using a domain based on the current record
model. Indeed activities are cross-model, like mail.message.
However as it is stored it generates are a lot of unnecessary computation
when activities are updated. Indeed when activities change the related
computation is triggered for every model inheriting from the activity
mixin. It then applies the domain in order to find record to update. This
generates a lot of void queries because only one will match the activity
model.
This commit makes the field not stored anymore. Indeed it is used mainly
in filters. Same functional result can be achieved using a computed and
searchable field.
We gain a lot of queries when creating activities as indicated by the
counter update in this commit. Creating a record inheriting from activity
mixin is a bit improved since there is less computed field to compute.
Creating an activity costs a lot less queries and we gain about 40%
in term of query required to create an activity when a lot of models are
using the mixin.
On community runbot it makes a gain of 2% when installing all modules.
This commit allows to save 13K queries on 587K (current) which is quite
good.
Only messages as comments and email template should keep a white backround since they are not internal.
It is also enough to display fields that changed on subtypes (update message) since emails will be reworked soon and more information will be given there. The support of 'always' track can be removed. (is now considered as a simple tracked field)
The use of self.env.ref('mail.mt_comment').id add some query and the performance tests needs to be update since message_format is called by notify_by_chat. This code will be refactor later
This commits adds two tests in test_mail performance tests related to
the write operation. Indeed create a often tested but there are less
performance tests related to the write. Those two tests are
* first one is a simple write on a chatter-enabled model without any
tracking or specific feature; nothing should happen when performing
a write in that kind of model;
* second one is a write on a complex tracking-enabled model linked to
templates and auto subscription. It allows to have some figures for
real world use case.
Due to last commits that aim at improving Odoo performance all complex
tests of test_mail now take less queries to execute. Counters are
updated to match the current runbot behavior.
There is quite a big improvement when posting using a template compared
to other tests. As there are not enough tests to ensure all results
are correct let us stay careful by saying "it seems there are
improvements".
Purpose
* move notify in message_post instead of always doing it at message
creation
Specifications
* message create should only create a message
* message post should be in charge of calling the notification process
Christophe Simonis