Feature Request: Adding posibility to autoloading callbacks

[fnaysee]

In file core/src/Commands/SystemCommands/CallbackqueryCommand.php We have this method :

 public function execute()
{
    //$callback_query = $this->getUpdate()->getCallbackQuery();
    //$user_id        = $callback_query->getFrom()->getId();
    //$query_id       = $callback_query->getId();
    //$query_data     = $callback_query->getData();
    // Call all registered callbacks.
    foreach (self::$callbacks as $callback) {
        $callback($this->getUpdate()->getCallbackQuery());
    }
    return Request::answerCallbackQuery(['callback_query_id' => $this->getUpdate()->getCallbackQuery()->getId()]);
}
/**
 * Add a new callback handler for callback queries.
 *
 * @param $callback
 */
public static function addCallbackHandler($callback)
{
    self::$callbacks[] = $callback;
}

If we add this code before looping through callbacks in execute method :

    $callbackArr = explode("_", $this->getUpdate()->getCallbackQuery()->getData());
    if(class_exists($callbackArr[0])){
        if(method_exists($callbackArr[0], $callbackArr[1])){
            call_user_func($callbackArr[0] . '::' . $callbackArr[1]);
        }
    }

Then we can auto call all the callbacks. Above code not tested that is just an idea, also it needs more lines to get the user commands directory. Then the user can defines callback names (callback_data) like this: ExampleCommand_ExampleStaticMethod

This way no more code for registering callbacks is needed.

Also it is possible to add a feature to register a list of classes that we have some static methods that are being called from callbackquery to prevent scanning all classes in the commands directory.

Thanks.

[sharkydog]

An alternative is circling in my head, but I don't have a clear picture how I want it yet. Something like:

1. if there is an open conversation (as in genericmessage) send the callback_query to the command that opened it through a method defined in the command (ex.: executeCallbackQuery()), or just execute the command, but then it has to check if the update is a message or a callback_query
2. if callback_query data is command like (/command something), execute command the same way as in 1. without active conversation

Both can be implemented in a custom CallbackqueryCommand and not in the core (if the user command is checking the update type).

But as there are several questions (and solutions) about this, it feels like there should be some helpers in the core.

I am currently doing this by executing the command with fake message spliced from callback_query parts through an executeCommandUpdate($command,$update) method, defined in a class extending Telegram class.

[noplanman]

@wp-src Thanks for the idea! I like things that make it easier for the end user 👍

Your approach implies that the class and method name is added to the callback data, something I tried to prevent by adding the addCallbackHandler method.

Remember, this value is limited to 64 characters.

So the shorter and more descriptive the better 😃 As for methods, they should be as long as necessary, to explain what's happening in them.

Also it is possible to add a feature to register a list of classes that we have some static methods that are being called from callbackquery to prevent scanning all classes in the commands directory.

So you mean that by having the class and method names listed in the callback query data, it could simply load the necessary class file directly, instead of loading all command classes and then calling it?

To sum it up, instead of using CallbackqueryCommand::addCallbackHandler($callback);, one would need to add the callback method to the callback_data, so it's just moving the "problem" to a different place.

Right?

