Repository: python-telegram-bot/python-telegram-bot
Pull request: Auto Pagination for answer_inline_query
As discussed in #2071.

Closes #2071

Problem statement:
[FEATURE] Auto-pagination for inline results
#### Is your feature request related to a problem? Please describe.
TG requires pagination for inline results using the `next_offset` parameter. Currently this has to be implemented manually, but the logic is always the same, so it's basically copy paste.

I think it would fit well into PTBs "more than just a wrapper" credo to provide a convenience method for that.

#### Describe the solution you'd like

Add a parameters `pagination` and `current_offset` to `answer_inline_query` to make PTB take care of that automically. it should be along the lines

``` python
offset = current_offset
max_length = 50
next_offset = None

if not offset:
    if len(results) > max_length :
        next_offset = len(results) - max_length 
        next_offset = next_offset or None
        results = results[:max_length]
elif len(results) > offset:
    results = results[offset:offset + max_length]
    if len(results) > offset + max_length:
        next_offset = offset + max_length
else:
    next_offset = ''
```

#### Describe alternatives you've considered
If we don't want to integrate this directly into `answer_inline_query`, we could at least add a function to `utils.helpers` , along the lines
```python
def paginate_inline_results(reslts: List[InlineQueryResult], current_offset: int) -> {'results': …, 'next_offset': …}
```
so one could call it like

```python
answer_inline_query(**paginate_inline_results(results, offset), …)
```

Hints from issue discussion:
@Bibo-Joshi I think I would preferer an opt out implementation with a flag into the function
Consider that expensive result generators might slow the execution down and be wasteful overall. I love the idea, and maybe an inbetween step should be a result generator that is passed the current `page` index, so that the developer can avoid overfetching but doesn't have to bother with the offset parameter. 

```
ResultFetcher = Callable[[int], Iterable[InlineResult]]
```


```
answer_ilq(my_get_more_results_callback)
```

This would become especially useful in an asyncio environment (in the future).
> Consider that expensive result generators might slow the execution down and be wasteful overall.

You mean "don't truncate a list of results, but just build the required results in the first place"? Makes sense for advanced use cases, for sure.

My concern with adding parameters to `answer_ilq` is that we're currently not fiddling with the official methods too much (everything has `timeout`, `Message` classmethods have `quote` and for some parameters we are more genereous, like `open()` instead of `InputFile` and `['button']` instead of `[ReplyButton()]`) and I wouldn't wanna go in a direction where just add a bunch of parameters everywhere. That's why I wanted this to be discussed before PRing :D

Adding to `answer_ilq`, we could go with adding `current_offset` and allowing for `results` to be a callback as @JosXa suggested. Then passing `current_offset` would mean that auto-pagination is to be used and only then would a callback be acceptable for `results`. This would me relatively minimal invasive.
In the `utils.helpers` scenario, we could basically do the same, or make `results` and `results_generator` two mutually exclusive parameters
> Adding to answer_ilq, we could go with adding current_offset and allowing for results to be a callback as @JosXa suggested. Then passing current_offset would mean that auto-pagination is to be used and only then would a callback be acceptable for results. 

For the `InlineQuery.answer()` shortcut we would have to replace the `current_offset` parameter by an `auto_pagination` boolean flag (or similar naming), as we'd use `current_offset=self.next_offset` for `self.bot.answer_inline_query` automatically.

Commit message: Tweak tests

Patch to review:
diff --git a/telegram/bot.py b/telegram/bot.py
index 86fb0ef6310..73cf533695f 100644
--- a/telegram/bot.py
+++ b/telegram/bot.py
@@ -39,6 +39,7 @@
                       ReplyMarkup, TelegramObject, WebhookInfo, GameHighScore, StickerSet,
                       PhotoSize, Audio, Document, Sticker, Video, Animation, Voice, VideoNote,
                       Location, Venue, Contact, InputFile, Poll, BotCommand)
+from telegram.constants import MAX_INLINE_QUERY_RESULTS
 from telegram.error import InvalidToken, TelegramError
 from telegram.utils.helpers import to_timestamp, DEFAULT_NONE
 from telegram.utils.request import Request
@@ -1526,6 +1527,7 @@ def answer_inline_query(self,
                             switch_pm_text=None,
                             switch_pm_parameter=None,
                             timeout=None,
+                            current_offset=None,
                             **kwargs):
         """
         Use this method to send answers to an inline query. No more than 50 results per query are
@@ -1533,8 +1535,12 @@ def answer_inline_query(self,
 
         Args:
             inline_query_id (:obj:`str`): Unique identifier for the answered query.
-            results (List[:class:`telegram.InlineQueryResult`)]: A list of results for the inline
-                query.
+            results (List[:class:`telegram.InlineQueryResult`] | Callable): A list of results for
+                the inline query. In case :attr:`current_offset` is passed, ``results`` may also be
+                a callable taking a single integer argument, being the 0-based current number of
+                the result pagination. It must return either a list of
+                :class:`telegram.InlineResult` instances or :obj:`None`, if there are no more
+                results.
             cache_time (:obj:`int`, optional): The maximum amount of time in seconds that the
                 result of the inline query may be cached on the server. Defaults to 300.
             is_personal (:obj:`bool`, optional): Pass :obj:`True`, if results may be cached on
@@ -1550,6 +1556,10 @@ def answer_inline_query(self,
             switch_pm_parameter (:obj:`str`, optional): Deep-linking parameter for the /start
                 message sent to the bot when user presses the switch button. 1-64 characters,
                 only A-Z, a-z, 0-9, _ and - are allowed.
+            current_offset (:obj:`str`, optional): The :attr:`telegram.InlineQuery.offset` of
+                the inline query to answer. If passed, PTB will automatically take care of
+                the pagination for you, i.e. pass set the correct ``next_offset`` and truncate the
+                results list/call the callable you passed.
             timeout (:obj:`int` | :obj:`float`, optional): If this value is specified, use it as
                 the read timeout from the server (instead of the one specified during creation of
                 the connection pool).
@@ -1573,6 +1583,32 @@ def answer_inline_query(self,
         """
         url = '{}/answerInlineQuery'.format(self.base_url)
 
+        if current_offset is not None and next_offset is not None:
+            raise ValueError('`current_offset` and `next_offset` are mutually exclusive!')
+
+        if current_offset is not None:
+            if current_offset == '':
+                current_offset = 0
+            else:
+                current_offset = int(current_offset)
+
+            next_offset = ''
+
+            if callable(results):
+                results = results(current_offset)
+                if not results:
+                    results = []
+                else:
+                    next_offset = current_offset + 1
+            else:
+                if len(results) > (current_offset + 1) * MAX_INLINE_QUERY_RESULTS:
+                    next_offset = current_offset + 1
+                    results = results[
+                        current_offset * MAX_INLINE_QUERY_RESULTS:
+                        current_offset * MAX_INLINE_QUERY_RESULTS + MAX_INLINE_QUERY_RESULTS]
+                else:
+                    results = results[current_offset * MAX_INLINE_QUERY_RESULTS:]
+
         for res in results:
             if res._has_parse_mode and res.parse_mode == DEFAULT_NONE:
                 if self.defaults:
@@ -1595,9 +1631,7 @@ def answer_inline_query(self,
                         res.input_message_content.disable_web_page_preview = None
 
         results = [res.to_dict() for res in results]
-
         data = {'inline_query_id': inline_query_id, 'results': results}
-
         if cache_time or cache_time == 0:
             data['cache_time'] = cache_time
         if is_personal:
diff --git a/telegram/inline/inlinequery.py b/telegram/inline/inlinequery.py
index 3c6df66a792..7648a2f6ee2 100644
--- a/telegram/inline/inlinequery.py
+++ b/telegram/inline/inlinequery.py
@@ -75,14 +75,21 @@ def de_json(cls, data, bot):
 
         return cls(bot=bot, **data)
 
-    def answer(self, *args, **kwargs):
+    def answer(self, *args, auto_pagination=False, **kwargs):
         """Shortcut for::
 
-            bot.answer_inline_query(update.inline_query.id, *args, **kwargs)
+            bot.answer_inline_query(update.inline_query.id,
+                                    *args,
+                                    current_offset = self.offset if auto_pagination else None,
+                                    **kwargs)
 
         Args:
-            results (List[:class:`telegram.InlineQueryResult`]): A list of results for the inline
-                query.
+            results (List[:class:`telegram.InlineQueryResult`] | Callable): A list of results for
+                the inline query. In case :attr:`auto_pagination` is set to :obj:`True`,
+                ``results`` may also be a callable taking a single integer argument, being the
+                0-based current number of the result pagination. It must return either a list of
+                :class:`telegram.InlineResult` instances or :obj:`None`, if there are no more
+                results.
             cache_time (:obj:`int`, optional): The maximum amount of time in seconds that the
                 result of the inline query may be cached on the server. Defaults to 300.
             is_personal (:obj:`bool`, optional): Pass :obj:`True`, if results may be cached on the
@@ -98,6 +105,14 @@ def answer(self, *args, **kwargs):
             switch_pm_parameter (:obj:`str`, optional): Deep-linking parameter for the /start
                 message sent to the bot when user presses the switch button. 1-64 characters,
                 only A-Z, a-z, 0-9, _ and - are allowed.
+            auto_pagination (:obj:`bool`, optional): If set to :obj:`True`, :attr:`offset` will be
+                passed as :attr:`current_offset` to :meth:telegram.Bot.answer_inline_query`.
+                Defaults to :obj:`False`.
 
         """
-        return self.bot.answer_inline_query(self.id, *args, **kwargs)
+        return self.bot.answer_inline_query(
+            self.id,
+            *args,
+            current_offset=self.offset if auto_pagination else None,
+            **kwargs
+        )
diff --git a/tests/test_bot.py b/tests/test_bot.py
index e708b45d3d9..b117d8db3c4 100644
--- a/tests/test_bot.py
+++ b/tests/test_bot.py
@@ -27,6 +27,7 @@
                       InlineKeyboardButton, InlineQueryResultArticle, InputTextMessageContent,
                       ShippingOption, LabeledPrice, ChatPermissions, Poll, BotCommand,
                       InlineQueryResultDocument, Dice, MessageEntity, ParseMode)
+from telegram.constants import MAX_INLINE_QUERY_RESULTS
 from telegram.error import BadRequest, InvalidToken, NetworkError, RetryAfter
 from telegram.utils.helpers import from_timestamp, escape_markdown
 from tests.conftest import expect_bad_request
@@ -54,6 +55,20 @@ def chat_permissions():
     return ChatPermissions(can_send_messages=False, can_change_info=False, can_invite_users=False)
 
 
+def inline_results_callback(page=None):
+    if not page:
+        return [InlineQueryResultArticle(i, str(i), None) for i in range(1, 254)]
+    elif page <= 5:
+        return [InlineQueryResultArticle(i, str(i), None)
+                for i in range(page * 5 + 1, (page + 1) * 5 + 1)]
+    return None
+
+
+@pytest.fixture(scope='class')
+def inline_results():
+    return inline_results_callback()
+
+
 class TestBot:
     @pytest.mark.parametrize('token', argvalues=[
         '123',
@@ -385,6 +400,97 @@ def test(_, url, data, *args, **kwargs):
                                                switch_pm_text='switch pm',
                                                switch_pm_parameter='start_pm')
 
+    def test_answer_inline_query_current_offset_error(self, bot, inline_results):
+        with pytest.raises(ValueError, match=('`current_offset` and `next_offset`')):
+            bot.answer_inline_query(1234,
+                                    results=inline_results,
+                                    next_offset=42,
+                                    current_offset=51)
+
+    def test_answer_inline_query_current_offset_1(self, monkeypatch, bot, inline_results):
+        # For now just test that our internals pass the correct data
+        def test(_, url, data, *args, **kwargs):
+            results = data['results']
+            length = len(results) == MAX_INLINE_QUERY_RESULTS
+            ids = all([int(res['id']) == 1 + i for i, res in enumerate(results)])
+            next_offset = data['next_offset'] == 1
+            return length and ids and next_offset
+
+        monkeypatch.setattr('telegram.utils.request.Request.post', test)
+
+        assert bot.answer_inline_query(1234, results=inline_results, current_offset='')
+
+        def test(_, url, data, *args, **kwargs):
+            results = data['results']
+            length = len(results) == MAX_INLINE_QUERY_RESULTS
+            ids = all([int(res['id']) == 51 + i for i, res in enumerate(results)])
+            next_offset = data['next_offset'] == 2
+            return length and ids and next_offset
+
+        monkeypatch.setattr('telegram.utils.request.Request.post', test)
+
+        assert bot.answer_inline_query(1234, results=inline_results, current_offset=1)
+
+        def test(_, url, data, *args, **kwargs):
+            results = data['results']
+            length = len(results) == 3
+            ids = all([int(res['id']) == 251 + i for i, res in enumerate(results)])
+            next_offset = data['next_offset'] == ''
+            return length and ids and next_offset
+
+        monkeypatch.setattr('telegram.utils.request.Request.post', test)
+
+        assert bot.answer_inline_query(1234, results=inline_results, current_offset=5)
+
+    def test_answer_inline_query_current_offset_2(self, monkeypatch, bot, inline_results):
+        # For now just test that our internals pass the correct data
+        def test(_, url, data, *args, **kwargs):
+            results = data['results']
+            length = len(results) == MAX_INLINE_QUERY_RESULTS
+            ids = all([int(res['id']) == 1 + i for i, res in enumerate(results)])
+            next_offset = data['next_offset'] == 1
+            return length and ids and next_offset
+
+        monkeypatch.setattr('telegram.utils.request.Request.post', test)
+
+        assert bot.answer_inline_query(1234, results=inline_results, current_offset=0)
+
+        inline_results = inline_results[:30]
+
+        def test(_, url, data, *args, **kwargs):
+            results = data['results']
+            length = len(results) == 30
+            ids = all([int(res['id']) == 1 + i for i, res in enumerate(results)])
+            next_offset = data['next_offset'] == ''
+            return length and ids and next_offset
+
+        monkeypatch.setattr('telegram.utils.request.Request.post', test)
+
+        assert bot.answer_inline_query(1234, results=inline_results, current_offset=0)
+
+    def test_answer_inline_query_current_offset_callback(self, monkeypatch, bot, caplog):
+        # For now just test that our internals pass the correct data
+        def test(_, url, data, *args, **kwargs):
+            results = data['results']
+            length = len(results) == 5
+            ids = all([int(res['id']) == 6 + i for i, res in enumerate(results)])
+            next_offset = data['next_offset'] == 2
+            return length and ids and next_offset
+
+        monkeypatch.setattr('telegram.utils.request.Request.post', test)
+
+        assert bot.answer_inline_query(1234, results=inline_results_callback, current_offset=1)
+
+        def test(_, url, data, *args, **kwargs):
+            results = data['results']
+            length = results == []
+            next_offset = data['next_offset'] == ''
+            return length and next_offset
+
+        monkeypatch.setattr('telegram.utils.request.Request.post', test)
+
+        assert bot.answer_inline_query(1234, results=inline_results_callback, current_offset=6)
+
     @flaky(3, 1)
     @pytest.mark.timeout(10)
     def test_get_user_profile_photos(self, bot, chat_id):
diff --git a/tests/test_inlinequery.py b/tests/test_inlinequery.py
index 0099bd78f63..9622d4fcb8a 100644
--- a/tests/test_inlinequery.py
+++ b/tests/test_inlinequery.py
@@ -68,6 +68,15 @@ def test(*args, **kwargs):
         monkeypatch.setattr(inline_query.bot, 'answer_inline_query', test)
         assert inline_query.answer()
 
+    def test_answer_auto_pagination(self, monkeypatch, inline_query):
+        def test(*args, **kwargs):
+            inline_query_id = args[0] == inline_query.id
+            offset = kwargs.get('current_offset') == inline_query.offset
+            return offset and inline_query_id
+
+        monkeypatch.setattr(inline_query.bot, 'answer_inline_query', test)
+        assert inline_query.answer(auto_pagination=True)
+
     def test_equality(self):
         a = InlineQuery(self.id_, User(1, '', False), '', '')
         b = InlineQuery(self.id_, User(1, '', False), '', '')
diff --git a/tests/test_official.py b/tests/test_official.py
index b804e4d7af4..7ef27bff9e1 100644
--- a/tests/test_official.py
+++ b/tests/test_official.py
@@ -78,6 +78,8 @@ def check_method(h4):
         ignored |= {'location'}  # Added for ease of use
     elif name == 'sendVenue':
         ignored |= {'venue'}  # Added for ease of use
+    elif name == 'answerInlineQuery':
+        ignored |= {'current_offset'}  # Added for ease of use
 
     assert (sig.parameters.keys() ^ checked) - ignored == set()
 


Code review comments:
diff_hunk: @@ -1573,6 +1583,32 @@ def answer_inline_query(self,
         """
         url = '{}/answerInlineQuery'.format(self.base_url)
 
+        if current_offset is not None and next_offset is not None:
+            raise ValueError('`current_offset` and `next_offset` are mutually exclusive!')
+
+        if current_offset is not None:
+            if current_offset == '':
+                current_offset = 0
+            else:
+                current_offset = int(current_offset)
+
+            next_offset = ''
+
+            if callable(results):
+                results = results(current_offset)
+                if not results:
+                    results = []
+                else:
+                    next_offset = current_offset + 1
+            else:
+                if len(results) > (current_offset + 1) * MAX_INLINE_QUERY_RESULTS:
+                    next_offset = current_offset + 1
+                    results = results[
+                        current_offset * MAX_INLINE_QUERY_RESULTS:
+                        current_offset * MAX_INLINE_QUERY_RESULTS + MAX_INLINE_QUERY_RESULTS]
path: telegram/bot.py
text: @user1:
This could just use `next_offset * MAX`, right?

@user1:
Actually `next_offset` could be used in a few places here

@author:
I see how to use it here, but I don't see where else to use it …

diff_hunk: @@ -1526,15 +1527,20 @@ def answer_inline_query(self,
                             switch_pm_text=None,
                             switch_pm_parameter=None,
                             timeout=None,
+                            current_offset=None,
                             **kwargs):
         """
         Use this method to send answers to an inline query. No more than 50 results per query are
         allowed.
 
         Args:
             inline_query_id (:obj:`str`): Unique identifier for the answered query.
-            results (List[:class:`telegram.InlineQueryResult`)]: A list of results for the inline
-                query.
+            results (List[:class:`telegram.InlineQueryResult`] | Callable): A list of results for
+                the inline query. In case :attr:`current_offset` is passed, ``results`` may also be
+                a callable taking a single integer argument, being the 0-based current number of
+                the result pagination. It must return either a list of
+                :class:`telegram.InlineResult` instances or :obj:`None`, if there are no more
path: telegram/bot.py
text: @user1:
```suggestion
                :class:`telegram.InlineResult` instances or :obj:`None` if there are no more
```

@author:
ok

diff_hunk: @@ -1550,6 +1556,10 @@ def answer_inline_query(self,
             switch_pm_parameter (:obj:`str`, optional): Deep-linking parameter for the /start
                 message sent to the bot when user presses the switch button. 1-64 characters,
                 only A-Z, a-z, 0-9, _ and - are allowed.
+            current_offset (:obj:`str`, optional): The :attr:`telegram.InlineQuery.offset` of
+                the inline query to answer. If passed, PTB will automatically take care of
+                the pagination for you, i.e. pass set the correct ``next_offset`` and truncate the
path: telegram/bot.py
text: @user1:
```suggestion
                the pagination for you, i.e. pass the correct ``next_offset`` and truncate the
```

diff_hunk: @@ -385,6 +400,97 @@ def test(_, url, data, *args, **kwargs):
                                                switch_pm_text='switch pm',
                                                switch_pm_parameter='start_pm')
 
+    def test_answer_inline_query_current_offset_error(self, bot, inline_results):
+        with pytest.raises(ValueError, match=('`current_offset` and `next_offset`')):
+            bot.answer_inline_query(1234,
+                                    results=inline_results,
+                                    next_offset=42,
+                                    current_offset=51)
+
+    def test_answer_inline_query_current_offset_1(self, monkeypatch, bot, inline_results):
path: tests/test_bot.py
text: @user1:
Again a case for parametrization. There are mutliple assertions in this test.

@author:
here i see it

diff_hunk: @@ -68,6 +68,15 @@ def test(*args, **kwargs):
         monkeypatch.setattr(inline_query.bot, 'answer_inline_query', test)
         assert inline_query.answer()
 
+    def test_answer_auto_pagination(self, monkeypatch, inline_query):
+        def test(*args, **kwargs):
+            inline_query_id = args[0] == inline_query.id
path: tests/test_inlinequery.py
text: @user1:
Might aswell make the assertions inside the callback directly. The `inline_query_id` naming doesn't imply that this is a boolean holding a test result. If you insist on returning it from the callback using `x and y`, then name it `inline_query_id_matches_expected` or so :)

diff_hunk: @@ -68,6 +68,15 @@ def test(*args, **kwargs):
         monkeypatch.setattr(inline_query.bot, 'answer_inline_query', test)
         assert inline_query.answer()
 
+    def test_answer_auto_pagination(self, monkeypatch, inline_query):
+        def test(*args, **kwargs):
+            inline_query_id = args[0] == inline_query.id
+            offset = kwargs.get('current_offset') == inline_query.offset
+            return offset and inline_query_id
+
+        monkeypatch.setattr(inline_query.bot, 'answer_inline_query', test)
path: tests/test_inlinequery.py
text: @user1:
```suggestion
    def test_answer_auto_pagination(self, monkeypatch, inline_query):
        def make_assertions(*args, **kwargs):
            inline_query_id = args[0] == inline_query.id
            offset = kwargs.get('current_offset') == inline_query.offset
            return offset and inline_query_id

        monkeypatch.setattr(inline_query.bot, 'answer_inline_query', make_assertions)
```

(in lack of a better name...)
In any case, don't name it `test`, that might be confusing (also to pytest discovery).

diff_hunk: @@ -1573,6 +1583,32 @@ def answer_inline_query(self,
         """
         url = '{}/answerInlineQuery'.format(self.base_url)
 
+        if current_offset is not None and next_offset is not None:
+            raise ValueError('`current_offset` and `next_offset` are mutually exclusive!')
+
+        if current_offset is not None:
+            if current_offset == '':
+                current_offset = 0
+            else:
+                current_offset = int(current_offset)
+
+            next_offset = ''
+
+            if callable(results):
+                results = results(current_offset)
path: telegram/bot.py
text: @user1:
I'm not a big fan of overriding the `results` here, as that changes the data type. Maybe we can introduce a new var for the actual list of items

@author:
changed

diff_hunk: @@ -1526,15 +1527,20 @@ def answer_inline_query(self,
                             switch_pm_text=None,
                             switch_pm_parameter=None,
                             timeout=None,
+                            current_offset=None,
                             **kwargs):
         """
         Use this method to send answers to an inline query. No more than 50 results per query are
         allowed.
 
         Args:
             inline_query_id (:obj:`str`): Unique identifier for the answered query.
-            results (List[:class:`telegram.InlineQueryResult`)]: A list of results for the inline
-                query.
+            results (List[:class:`telegram.InlineQueryResult`] | Callable): A list of results for
+                the inline query. In case :attr:`current_offset` is passed, ``results`` may also be
+                a callable taking a single integer argument, being the 0-based current number of
path: telegram/bot.py
text: @user1:
"... may also be a callable that accepts the current page index starting from 0."
? :)

@author:
jup, that's probably it bit less convoluted.