From 4c763e6bca864d710c98ce7510169772a2dea55b Mon Sep 17 00:00:00 2001
From: Xavier Dubuc <xdu@odoo.com>
Date: Mon, 22 Jun 2020 15:59:16 +0000
Subject: [PATCH] [FIX] mail: clarify composer placeholder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Also moved isLog from composer component prop to composer model field.

task-2280159 & task-2250887

closes odoo/odoo#53645

X-original-commit: ba41b1de5edc2b3153a5b20a4f11778a3f724ade
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
---
 .../static/src/components/chatter/chatter.xml |   1 -
 .../chatter_topbar/chatter_topbar.js          |  11 +-
 .../chatter_topbar/chatter_topbar.xml         |   4 +-
 .../chatter_topbar/chatter_topbar_tests.js    | 137 ++++++++++++++++++
 .../src/components/composer/composer.js       |  11 +-
 .../src/components/composer/composer.xml      |   4 +-
 .../src/components/composer/composer_tests.js |  83 ++++++++++-
 .../composer_text_input.js                    |  16 ++
 .../composer_text_input.xml                   |   2 +-
 .../mail/static/src/models/chatter/chatter.js |  26 ++--
 .../static/src/models/composer/composer.js    |  50 ++-----
 11 files changed, 273 insertions(+), 72 deletions(-)

diff --git a/addons/mail/static/src/components/chatter/chatter.xml b/addons/mail/static/src/components/chatter/chatter.xml
index f630eaf7634..ad5751cdbb0 100644
--- a/addons/mail/static/src/components/chatter/chatter.xml
+++ b/addons/mail/static/src/components/chatter/chatter.xml
@@ -17,7 +17,6 @@
                         hasTextInputSendOnEnterEnabled="false"
                         isCompact="false"
                         isExpandable="true"
-                        isLog="chatter.isComposerLog"
                         t-on-o-message-posted="_onComposerMessagePosted"
                         t-ref="composer"
                     />
diff --git a/addons/mail/static/src/components/chatter_topbar/chatter_topbar.js b/addons/mail/static/src/components/chatter_topbar/chatter_topbar.js
index c4b088b7709..37b28b4970e 100644
--- a/addons/mail/static/src/components/chatter_topbar/chatter_topbar.js
+++ b/addons/mail/static/src/components/chatter_topbar/chatter_topbar.js
@@ -23,6 +23,7 @@ class ChatterTopbar extends Component {
             return {
                 areThreadAttachmentsLoaded: thread && thread.areAttachmentsLoaded,
                 chatter: chatter ? chatter.__state : undefined,
+                composer: chatter && chatter.composer ? chatter.composer.__state : undefined,
                 threadAttachmentsAmount: threadAttachments.length,
             };
         });