I'll add an idea I've been working on (since #540), which tries to tackle the same issue: In a bot I'm busy developing, I've updated the addCallbackHandler method to also accept an ID, like this: addCallbackHandler($callback, $id) Then I add that ID to the callback_data, making sure that only appropriate callbacks are called, not all of them (as it is now), like: my_id;more_callback_data

@sharkydog Conversations and callback queries are totally different things, which can be used together for certain things. So I'm not sure I fully understand your thinking, would you mind clarifying with a simple example?

[fnaysee]

@noplanman Thanks for your response, but 64 characters is more than what we need! Look at this: MyVeryLongClassName_MyVeryLongStaticMethodName It is just 46 characters. Anyway i think not just in callbackquery even in Genericmessage we need similar functionality so library can loads all classes automatically even based on non command strings(for example defined by user before calling handle()). or at least there could be an array structure that we feed it to the library before calling handle(), Which describes our normal keyboard menus hierarchy. Thanks.

[sharkydog]

@noplanman Not a totally different things if you start a conversation in a command and then send an inline keyboard, then in CallbackqueryCommand you could load the conversation, get the command (just like genericmessage) then execute it. The problem is, that command will receive a callback_query update, not a message.

[noplanman]

@sharkydog Ok, I see what you mean, but still they are different things 😁

Keep in mind, inline keyboards can be executed at any point! So lets say there is a keyboard a few messages back that is linked to a conversation that is finished or cancelled, what then? How do you manage those cases?

I personally feel it's quite tricky to keep this clean.

@wp-src Agreed, 64 should be more than enough for most cases. Just trying to not build in a fixed limitation. I feel the user should be able to use as many of those characters as (s)he wants to.

The commands are all loaded when the update object is processed. Or do you mean something else?

[sharkydog]

@noplanman If the conversation ended, then callbackquery command would not be able to determine the command and can fall back to a registered callback or do nothing. Or if callback_data is formatted like a command (ex "/echo hello"), this can be directly executed, if the command is aware it is getting a callback query update and not a message.

I still don't think it's a great idea to call a bunch of callbacks for plain data.

[sharkydog]

The tricky part is in groups, genericmessage handling is just as tricky, the two (genericmessage and callbackquery) are essentially (almost) the same thing.

[noplanman]

I still don't think it's a great idea to call a bunch of callbacks for plain data.

Absolutely agree with that. The current implementation is very basic and requires the user to do some filtering when using a bunch of callbacks. My idea above for adding an ID to reduce the calls works very well though, as it only calls what needs to be called.

I see now how you use the callback data 👍

Yep @ essentially the same thing, just coming from a different source.

[sharkydog]

The idea about an ID is sort of special formatting, which is outside of any telegram api specs and should be left to the individual to decide. BUT, you as a core developer of this bot, along with some help could agree on some format that can forward the callback data to a command (like the one that send the inline keyboard). Think of it as an extension, a plugin, for example a new InlineKeyboard class that takes a command name and data, then callback query command would recognize and handle that format.

[sharkydog]

And I still think that along with callbacks and specials formatting/filtering, CallbackqueryCommand should also check for a conversation and execute the command that started it (like in GenericmessageCommand), maybe using a special method (executeCallbackQuery() or whatever) so the command would not have to check if it got message or callback_query. Think of a "/survey" command, would be bulky to send back the answers through callbacks and IDs in callback data.

[fnaysee]

The commands are all loaded when the update object is processed. Or do you mean something else?

This is ok, but i mean sth else, For exmple before calling handle we set $telegram->setStartCommand('start'); Then in command u add two abstract methods called keyboard and inlineKeyboard which are not initialized or they both return null: Then we implement them like this:

public static function inlineKeyboard(){
    return [['text'=>'String', 'targetCommand'=>'self', 'targetCallback' =>'anStaticMethod', 'callbackData' =>    'aUniqueIDForThisButton']]
}

Or in above example method library can creates a callback data automatically based on Command and Callback names. And we get the keyboard by calling $this->getKeyboard(). Then when a button is being clicked library must stores current command and callback name from that button in a conversation and so on...

Edit: Also for checking the state we add a property called state in our command and library checks to see if there is an state in conversation notes or not, if there was an state loads it else reads the property from class else default value ! and we change the state by calling $this->setState(2); which auto updates the db, so we didn't need work with conversation for basic tasks like changing state, but we must be able to add more notes if required.

[sharkydog]

My take on this: https://github.com/sharkydog/php-telegram-bot/blob/callbackquery/src/Commands/SystemCommands/CallbackqueryCommand.php Not tested and probably not finished yet.

Edit: Now it's tested and works

Edit: coupled with this trait to make it easier for commands

trait ShdTelegramExecCallback {
    public function executeCallbackQuery() {
        $query = $this->getCallbackQuery();

        $update = [
            'update_id' => 0,
            'callback_query' => $query->getRawData(),
            'message'   => [
                'message_id' => 0,
                'from' => $query->getFrom()->getRawData(),
                'date' => time(),
                'text' => $query->getData()
            ]
        ];

        if($query->getMessage()) {
            $update['message']['chat'] = $query->getMessage()->getChat()->getRawData();
        } else {
            $update['message']['chat'] = [
                'id' => $update['message']['from']['id'],
                'first_name' => $update['message']['from']['first_name'],
                'username' => $update['message']['from']['username'],
                'type' => 'private'
            ];
        }

        $this->update = new Longman\TelegramBot\Entities\Update(
            $update,
            $this->telegram->getBotUsername()
        );

        return $this->execute();
    }
}