Skip to content

Small cleanup fixes #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 13, 2016
Merged

Small cleanup fixes #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8855585
Remove unused $command_is_provided variable.
noplanman Mar 10, 2016
92608fd
Add clear() method to reset internal conversation variables when canc…
noplanman Mar 10, 2016
7db0a91
Fix how the /sendtochannel command is ended.
noplanman Mar 11, 2016
b8e3b4b
Fix exception handling for ConversationDB.
noplanman Mar 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/Commands/AdminCommands/SendtochannelCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,18 +215,20 @@ public function execute()
$this->conversation->notes['last_message_id'] = $message->getMessageId();
// no break
case 5:
$this->conversation->stop();
$data['reply_markup'] = new ReplyKeyBoardHide(['selective' => true]);

if ($this->conversation->notes['post_message']) {
$data['text'] = $this->publish(new Message($this->conversation->notes['message'], 'anystring'), $this->conversation->notes['channel'], $this->conversation->notes['caption']);
$result = Request::sendMessage($data);
break;
$data['text'] = $this->publish(
new Message($this->conversation->notes['message'], 'anystring'),
$this->conversation->notes['channel'],
$this->conversation->notes['caption']
);
} else {
$data['text'] = 'Abort by user, message not sent..';
}

$data['text'] = 'Abort by user, message not sent..';
$this->conversation->stop();
$result = Request::sendMessage($data);
break;
}
return $result;
}
Expand Down
33 changes: 17 additions & 16 deletions src/Conversation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,6 @@ class Conversation
*/
protected $command;

/**
* Command has been provided
*
* @var string
*/
protected $command_is_provided;

/**
* Conversation contructor to initialize a new conversation
*
Expand All @@ -80,26 +73,35 @@ public function __construct($user_id, $chat_id, $command = null)
$this->chat_id = $chat_id;
$this->command = $command;

$this->command_is_provided = ($command !== null);

//Try to load an existing conversation if possible
if (!$this->load() && $this->command_is_provided) {
if (!$this->load() && $command !== null) {
//A new conversation start
$this->start();
}
}

/**
* Load the conversation from the database
* Clear all conversation variables.
*
* @return bool
* @return bool Always return true, to allow this method in an if statement.
*/
protected function load()
protected function clear()
{
$this->conversation = null;
$this->protected_notes = null;
$this->notes = null;

return true;
}

/**
* Load the conversation from the database
*
* @return bool
*/
protected function load()
{

//Select an active conversation
$conversation = ConversationDB::selectConversation($this->user_id, $this->chat_id, 1);
if (isset($conversation[0])) {
Expand All @@ -111,7 +113,6 @@ protected function load()

if ($this->command !== $this->conversation['command']) {
$this->cancel();
$this->conversation = null;
return false;
}

Expand Down Expand Up @@ -162,7 +163,7 @@ protected function start()
*/
public function stop()
{
return $this->updateStatus('stopped');
return ($this->updateStatus('stopped') && $this->clear());
}

/**
Expand All @@ -172,7 +173,7 @@ public function stop()
*/
public function cancel()
{
return $this->updateStatus('cancelled');
return ($this->updateStatus('cancelled') && $this->clear());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a stop the conversation I can leave conversation data inside instead to reset everything.

php-telegram-bot#104 (comment)
I my last comment i said that could be a nice idea to have a var for each column of the database instead of store everything inside the '$this->conversation' var
something like :

protected id;
protected status;
ecc

This can be useful because after updating we can just update the status var inside the class.
Do you think that this can work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between stop() and cancel()?
The way I understood it, is that a stopped conversation is one that's ended normally and a cancelled conversation has been cancelled somewhere in the middle of a conversation.
Is this right?

Since there's no way to "resume" a conversation at the moment, both stop() and cancel() stop and end the conversation, marking it as "finished" so to speak. So either one would need to be restarted as a new conversation afterwards.

Now about the separate variables in the class:
It could work both ways. Either as you say, or to modify the update() method to update the $conversation variable for us.

I still think it would be useful to have a specific getter and setter for the notes and a revert() command or something to undo any changes and not update them to the DB, but we'll discuss this in a later stage.

}

/**
Expand Down
7 changes: 4 additions & 3 deletions src/ConversationDB.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace Longman\TelegramBot;

use Longman\TelegramBot\DB;
use Longman\TelegramBot\Exception\TelegramException;

/**
* Class ConversationDB
Expand Down Expand Up @@ -63,7 +64,7 @@ public static function selectConversation($user_id, $chat_id, $limit = null)

$results = $sth->fetchAll(\PDO::FETCH_ASSOC);

} catch (PDOException $e) {
} catch (\Exception $e) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PDOException didn't work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, unfortunately not 😕

This is the exception I get, which is not caught:

PDOStatement::execute(): SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (`telegrambot`.`conversation`, CONSTRAINT `conversation_ibfk_1` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`))

I checked the PDO documentation and found that the PDOException is the only exception type available, but the one being thrown is not of that type. If you can find a more specific one to catch that would be great, but I didn't find any...

throw new TelegramException($e->getMessage());
}
return $results;
Expand Down Expand Up @@ -106,7 +107,7 @@ public static function insertConversation($user_id, $chat_id, $command)
$sth->bindParam(':date', $created_at);

$status = $sth->execute();
} catch (PDOException $e) {
} catch (\Exception $e) {
throw new TelegramException($e->getMessage());
}
return $status;
Expand Down Expand Up @@ -178,7 +179,7 @@ public static function update($table, array $fields_values, array $where_fields_
try {
$sth = self::$pdo->prepare($query);
$status = $sth->execute($tokens);
} catch (PDOException $e) {
} catch (\Exception $e) {
throw new TelegramException($e->getMessage());
}
return $status;
Expand Down