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

Treat list custom json facebook #9567

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
2cebc45
Add list handling in the send_custom_json method.
silvasara Nov 11, 2020
f69f4d2
Fix lint problem
silvasara Dec 7, 2020
728dc91
adding tests and create a facebook file test
jppgomes Mar 11, 2021
d2e50c2
Fixed issue (#5657) in MessengerBot.send_custom_json
wavymazy Mar 21, 2021
9f3bfcc
Merge branch 'main' into treat-list-custom-json-facebook
HotThoughts Apr 6, 2021
d8c8b66
Merge branch 'main' into treat-list-custom-json-facebook
HotThoughts Apr 8, 2021
aa7b4eb
add a support guide for macOS users regarding rasa development instal…
ErickGiffoni Aug 7, 2021
3966ece
update README with a link to the new macOS support guide
ErickGiffoni Aug 7, 2021
a21deeb
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 7, 2021
f007a30
Apply suggestions from code review
ErickGiffoni Aug 7, 2021
fcf7893
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 9, 2021
9ae8c35
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 10, 2021
a87ab3d
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 12, 2021
773e821
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 16, 2021
0006416
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 17, 2021
317dc7b
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 18, 2021
de28d04
Merge 2.8.2 to main (#9285)
aeshky Aug 6, 2021
0b7485f
Apply suggestions from code review
ErickGiffoni Aug 7, 2021
cef6c77
Merge branch 'macOS-compiler-issues' of https://github.com/FGA-GCES/r…
ErickGiffoni Aug 31, 2021
4b6e650
Merge branch 'main' into macOS-compiler-issues
ErickGiffoni Aug 31, 2021
1dcfd33
Merge branch 'main' of git://github.com/RasaHQ/rasa into HEAD
ErickGiffoni Sep 7, 2021
0d05a9d
Merge commit '1dcfd33e71b' into macOS-compiler-issues
ErickGiffoni Sep 7, 2021
4993ea1
Merge branch 'macOS-compiler-issues' of https://github.com/FGA-GCES/r…
ErickGiffoni Sep 7, 2021
d1a03d1
Merge branch 'treat-list-custom-json-facebook' of https://github.com/…
ErickGiffoni Sep 7, 2021
ee0ddb2
remove the unused imports and comments
ErickGiffoni Sep 7, 2021
49174dd
uses a Union for the json_message, as suggested here #8332
ErickGiffoni Sep 7, 2021
8a642d2
removing irrelevant file
ErickGiffoni Sep 8, 2021
831e3a3
updates README with branch main
ErickGiffoni Sep 8, 2021
841e6e7
updates file test_unexpected_intent_policy with branch main
ErickGiffoni Sep 8, 2021
f8b3a28
updates file test_ted_policy with branch main
ErickGiffoni Sep 8, 2021
0214197
Merge branch 'main' into treat-list-custom-json-facebook
ErickGiffoni Sep 8, 2021
895a068
Merge branch 'main' into treat-list-custom-json-facebook
ErickGiffoni Sep 9, 2021
a11a6b2
pass recipient_id as return value when pop fails
ErickGiffoni Sep 21, 2021
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,4 +404,4 @@ Copyright 2021 Rasa Technologies GmbH. [Copy of the license](LICENSE.txt).

A list of the Licenses of the dependencies of the project can be found at
the bottom of the
[Libraries Summary](https://libraries.io/github/RasaHQ/rasa).
[Libraries Summary](https://libraries.io/github/RasaHQ/rasa).
1 change: 1 addition & 0 deletions changelog/5657.improvement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add list handling in the `send_custom_json` method on `channels/facebook.py`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this changelog entry should actually be a bugfix rather than improvement based on the original issue. It would also be great if you could give more details around the bug since it isn't very clear for me in which circumstances would the json_message be of type list rather than dict.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Done in a new branch rebase-treat-custom-json-fb.
I haven't pushed it yet, but soon I will and I'll also open the new PR.

11 changes: 7 additions & 4 deletions rasa/core/channels/facebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import rasa.shared.utils.io
from sanic import Blueprint, response
from sanic.request import Request
from typing import Text, List, Dict, Any, Callable, Awaitable, Iterable, Optional
from typing import Text, List, Dict, Any, Callable, Awaitable, Iterable, Optional, Union

from rasa.core.channels.channel import UserMessage, OutputChannel, InputChannel
from sanic.response import HTTPResponse
Expand Down Expand Up @@ -276,11 +276,14 @@ async def send_elements(
self.messenger_client.send(payload, recipient_id, "RESPONSE")

async def send_custom_json(
self, recipient_id: Text, json_message: Dict[Text, Any], **kwargs: Any
self,
recipient_id: Text,
json_message: Union[List, Dict[Text, Any]],
**kwargs: Any,
) -> None:
"""Sends custom json data to the output."""

recipient_id = json_message.pop("sender", {}).pop("id", None) or recipient_id
if isinstance(json_message, dict) and "sender" in json_message.keys():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs a check if json_message is list and assigning recipient_id to the extracted id from json_message.
Potentially also json_message needs to be updated to be of dict type when being given as an argument to self.messenger_client.send(), not entirely sure since Facebook code doesn't have type annotations.
I'd advise researching a bit how to test that this works with MessengerClient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, except the part regarding json_message being updated to dict.
It seams to be ok passing a list to self.messenger_client.send().

recipient_id = json_message.pop("sender", {}).pop("id", recipient_id)

self.messenger_client.send(json_message, recipient_id, "RESPONSE")

Expand Down
91 changes: 91 additions & 0 deletions tests/test_facebook.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import logging
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please shift this file into the correct subdirectory: tests/core/channels?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

from rasa.core.channels.facebook import MessengerBot
from fbmessenger import MessengerClient

from _pytest.monkeypatch import MonkeyPatch

import rasa.core.run
from rasa.core import utils


# this is needed so that the tests included as code examples look better
from tests.utilities import json_of_latest_request, latest_request

logger = logging.getLogger(__name__)


def test_facebook_channel():
from rasa.core.channels.facebook import FacebookInput

input_channel = FacebookInput(
fb_verify="YOUR_FB_VERIFY",
# you need tell facebook this token, to confirm your URL
fb_secret="YOUR_FB_SECRET", # your app secret
fb_access_token="YOUR_FB_PAGE_ACCESS_TOKEN"
# token for the page you subscribed to
)

s = rasa.core.run.configure_app([input_channel], port=5004)

routes_list = utils.list_routes(s)

assert routes_list["fb_webhook.health"].startswith("/webhooks/facebook")
assert routes_list["fb_webhook.webhook"].startswith("/webhooks/facebook/webhook")


async def test_facebook_send_custom_json():
# This function tests cases when the custom json is a list
# The send_custom_json function doesn't return anything. Rather
# it calls an object MessengerClient, that will
# then make a post request.
# Since the purpose is to test the extraction of the recipient_id
# by the MessengerBot.send_custom_json_list we
# modify MessengerClient (from the fbmessenger pypackage) to
# return the recipient ID.

json_without_id = {
"blocks": [
{"type": "title", "text": {"text": "Conversation progress"}},
{
"type": "progression_bar",
"text": {"text": "progression 1", "level": "1"},
},
]
}
json_with_id = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should also check that send_custom_json works with json_message as list.
I would also suggest using pytest.mark.parametrize for different json inputs and expected values of recipient_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"blocks": [
{"type": "title", "text": {"text": "Conversation progress"}},
{
"type": "progression_bar",
"text": {"text": "progression 1", "level": "1"},
},
],
"sender": {"id": "test_json_id"},
}

class TestableMessengerClient(MessengerClient):
def __init__(self, page_access_token, **kwargs):
self.recipient_id = ""
super(TestableMessengerClient, self).__init__(page_access_token, **kwargs)

def send(
self,
payload,
recipient_id,
messaging_type="RESPONSE",
notification_type="REGULAR",
timeout=None,
tag=None,
):
self.recipient_id = recipient_id

messenger_client = TestableMessengerClient(page_access_token="test_token")
messenger_bot = MessengerBot(messenger_client)
await messenger_bot.send_custom_json(
recipient_id="test_id", json_message=json_without_id
)
assert messenger_bot.messenger_client.recipient_id == "test_id"
await messenger_bot.send_custom_json(
recipient_id="test_id", json_message=json_with_id
)
assert messenger_bot.messenger_client.recipient_id == "test_json_id"