sergey111991kv / echobot

echoBot with standart commands

EchoBot

This bot sends messages back to telegram and vk Bot can repeat last message and if you need to change count repeate

            /repeat

If you want to get help message

             /help

Configuration:

All configuration are in the src/Config.hs and you need to write in your customization or if need I can rewrite project with change "bot.config":

    help message
    count repeat message
    log configuration ( logFile - where to write?, logLevelForFile - recording level(Debug, Warning ,Error), logConsole  - is write to console?)

and some specific option for every bot just need only for administrator

For start:

for start bot you need to create file "bot.config" like file from folder templates, to replaced "token" with your token

How to use

All you need to start project:

create file

            "bot.config" 

like in

            templates/bot.config.template 

write your option and script to start project, like this:

            TelegramToken 889:9jhhjj
            VKtoken dsfdsfsdfsadfasfasddsfsd345
            RepeatCount 2
            HelpMessage "default help message"

take script in terminal:

            stack build

Structure of project.

    All logic divided for 2 group: Bot  (main logic) and Adapter (implementation). 
    I used for ReaderT design pattern. 
    Adapter/Telegram - all logic for Telegram.
    Adapter/VK - all logic for VK.
    Keyboard VK is in keyboard.json
    log-journalTest and  log-journal - files for write log in testing and running programm
    Config - default option  

Test.

I used spec library for testing app and empty structure.

echobot's Issues

имплементация tryGetCountRepeat

в Adapter.Tel.TelEchoBot и Adapter.VK.VKEchoBot tryGetCountRepeat практически одинаковые, стоит их вынести. А так же стрнное вычисление ответа через lookup. Почему бы просто не делать readMaybe msg, а затем проверять чтобы 0 < n <= 5?

Именование

Должен быть единый стиль именования.
На участке кода, предоставленном ниже, я насчитал 4 различных представления message :)

echoBot/src/Bot/EchoBot.hs

Lines 56 to 72 in 2ebb8bd

	handBotMsg ::
	(MonadIO m, EchoBot m) => BotMessage -> m (Either Error ())
	handBotMsg (BotMessage mess) = do
	txt <- getNameAdapter
	let txtMess = textMsg mess
	isWait <- isWaitForRepeat
	case isWait of
	Left err -> do
	writeLog Error (errorText err)
	return $ Left err
	Right boolWait -> do
	if boolWait
	then do
	case txtMess of
	"/help" -> do
	helpMes <- msgHelp
	sendMsgHelpTo' helpMes (BotMessage mess)

Самостоятельная работа модулей

При отсутствии одного из токенов, другой модуль продолжает работать. В случае с неработающим вк это спамящиеся раз в секунду сообщения NotAnswer VK, в случае с неработающей телегой это NotAnswer Telegram и огромные сообщения HttpExceptionRequest

Парсинг сообщения ВК

Есть instance FromJSON MessageVK с парсингом через массив, почему так?
Этот же функционал дублируется в Adapter.VK.VKBot.parseArray

Long poll delay

Сейчас нет никакой задержки по обновлению сообщений у вк. Нужна хотя бы 5-30 секунд.

Build system

Запуская через stack ghci я заметил что там очень много варнингов от -Wall, которых нет при компиляции через stack build. Вместе с этим при запуске компиляции пишет следующее:

Warning: /tmp/echoBot/stack.yaml: Unrecognized field in ProjectAndConfigMonoid: pure
/tmp/echoBot/echoBot.cabal was modified manually. Ignoring /tmp/echoBot/package.yaml in favor of the cabal file.
If you want to use the package.yaml file instead of the cabal file,
then please delete the cabal file.

При удалении .cabal файла ругается что package.yaml невалидный. Надо со всем этим разобраться и привести в порядок, чтобы не было никаких ошибок, и исправить все варнинги, которые сейчас видны только при запуске через ghci

Дублирующийся код

В src.Adapter.VK.VKBot.getMsgLast обе ветви case tsLast . longConfig $ config st of на 99% одинаковы.
Там же рядом getInitialConfig и getWithOldConfig на 80% одинаковы
В Bot.Bot все три функции на 90% одинаковы

Обработка ошибок

Здесь функции возвращают тип с ошибкой, но ошибки игнорируются, не стоит так делать.
Если ошибки обрабатываются в самих функциях, то они не должны возвращать те же самые ошибки

echoBot/src/Bot/EchoBot.hs

Lines 99 to 106 in 2ebb8bd

	processEchoBot :: (MonadIO m, EchoBot m) => m ()
	processEchoBot = do
	resultGet <- getMsgLast'
	case resultGet of
	Left _ -> processEchoBot
	Right mess -> do
	_ <- handBotMsg mess
	processEchoBot

Починить клавиатуру Telegram

Сейчас Adapter.Tel.TelEchoBot.sendKeyboard сериализует KeyboardTel в строку, а затем эту строку запихивает в поле reply_markup. Из-за этого клавиатура не работает. Нужно посылать объект, а не объект закодированный в строку

Непонятные название переменных

txt - слишком уж общее название.

echoBot/src/Bot/EchoBot.hs

Line 59 in 2ebb8bd

txt <- getNameAdapter

То есть на этой строке

echoBot/src/Bot/EchoBot.hs

Line 84 in 2ebb8bd

writeLog Debug (txt ++ " tryGetCountRepeat is done ")

совсем не понятно что за строка у нас получится в результате.
Нужно пройтись по обоим проектам и посмотреть, достаточно ли информативны названия

Зависимости

Все ли зависимости используются и необходимы?
Сборка с нуля достаточно долгая, стоит проверить этот момент, возможно тут есть лишние пакеты

echoBot/package.yaml

Line 22 in 2ebb8bd

dependencies:

Неудачное название для класса

Название класса не совсем коррелирует с назначением и только путает, на мой взгляд
BotCompatibleMessage или просто BotMessage подойдут лучше

echoBot/src/Bot/Message.hs

Line 9 in 2ebb8bd

class HandleBotMessage a where

Переименовать /count в /repeat

Часть функциональных требований

Ревью

Привет, посмотрел проект, вот что надо исправить:

Не соблюдены все требования:

1. В README должно быть описание того, как разворачивать проект локально, как его запускать, должна быть описана базовая архитектура проекта так, чтобы новичкам было понятно. Всё это на английском.

2. Все настройки(токены, ответные сообщения, кол-во повторов при запуске) должны быть вынесены в конфиг. Сейчас во первых ты свои токены пушишь в репозиторий, а это очень-очень плохо, во вторых чтобы применить какие-то изменения тебе придётся заново компилировать проект.

3. В .gitignore добавить правила для папок .gitignore, .history, логов и конфига.

4. Проект должен быть скомпилирован с флагами -Wall и -Werror без ошибок.

5. Весь код должен быть проверен через hlint. У меня нашло 33 ворнинга.

Теперь по поводу кода:

1. У всего должно быть понятное имя. Например у тебя в коде есть модуль Lib и функция someFunc, надо переименовать так, чтобы было понятно, что делает someFunc и что примерно лежит в Lib.

2. Логи пишутся log-journal, при этом есть файл log-journal.txt. Писать надо во второй, первый удалить.

3. Оставлять за собой закомментированный код нельзя. Если очень надо, то рядом надо написать причину.

4. В некоторых местах ты используешь для отступа 4 пробела, а в некоторых 2. Необходимо придерживаться единого стиля по всему проекту. Рекомендую 2 пробела, т. к. мы используем 2. Также если имя содержит несколько слов, то где-то ты разделяешь их нижним подчёркиванием, а где-то каждое последующее слово начинается с заглавной буквы. Здесь также нужно выбрать единый стиль(рекомендую второе).

5. Постарайся писать более информативные сообщения для коммитов. Например, увидел сообщение "l" или сообщение "1-test". Лучше писать "fixed VK tests" или "added Telegram tests".

6. Есть неочевидные сокращения и ошибки в словах, постарайся избавиться от этого. Например "Mess"(лучше "Msg") или "loging"(правильно "logging").

Продумать архитектуру

Смотри, сейчас много функций у тебя возвращает m (Either Error a), чтобы развернуть результат такой функции у тебя везде такая конструкция:

res <- funWithErr
case res of
  Left err -> do                                                                                 
    writeLogE $ errorText err                                                                    
    return $ Left err
  Right a -> someFunction a

Я предлагаю что то с этим делать, например, добавить в твой стек эффект ошибки, это позволило бы вышеприведенный кусок кода сократить до funWithErr >>= someFunction, а печать ошибки в лог делать гденибудь снаружи.

Именование

Не совсем понимаю назначение To в названиях.
Если смотреть на вызов функции не видя сигнатуры, то кажется, что нужено передать аргументом id назначение

echoBot/src/Bot/Bot.hs

Line 28 in 2ebb8bd

sendMsgTo' :: Bot m => BotMessage -> m (Either Error ())

Пофиксить импорты в тестах

https://github.com/Sergey111991KV/echoBot/blob/master/test/Bot/BotSpec.hs#L8

https://github.com/Sergey111991KV/echoBot/blob/master/test/Bot/EchoBotSpec.hs#L11

Надо поправить импорты)

Ревью

1. Зачем ты сравниваешь LogWrite'ы через caseOfWriteLog (кстати, странное имя) когда у тебя задерайвлен для них Ord?

2. Сейчас программа полностью зависит от успешности треда с VK ботом, если он упадет, то и Телега упадет тоже. Стоит сделать так чтобы хотя бы можно было запустить любой из ботов по отдельности. См. concurrently.
   "Nessesary take new module to put y/n for restart project or only one module" -- совершенно непонятно что имеется ввиду

3. Bot.Error.errorText переделать через case of или pattern matching

4. Bot.Request.buildRequest тоже самое что Adapter.Tel.TelRequest.buildRequest

5. Bug-prone сериализация в Adapter.Tel.TelRequest.buildRequestBody, почему бы не использовать encode из Aeson'а? Что будет если в strToParam параметр s будет содержать "? Что получится в show s, где s это String или Text?

6. Уточнить, не нужно ли в Adapter.VK.VKRequest.keyboardVK после сериализации keybord'a пропустить его через Network.URI.Encode.encode, аналогично для parVal из Bot.Request.buildQueryParams и вообще везде где собирается строка с query параметрами. Если использовать не ASCII символы это может вызвать проблемы.

7. В Adapter.VK.VKBot.getNewStateLongPool и Adapter.VK.VKBot.setNewTs происходит слишком громоздкая перепаковка, хотя меняется только одно поле. Поменять одно поле у рекорда можно так:

modifiedRecord = oldRecord { field1 = newvalue }

8. Bot.EchoBot.kostylIO вероятно не работает так как задумывалось. Как насчет:

liftIO (threadDelay 1000) >> processEchoBot

9. Переименовать README.md в README.txt или оформить README.md в синтаксисе markdown. Для превьювинга md я использую grip.