Fixed _usersApproval, fixed changeThreadImage methods, more tests

fbchat/client.py

@@ -1300,7 +1300,7 @@ class Client(object):
     def _usersApproval(self, user_ids, approve, thread_id=None):
         thread_id, thread_type = self._getThread(thread_id, None)
 
-        user_ids = require_list(user_ids)
+        user_ids = list(require_list(user_ids))
 
         j = self.graphql_request(GraphQL(doc_id='1574519202665847', params={
             'data': {
@@ -1333,7 +1333,7 @@ class Client(object):
         """
         self._usersApproval(user_ids, False, thread_id)
 
-    def changeThreadImage(self, image_id, thread_id=None, thread_type=ThreadType.USER):
+    def changeThreadImage(self, image_id, thread_id=None, thread_type=ThreadType.GROUP):
         """
         Changes a thread image from an image id
 
@@ -1348,15 +1348,16 @@ class Client(object):
 
         if thread_type != ThreadType.GROUP:
             raise FBchatUserError('Can only change the image of group threads')
-        else:
-            data = {
-                'thread_image_id': image_id,
-                'thread_id': thread_id
-            }
 
-            j = self._post(self.req_url.THREAD_IMAGE, data, fix_request=True, as_json=True)
+        data = {
+            'thread_image_id': image_id,
+            'thread_id': thread_id
+        }
 
-    def changeThreadImageRemote(self, image_url, thread_id=None, thread_type=ThreadType.USER):
+        j = self._post(self.req_url.THREAD_IMAGE, data, fix_request=True, as_json=True)
+        return image_id
+
+    def changeThreadImageRemote(self, image_url, thread_id=None, thread_type=ThreadType.GROUP):
         """
         Changes a thread image from a URL
 
@@ -1367,12 +1368,10 @@ class Client(object):
         :raises: FBchatException if request failed
         """
 
-        with get_files_from_urls([image_url]) as files:
-            (image_id, mimetype), = self._upload(files)
+        (image_id, mimetype), = self._upload(get_files_from_urls([image_url]))
+        return self.changeThreadImage(image_id, thread_id, thread_type)
 
-        self.changeThreadImage(image_id, thread_id, thread_type)
-
-    def changeThreadImageLocal(self, image_path, thread_id=None, thread_type=ThreadType.USER):
+    def changeThreadImageLocal(self, image_path, thread_id=None, thread_type=ThreadType.GROUP):
         """
         Changes a thread image from a local path
 
@@ -1386,7 +1385,7 @@ class Client(object):
         with get_files_from_paths([image_path]) as files:
             (image_id, mimetype), = self._upload(files)
 
-        self.changeThreadImage(image_id, thread_id, thread_type)
+        return self.changeThreadImage(image_id, thread_id, thread_type)
 
     def changeThreadTitle(self, title, thread_id=None, thread_type=ThreadType.USER):
         """
@@ -2071,7 +2070,7 @@ class Client(object):
                             if fetch_data.get("__typename") == "ThreadImageMessage":
                                 # Thread image change
                                 image_metadata = fetch_data.get("image_with_metadata")
-                                image_id = image_metadata["legacy_attachment_id"] if image_metadata else None
+                                image_id = int(image_metadata["legacy_attachment_id"]) if image_metadata else None
                                 self.onImageChange(mid=mid, author_id=author_id, new_image=image_id, thread_id=thread_id,
                                                    thread_type=ThreadType.GROUP, ts=ts)
 

tests/test_fetch.py

@@ -86,9 +86,7 @@ def test_fetch_info(client1, group):
 
 
 def test_fetch_image_url(client):
-    url = path.join(path.dirname(__file__), "resources", "image.png")
-
-    client.sendLocalImage(url)
+    client.sendLocalFiles([path.join(path.dirname(__file__), "resources", "image.png")])
     message, = client.fetchThreadMessages(limit=1)
 
     assert client.fetchImageUrl(message.attachments[0].uid)

tests/test_send.py

@@ -102,3 +102,8 @@ def test_send_remote_files(client, catch_event, compare):
     assert compare(x, mid=mid, message=text)
     assert subset(vars(x.res["message_object"]), uid=mid, author=client.uid, text=text)
     assert len(x.res["message_object"].attachments) == len(files)
+
+
+@pytest.mark.parametrize('wave_first', [True, False])
+def test_wave(client, wave_first):
+    client.wave(wave_first)

tests/test_thread_interraction.py

@@ -12,7 +12,7 @@ from fbchat.models import (
     ThreadColor,
 )
 from utils import random_hex, subset
-from os import environ
+from os import path
 
 
 def test_remove_from_and_add_to_group(client1, client2, group, catch_event):
@@ -47,10 +47,7 @@ def test_remove_from_and_add_admins_to_group(client1, client2, group, catch_even
         )
 
 
-@pytest.mark.xfail(
-    raises=FBchatFacebookError, reason="Apparently changeThreadTitle is broken"
-)
-def test_change_title(client1, catch_event, group):
+def test_change_title(client1, group, catch_event):
     title = random_hex()
     with catch_event("onTitleChange") as x:
         mid = client1.changeThreadTitle(title, group["id"])
@@ -71,17 +68,33 @@ def test_change_nickname(client, client_all, catch_event, compare):
     assert compare(x, changed_for=client_all.uid, new_nickname=nickname)
 
 
-@pytest.mark.parametrize("emoji", ["😀", "😂", "😕", "😍"])
+@pytest.mark.parametrize("emoji", [
+    "😀",
+    "😂",
+    "😕",
+    "😍",
+    pytest.mark.xfail("🙃", raises=FBchatFacebookError),
+    pytest.mark.xfail("not an emoji", raises=FBchatFacebookError)
+])
 def test_change_emoji(client, catch_event, compare, emoji):
     with catch_event("onEmojiChange") as x:
         client.changeThreadEmoji(emoji)
     assert compare(x, new_emoji=emoji)
 
 
-@pytest.mark.xfail(raises=FBchatFacebookError)
-@pytest.mark.parametrize("emoji", ["🙃", "not an emoji"])
-def test_change_emoji_invalid(client, emoji):
-    client.changeThreadEmoji(emoji)
+def test_change_thread_image_local(client1, group, catch_event):
+    url = path.join(path.dirname(__file__), "resources", "image.png")
+    with catch_event("onImageChange") as x:
+        image_id = client1.changeThreadImageLocal(url, group["id"])
+    assert subset(x.res, new_image=image_id, author_id=client1.uid, thread_id=group["id"])
+
+
+# To be changed when merged into master
+def test_change_thread_image_remote(client1, group, catch_event):
+    url = "https://github.com/carpedm20/fbchat/raw/master/tests/image.png"
+    with catch_event("onImageChange") as x:
+        image_id = client1.changeThreadImageRemote(url, group["id"])
+    assert subset(x.res, new_image=image_id, author_id=client1.uid, thread_id=group["id"])
 
 
 @pytest.mark.parametrize(
@@ -99,9 +112,7 @@ def test_change_color(client, catch_event, compare, color):
     assert compare(x, new_color=color)
 
 
-@pytest.mark.xfail(
-    raises=FBchatFacebookError, strict=False, reason="Should fail, but doesn't"
-)
+@pytest.mark.xfail(raises=FBchatFacebookError, reason="Should fail, but doesn't")
 def test_change_color_invalid(client):
     class InvalidColor:
         value = "#0077ff"
@@ -114,3 +125,16 @@ def test_typing_status(client, catch_event, compare, status):
     with catch_event("onTyping") as x:
         client.setTypingStatus(status)
     assert compare(x, status=status)
+
+
+@pytest.mark.parametrize('require_admin_approval', [True, False])
+def test_change_approval_mode(client1, group, catch_event, require_admin_approval):
+    with catch_event("onApprovalModeChange") as x:
+        client1.changeGroupApprovalMode(require_admin_approval, group["id"])
+
+    assert subset(
+        x.res,
+        approval_mode=require_admin_approval,
+        author_id=client1.uid,
+        thread_id=group["id"],
+    )