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

[FIX] mail: enable composer after message sent #28475

Closed
wants to merge 1 commit into
base: 12.0
from

Conversation

Projects
None yet
3 participants
@nle-odoo
Contributor

nle-odoo commented Nov 7, 2018

When a message is sent, the composer sent button is disabled (to prevent
multiple emissions) but then was not enabled back.

It is not much of an issue in the basic composer since ENTER key still
sent message, but in extended composer CTRL+ENTER has to be used (and
is less easily known).

issue found when checking opw-1904029

@robodoo robodoo added the seen 🙂 label Nov 7, 2018

@nle-odoo nle-odoo added OE and removed seen 🙂 labels Nov 7, 2018

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 7, 2018

[FIX] mail: enable composer after message sent
When a message is sent, the composer sent button is disabled (to prevent
multiple emissions) but then was not enabled back.

It is not much of an issue in the basic composer since ENTER key still
sent message, but in extended composer CTRL+ENTER has to be used (and
is less easily known).

issue found when checking opw-1904029
closes odoo#28475

@nle-odoo nle-odoo requested a review from alexkuhn Nov 7, 2018

@nle-odoo

This comment has been minimized.

Contributor

nle-odoo commented Nov 7, 2018

Hi @alexkuhn

This is not for a ticket, but when testing the other PR (thanks for review) I saw this issue that the send button only worked with the first message. After it is disabled and only enter or control+enter (for extended composer) worked.

Thanks.

@robodoo robodoo added the CI 🤖 label Nov 7, 2018

@alexkuhn

That's indeed an issue, this is a revision of the following fix: c5dc0b2

@alexkuhn

This comment has been minimized.

Contributor

alexkuhn commented Nov 7, 2018

robodoo r+

@robodoo robodoo added the r+ 👌 label Nov 7, 2018

robodoo pushed a commit that referenced this pull request Nov 7, 2018

[FIX] mail: enable composer after message sent
When a message is sent, the composer sent button is disabled (to prevent
multiple emissions) but then was not enabled back.

It is not much of an issue in the basic composer since ENTER key still
sent message, but in extended composer CTRL+ENTER has to be used (and
is less easily known).

issue found when checking opw-1904029
closes #28475

@robodoo robodoo added the merging 👷 label Nov 7, 2018

@nle-odoo

This comment has been minimized.

Contributor

nle-odoo commented Nov 7, 2018

robodoo r-

Ok I did not see this in history.

I will check further since reverting totally c5dc0b2 in 12.0 the issue it solved is not reintroduced.

@robodoo

This comment has been minimized.

Contributor

robodoo commented Nov 7, 2018

I'm sorry, @nle-odoo. r- makes no sense in the current PR state.

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 8, 2018

[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

issue found when checking opw-1904029
closes odoo#28475

@robodoo robodoo removed the CI 🤖 label Nov 8, 2018

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 8, 2018

[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

Without change in code, added test fails with:
 "Message with failed delivery is not removed" (result: "")
 "Send button should be enabled after failure" (result: true)

issue found when checking opw-1904029
closes odoo#28475

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 8, 2018

[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

Without change in code, added test fails with:
 "Message with failed delivery is not removed" (result: "")
 "Send button should be enabled after failure" (result: true)

issue found when checking opw-1904029
closes odoo#28475

@robodoo robodoo added the CI 🤖 label Nov 8, 2018

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 8, 2018

[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

Without change in code, added test fails with:
 "Message with failed delivery is not removed" (result: "")
 "Send button should be enabled after failure" (result: true)

issue found when checking opw-1904029
closes odoo#28475

@robodoo robodoo removed the CI 🤖 label Nov 8, 2018

@nle-odoo

This comment has been minimized.

Contributor

nle-odoo commented Nov 8, 2018

@alexkuhn

It should be ready now in this PR.

I have version for 11.0 in #28506 without the test (since the issue of the test has not happened in 11.0, and the test would be different in 11.0, 11.3, 12.0 which is a pain).

Show resolved Hide resolved addons/mail/static/src/js/chatter.js Outdated
Show resolved Hide resolved addons/mail/static/src/js/chatter.js Outdated
Show resolved Hide resolved addons/mail/static/src/js/chatter.js Outdated
Show resolved Hide resolved addons/mail/static/tests/chatter_tests.js Outdated
Show resolved Hide resolved addons/mail/static/tests/chatter_tests.js
}
return this._super(route, args);
},
intercepts: {

This comment has been minimized.

@alexkuhn

alexkuhn Nov 8, 2018

Contributor

you can remove this

(this was necessary when the mail service was not available in tests, which is no longer the case in v11.2+)

This comment has been minimized.

@nle-odoo

nle-odoo Nov 8, 2018

Contributor

ok, I have also removed the one from the test case over this one

Show resolved Hide resolved addons/mail/static/tests/chatter_tests.js Outdated
Show resolved Hide resolved addons/mail/static/tests/chatter_tests.js Outdated
Show resolved Hide resolved addons/mail/static/tests/chatter_tests.js Outdated

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 8, 2018

[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

Without change in code, added test fails with:
 "Message with failed delivery is not removed" (result: "")
 "Send button should be enabled after failure" (result: true)

issue found when checking opw-1904029
closes odoo#28475

@robodoo robodoo added the CI 🤖 label Nov 8, 2018

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 8, 2018

[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

Without change in code, added test fails with:
 "Should keep unsent message in the composer on failure" (result: "")
 "Send button should be re-enabled on message post failure" (result: true)

issue found when checking opw-1904029
closes odoo#28475

@robodoo robodoo removed the CI 🤖 label Nov 8, 2018

Show resolved Hide resolved addons/mail/static/tests/chatter_tests.js Outdated
[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

Without change in code, added test fails with:
 "Should keep unsent message in the composer on failure" (result: "")
 "Send button should be re-enabled on message post failure" (result: true)

issue found when checking opw-1904029
closes #28475
@nle-odoo

This comment has been minimized.

Contributor

nle-odoo commented Nov 9, 2018

robodoo r+

ok @alexkuhn thanks

robodoo pushed a commit that referenced this pull request Nov 9, 2018

[FIX] mail: enable composer after message sent
c5dc0b2 disabled send button on composer sent.

But this is only needed for chatter composer, other composers directly
send the message and are not refreshed asynchronously.

So for other composer, the send button worked one time but then was
disabled and the only way to sent was ENTER (on basic composer) or
CTRL+ENTER (on extended composer).

With this fix, disabling the button is done only for chatter composer.
Also the behavior of not clearing the message if there was an error is
introduced back.

Without change in code, added test fails with:
 "Should keep unsent message in the composer on failure" (result: "")
 "Send button should be re-enabled on message post failure" (result: true)

issue found when checking opw-1904029
closes #28475
@robodoo

This comment has been minimized.

Contributor

robodoo commented Nov 9, 2018

Merged, thanks!

@robodoo robodoo closed this Nov 9, 2018

@nle-odoo nle-odoo deleted the odoo-dev:12.0-staging-nle branch Nov 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment