Pause / resume functionality

[asafov]

Pause / resume functionality for #1099

[asafov]

Call example:
$this->conversation = new Conversation($chatId, $chatId, $this->getName(), true);

[noplanman]

Thanks for the PR @asafov

I've noticed that you've worked on the master branch and pointed this PR to master as well.

As all work is done on the develop branch and only finished releases go into master, please rebase your commits (onto the develop branch) and change the PR to point to develop.

🙏

Looking forward to give this a test afterwards 😃

[asafov]

Thanks for the PR @asafov

I've noticed that you've worked on the master branch and pointed this PR to master as well.

As all work is done on the develop branch and only finished releases go into master, please rebase your commits (onto the develop branch) and change the PR to point to develop.

Looking forward to give this a test afterwards

Done.

[noplanman]

As far as i recall how conversations are set up, there can only be 1 conversation per user per command, so there can never be multiple conversations happening for the same command.
For this reason, the state of a conversation can only be a single state, never multiple ones.

With that in mind, a paused conversation is the same like an active one, but with a paused status. So when requesting a conversation, there is no need to know in which state the conversation is in, as there will only be 1 to return anyway, and then the state can be read from that.

So looking at the PR, it seems that most of the extra code for the paused status isn't necessary and can be accomplished much more easily.

If I've missed something or have it wrong, please explain 🙏

[asafov]

So when user start conversation just get active or paused like this?

WHERE (status = :status OR status = :pstatus)
....
$sth->bindValue(':status', 'active');
$sth->bindValue(':pstatus', 'paused');

[noplanman]

@asafov Looking better indeed 👍

Just a few things:

1. When pausing and resuming, the conversation doesn't need to be cleared, as that unnecessarily re-fetches all data from the DB.
2. When updating the status, the status of the $conversation array need to be updated too (as it wouldn't be reloaded after changing point 1.)

Looking at the conversation feature in general, I think it would be great to export it into a separate little package and redesign it a bit. For instance, having the $conversation variable seems a bit weird and it probably makes sense to extract all data directly into the Conversation class, to not have that second layer.

What do you think?

We could then also extend the class to have getStatus or even isActive/isPaused methods.

[asafov]

All fixes it applied. Can anybody check it?

[asafov]

Guys?

[noplanman]

@asafov I've updated the code a tiny bit, could you have a look please?

If all is ok with your tests, we can get this merged 💪

[noplanman]

After some deliberation, it looks like the conversations don't need a pause state per se.

What should be updated though, is to allow parallel commands to be active, 1 per command.
At the moment any active conversation gets cancelled when a new one is started.

Also it would make sense to extract the conversations into a separate package entirely.

Thanks @asafov for your input on this 🙏