Skip to content
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.

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

Conversation bot improvements #11

Open
anpryl opened this issue Jun 2, 2018 · 4 comments
Open

Conversation bot improvements #11

anpryl opened this issue Jun 2, 2018 · 4 comments

Comments

@anpryl
Copy link
Contributor

anpryl commented Jun 2, 2018

currentChatId always returns Maybe ChatId but in conversationBot we always have ChatId. It leads to meaningless boilerplate code every time ChatId is needed in handlers.
What do you think about ConversationM?
Here is example:

newtype ConversationM a =
  ConversationM { _runConversationM :: ReaderT (ChatId, Update) ClientM a }

@fizruk
Is there are any hidden issues which will block this improvement?
If you are OK with this I will try to create prototype next week.

@fizruk
Copy link
Owner

fizruk commented Jun 3, 2018

Can you elaborate on the boilerplate you get? I see how it can arise, but I'd like to know more about your specific case.

I am not sure where do you expect ConversationM to arise. Note that you don't always have a ChatId (when Update is not chat-related) or Update (e.g. in bot jobs).

@fizruk
Copy link
Owner

fizruk commented Jun 3, 2018

@anpryl note that I am working on a new release with a lot of new nice features on improvements branch. Perhaps I've implemented a solution to your problem already.

@anpryl
Copy link
Contributor Author

anpryl commented Jun 4, 2018

In my case, I need to store a mapping (in database) between UserID in my system and telegram's ChatID. So in most handlers I need ChatId to lookup user data and it leads to boiler plate like

mchatID <- currecntChatId
case mchatID of
  Just chatID -> do something
  Nothing -> pure NoOp

I understand that update can be without ChatId. But is it possible in conversation mode? We need ChatId to lookup conversation from HashMap. After this we definitely have ChatId inside handlers.

@fizruk
Copy link
Owner

fizruk commented Jun 4, 2018

I think I see your point, but I'll have to think a little more about it.

I think we can actually make Maybe Update (or rather BotContext since v0.2) an argument to BotM:

newtype BotM context a = BotM { _runBotM :: ReaderT context ClientM a }

newtype Eff action model = Eff { _runEff :: Writer [BotM BotContext action] model }

With this we might use something similar to magnify to zoom into chatId or any other part of an update (when possible).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants