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

strip responses before splitting them on newlines #5362

Merged
merged 4 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/5361.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed bug where starting or ending a response with ``\n\n`` led to one of the responses returned being empty.
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@
"fixed_sidebar": True,
"product": "Rasa",
"base_url": "https://rasa.com/docs/rasa/",
"canonical_url": "https://rasa.com/docs/rasa/"
"canonical_url": "https://rasa.com/docs/rasa/",
}
# html_theme_options = {}

Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/botframework.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ async def send(self, message_data: Dict[Text, Any]) -> None:
async def send_text_message(
self, recipient_id: Text, text: Text, **kwargs: Any
) -> None:
for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
text_message = {"text": message_part}
message = self.prepare_message(recipient_id, text_message)
await self.send(message)
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ async def _persist_message(self, message: Dict[Text, Any]) -> None:
async def send_text_message(
self, recipient_id: Text, text: Text, **kwargs: Any
) -> None:
for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
await self._persist_message(self._message(recipient_id, text=message_part))

async def send_image_url(
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/facebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ async def send_text_message(
) -> None:
"""Send a message through this channel."""

for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
self.send(recipient_id, FBText(text=message_part))

async def send_image_url(
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/mattermost.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def _post_data_to_channel(self, data) -> Response:
async def send_text_message(
self, recipient_id: Text, text: Text, **kwargs: Any
) -> None:
for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
self._post_message_to_channel(self.bot_channel, message_part)

async def send_custom_json(
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/rocketchat.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async def send_text_message(
) -> None:
"""Send message to output channel"""

for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
self.rocket.chat_post_message(message_part, room_id=recipient_id)

async def send_image_url(
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ async def send_text_message(
self, recipient_id: Text, text: Text, **kwargs: Any
) -> None:
recipient = self.slack_channel or recipient_id
for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
await self.client.chat_postMessage(
channel=recipient, as_user=True, text=message_part, type="mrkdwn",
)
Expand Down
4 changes: 2 additions & 2 deletions rasa/core/channels/socketio.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ async def send_text_message(
) -> None:
"""Send a message through this channel."""

for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
await self._send_message(self.sid, {"text": message_part})

async def send_image_url(
Expand All @@ -66,7 +66,7 @@ async def send_text_with_buttons(
# split text and create a message for each text fragment
# the `or` makes sure there is at least one message we can attach the quick
# replies to
message_parts = text.split("\n\n") or [text]
message_parts = text.strip().split("\n\n") or [text]
messages = [{"text": message, "quick_replies": []} for message in message_parts]

# attach all buttons to the last text fragment
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/telegram.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def __init__(self, access_token: Optional[Text]) -> None:
async def send_text_message(
self, recipient_id: Text, text: Text, **kwargs: Any
) -> None:
for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
self.send_message(recipient_id, message_part)

async def send_image_url(
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/twilio.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ async def send_text_message(
"""Sends text message"""

message_data = {"to": recipient_id, "from_": self.twilio_number}
for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
message_data.update({"body": message_part})
await self._send_message(message_data)

Expand Down
2 changes: 1 addition & 1 deletion rasa/core/channels/webexteams.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async def send_text_message(
self, recipient_id: Text, text: Text, **kwargs: Any
) -> None:
recipient = self.room or recipient_id
for message_part in text.split("\n\n"):
for message_part in text.strip().split("\n\n"):
self.api.messages.create(roomId=recipient, text=message_part)

async def send_image_url(
Expand Down
28 changes: 22 additions & 6 deletions tests/core/test_channels.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ def fake_send_message(*args, **kwargs):

async def test_send_response(default_channel, default_tracker):
text_only_message = {"text": "hey"}
multiline_text_message = {
"text": "This message should come first: \n\nThis is message two \nThis as well\n\n"
}
image_only_message = {"image": "https://i.imgur.com/nGF1K8f.jpg"}
text_and_image_message = {
"text": "look at this",
Expand All @@ -68,32 +71,45 @@ async def test_send_response(default_channel, default_tracker):
}

await default_channel.send_response(default_tracker.sender_id, text_only_message)
await default_channel.send_response(
default_tracker.sender_id, multiline_text_message
)
await default_channel.send_response(default_tracker.sender_id, image_only_message)
await default_channel.send_response(
default_tracker.sender_id, text_and_image_message
)
await default_channel.send_response(default_tracker.sender_id, custom_json_message)
collected = default_channel.messages

assert len(collected) == 6
assert len(collected) == 8

# text only message
assert collected[0] == {"recipient_id": "my-sender", "text": "hey"}

# image only message
# multiline text message, should split on '\n\n'
assert collected[1] == {
"recipient_id": "my-sender",
"image": "https://i.imgur.com/nGF1K8f.jpg",
"text": "This message should come first: ",
}
assert collected[2] == {
"recipient_id": "my-sender",
"text": "This is message two \nThis as well",
}

# text & image combined - will result in two messages
assert collected[2] == {"recipient_id": "my-sender", "text": "look at this"}
# image only message
assert collected[3] == {
"recipient_id": "my-sender",
"image": "https://i.imgur.com/T5xVo.jpg",
"image": "https://i.imgur.com/nGF1K8f.jpg",
}

# text & image combined - will result in two messages
assert collected[4] == {"recipient_id": "my-sender", "text": "look at this"}
assert collected[5] == {
"recipient_id": "my-sender",
"image": "https://i.imgur.com/T5xVo.jpg",
}
assert collected[6] == {"recipient_id": "my-sender", "text": "look at this"}
assert collected[7] == {
"recipient_id": "my-sender",
"custom": {"some_random_arg": "value", "another_arg": "value2"},
}
Expand Down