[IMP] mail: send cache control header on avatars

Before this PR cache control header for avatar was set to no-cache which is not
the right value.
This PR mark the avatar routes response as immutable when a unique string is
present. We also use the partner write_date as unique string when possible.

task-3551636

closes odoo/odoo#141326

X-original-commit: 3de4f1f1225229cbdb0512df5790ce491f04ea7e
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>

addons/mail/controllers/discuss/binary.py

@@ -16,29 +16,29 @@ class BinaryController(Binary):
         auth="public",
     )
     @add_guest_to_context
-    def discuss_channel_partner_avatar_128(self, channel_id, partner_id, **kwargs):
+    def discuss_channel_partner_avatar_128(self, channel_id, partner_id, unique=False):
         channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
         partner = request.env["res.partner"].browse(partner_id).exists()
         domain = [("channel_id", "=", channel_id), ("partner_id", "=", partner_id)]
         if channel and partner and request.env["discuss.channel.member"].search(domain):
             # sudo: res.partner - the partner is in the same channel as the current user, so they can see their avatar
             return (
-                request.env["ir.binary"]._get_image_stream_from(partner.sudo(), field_name="avatar_128").get_response()
+                request.env["ir.binary"]._get_image_stream_from(partner.sudo(), field_name="avatar_128").get_response(immutable=True if unique else False)
             )
-        return self.content_image(model="res.partner", id=partner_id, field="avatar_128")
+        return self.content_image(model="res.partner", id=partner_id, field="avatar_128", unique=unique)
 
     @http.route(
         "/discuss/channel/<int:channel_id>/guest/<int:guest_id>/avatar_128", methods=["GET"], type="http", auth="public"
     )
     @add_guest_to_context
-    def discuss_channel_guest_avatar_128(self, channel_id, guest_id, **kwargs):
+    def discuss_channel_guest_avatar_128(self, channel_id, guest_id, unique=False):
         channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
         guest = request.env["mail.guest"].browse(guest_id).exists()
         domain = [("channel_id", "=", channel_id), ("guest_id", "=", guest_id)]
         if channel and guest and request.env["discuss.channel.member"].search(domain):
             # sudo: mail.guest - the guest is in the same channel as the current user, so they can see their avatar
-            return request.env["ir.binary"]._get_image_stream_from(guest.sudo(), field_name="avatar_128").get_response()
-        return self.content_image(model="mail.guest", id=guest_id, field="avatar_128")
+            return request.env["ir.binary"]._get_image_stream_from(guest.sudo(), field_name="avatar_128").get_response(immutable=True if unique else False)
+        return self.content_image(model="mail.guest", id=guest_id, field="avatar_128", unique=unique)
 
     @http.route(
         "/discuss/channel/<int:channel_id>/attachment/<int:attachment_id>", methods=["GET"], type="http", auth="public"
@@ -61,8 +61,8 @@ class BinaryController(Binary):
 
     @http.route("/discuss/channel/<int:channel_id>/avatar_128", methods=["GET"], type="http", auth="public")
     @add_guest_to_context
-    def discuss_channel_avatar_128(self, channel_id, **kwargs):
-        return self.content_image(model="discuss.channel", id=channel_id, field="avatar_128")
+    def discuss_channel_avatar_128(self, channel_id, unique=False):
+        return self.content_image(model="discuss.channel", id=channel_id, field="avatar_128", unique=unique)
 
     @http.route(
         [

addons/mail/models/discuss/mail_guest.py

@@ -6,6 +6,7 @@ from datetime import datetime, timedelta
 from functools import wraps
 from inspect import Parameter, signature
 
+import odoo
 from odoo.tools import consteq, get_lang
 from odoo import _, api, fields, models
 from odoo.http import request
@@ -163,7 +164,7 @@ class MailGuest(models.Model):
 
     def _guest_format(self, fields=None):
         if not fields:
-            fields = {'id': True, 'name': True, 'im_status': True}
+            fields = {'id': True, 'name': True, 'im_status': True, "write_date": True}
         guests_formatted_data = {}
         for guest in self:
             data = {}
@@ -173,6 +174,8 @@ class MailGuest(models.Model):
                 data['name'] = guest.name
             if 'im_status' in fields:
                 data['im_status'] = guest.im_status
+            if "write_date" in fields:
+                data["write_date"] = odoo.fields.Datetime.to_string(guest.write_date)
             data['type'] = "guest"
             guests_formatted_data[guest] = data
         return guests_formatted_data

addons/mail/models/res_partner.py

@@ -3,6 +3,7 @@
 
 import re
 
+import odoo
 from odoo import _, api, fields, models, tools
 from odoo.osv import expression
 
@@ -213,7 +214,7 @@ class Partner(models.Model):
     def mail_partner_format(self, fields=None):
         partners_format = dict()
         if not fields:
-            fields = {'id': True, 'name': True, 'email': True, 'active': True, 'im_status': True, 'is_company': True, 'user': {}}
+            fields = {'id': True, 'name': True, 'email': True, 'active': True, 'im_status': True, 'is_company': True, 'user': {}, "write_date": True}
         for partner in self:
             data = {}
             if 'id' in fields:
@@ -228,6 +229,8 @@ class Partner(models.Model):
                 data['im_status'] = partner.im_status
             if 'is_company' in fields:
                 data['is_company'] = partner.is_company
+            if "write_date" in fields:
+                data["write_date"] = odoo.fields.Datetime.to_string(partner.write_date)
             if 'user' in fields:
                 internal_users = partner.user_ids - partner.user_ids.filtered('share')
                 main_user = internal_users[0] if len(internal_users) > 0 else partner.user_ids[0] if len(partner.user_ids) > 0 else self.env['res.users']

addons/mail/static/src/core/common/persona_model.js

@@ -61,6 +61,8 @@ export class Persona extends Record {
     /** @type {ImStatus} */
     im_status;
     isAdmin = false;
+    /** @type {string} */
+    write_date;
 
     /**
      * @returns {boolean}

addons/mail/static/src/core/common/thread_service.js

@@ -854,15 +854,19 @@ export class ThreadService {
         if (!persona) {
             return DEFAULT_AVATAR;
         }
+        const urlParams = {};
+        if (persona.write_date) {
+            urlParams.unique = persona.write_date;
+        }
         if (persona.is_company === undefined && this.store.self?.user?.isInternalUser) {
             this.personaService.fetchIsCompany(persona);
         }
         if (thread?.model === "discuss.channel") {
             if (persona.type === "partner") {
-                return url(`/discuss/channel/${thread.id}/partner/${persona.id}/avatar_128`);
+                return url(`/discuss/channel/${thread.id}/partner/${persona.id}/avatar_128`, urlParams);
             }
             if (persona.type === "guest") {
-                return url(`/discuss/channel/${thread.id}/guest/${persona.id}/avatar_128`);
+                return url(`/discuss/channel/${thread.id}/guest/${persona.id}/avatar_128`, urlParams);
             }
         }
         if (persona.type === "partner" && persona?.id) {
@@ -870,6 +874,7 @@ export class ThreadService {
                 field: "avatar_128",
                 id: persona.id,
                 model: "res.partner",
+                ...urlParams,
             });
             return avatar;
         }
@@ -878,6 +883,7 @@ export class ThreadService {
                 field: "avatar_128",
                 id: persona.user.id,
                 model: "res.users",
+                ...urlParams,
             });
             return avatar;
         }

addons/mail/static/src/discuss/core/common/thread_model_patch.js

@@ -66,7 +66,10 @@ patch(Thread.prototype, {
             );
         }
         if (this.type === "chat") {
-            return `/web/image/res.partner/${this.chatPartner.id}/avatar_128`;
+            return url(
+                `/web/image/res.partner/${this.chatPartner.id}/avatar_128`,
+                assignDefined({}, { unique: this.chatPartner.write_date })
+            );
         }
         return super.imgUrl;
     },

addons/mail/static/tests/discuss_app/sidebar_tests.js

@@ -1000,7 +1000,7 @@ QUnit.test("chat - avatar: should have correct avatar", async () => {
     openDiscuss();
 
     await contains(".o-mail-DiscussSidebarChannel img");
-    await contains(`img[data-src='/web/image/res.partner/${partnerId}/avatar_128']`);
+    await contains(`img[data-src$='/web/image/res.partner/${partnerId}/avatar_128']`);
 });
 
 QUnit.test("chat should be sorted by last activity time [REQUIRE FOCUS]", async () => {

addons/mail/tests/discuss/test_message_controller.py

@@ -5,6 +5,8 @@ import json
 import odoo
 from odoo.tools import mute_logger, date_utils
 from odoo.tests import HttpCase
+from odoo.http import STATIC_CACHE_LONG
+from odoo import Command
 
 
 @odoo.tests.tagged("-at_install", "post_install")
@@ -342,3 +344,44 @@ class TestMessageController(HttpCase):
             self.env["res.partner"].search_count([('email', '=', "john2@test.be")]),
             "'mail/message/post' does not create another user if there's already a user with matching email",
         )
+
+    def test_mail_cache_control_header(self):
+        testuser = self.env['res.users'].create({
+            'email': 'testuser@testuser.com',
+            'groups_id': [Command.set([self.ref('base.group_portal')])],
+            'name': 'Test User',
+            'login': 'testuser',
+            'password': 'testuser',
+        })
+        test_user = self.authenticate("testuser", "testuser")
+        partner = self.env["res.users"].browse(test_user.uid).partner_id
+        self.channel.add_members(testuser.partner_id.ids)
+        res = self.url_open(
+            url=f"/discuss/channel/{self.channel.id}/avatar_128?unique={self.channel._get_avatar_cache_key()}"
+        )
+        self.assertEqual(res.headers["Cache-Control"], f"public, max-age={STATIC_CACHE_LONG}")
+
+        res = self.url_open(
+            url=f"/discuss/channel/{self.channel.id}/avatar_128"
+        )
+        self.assertEqual(res.headers["Cache-Control"], "no-cache")
+
+        res = self.url_open(
+            url=f"/discuss/channel/{self.channel.id}/partner/{partner.id}/avatar_128?unique={partner.write_date.isoformat()}"
+        )
+        self.assertEqual(res.headers["Cache-Control"], f"public, max-age={STATIC_CACHE_LONG}")
+
+        res = self.url_open(
+            url=f"/discuss/channel/{self.channel.id}/partner/{partner.id}/avatar_128"
+        )
+        self.assertEqual(res.headers["Cache-Control"], "no-cache")
+
+        res = self.url_open(
+            url=f"/discuss/channel/{self.channel.id}/guest/{self.guest.id}/avatar_128?unique={self.guest.write_date.isoformat()}"
+        )
+        self.assertEqual(res.headers["Cache-Control"], f"public, max-age={STATIC_CACHE_LONG}")
+
+        res = self.url_open(
+            url=f"/discuss/channel/{self.channel.id}/guest/{self.guest.id}/avatar_128"
+        )
+        self.assertEqual(res.headers["Cache-Control"], "no-cache")