From 5eb48917fbeb2e414bead1a48574ab7c35f9866a Mon Sep 17 00:00:00 2001 From: Laura Kajpust Date: Fri, 4 Jan 2019 16:55:34 -0600 Subject: [PATCH 1/3] Create reminder worker --- app/workers/reminder_worker.rb | 26 ++++++++++++++++++++++++++ config/schedule.yml | 4 ++++ 2 files changed, 30 insertions(+) create mode 100644 app/workers/reminder_worker.rb diff --git a/app/workers/reminder_worker.rb b/app/workers/reminder_worker.rb new file mode 100644 index 0000000..65dd9cc --- /dev/null +++ b/app/workers/reminder_worker.rb @@ -0,0 +1,26 @@ +# TODO: Move worker logic into service +require 'post_to_slack' +class ReminderWorker + include Sidekiq::Worker + def perform + + # First check there is an active question + if Question.open.any? + # get all active users and the active question + all_users = User.where(active: true) + todays_question = Question.open.first + if all_users + all_users.each do |user| + # Has this user answered? + if question.responses.where(user: user).none? + 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.channel, message, attachments ) + end + end + end + end + + end +end \ No newline at end of file diff --git a/config/schedule.yml b/config/schedule.yml index 56ac9d4..f8ae7fd 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -2,6 +2,10 @@ send_todays_question: cron: "0 8 * * 1,3,5 America/Chicago" class: "QuestionWorker" +remind_active_users: + cron: "0 12 * * 1-5 America/Chicago" + class: "ReminderWorker" + post_all_results: cron: "0 14 * * 1,3,5 America/Chicago" class: "ResultsWorker" \ No newline at end of file From 59f4f26b7f9f36245c6b092192ab6ec2c86df5cf Mon Sep 17 00:00:00 2001 From: Laura Kajpust Date: Sun, 17 Feb 2019 12:42:27 -0600 Subject: [PATCH 2/3] Create tests --- .rspec | 1 + Gemfile | 6 ++ Gemfile.lock | 33 ++++++++++ app/workers/reminder_worker.rb | 4 +- config/schedule.yml | 2 +- spec/factories/question.rb | 19 ++++++ spec/factories/response.rb | 7 ++ spec/factories/user.rb | 10 +++ spec/rails_helper.rb | 57 +++++++++++++++++ spec/spec_helper.rb | 96 ++++++++++++++++++++++++++++ spec/workers/reminder_worker_spec.rb | 25 ++++++++ 11 files changed, 257 insertions(+), 3 deletions(-) create mode 100644 .rspec create mode 100644 spec/factories/question.rb create mode 100644 spec/factories/response.rb create mode 100644 spec/factories/user.rb create mode 100644 spec/rails_helper.rb create mode 100644 spec/spec_helper.rb create mode 100644 spec/workers/reminder_worker_spec.rb diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..c99d2e7 --- /dev/null +++ b/.rspec @@ -0,0 +1 @@ +--require spec_helper diff --git a/Gemfile b/Gemfile index 273ad5c..8f64595 100644 --- a/Gemfile +++ b/Gemfile @@ -39,12 +39,18 @@ gem 'sidekiq' gem 'sidekiq-cron' gem 'honeybadger', '~> 4.0' +# Testing +gem 'factory_bot_rails' +gem 'faker' + group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] # Adds support for Capybara system testing and selenium driver gem 'capybara', '~> 2.13' gem 'selenium-webdriver' + gem 'rspec', '~> 3.6.0' + gem 'rspec-rails', '~> 3.6.0' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 542828e..8efc6aa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -63,10 +63,18 @@ GEM concurrent-ruby (1.0.5) connection_pool (2.2.2) crass (1.0.4) + diff-lcs (1.3) erubi (1.7.1) et-orbi (1.1.6) tzinfo execjs (2.7.0) + factory_bot (5.0.1) + activesupport (>= 4.2.0) + factory_bot_rails (5.0.1) + factory_bot (~> 5.0.0) + railties (>= 4.2.0) + faker (1.9.3) + i18n (>= 0.7) ffi (1.9.25) figaro (1.1.1) thor (~> 0.14) @@ -142,6 +150,27 @@ GEM rb-inotify (0.9.10) ffi (>= 0.5.0, < 2) redis (4.1.0) + rspec (3.6.0) + rspec-core (~> 3.6.0) + rspec-expectations (~> 3.6.0) + rspec-mocks (~> 3.6.0) + rspec-core (3.6.0) + rspec-support (~> 3.6.0) + rspec-expectations (3.6.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.6.0) + rspec-mocks (3.6.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.6.0) + rspec-rails (3.6.1) + actionpack (>= 3.0) + activesupport (>= 3.0) + railties (>= 3.0) + rspec-core (~> 3.6.0) + rspec-expectations (~> 3.6.0) + rspec-mocks (~> 3.6.0) + rspec-support (~> 3.6.0) + rspec-support (3.6.0) ruby_dep (1.5.0) rubyzip (1.2.2) sass (3.6.0) @@ -205,6 +234,8 @@ DEPENDENCIES byebug capybara (~> 2.13) coffee-rails (~> 4.2) + factory_bot_rails + faker figaro honeybadger (~> 4.0) httparty @@ -213,6 +244,8 @@ DEPENDENCIES pg puma (~> 3.7) rails (~> 5.1.4) + rspec (~> 3.6.0) + rspec-rails (~> 3.6.0) sass-rails (~> 5.0) selenium-webdriver sidekiq diff --git a/app/workers/reminder_worker.rb b/app/workers/reminder_worker.rb index 65dd9cc..abb6c75 100644 --- a/app/workers/reminder_worker.rb +++ b/app/workers/reminder_worker.rb @@ -12,11 +12,11 @@ def perform if all_users all_users.each do |user| # Has this user answered? - if question.responses.where(user: user).none? + if todays_question.responses.where(user: user).none? 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.channel, message, attachments ) + PostToSlack.post_slack_msg( user.ucode, message, attachments ) end end end diff --git a/config/schedule.yml b/config/schedule.yml index f8ae7fd..543887d 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -3,7 +3,7 @@ send_todays_question: class: "QuestionWorker" remind_active_users: - cron: "0 12 * * 1-5 America/Chicago" + cron: "0 12 * * 1,3,5 America/Chicago" class: "ReminderWorker" post_all_results: diff --git a/spec/factories/question.rb b/spec/factories/question.rb new file mode 100644 index 0000000..ebab11c --- /dev/null +++ b/spec/factories/question.rb @@ -0,0 +1,19 @@ +FactoryBot.define do + factory :question, class: Question do + user { FactoryBot.create(:user) } + question { Faker::Lorem.question } + + trait :with_answers do + after :create do |question| + FactoryBot.create(:response, answer: Faker::Lorem.sentence, question: question) + FactoryBot.create(:response, answer: Faker::Lorem.sentence, question: question) + FactoryBot.create(:response, answer: Faker::Lorem.sentence, question: question) + end + end + + trait :is_open do + open { true } + end + + end +end \ No newline at end of file diff --git a/spec/factories/response.rb b/spec/factories/response.rb new file mode 100644 index 0000000..930aa0a --- /dev/null +++ b/spec/factories/response.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :response, class: Response do + user { FactoryBot.create(:user) } + answer { Faker::Lorem.sentence } + association :question + end +end \ No newline at end of file diff --git a/spec/factories/user.rb b/spec/factories/user.rb new file mode 100644 index 0000000..7f3a440 --- /dev/null +++ b/spec/factories/user.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :user, class: User do + ucode { Faker::Lorem.characters(8) } + active { true } + + trait :inactive do + active { false } + end + end +end \ No newline at end of file diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb new file mode 100644 index 0000000..46ba8e0 --- /dev/null +++ b/spec/rails_helper.rb @@ -0,0 +1,57 @@ +# This file is copied to spec/ when you run 'rails generate rspec:install' +require 'spec_helper' +ENV['RAILS_ENV'] ||= 'test' +require File.expand_path('../../config/environment', __FILE__) +# Prevent database truncation if the environment is production +abort("The Rails environment is running in production mode!") if Rails.env.production? +require 'rspec/rails' +# Add additional requires below this line. Rails is not loaded until this point! + +# Requires supporting ruby files with custom matchers and macros, etc, in +# spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are +# run as spec files by default. This means that files in spec/support that end +# in _spec.rb will both be required and run as specs, causing the specs to be +# run twice. It is recommended that you do not name files matching this glob to +# end with _spec.rb. You can configure this pattern with the --pattern +# option on the command line or in ~/.rspec, .rspec or `.rspec-local`. +# +# The following line is provided for convenience purposes. It has the downside +# of increasing the boot-up time by auto-requiring all files in the support +# directory. Alternatively, in the individual `*_spec.rb` files, manually +# require only the support files necessary. +# +# Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f } + +# Checks for pending migration and applies them before tests are run. +# If you are not using ActiveRecord, you can remove this line. +ActiveRecord::Migration.maintain_test_schema! + +RSpec.configure do |config| + # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures + config.fixture_path = "#{::Rails.root}/spec/fixtures" + + # If you're not using ActiveRecord, or you'd prefer not to run each of your + # examples within a transaction, remove the following line or assign false + # instead of true. + config.use_transactional_fixtures = true + + # RSpec Rails can automatically mix in different behaviours to your tests + # based on their file location, for example enabling you to call `get` and + # `post` in specs under `spec/controllers`. + # + # You can disable this behaviour by removing the line below, and instead + # explicitly tag your specs with their type, e.g.: + # + # RSpec.describe UsersController, :type => :controller do + # # ... + # end + # + # The different available types are documented in the features, such as in + # https://relishapp.com/rspec/rspec-rails/docs + config.infer_spec_type_from_file_location! + + # Filter lines from Rails gems in backtraces. + config.filter_rails_from_backtrace! + # arbitrary gems may also be filtered via: + # config.filter_gems_from_backtrace("gem name") +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 0000000..ce33d66 --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,96 @@ +# This file was generated by the `rails generate rspec:install` command. Conventionally, all +# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. +# The generated `.rspec` file contains `--require spec_helper` which will cause +# this file to always be loaded, without a need to explicitly require it in any +# files. +# +# Given that it is always loaded, you are encouraged to keep this file as +# light-weight as possible. Requiring heavyweight dependencies from this file +# will add to the boot time of your test suite on EVERY test run, even for an +# individual file that may not need all of that loaded. Instead, consider making +# a separate helper file that requires the additional dependencies and performs +# the additional setup, and require it from the spec files that actually need +# it. +# +# See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration +RSpec.configure do |config| + # rspec-expectations config goes here. You can use an alternate + # assertion/expectation library such as wrong or the stdlib/minitest + # assertions if you prefer. + config.expect_with :rspec do |expectations| + # This option will default to `true` in RSpec 4. It makes the `description` + # and `failure_message` of custom matchers include text for helper methods + # defined using `chain`, e.g.: + # be_bigger_than(2).and_smaller_than(4).description + # # => "be bigger than 2 and smaller than 4" + # ...rather than: + # # => "be bigger than 2" + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + end + + # rspec-mocks config goes here. You can use an alternate test double + # library (such as bogus or mocha) by changing the `mock_with` option here. + config.mock_with :rspec do |mocks| + # Prevents you from mocking or stubbing a method that does not exist on + # a real object. This is generally recommended, and will default to + # `true` in RSpec 4. + mocks.verify_partial_doubles = true + end + + # This option will default to `:apply_to_host_groups` in RSpec 4 (and will + # have no way to turn it off -- the option exists only for backwards + # compatibility in RSpec 3). It causes shared context metadata to be + # inherited by the metadata hash of host groups and examples, rather than + # triggering implicit auto-inclusion in groups with matching metadata. + config.shared_context_metadata_behavior = :apply_to_host_groups + +# The settings below are suggested to provide a good initial experience +# with RSpec, but feel free to customize to your heart's content. +=begin + # This allows you to limit a spec run to individual examples or groups + # you care about by tagging them with `:focus` metadata. When nothing + # is tagged with `:focus`, all examples get run. RSpec also provides + # aliases for `it`, `describe`, and `context` that include `:focus` + # metadata: `fit`, `fdescribe` and `fcontext`, respectively. + config.filter_run_when_matching :focus + + # Allows RSpec to persist some state between runs in order to support + # the `--only-failures` and `--next-failure` CLI options. We recommend + # you configure your source control system to ignore this file. + config.example_status_persistence_file_path = "spec/examples.txt" + + # Limits the available syntax to the non-monkey patched syntax that is + # recommended. For more details, see: + # - http://rspec.info/blog/2012/06/rspecs-new-expectation-syntax/ + # - http://www.teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/ + # - http://rspec.info/blog/2014/05/notable-changes-in-rspec-3/#zero-monkey-patching-mode + config.disable_monkey_patching! + + # Many RSpec users commonly either run the entire suite or an individual + # file, and it's useful to allow more verbose output when running an + # individual spec file. + if config.files_to_run.one? + # Use the documentation formatter for detailed output, + # unless a formatter has already been configured + # (e.g. via a command-line flag). + config.default_formatter = "doc" + end + + # Print the 10 slowest examples and example groups at the + # end of the spec run, to help surface which specs are running + # particularly slow. + config.profile_examples = 10 + + # Run specs in random order to surface order dependencies. If you find an + # order dependency and want to debug it, you can fix the order by providing + # the seed, which is printed after each run. + # --seed 1234 + config.order = :random + + # Seed global randomization in this process using the `--seed` CLI option. + # Setting this allows you to use `--seed` to deterministically reproduce + # test failures related to randomization by passing the same `--seed` value + # as the one that triggered the failure. + Kernel.srand config.seed +=end +end diff --git a/spec/workers/reminder_worker_spec.rb b/spec/workers/reminder_worker_spec.rb new file mode 100644 index 0000000..209ddf0 --- /dev/null +++ b/spec/workers/reminder_worker_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' +require 'sidekiq/testing' + +RSpec.describe ReminderWorker, type: :worker do + subject { ReminderWorker.new.perform() } + + # The unanaswered user is the one who wrote the question + # Generates 4 active users, 3 in :with_answers, 1 as the question's author + let!(:question) { FactoryBot.create(:question, :with_answers, :is_open) } + let!(:inactive_users) { FactoryBot.create_list(:user, 2, :inactive) } + + context 'reminds users to answer' do + it 'has one question with 2 answers' do + subject + expect(Question.all.count).to eq(1) + expect(Question.first.responses.count).to eq(3) + end + + it 'only reminds active users who havent answered' do + expect(PostToSlack).to receive(:post_slack_msg).once + subject + end + end + +end \ No newline at end of file From 18ae6bdf190b06c76fdf8778b444d163ef5184e4 Mon Sep 17 00:00:00 2001 From: Laura Kajpust Date: Sat, 23 Feb 2019 12:49:53 -0600 Subject: [PATCH 3/3] Cleanup worker (new spec WIP) --- app/models/user.rb | 4 ++++ app/workers/reminder_worker.rb | 27 ++++++++++++--------------- spec/workers/reminder_worker_spec.rb | 2 ++ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 043ab65..e2aa87b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,3 +1,7 @@ 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 diff --git a/app/workers/reminder_worker.rb b/app/workers/reminder_worker.rb index abb6c75..e5a4b55 100644 --- a/app/workers/reminder_worker.rb +++ b/app/workers/reminder_worker.rb @@ -5,21 +5,18 @@ class ReminderWorker def perform # First check there is an active question - if Question.open.any? - # get all active users and the active question - all_users = User.where(active: true) - todays_question = Question.open.first - if all_users - all_users.each do |user| - # Has this user answered? - if todays_question.responses.where(user: user).none? - 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 ) - end - end - end + return if Question.open.none? + # get all active users and the active question + all_users = User.hasnt_answered + todays_question = Question.open.first + + return if all_users.none? + all_users.each do |user| + # Has this user answered? + 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) end end diff --git a/spec/workers/reminder_worker_spec.rb b/spec/workers/reminder_worker_spec.rb index 209ddf0..960cf8c 100644 --- a/spec/workers/reminder_worker_spec.rb +++ b/spec/workers/reminder_worker_spec.rb @@ -6,6 +6,8 @@ # The unanaswered user is the one who wrote the question # Generates 4 active users, 3 in :with_answers, 1 as the question's author + # Generate an inactive question to be sure doesn't check that one + let!(:old_question) { FactoryBot.create(:question,) } let!(:question) { FactoryBot.create(:question, :with_answers, :is_open) } let!(:inactive_users) { FactoryBot.create_list(:user, 2, :inactive) }