Красилов Константин - 3 #57

kkrasilov · 2021-05-03T14:47:43Z

Фамилия Имя

Красилов Консантин

Email

kkrasilov@gmail.com

Номер домашнего задания

3

Ссылка на видео с демо работы

https://youtu.be/q92DMbnwDac

Грустно вышло с тестами :(
Они работали просто с гемом active_record, а c sinatra-activerecord не захотели взлетать.
И не успел часть тестов сделать, где нужно было реализовать связи между таблицами.

hound

Some files could not be reviewed due to errors:

Error: We found some problems with your configuration file: [/Attribute] key ...

Error: We found some problems with your configuration file: [/Attribute] key 'Attribute:' is undefined., [/DuplicateMethodCall] key 'DuplicateMethodCall:' is undefined., [/FeatureEnvy] key 'FeatureEnvy:' is undefined., [/IrresponsibleModule] key 'IrresponsibleModule:' is undefined., [/PrimaDonnaMethod] key 'PrimaDonnaMethod:' is undefined., [/TooManyStatements] key 'TooManyStatements:' is undefined., [/app/workers] key 'app/workers:' is undefined., [/db/migrate] key 'db/migrate:' is undefined.

hound · 2021-05-03T14:48:05Z

Konstantin_Krasilov/3/app/models/user_word.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+class UserWord < ActiveRecord::Base


Rails/ApplicationRecord: Models should subclass ApplicationRecord.

Konstantin_Krasilov/3/Rakefile

Konstantin_Krasilov/3/app/models/user.rb

hound · 2021-05-03T14:48:07Z

Konstantin_Krasilov/3/app/models/word.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class Word < ActiveRecord::Base


Rails/ApplicationRecord: Models should subclass ApplicationRecord.

hound

Some files could not be reviewed due to errors:

Error: We found some problems with your configuration file: [/Attribute] key ...

Error: We found some problems with your configuration file: [/Attribute] key 'Attribute:' is undefined., [/DuplicateMethodCall] key 'DuplicateMethodCall:' is undefined., [/FeatureEnvy] key 'FeatureEnvy:' is undefined., [/IrresponsibleModule] key 'IrresponsibleModule:' is undefined., [/PrimaDonnaMethod] key 'PrimaDonnaMethod:' is undefined., [/TooManyStatements] key 'TooManyStatements:' is undefined., [/app/workers] key 'app/workers:' is undefined., [/db/migrate] key 'db/migrate:' is undefined.

hound

Some files could not be reviewed due to errors:

Error: We found some problems with your configuration file: [/Attribute] key ...

Error: We found some problems with your configuration file: [/Attribute] key 'Attribute:' is undefined., [/DuplicateMethodCall] key 'DuplicateMethodCall:' is undefined., [/FeatureEnvy] key 'FeatureEnvy:' is undefined., [/IrresponsibleModule] key 'IrresponsibleModule:' is undefined., [/PrimaDonnaMethod] key 'PrimaDonnaMethod:' is undefined., [/TooManyStatements] key 'TooManyStatements:' is undefined., [/app/workers] key 'app/workers:' is undefined., [/db/migrate] key 'db/migrate:' is undefined.

Konstantin_Krasilov/3/Gemfile

Saicheg · 2021-05-12T15:08:31Z

Konstantin_Krasilov/3/Rakefile

+require 'sinatra/activerecord/rake'
+require './app'
+
+namespace :db do


Не очень понятно, если честно, что ты хотел тут сказать. Выглядит как ненужный код.

Я его добавил чтобы была возможность работать с таксками гема sinatra/activerecord/rake - rake db:migrate, rake db:seed.
https://github.com/sinatra-activerecord/sinatra-activerecord#setup

Saicheg · 2021-05-12T15:10:17Z

Konstantin_Krasilov/3/app.rb

+  request.body.rewind
+  data = JSON.parse(request.body.read)
+
+  chat_id = data['message']['chat']['id']


Чисто в теории у тебя тут очень легко может случиться ошибка, которую ты не обработаешь. Например chat ключа не будет в сообщении, оно вернет nil попробует взять ['id'] и получится беда.

Чтобы от такого застраховаться - пойди узнать про метод dig у хеша. И не забывай про обработку ошибок.

Как лучше обрабатывать ошибки?

Просто добавить

chat_id = data.dig('message', 'chat', 'id') raise StandardError, 'Sorry, chat id cannot be blank' if chat_id.nil?

или обернуть все в begin -

begin chat_id = data.dig('message', 'chat', 'id') raise StandardError if chat_id.nil? rescue puts 'Error - Sorry, chat id cannot be blank' end

чтобы программа продолжала работать.

И с ошибками ActiveRecord, сделать -

def set_user user = User.find_or_create_by!(telegram_id: chat_id) rescue ActiveRecord::NotNullViolation => e puts "Error - #{e}" end

или просто добавить метод find_or_create_by! без rescue?

Saicheg · 2021-05-12T15:10:50Z

Konstantin_Krasilov/3/app.rb

+require_relative 'app/models/user'
+require_relative 'app/services/telegram/conversation'
+
+Dotenv.load


Лайк за использование ENV переменных 👍

Saicheg · 2021-05-12T15:11:42Z

Konstantin_Krasilov/3/app.rb

+  chat_id = data['message']['chat']['id']
+  message = data['message']['text']
+
+  user = User.find_or_create_by(telegram_id: chat_id)


find_or_create_by если не создаст пользователя, вернет тебе false и будет беда. Лучше всего если не обрабатываешь ошибки в ActiveRecord использовать методы с восклицательным знаком find_or_create_by!

Saicheg · 2021-05-12T15:13:04Z

Konstantin_Krasilov/3/app.rb

+
+  case message
+  when '/start'
+    user.send_max_word!


Понятно что ты тут пытался сделать какую-то машину состояний. Так как ты тоже можно, но лучше посмотреть в сторону https://github.com/aasm/aasm

Saicheg · 2021-05-12T15:16:17Z

Konstantin_Krasilov/3/app/services/telegram/conversation.rb

+    end
+
+    def call
+      send(@user.conversation_status)


У тебя сейчас в приложении появилась очень странная неприятность. Получается что работа твоего класса Telegram::Coversation зависит от названия статусов в модели. Так делать нельзя, хоть и может казаться что это красиво. Завтра кто-нибудь решит переименовать все статусы и будет большая беда.

Можно разбить на отдельные сервисы - Telegram::Start, Telegram::SendMaxWord, Telegram::SendSmiley?

Saicheg · 2021-05-12T15:17:02Z

Konstantin_Krasilov/3/app/services/telegram/conversation.rb

+    def send_max_word
+      return RESPONSE[:max_word_error] unless (1..6).cover?(@message.to_i)
+
+      @user.update(max_words: @message, conversation_status: 'conversation_break')


@user.update!

Saicheg · 2021-05-12T15:19:16Z

Konstantin_Krasilov/3/app/services/telegram/lesson.rb

+      @api.sendMessage(@user.telegram_id, @word.to_s)
+    end
+
+    def waiting_answer


А вот это очень опасная архитектура этой фишки. Проблема заключается в том что ты с помощью крона запускаешь каждые N минут процесс. И если пользователь тебе не ответил - тупо его держишь. Это будет работать пока у тебя парочку пользователей, но когда у тебя их станет хотя бы пару десятков - твой сервис упадет.

Saicheg · 2021-05-12T15:20:06Z

Konstantin_Krasilov/3/app/services/telegram/lesson.rb

+    def call
+      find_and_added_word_to_user
+      send_word
+      waiting_answer


Тут надо было перключать пользователя в статус "ждет ответа" и просто выходить.

hound

Some files could not be reviewed due to errors:

Error: We found some problems with your configuration file: [/Attribute] key ...

Error: We found some problems with your configuration file: [/Attribute] key 'Attribute:' is undefined., [/DuplicateMethodCall] key 'DuplicateMethodCall:' is undefined., [/FeatureEnvy] key 'FeatureEnvy:' is undefined., [/IrresponsibleModule] key 'IrresponsibleModule:' is undefined., [/PrimaDonnaMethod] key 'PrimaDonnaMethod:' is undefined., [/TooManyStatements] key 'TooManyStatements:' is undefined., [/app/workers] key 'app/workers:' is undefined., [/db/migrate] key 'db/migrate:' is undefined.

hound · 2021-05-25T18:45:57Z

Konstantin_Krasilov/3/Gemfile

+gem 'sinatra'
+gem 'sinatra-activerecord'
+gem 'telegramAPI'
+gem 'pry'


Bundler/OrderedGems: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem pry should appear before telegramAPI.

hound · 2021-05-25T18:45:57Z

Konstantin_Krasilov/3/app.rb

+
+Dotenv.load
+
+WELCOME = 'Привет! Я бот, который помогает учить новые английские слова каждый день. Давай сперва определимся,' \


Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.

hound · 2021-05-25T18:45:57Z

Konstantin_Krasilov/3/app/models/user.rb

+    end
+
+    event :learn do
+      transitions from: [:waiting_max_word, :waiting_smiley, :sleeping], to: :learning


Style/SymbolArray: Use %i or %I for an array of symbols.

hound · 2021-05-25T18:45:57Z

Konstantin_Krasilov/3/app/models/user.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+require 'aasm'


Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.

hound · 2021-05-25T18:45:57Z

Konstantin_Krasilov/3/lib/tasks/telegram.rake

+  end
+
+  desc 'Reminder for answer to user'
+  task :reminder_for_answer do


Rails/RakeEnvironment: Include :environment task as a dependency for all Rake tasks.

hound · 2021-05-25T18:45:58Z

Konstantin_Krasilov/3/lib/tasks/telegram.rake

+  end
+
+  desc 'Start a lesson with a user'
+  task :start_lesson do


Rails/RakeEnvironment: Include :environment task as a dependency for all Rake tasks.

hound · 2021-05-25T18:45:58Z

Konstantin_Krasilov/3/lib/tasks/telegram.rake

+namespace :telegram do
+  desc 'Start new training day'
+  task :start_new_training_day do
+    User.update_all(aasm_state: 'learning')


Rails/SkipsModelValidations: Avoid using update_all because it skips validations.

hound · 2021-05-25T18:45:58Z

Konstantin_Krasilov/3/lib/tasks/telegram.rake

+
+namespace :telegram do
+  desc 'Start new training day'
+  task :start_new_training_day do


Rails/RakeEnvironment: Include :environment task as a dependency for all Rake tasks.

hound · 2021-05-25T18:45:58Z

Konstantin_Krasilov/3/app/services/telegram/send_smiley.rb

+    private
+
+    def update_user!
+      @user.user_words.where(created_at: Date.today.all_day).count >= @user.max_words ? @user.sleep! : @user.lesson!


Rails/Date: Do not use Date.today without zone. Use Time.zone.today instead.

kkrasilov added 14 commits May 3, 2021 19:43

Initial commit

acf18b8

Added connection database

9f4d54f

Added migration and model user

1e1bc2a

Added migration and model word

2c00fc6

Added migration and model user_words

7c1b9ae

Added associations

daf5946

Added seed

12c8eca

Added telegram conversation service

8736004

Added telegram lesson service

a1b7a1c

Added telegram lesson task and Rakefile

489069b

Added gem whenever and schedule.rb

8163986

Added gem rspec and factory_bot

fb81bb9

Added conversation spec

d81b5d8

Added Readme.md

56e0308

hound bot reviewed May 3, 2021

View reviewed changes

kkrasilov added 2 commits May 3, 2021 23:39

Sorted gems in an alphabetical (hound fix)

c9bba57

Used ascii comments and range#cover (hound fix)

79d23cb

hound bot reviewed May 3, 2021

View reviewed changes

olgamalofeeva requested a review from Saicheg May 4, 2021 18:46

olgamalofeeva assigned Saicheg May 4, 2021

Saicheg requested changes May 12, 2021

View reviewed changes

kkrasilov added 5 commits May 25, 2021 23:33

Added aasm gem

08da707

Added separate conversation services

2714874

Change telegram lesson service

bb18cb6

Added reminder service

0f131e8

Added new task

7c3b816

hound bot reviewed May 25, 2021

View reviewed changes

		@@ -0,0 +1,6 @@
		# frozen_string_literal: true

		class UserWord < ActiveRecord::Base

		@@ -0,0 +1,10 @@
		# frozen_string_literal: true

		class Word < ActiveRecord::Base


		Dotenv.load

		WELCOME = 'Привет! Я бот, который помогает учить новые английские слова каждый день. Давай сперва определимся,' \

Красилов Константин - 3 #57

Are you sure you want to change the base?

Красилов Константин - 3 #57

Uh oh!

Conversation

kkrasilov commented May 3, 2021

Фамилия Имя

Email

Номер домашнего задания

Ссылка на видео с демо работы

Комментарии

Uh oh!

hound bot left a comment

Choose a reason for hiding this comment

Uh oh!

hound bot May 3, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hound bot May 3, 2021

Choose a reason for hiding this comment

Uh oh!

hound bot left a comment

Choose a reason for hiding this comment

Uh oh!

hound bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkrasilov May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkrasilov May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkrasilov May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hound bot left a comment

Choose a reason for hiding this comment

Uh oh!

hound bot May 25, 2021

Choose a reason for hiding this comment

Uh oh!

hound bot May 25, 2021

Choose a reason for hiding this comment

Uh oh!

hound bot May 25, 2021

Choose a reason for hiding this comment

Uh oh!

hound bot May 25, 2021

Choose a reason for hiding this comment

Uh oh!

hound bot May 25, 2021

Choose a reason for hiding this comment

Uh oh!

hound bot May 25, 2021

Choose a reason for hiding this comment

kkrasilov May 20, 2021 •

edited

Loading

kkrasilov May 20, 2021 •

edited

Loading

kkrasilov May 20, 2021 •

edited

Loading