Repository: python-telegram-bot/python-telegram-bot
Pull request: Add property `Update.effective_sender`
Closes #4085.

- [x] Added ``.. versionadded:: NEXT.VERSION``, ``.. versionchanged:: NEXT.VERSION`` or ``.. deprecated:: NEXT.VERSION`` to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
- [x] Created new or adapted existing unit tests
- [x] Documented code changes according to the `CSI standard <https://standards.mousepawmedia.com/en/stable/csi.html>`__
- [ ] ~Added myself alphabetically to ``AUTHORS.rst`` (optional)~
- [ ] ~Added new classes & modules to the docs and all suitable ``__all__`` s~
- [ ] ~Checked the `Stability Policy <https://docs.python-telegram-bot.org/stability_policy.html>`_ in case of deprecations or changes to documented behavior~

-  If relevant:
   - [x] Added or updated documentation for the changed class(es) and/or method(s)

Problem statement:
[FEATURE] Add Update.effective_sender
We have `Update.effective_user` for the sender of the message. This attribute however misses out on the anonymous sender info provided in `Message.sender_chat`, `MessageReactionUpdated.action_chat` and `Poll.voter_chat`.

I would propose adding a new shortcut to the `Update` class, `effective_sender`. It either returns the sender chats above or effective_user, if they are not set.

Hints from issue discussion:
Hi, I would like to work on this

Alright, have fun!
@Poolitzer Please keep in mind that this is dependent on the ongoing API 7.0 pr
Oh right, you should base your branch on this. So either wait until it is merged with master and then do the PR or branch it on the api branch and PR towards it if you are eager to do it now.
I can wait. Until then I can take a look at the code base and understand the req better

> I can wait. Until then I can take a look at the code base and understand the req better

@nemo-1999 you may start working on this now since it's already merged to master

Commit message: Add tests.

Patch to review:
diff --git a/telegram/_update.py b/telegram/_update.py
index 566ca9cfd3f..151bde24161 100644
--- a/telegram/_update.py
+++ b/telegram/_update.py
@@ -18,7 +18,7 @@
 # along with this program.  If not, see [http://www.gnu.org/licenses/].
 """This module contains an object that represents a Telegram Update."""
 
-from typing import TYPE_CHECKING, Final, List, Optional
+from typing import TYPE_CHECKING, Final, List, Optional, Union
 
 from telegram import constants
 from telegram._callbackquery import CallbackQuery
