[ Feature] Remind active users before sending results #6
Conversation
delusioninabox
force-pushed
the
feature/final-reminder
branch
from
e468218 to
59f4f26
Compare
|
Need to mock out the |
| include Sidekiq::Worker | ||
| def perform | ||
|
|
||
| # First check there is an active question |
There was a problem hiding this comment.
We should use guard clauses instead of nesting conditionals. If there are no open questions or no active users we can return from this method. Then the following code will be much more straightforward to read
app/workers/reminder_worker.rb
Outdated
| if all_users | ||
| all_users.each do |user| | ||
| # Has this user answered? | ||
| if todays_question.responses.where(user: user).none? |
There was a problem hiding this comment.
It may be better to add a scope or class method to the user model to query only users who haven't answered today's question from the DB instead of loading all users from the DB and then checking if they've answered
There was a problem hiding this comment.
Trying to write this as a scope is making my head spin. 😭I can get where they haven't answered, but once I try to include the criteria of only where the question is open, it all breaks.
class User < ApplicationRecord
has_many :responses
scope :hasnt_answered, -> {
where(active: true).left_outer_joins(:responses => :question).where(responses: {user_id: nil, question: {open: true}})
}
end
app/workers/reminder_worker.rb
Outdated
| message = ":bow: Sorry to disturb your work…I haven’t received an answer from you yet. :pray: I'm a good listener if you have a few minutes. :ear:" | ||
| attachments = [] | ||
| # Get all active users and DM them the question | ||
| PostToSlack.post_slack_msg( user.ucode, message, attachments ) |
There was a problem hiding this comment.
Can you remove the padding spaces inside of parens? It's against our style guide
|
This looks pretty good. I think Lawrence had some really good feedback for you. |
Requirements
Tests