@@ -66,7 +67,10 @@ class ChatterTopbar extends Component {
      * @param {MouseEvent} ev
      */
     _onClickLogNote(ev) {
-        if (this.chatter.isComposerVisible && this.chatter.isComposerLog) {
+        if (!this.chatter.composer) {
+            return;
+        }
+        if (this.chatter.isComposerVisible && this.chatter.composer.isLog) {
             this.chatter.update({ isComposerVisible: false });
         } else {
             this.chatter.showLogNote();
@@ -106,7 +110,10 @@ class ChatterTopbar extends Component {
      * @param {MouseEvent} ev
      */
     _onClickSendMessage(ev) {
-        if (this.chatter.isComposerVisible && !this.chatter.isComposerLog) {
+        if (!this.chatter.composer) {
+            return;
+        }
+        if (this.chatter.isComposerVisible && !this.chatter.composer.isLog) {
             this.chatter.update({ isComposerVisible: false });
         } else {
             this.chatter.showSendMessage();
diff --git a/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml b/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml
index 67dfe4ea844..3512f93332e 100644
--- a/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml
+++ b/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml
@@ -9,7 +9,7 @@
                         <button class="btn btn-link o_ChatterTopbar_button o_ChatterTopbar_buttonSendMessage"
                             type="button"
                             t-att-class="{
-                                'o-active': chatter.isComposerVisible and !chatter.isComposerLog,
+                                'o-active': chatter.isComposerVisible and chatter.composer and !chatter.composer.isLog,
                                 'o-bordered': chatter.hasExternalBorder,
                             }"
                             t-att-disabled="chatter.isDisabled"
@@ -20,7 +20,7 @@
                         <button class="btn btn-link o_ChatterTopbar_button o_ChatterTopbar_buttonLogNote"
                             type="button"
                             t-att-class="{
-                                'o-active': chatter.isComposerVisible and chatter.isComposerLog,
+                                'o-active': chatter.isComposerVisible and chatter.composer and chatter.composer.isLog,
                                 'o-bordered': chatter.hasExternalBorder,
                             }"
                             t-att-disabled="chatter.isDisabled"
diff --git a/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js b/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js
index c6c38c081b6..352566423f5 100644
--- a/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js
+++ b/addons/mail/static/src/components/chatter_topbar/chatter_topbar_tests.js
@@ -583,6 +583,143 @@ QUnit.test('rendering with multiple channel followers', async function (assert)
     );
 });
 
+QUnit.test('log note/send message switching', async function (assert) {
+    assert.expect(8);
+
+    await this.start();
+    const chatter = this.env.models['mail.chatter'].create({
+        threadId: 100,
+        threadModel: 'res.partner',
+    });
+    await this.createChatterTopbarComponent(chatter);
+    assert.containsOnce(
+        document.body,
+        '.o_ChatterTopbar_buttonSendMessage',
+        "should have a 'Send Message' button"
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonSendMessage'),
+        'o-active',
+        "'Send Message' button should not be active"
+    );
+    assert.containsOnce(
+        document.body,
+        '.o_ChatterTopbar_buttonLogNote',
+        "should have a 'Log Note' button"
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonLogNote'),
+        'o-active',
+        "'Log Note' button should not be active"
+    );
+
+    await afterNextRender(() =>
+        document.querySelector(`.o_ChatterTopbar_buttonSendMessage`).click()
+    );
+    assert.hasClass(
+        document.querySelector('.o_ChatterTopbar_buttonSendMessage'),
+        'o-active',
+        "'Send Message' button should be active"
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonLogNote'),
+        'o-active',
+        "'Log Note' button should not be active"
+    );
+
+    await afterNextRender(() =>
+        document.querySelector(`.o_ChatterTopbar_buttonLogNote`).click()
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonSendMessage'),
+        'o-active',
+        "'Send Message' button should not be active"
+    );
+    assert.hasClass(
+        document.querySelector('.o_ChatterTopbar_buttonLogNote'),
+        'o-active',
+        "'Log Note' button should be active"
+    );
+});
+
+QUnit.test('log note toggling', async function (assert) {
+    assert.expect(4);
+
+    await this.start();
+    const chatter = this.env.models['mail.chatter'].create({
+        threadId: 100,
+        threadModel: 'res.partner',
+    });
+    await this.createChatterTopbarComponent(chatter);
+    assert.containsOnce(
+        document.body,
+        '.o_ChatterTopbar_buttonLogNote',
+        "should have a 'Log Note' button"
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonLogNote'),
+        'o-active',
+        "'Log Note' button should not be active"
+    );
+
+    await afterNextRender(() =>
+        document.querySelector(`.o_ChatterTopbar_buttonLogNote`).click()
+    );
+    assert.hasClass(
+        document.querySelector('.o_ChatterTopbar_buttonLogNote'),
+        'o-active',
+        "'Log Note' button should be active"
+    );
+
+    await afterNextRender(() =>
+        document.querySelector(`.o_ChatterTopbar_buttonLogNote`).click()
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonLogNote'),
+        'o-active',
+        "'Log Note' button should not be active"
+    );
+});
+
+QUnit.test('send message toggling', async function (assert) {
+    assert.expect(4);
+
+    await this.start();
+    const chatter = this.env.models['mail.chatter'].create({
+        threadId: 100,
+        threadModel: 'res.partner',
+    });
+    await this.createChatterTopbarComponent(chatter);
+    assert.containsOnce(
+        document.body,
+        '.o_ChatterTopbar_buttonSendMessage',
+        "should have a 'Send Message' button"
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonSendMessage'),
+        'o-active',
+        "'Send Message' button should not be active"
+    );
+
+    await afterNextRender(() =>
+        document.querySelector(`.o_ChatterTopbar_buttonSendMessage`).click()
+    );
+    assert.hasClass(
+        document.querySelector('.o_ChatterTopbar_buttonSendMessage'),
+        'o-active',
+        "'Send Message' button should be active"
+    );
+
+    await afterNextRender(() =>
+        document.querySelector(`.o_ChatterTopbar_buttonSendMessage`).click()
+    );
+    assert.doesNotHaveClass(
+        document.querySelector('.o_ChatterTopbar_buttonSendMessage'),
+        'o-active',
+        "'Send Message' button should not be active"
+    );
+});
+
 });
 });
 });
