-
Notifications
You must be signed in to change notification settings - Fork 0
Lesson18 #19
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
base: main
Are you sure you want to change the base?
Conversation
| end | ||
| end | ||
|
|
||
| def time_expired? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
можно это в метод модели разместить
| @test_passage.accept!(params[:answer_ids]) | ||
|
|
||
| if @test_passage.completed? | ||
| if time_expired? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а надо ли делать отдельный иф под то что время вышло? мне кажется можно расширить условие просто изначальное if @test_passage.completed? || time_expired?
app/services/badge_assigner.rb
Outdated
|
|
||
| def call | ||
| Badge.where(rule: RULES).each do |badge| | ||
| assign_badge(badge) if rule_satisfied?(badge.rule) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Выдать бэйдж после успешного прохождения всех тестов из категории Backend
Выдать бэйдж после успешного прохождения всех тестов определённого уровня
задание подразумевает что можно задавать параметры для баджей. что награду дать за определенную категорию или за уровень. в твоей реализации ты за каждую категорию/уровень будешь выдавать бадж. а может быть так что за какие-то категории/уврони ты этого делать не хочешь
надо добавить параметр в твои баджи
app/services/badge_assigner.rb
Outdated
|
|
||
| def all_category_tests_passed? | ||
| category = @test_passage.test.category | ||
| category.tests.all? { |test| test_successful?(test) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
у тебя тут много рубишной обработки результатов. и successful_passage? может быть довольно тяжелым методом. надо максимально все вынести на уровень БД, чтобы ее ресурсами получить максимум подготовленных данных.
у тебя тут по сути 3 действия должно быть
1 - получить все тесты категории/уровня
2 - получить все пройденные тесты этой категории/уровня
3 - сравнить между собой 1 и 2
app/services/badge_assigner.rb
Outdated
| end | ||
|
|
||
| def rule_satisfied?(rule) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишняя пустая строка
app/services/badge_assigner.rb
Outdated
| def rule_satisfied?(rule) | ||
|
|
||
| if RULES.include?(rule) | ||
| send("#{rule}?") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
отступа не хватает
|
|
||
| def set_flash_alert | ||
| flash[:alert] = | ||
| if @test_passage.time_expired? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
у тебя только что была эта проверка на 15ой строке, и тут же в методе следующем ты о5 ее делаешь. выглядит неоптимально. время выполнения х2
|
|
||
| before_validation :before_validation_set_first_question, on: %i[create update] | ||
|
|
||
| scope :successful, lambda { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
очень сложное решение. просто во время завершения теста сохрани результат в базу успешно или не успешно тест пройден. новое поле в БД с boolean типом данных. тогда выбор прохождений будет очень простой
| .group('test_passages.id') | ||
| .having('correct_questions * 100.0 / COUNT(questions.id) >= ?', COMPLETION_SCORE) | ||
| } | ||
| scope :category, lambda { |category_id| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
много ненужных скоупов. они нужны когда функциоанльность запроса требуется в нескольких точках приложения. у тебя она требуется только в сервисе, можно там и оставить запросы пока что
| send("#{rule}?", value) | ||
| end | ||
|
|
||
| def all_category_tests_passed?(category_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тут будем придерживаться того условия что админ будет заполнять значение не по id категорий, а по их названиям. помнишь было задание поиск тестов по названию категории. вот тут метод реализованный там нам и пригодиться
|
|
||
| def all_category_tests_passed?(category_id) | ||
| completed_ids = TestPassage.completed.successful.category(category_id).pluck(:test_id) | ||
| test_ids = Test.where(category: category_id).pluck(:id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тут проще найти сначала все тесты категории.
и потом этот результат использовать для поиска успешных пройденных, чтобы не джойнить лишние таблицы(категории)
| } | ||
|
|
||
| scope :attempts_count, lambda { |user_id, test_id| | ||
| completed.where(user_id: user_id, test_id: test_id).count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
скоуп должен возвращать объект, позволяющий продолжать вызывать цепочку скопуов или методво ActiveRecord. в твоем случае ты возвращаешь число и ломаешь эту возможность
No description provided.