php-telegram-bot / core

PHP Telegram Bot based on the official Telegram Bot API
MIT License
3.9k stars 955 forks source link

Command repeats twice #449

Closed Sogl closed 7 years ago

Sogl commented 7 years ago

Hello!

I have token command for user registration:

<?php

namespace Longman\TelegramBot\Commands\UserCommands;

use Longman\TelegramBot\Commands\UserCommand;
use Longman\TelegramBot\Request;
use Kwf_Model_Abstract;
use Longman\TelegramBot\TelegramLog;

class TokenCommand extends UserCommand
{
    protected $name = 'token';
    protected $description = 'Registration with token';
    protected $usage = '/token <user_token>';
    protected $version = '1.0.0';
    protected $enabled = true;
    protected $public = true;

    public function execute()
    {
        $message = $this->getMessage();

        TelegramLog::initErrorLog(getcwd() . '/telegram_debug.log');

        if ($message->getDate() < strtotime("-5 minutes")) {

            return Request::emptyResponse();
        } else {

            $chatId = $message->getChat()->getId();
            $fromId = $message->getFrom()->getId();

            $text = trim($message->getText(true));

            if ($text === '') {

                $reply = 'Token is not set. Usage: ' . $this->getUsage();
            } else {

                //Send chat action
                Request::sendChatAction(['chat_id' => $chatId, 'action' => 'typing']);

                $employeesModel = Kwf_Model_Abstract::getInstance('Employees');
                $employeesSelect = $employeesModel->select()->whereEquals('telegramToken', $text);
                $employee = $employeesModel->getRow($employeesSelect);

                TelegramLog::error($employee);

                if (count($employee) > 0) {

                    TelegramLog::error('Reg:');
                    TelegramLog::error($employee->telegramRegistered);

                    if ($employee->telegramRegistered) {

                        TelegramLog::error('Registered');

                        $reply = '❌ Already registered. You do not need to do this twice.';
                        TelegramLog::error($reply);
                        exit;
                    } else {

                        TelegramLog::error('Not registered');

                        $employeesSelect = $employeesModel->select()->whereEquals('telegramId', $fromId);
                        $employees = $employeesModel->getRows($employeesSelect);

                        if (count($employees) > 0) {

                            $reply = '❌ Already registered.';
                        } else {

                            //reg flag
                            $employee->telegramRegistered = true;
                            //set telegram id
                            $employee->telegramId = $message->getFrom()->getId();
                            //set nick
                            $employee->telegramUsername = $message->getFrom()->getUsername();
                            //save
                            $employee->save();

                            $reply = "✅ Glad to see you, {$employee->lastname} {$employee->firstname} {$employee->middlename}! Thx for registration.";
                        }
                    }
                } else {

                    $reply = '❌ User with this token not found. Check your token.';
                }
            }

            $data = [
                'chat_id' => $chatId,
                'text'    => $reply
            ];

            return Request::sendMessage($data);
        }
    }
}

All worked fine for a long time, but some time ago for every new user registration this message was sent:

❌ Already registered. You do not need to do this twice.

I added some TelegramLog::error commands and what I saw in logs:

[2017-04-04 16:31:35] bot_log.ERROR: John Doe [] []
[2017-04-04 16:31:35] bot_log.ERROR: Reg: [] []
[2017-04-04 16:31:35] bot_log.ERROR:  [] []
[2017-04-04 16:31:35] bot_log.ERROR: Not registered [] []
[2017-04-04 16:31:38] bot_log.ERROR: John Doe [] []
[2017-04-04 16:31:38] bot_log.ERROR: Reg: [] []
[2017-04-04 16:31:38] bot_log.ERROR: 1 [] []
[2017-04-04 16:31:38] bot_log.ERROR: Registered [] []
[2017-04-04 16:31:38] bot_log.ERROR: ❌ Already registered. You do not need to do this twice. [] []

As you can see, first time (16:31:35) system said that user isn't registered, but somehow this command worked second time after 3 seconds (16:31:38) and re-checked that user just registered! But user see error message and doesn't understand what he did wrong.

Why is this happening?

Env: PHP 5.6 Php-telegram-bot v. 0.33

noplanman commented 7 years ago

Does this happen every time a new user registers? Or just sometimes? Also, enable and check your update log, to see if the exact same request comes through twice from Telegram, or if it's the library itself calling it twice.

Also, I'd recommend to refactor your command a bit to make it easier to understand, which also helps to find bugs (if any) 😊

Sogl commented 7 years ago

Does this happen every time a new user registers? Or just sometimes?

Every time when user registers

Also, enable and check your update log, to see if the exact same request comes through twice from Telegram, or if it's the library itself calling it twice.

Do you mean debug log enabling?

noplanman commented 7 years ago

Do you mean debug log enabling?

No, I mean the update log, which saves all the incoming requests from Telegram:

TelegramLog::initUpdateLog(__DIR__ . '/update.log');

Each message sent to your bot should be logged in that file, so that you know exactly what is coming from Telegram.

Sogl commented 7 years ago

Solved. Actually it was two requests from Telegram because first request failed (logging user actions in my ERP didn't work if the user isn't logged in). I don't know why Telegram sends request again after fail.

Also, I'd recommend to refactor your command a bit to make it easier to understand, which also helps to find bugs (if any)

What exactly do you mean by that? Use return instead of nested if conditions?

noplanman commented 7 years ago

Actually it was two requests from Telegram because first request failed

I had a feeling this was the case.

Regarding the refactor, yes, less nesting and more returning 👍 It's just much easier to understand the flow of the code, instead of having to scroll all the way down to the various else statements to see what's happening.