@@ -224,6 +224,7 @@ class Update(TelegramObject):
     __slots__ = (
         "_effective_chat",
         "_effective_message",
+        "_effective_sender",
         "_effective_user",
         "callback_query",
         "channel_post",
@@ -371,6 +372,7 @@ def __init__(
         self.message_reaction_count: Optional[MessageReactionCountUpdated] = message_reaction_count
 
         self._effective_user: Optional[User] = None
+        self._effective_sender: Optional[Union["User", "Chat"]] = None
         self._effective_chat: Optional[Chat] = None
         self._effective_message: Optional[Message] = None
 
@@ -444,6 +446,56 @@ def effective_user(self) -> Optional["User"]:
         self._effective_user = user
         return user
 
+    @property
+    def effective_sender(self) -> Optional[Union["User", "Chat"]]:
+        """
+        :class:`telegram.User` or :class:`telegram.Chat`: The user or chat that sent this update,
+        no matter what kind of update this is.
+
+        Note:
+            * When the user is anonymous this is either :class:`telegram.Chat` or :obj:`None`.
+            * If the user is not anonymous it's either :class:`telegram.User` or :obj:`None`.
+
+        If no user whatsoever is associated with this update, this gives :obj:`None`. This
+        is the case if any of
+
+        * :attr:`channel_post`
+        * :attr:`edited_channel_post`
+        * :attr:`poll`
+        * :attr:`chat_boost`
+        * :attr:`removed_chat_boost`
+        * :attr:`message_reaction_count`
+
+        is present.
+
+        Example:
+            * If :attr:`message` is present, this will give either
+              :attr:`telegram.Message.from_user` or :attr:`telegram.Message.sender_chat`.
+            * If :attr:`poll_answer` is present, this will give either
+              :attr:`telegram.PollAnswer.user` or :attr:`telegram.PollAnswer.voter_chat`.
+
+        .. versionadded:: NEXT.VERSION
+        """
+        if self._effective_sender:
+            return self._effective_sender
+
+        sender: Optional[Union["User", "Chat"]] = None
+
+        if message := (self.message or self.edited_message):
+            sender = message.sender_chat
+
+        elif self.poll_answer:
+            sender = self.poll_answer.voter_chat
+
+        elif self.message_reaction:
+            sender = self.message_reaction.actor_chat
+
+        if sender is None:
+            sender = self.effective_user
+
+        self._effective_sender = sender
+        return sender
+
     @property
     def effective_chat(self) -> Optional["Chat"]:
         """
diff --git a/tests/test_update.py b/tests/test_update.py
index 7c7a35a4c02..139192b346e 100644
--- a/tests/test_update.py
+++ b/tests/test_update.py
@@ -51,7 +51,14 @@
 from telegram.warnings import PTBUserWarning
 from tests.auxil.slots import mro_slots
 
-message = Message(1, datetime.utcnow(), Chat(1, ""), from_user=User(1, "", False), text="Text")
+message = Message(
+    1,
+    datetime.utcnow(),
+    Chat(1, ""),
+    from_user=User(1, "", False),
+    text="Text",
+    sender_chat=Chat(1, ""),
+)
 chat_member_updated = ChatMemberUpdated(
     Chat(1, "chat"),
     User(1, "", False),
@@ -93,6 +100,7 @@
     old_reaction=(ReactionTypeEmoji("👍"),),
     new_reaction=(ReactionTypeEmoji("👍"),),
     user=User(1, "name", False),
+    actor_chat=Chat(1, ""),
 )
 
 
@@ -261,6 +269,47 @@ def test_effective_user(self, update):
         else:
             assert user is None
 
+    def test_effective_sender(self, update):
+        # Test that it's sometimes None per docstring
+        sender = update.effective_sender
+        if not (
+            update.channel_post is not None
+            or update.edited_channel_post is not None
+            or update.poll is not None
+            or update.chat_boost is not None
+            or update.removed_chat_boost is not None
+            or update.message_reaction_count is not None
+        ):
+            assert sender.id == 1
+        else:
+            assert sender is None
+
+    def test_effective_sender_anonymous(self, update):
+        # remove none anonymous user
+        if message := (update.message or update.edited_message):
+            message._unfreeze()
+            message.from_user = None
+        elif reaction := (update.message_reaction):
+            reaction._unfreeze()
+            reaction.user = None
+        elif answer := (update.poll_answer):
+            answer._unfreeze()
+            answer.user = None
+
+        # Test that it's sometimes None per docstring
+        sender = update.effective_sender
+        if not (
+            update.channel_post is not None
+            or update.edited_channel_post is not None
+            or update.poll is not None
+            or update.chat_boost is not None
+            or update.removed_chat_boost is not None
+            or update.message_reaction_count is not None
+        ):
+            assert sender.id == 1
+        else:
+            assert sender is None
+
     def test_effective_message(self, update):
         # Test that it's sometimes None per docstring
         eff_message = update.effective_message


Code review comments:
diff_hunk: @@ -444,6 +446,56 @@ def effective_user(self) -> Optional["User"]:
         self._effective_user = user
         return user
 
+    @property
+    def effective_sender(self) -> Optional[Union["User", "Chat"]]:
+        """
+        :class:`telegram.User` or :class:`telegram.Chat`: The user or chat that sent this update,
+        no matter what kind of update this is.
+
+        Note:
+            * When the user is anonymous this is either :class:`telegram.Chat` or :obj:`None`.
+            * If the user is not anonymous it's either :class:`telegram.User` or :obj:`None`.
+
+        If no user whatsoever is associated with this update, this gives :obj:`None`. This
+        is the case if any of
+
+        * :attr:`channel_post`
+        * :attr:`edited_channel_post`
path: telegram/_update.py
text: @user1:
I strongly suggest using `message.sender_chat` also in these cases. Not including a field called `sender_chat` in this property doesn't make much sense to me :D

diff_hunk: @@ -261,6 +269,47 @@ def test_effective_user(self, update):
         else:
             assert user is None
 
+    def test_effective_sender(self, update):
+        # Test that it's sometimes None per docstring
+        sender = update.effective_sender
+        if not (
+            update.channel_post is not None
+            or update.edited_channel_post is not None
+            or update.poll is not None
+            or update.chat_boost is not None
+            or update.removed_chat_boost is not None
+            or update.message_reaction_count is not None
+        ):
+            assert sender.id == 1
path: tests/test_update.py
text: @user1:
I would have expected something like `assert isinstance(sender, Chat/User)` here making sure that we're getting the right attribute for the right use cases 🤔 

@author:
Right. this is why i intended to have two tests, although currently not working as you suggest..

`test_effective_sender_non_anonymous`: makes sure sender is either `User` or `None`
`test_effective_sender_anonymous`: makes sure sender is either `Chat`, `User` or `None`

i'll make them them more clear 😅