diff --git a/addons/mail/static/src/components/composer/composer.js b/addons/mail/static/src/components/composer/composer.js
index 63d85a0fb95..3fd82216b05 100644
--- a/addons/mail/static/src/components/composer/composer.js
+++ b/addons/mail/static/src/components/composer/composer.js
@@ -124,7 +124,7 @@ class Composer extends Component {
     get hasHeader() {
         return (
             (this.props.hasThreadName && this.composer.thread) ||
-            (this.props.hasFollowers && !this.props.isLog)
+            (this.props.hasFollowers && !this.composer.isLog)
         );
     }
 
@@ -150,9 +150,10 @@ class Composer extends Component {
      * @private
      */
     async _postMessage() {
-        // TODO: take suggested recipients into account
-        await this.composer.postMessage({ isLog: this.props.isLog });
+        // TODO: take suggested recipients into account (task-2283356)
+        await this.composer.postMessage();
         // TODO: we might need to remove trigger and use the store to wait for the post rpc to be done
+        // task-2252858
         this.trigger('o-message-posted');
     }
 
@@ -206,7 +207,7 @@ class Composer extends Component {
             default_body: mailUtils.escapeAndCompactTextContent(this.composer.textInputContent),
             default_attachment_ids: attachmentIds,
             // default_partner_ids: partnerIds,
-            default_is_log: this.props.isLog,
+            default_is_log: this.composer.isLog,
             mail_post_autofollow: true,
         };
 
@@ -316,7 +317,6 @@ Object.assign(Composer, {
         isCompact: true,
         isDiscardOnClickAway: false,
         isExpandable: false,
-        isLog: false,
     },
     props: {
         attachmentLocalIds: {
@@ -359,7 +359,6 @@ Object.assign(Composer, {
         isCompact: Boolean,
         isDiscardOnClickAway: Boolean,
         isExpandable: Boolean,
-        isLog: Boolean,
     },
     template: 'mail.Composer',
 });
diff --git a/addons/mail/static/src/components/composer/composer.xml b/addons/mail/static/src/components/composer/composer.xml
index 8aa429d7670..2845466750f 100644
--- a/addons/mail/static/src/components/composer/composer.xml
+++ b/addons/mail/static/src/components/composer/composer.xml
@@ -31,7 +31,7 @@
                                 on: <b><t t-esc="composer.thread.displayName"/></b>
                             </span>
                         </t>
-                        <t t-if="props.hasFollowers and !props.isLog">
+                        <t t-if="props.hasFollowers and !composer.isLog">
                             <!-- Text for followers -->
                             <small class="o_Composer_followers">
                                 <b class="text-muted">To: </b>
@@ -141,7 +141,7 @@
                     type="button"
                     t-on-click="_onClickSend"
                 >
-                    <t t-if="!env.messaging.device.isMobile"><t t-if="props.isLog">Log</t><t t-else="">Send</t></t>
+                    <t t-if="!env.messaging.device.isMobile"><t t-if="composer.isLog">Log</t><t t-else="">Send</t></t>
                 </button>
             </t>
             <t t-if="!env.messaging.device.isMobile and props.hasDiscardButton">
diff --git a/addons/mail/static/src/components/composer/composer_tests.js b/addons/mail/static/src/components/composer/composer_tests.js
index e506e99a48e..c62242360a6 100644
--- a/addons/mail/static/src/components/composer/composer_tests.js
+++ b/addons/mail/static/src/components/composer/composer_tests.js
@@ -58,12 +58,87 @@ QUnit.module('composer_tests.js', {
     },
 });
 
-QUnit.test('composer text input: basic rendering', async function (assert) {
+QUnit.test('composer text input: basic rendering when posting a message', async function (assert) {
     assert.expect(5);
 
     await this.start();
-    const composer = this.env.models['mail.composer'].create();
-    await this.createComposerComponent(composer);
+    const thread = this.env.models['mail.thread'].create({
+        composer: [['create', { isLog: false }]],
+        id: 20,
+        model: 'res.partner',
+    });
+    await this.createComposerComponent(thread.composer);
+    assert.strictEqual(
+        document.querySelectorAll('.o_Composer').length,
+        1,
+        "should have composer in discuss thread"
+    );
+    assert.strictEqual(
+        document.querySelectorAll('.o_Composer_textInput').length,
+        1,
+        "should have text input inside discuss thread composer"
+    );
+    assert.ok(
+        document.querySelector('.o_Composer_textInput').classList.contains('o_ComposerTextInput'),
+        "composer text input of composer should be a ComposerTextIput component"
+    );
+    assert.strictEqual(
+        document.querySelectorAll(`.o_ComposerTextInput_textarea`).length,
+        1,
+        "should have editable part inside composer text input"
+    );
+    assert.strictEqual(
+        document.querySelector(`.o_ComposerTextInput_textarea`).placeholder,
+        "Send a message to followers...",
+        "should have 'Send a message to followers...' as placeholder composer text input"
+    );
+});
+
+QUnit.test('composer text input: basic rendering when logging note', async function (assert) {
+    assert.expect(5);
+
+    await this.start();
+    const thread = this.env.models['mail.thread'].create({
+        composer: [['create', { isLog: true }]],
+        id: 20,
+        model: 'res.partner',
+    });
+    await this.createComposerComponent(thread.composer);
+    assert.strictEqual(
+        document.querySelectorAll('.o_Composer').length,
+        1,
+        "should have composer in discuss thread"
+    );
+    assert.strictEqual(
+        document.querySelectorAll('.o_Composer_textInput').length,
+        1,
+        "should have text input inside discuss thread composer"
+    );
+    assert.ok(
+        document.querySelector('.o_Composer_textInput').classList.contains('o_ComposerTextInput'),
+        "composer text input of composer should be a ComposerTextIput component"
+    );
+    assert.strictEqual(
+        document.querySelectorAll(`.o_ComposerTextInput_textarea`).length,
+        1,
+        "should have editable part inside composer text input"
+    );
+    assert.strictEqual(
+        document.querySelector(`.o_ComposerTextInput_textarea`).placeholder,
+        "Log an internal note...",
+        "should have 'Log an internal note...' as placeholder in composer text input if composer is log"
+    );
+});
+
+QUnit.test('composer text input: basic rendering when linked thread is a mail.channel', async function (assert) {
+    assert.expect(5);
+
+    await this.start();
+    const thread = this.env.models['mail.thread'].create({
+        id: 20,
+        model: 'mail.channel',
+    });
+    await this.createComposerComponent(thread.composer);
     assert.strictEqual(
         document.querySelectorAll('.o_Composer').length,
         1,
@@ -86,7 +161,7 @@ QUnit.test('composer text input: basic rendering', async function (assert) {
     assert.strictEqual(
         document.querySelector(`.o_ComposerTextInput_textarea`).placeholder,
         "Write something...",
-        "should have placeholder in note editable of composer text input"
+        "should have 'Write something...' as placeholder in composer text input if composer is for a 'mail.channel'"
     );
 });
 
diff --git a/addons/mail/static/src/components/composer_text_input/composer_text_input.js b/addons/mail/static/src/components/composer_text_input/composer_text_input.js
index 1be0e77a98b..279d5a6ae93 100644
--- a/addons/mail/static/src/components/composer_text_input/composer_text_input.js
+++ b/addons/mail/static/src/components/composer_text_input/composer_text_input.js
@@ -65,6 +65,22 @@ class ComposerTextInput extends Component {
         return this.env.models['mail.composer'].get(this.props.composerLocalId);
     }
 
+    /**
+     * @returns {string}
+     */
+    get textareaPlaceholder() {
+        if (!this.composer) {
+            return "";
+        }
+        if (this.composer.thread && this.composer.thread.model !== 'mail.channel') {
+            if (this.composer.isLog) {
+                return this.env._t("Log an internal note...");
+            }
+            return this.env._t("Send a message to followers...");
+        }
+        return this.env._t("Write something...");
+    }
+
     focusout() {
         this.saveStateInStore();
         this._textareaRef.el.blur();
diff --git a/addons/mail/static/src/components/composer_text_input/composer_text_input.xml b/addons/mail/static/src/components/composer_text_input/composer_text_input.xml
index 949f1065a56..dfeae27fc61 100644
--- a/addons/mail/static/src/components/composer_text_input/composer_text_input.xml
+++ b/addons/mail/static/src/components/composer_text_input/composer_text_input.xml
@@ -36,7 +36,7 @@
                         </div>
                     </div>
                 </t>
-                <textarea class="o_ComposerTextInput_textarea" t-esc="composer.textInputContent" placeholder="Write something..." t-on-focusin="_onFocusinTextarea" t-on-focusout="_onFocusoutTextarea" t-on-keydown="_onKeydownTextarea" t-on-keyup="_onKeyupTextarea" t-on-input="_onInputTextarea" t-ref="textarea"/>
+                <textarea class="o_ComposerTextInput_textarea" t-esc="composer.textInputContent" t-att-placeholder="textareaPlaceholder" t-on-focusin="_onFocusinTextarea" t-on-focusout="_onFocusoutTextarea" t-on-keydown="_onKeydownTextarea" t-on-keyup="_onKeyupTextarea" t-on-input="_onInputTextarea" t-ref="textarea"/>
             </t>
         </div>
     </t>
diff --git a/addons/mail/static/src/models/chatter/chatter.js b/addons/mail/static/src/models/chatter/chatter.js
index 59f197ded1c..59ba524f23b 100644
--- a/addons/mail/static/src/models/chatter/chatter.js
+++ b/addons/mail/static/src/models/chatter/chatter.js
@@ -2,7 +2,7 @@ odoo.define('mail/static/src/models/chatter/chatter.js', function (require) {
 'use strict';
 
 const { registerNewModel } = require('mail/static/src/model/model_core.js');
-const { attr, one2many, one2one } = require('mail/static/src/model/model_field.js');
+const { attr, many2one, one2many, one2one } = require('mail/static/src/model/model_field.js');
 
 function factory(dependencies) {
 
@@ -72,19 +72,19 @@ function factory(dependencies) {
         }
 
         showLogNote() {
-            this.update({
-                isComposerLog: true,
-                isComposerVisible: true,
+            this.update({ isComposerVisible: true });
+            this.thread.composer.update({
+                isDoFocus: true,
+                isLog: true,
             });
-            this.thread.composer.update({ isDoFocus: true });
         }
 
         showSendMessage() {
-            this.update({
-                isComposerLog: false,
-                isComposerVisible: true,
+            this.update({ isComposerVisible: true });
+            this.thread.composer.update({
+                isDoFocus: true,
+                isLog: false,
             });
-            this.thread.composer.update({ isDoFocus: true });
         }
 
         toggleActivityBoxVisibility() {
@@ -249,6 +249,9 @@ function factory(dependencies) {
         activitiesState: attr({
             related: 'activities.state',
         }),
+        composer: many2one('mail.composer', {
+            related: 'thread.composer',
+        }),
         context: attr({
             default: {},
         }),
@@ -280,9 +283,6 @@ function factory(dependencies) {
         isAttachmentBoxVisible: attr({
             default: false,
         }),
-        isComposerLog: attr({
-            default: true,
-        }),
         isComposerVisible: attr({
             default: false,
         }),
@@ -301,7 +301,7 @@ function factory(dependencies) {
             compute: '_computeOverdueActivities',
             dependencies: ['activitiesState'],
         }),
-        thread: one2one('mail.thread', {
+        thread: many2one('mail.thread', {
             related: 'threadViewer.thread',
         }),
         threadAttachmentCount: attr({
diff --git a/addons/mail/static/src/models/composer/composer.js b/addons/mail/static/src/models/composer/composer.js
index fa6e2698eb0..86dfe78aa5e 100644
--- a/addons/mail/static/src/models/composer/composer.js
+++ b/addons/mail/static/src/models/composer/composer.js
@@ -85,40 +85,11 @@ function factory(dependencies) {
         }
 
         /**
-         * Post a message in provided composer's thread with given data.
-         *
-         * @param {Object} data
-         * @param {string[]} data.attachments
-         * @param {*[]} data.canned_response_ids
-         * @param {integer[]} data.channel_ids
-         * @param {boolean} [data.isLog=false]
-         * @param {string} data.subject
-         * @param {integer} [data.subtype_id]
-         * @param {string} [data.subtype_xmlid='mail.mt_comment']
-         * @param {Object} [options]
-         * @param {integer} options.res_id
-         * @param {string} options.res_model
+         * Post a message in provided composer's thread based on current composer fields values.
          */
-        async postMessage(data, options) {
+        async postMessage() {
             const thread = this.thread;
             this.thread.unregisterCurrentPartnerIsTyping({ immediateNotify: true });
-            if (thread.model === 'mail.box') {
-                const { res_id, res_model } = options;
-                const otherThread = this.env.models['mail.thread'].find(thread =>
-                    thread.id === res_id &&
-                    thread.model === res_model
-                );
-                return otherThread.composer.postMessage(Object.assign({}, data));
-            }
-            const {
-                canned_response_ids,
-                channel_ids = [],
-                context,
-                isLog = false,
-                subject,
-                subtype_id,
-                // subtype_xmlid='mail.mt_comment',
-            } = data;
             const escapedAndCompactContent = escapeAndCompactTextContent(this.textInputContent);
             let body = escapedAndCompactContent.replace(/&nbsp;/g, ' ').trim();
             // This message will be received from the mail composer as html content
@@ -151,16 +122,7 @@ function factory(dependencies) {
                 }));
             } else {
                 Object.assign(postData, {
-                    channel_ids: channel_ids.map(channelId => [4, channelId, false]),
-                    canned_response_ids
-                });
-                if (subject) {
-                    postData.subject = subject;
-                }
-                Object.assign(postData, {
-                    context,
-                    subtype_id,
-                    subtype_xmlid: isLog ? 'mail.mt_note' : 'mail.mt_comment',
+                    subtype_xmlid: this.isLog ? 'mail.mt_note' : 'mail.mt_comment',
                 });
                 messageId = await this.async(() => this.env.services.rpc({
                     model: thread.model,
@@ -567,6 +529,12 @@ function factory(dependencies) {
         isDoFocus: attr({
             default: false,
         }),
+        /**
+         * If true composer will log a note, else a comment will be posted.
+         */
+        isLog: attr({
+            default: false,
+        }),
         mainSuggestedPartners: many2many('mail.partner'),
         mentionedPartners: many2many('mail.partner', {
             compute: '_computeMentionedPartners',