diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 88d42d269..b026f6180 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --auto-gen-only-exclude --no-exclude-limit` -# on 2026-07-28 16:48:26 UTC using RuboCop version 1.88.2. +# on 2026-07-30 08:58:12 UTC using RuboCop version 1.88.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -18,7 +18,6 @@ Capybara/RSpec/VisibilityMatcher: Exclude: - 'spec/components/chapters_sidebar_component_spec.rb' - # Offense count: 2 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: AllowSafeAssignment. @@ -52,13 +51,6 @@ Lint/UnderscorePrefixedVariableName: Exclude: - 'spec/models/group_spec.rb' -# Offense count: 5 -# This cop supports unsafe autocorrection (--autocorrect-all). -Lint/UselessTimes: - Exclude: - - 'spec/models/event_spec.rb' - - 'spec/models/workshop_spec.rb' - # Offense count: 32 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes, Max. Metrics/AbcSize: @@ -102,7 +94,7 @@ Metrics/ClassLength: - 'lib/omniauth/strategies/codebar.rb' - 'lib/tasks/setup.rake' -# Offense count: 9 +# Offense count: 8 # Configuration parameters: AllowedMethods, AllowedPatterns, Max. Metrics/CyclomaticComplexity: Exclude: @@ -110,7 +102,6 @@ Metrics/CyclomaticComplexity: - 'app/controllers/auth_services_controller.rb' - 'app/controllers/invitations_controller.rb' - 'app/controllers/workshop_invitation_controller.rb' - - 'app/presenters/how_you_found_us_presenter.rb' - 'app/services/invitation_manager.rb' - 'lib/flodesk.rb' - 'lib/omniauth/strategies/codebar.rb' @@ -164,7 +155,6 @@ Naming/AccessorMethodName: - 'app/controllers/admin/workshops_controller.rb' - 'app/controllers/feedback_controller.rb' - # Offense count: 1 # Configuration parameters: EnforcedStyle, AllowedPatterns, ForbiddenIdentifiers, ForbiddenPatterns. # SupportedStyles: snake_case, camelCase @@ -189,45 +179,18 @@ Naming/PredicatePrefix: - 'app/policies/event_policy.rb' - 'app/policies/workshop_policy.rb' - - # Offense count: 2 RSpec/AnyInstance: Exclude: - 'spec/controllers/member/details_controller_spec.rb' - 'spec/support/helpers/login_helpers.rb' - # Offense count: 1 # Configuration parameters: Max, CountAsOne. RSpec/ExampleLength: Exclude: - 'spec/features/admin/event_spec.rb' -# Offense count: 68 -# Configuration parameters: EnforcedStyle. -# SupportedStyles: have_received, receive -RSpec/MessageSpies: - Exclude: - - 'spec/controllers/admin/members_controller_spec.rb' - - 'spec/controllers/payments_controller_spec.rb' - - 'spec/features/admin/workshops_spec.rb' - - 'spec/features/subscribing_to_newsletter_spec.rb' - - 'spec/helpers/email_header_helper_spec.rb' - - 'spec/lib/services/mailing_list_spec.rb' - - 'spec/lib/tasks/feedback_rake_spec.rb' - - 'spec/lib/tasks/mailing_list_rake_spec.rb' - - 'spec/lib/tasks/reminders_meeting_rake_spec.rb' - - 'spec/lib/tasks/reminders_workshop_rake_spec.rb' - - 'spec/presenters/meeting_presenter_spec.rb' - - 'spec/presenters/member_presenter_spec.rb' - - 'spec/presenters/sponsor_presenter_spec.rb' - - 'spec/presenters/virtual_workshop_presenter_spec.rb' - - 'spec/presenters/workshop_presenter_spec.rb' - - 'spec/services/invitation_manager_spec.rb' - - 'spec/support/shared_examples/behaves_like_an_invitation.rb' - - 'spec/support/shared_examples/behaves_like_sending_workshop_emails.rb' - # Offense count: 3 # Configuration parameters: Max. RSpec/MultipleExpectations: @@ -236,34 +199,6 @@ RSpec/MultipleExpectations: - 'spec/features/admin/sponsor_spec.rb' - 'spec/lib/omniauth/strategies/codebar_spec.rb' -# Offense count: 19 -RSpec/StubbedMock: - Exclude: - - 'spec/controllers/admin/members_controller_spec.rb' - - 'spec/controllers/payments_controller_spec.rb' - - 'spec/features/admin/workshops_spec.rb' - - 'spec/features/subscribing_to_newsletter_spec.rb' - - 'spec/lib/services/mailing_list_spec.rb' - - 'spec/lib/tasks/feedback_rake_spec.rb' - - 'spec/lib/tasks/mailing_list_rake_spec.rb' - - 'spec/lib/tasks/reminders_meeting_rake_spec.rb' - - 'spec/lib/tasks/reminders_workshop_rake_spec.rb' - - 'spec/presenters/meeting_presenter_spec.rb' - -# Offense count: 38 -# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. -RSpec/VerifiedDoubles: - Exclude: - - 'spec/controllers/admin/member_search_controller_spec.rb' - - 'spec/features/subscribing_to_newsletter_spec.rb' - - 'spec/lib/services/mailing_list_spec.rb' - - 'spec/policies/application_policy_spec.rb' - - 'spec/presenters/event_presenter_spec.rb' - - 'spec/presenters/virtual_workshop_presenter_spec.rb' - - 'spec/presenters/workshop_presenter_spec.rb' - - - # Offense count: 5 Rails/HasAndBelongsToMany: Exclude: @@ -291,7 +226,6 @@ Rails/HelperInstanceVariable: Exclude: - 'app/helpers/email_helper.rb' - # Offense count: 2 # Configuration parameters: IgnoreScopes. Rails/InverseOf: @@ -321,7 +255,6 @@ Rails/SkipsModelValidations: - 'app/controllers/workshop_invitation_controller.rb' - 'app/services/invitation_logger.rb' - # Offense count: 8 Rails/UniqueValidationWithoutIndex: Exclude: @@ -332,12 +265,3 @@ Rails/UniqueValidationWithoutIndex: - 'app/models/meeting_invitation.rb' - 'app/models/workshop_invitation.rb' - 'app/models/workshop_sponsor.rb' - - - - - - - - - diff --git a/spec/controllers/admin/member_search_controller_spec.rb b/spec/controllers/admin/member_search_controller_spec.rb index 6ac69abc8..07faa90f9 100644 --- a/spec/controllers/admin/member_search_controller_spec.rb +++ b/spec/controllers/admin/member_search_controller_spec.rb @@ -43,7 +43,7 @@ end context 'when an admin user searches and there are multiple results' do - let(:fake_romeo) { double('Member', id: 2, name: 'Romeo', surname: 'Capulet') } + let(:fake_romeo) { instance_double(Member, id: 2, name: 'Romeo', surname: 'Capulet') } before do allow(Member).to receive(:find_members_by_name).with('e').and_return(fake_relation) diff --git a/spec/controllers/admin/members_controller_spec.rb b/spec/controllers/admin/members_controller_spec.rb index 715ded2e8..05e96c118 100644 --- a/spec/controllers/admin/members_controller_spec.rb +++ b/spec/controllers/admin/members_controller_spec.rb @@ -85,11 +85,14 @@ it 'sends an attendance warning email' do mailer = double(deliver_now: true) - expect(MemberMailer).to receive(:attendance_warning) + allow(MemberMailer).to receive(:attendance_warning) .with(member, member.email) .and_return(mailer) get :send_attendance_email, params: { member_id: member.id } + + expect(MemberMailer).to have_received(:attendance_warning) + .with(member, member.email) end it 'redirects to the member page' do @@ -126,11 +129,14 @@ it 'sends an eligibility check email' do mailer = double(deliver_now: true) - expect(MemberMailer).to receive(:eligibility_check) + allow(MemberMailer).to receive(:eligibility_check) .with(member, member.email) .and_return(mailer) get :send_eligibility_email, params: { member_id: member.id } + + expect(MemberMailer).to have_received(:eligibility_check) + .with(member, member.email) end it 'redirects to the member page' do diff --git a/spec/controllers/payments_controller_spec.rb b/spec/controllers/payments_controller_spec.rb index 6950159a0..026f75492 100644 --- a/spec/controllers/payments_controller_spec.rb +++ b/spec/controllers/payments_controller_spec.rb @@ -10,7 +10,7 @@ describe 'POST #create' do context 'with valid parameters' do it 'creates a Stripe customer and charge' do - expect(Stripe::Customer).to receive(:create).with( + allow(Stripe::Customer).to receive(:create).with( email: 'john@example.com', description: 'John Doe', source: 'tok_123' @@ -25,6 +25,11 @@ } } expect(response).to be_successful + expect(Stripe::Customer).to have_received(:create).with( + email: 'john@example.com', + description: 'John Doe', + source: 'tok_123' + ) end end diff --git a/spec/features/admin/workshops_spec.rb b/spec/features/admin/workshops_spec.rb index e656f6a60..df7eac7a2 100644 --- a/spec/features/admin/workshops_spec.rb +++ b/spec/features/admin/workshops_spec.rb @@ -183,21 +183,23 @@ context 'when sending invitations to attendees' do scenario 'for a workshop' do workshop = Fabricate(:workshop) - expect(InvitationManager).to receive(:new).and_return(double.as_null_object) + allow(InvitationManager).to receive(:new).and_return(double.as_null_object) visit admin_workshop_send_invites_path(workshop) click_on 'Students' + expect(InvitationManager).to have_received(:new) expect(page).to have_text('Invitations to students are being emailed out') end scenario 'for a virtual workshop' do workshop = Fabricate(:virtual_workshop) - expect(InvitationManager).to receive(:new).and_return(double.as_null_object) + allow(InvitationManager).to receive(:new).and_return(double.as_null_object) visit admin_workshop_send_invites_path(workshop) click_on 'Students' + expect(InvitationManager).to have_received(:new) expect(page).to have_text('Invitations to students are being emailed out') end end diff --git a/spec/features/subscribing_to_newsletter_spec.rb b/spec/features/subscribing_to_newsletter_spec.rb index 23a84c4e0..62c1648aa 100644 --- a/spec/features/subscribing_to_newsletter_spec.rb +++ b/spec/features/subscribing_to_newsletter_spec.rb @@ -13,9 +13,9 @@ context 'when a new member' do scenario 'is subscribed to the newsletter by default' do - mailing_list = double(:mailing_list) - expect(Services::MailingList).to receive(:new).and_return(mailing_list) - expect(mailing_list).to receive(:subscribe).with('jane@codebar.io', 'Jane', 'Doe') + mailing_list = instance_double(Services::MailingList) + allow(Services::MailingList).to receive(:new).and_return(mailing_list) + allow(mailing_list).to receive(:subscribe).with('jane@codebar.io', 'Jane', 'Doe') visit new_member_path click_on 'Join us as a coach' @@ -30,12 +30,14 @@ find_by_id('member_how_you_found_us_from_a_friend').click click_on 'Next' + + expect(mailing_list).to have_received(:subscribe).with('jane@codebar.io', 'Jane', 'Doe') end scenario 'can opt out of the newsletter' do - mailing_list = double(:mailing_list) - expect(Services::MailingList).to receive(:new).and_return(mailing_list) - expect(mailing_list).to receive(:unsubscribe).with('jane@codebar.io') + mailing_list = instance_double(Services::MailingList) + allow(Services::MailingList).to receive(:new).and_return(mailing_list) + allow(mailing_list).to receive(:unsubscribe).with('jane@codebar.io') visit new_member_path click_on 'Join us as a coach' @@ -53,6 +55,8 @@ uncheck 'newsletter' click_on 'Next' + + expect(mailing_list).to have_received(:unsubscribe).with('jane@codebar.io') end end @@ -62,13 +66,14 @@ login member - mailing_list = double(:mailing_list) - expect(Services::MailingList).to receive(:new).and_return(mailing_list) - expect(mailing_list).to receive(:subscribe) + mailing_list = instance_double(Services::MailingList) + allow(Services::MailingList).to receive(:new).and_return(mailing_list) + allow(mailing_list).to receive(:subscribe) visit subscriptions_path click_on 'Subscribe to newsletter' + expect(mailing_list).to have_received(:subscribe) expect(page).to have_text('You have subscribed to codebar\'s newsletter') end @@ -77,13 +82,14 @@ login member - mailing_list = double(:mailing_list) - expect(Services::MailingList).to receive(:new).and_return(mailing_list) - expect(mailing_list).to receive(:unsubscribe) + mailing_list = instance_double(Services::MailingList) + allow(Services::MailingList).to receive(:new).and_return(mailing_list) + allow(mailing_list).to receive(:unsubscribe) visit subscriptions_path click_on 'Unsubscribe from newsletter' + expect(mailing_list).to have_received(:unsubscribe) expect(page).to have_text('You have unsubscribed from codebar\'s newsletter') end @@ -92,20 +98,20 @@ login member - mailing_list = double(:mailing_list) - expect(Services::MailingList).to receive(:new).and_return(mailing_list) - expect(mailing_list).to receive(:subscribe) + mailing_list = instance_double(Services::MailingList) + allow(Services::MailingList).to receive(:new).and_return(mailing_list) + allow(mailing_list).to receive(:subscribe) + allow(mailing_list).to receive(:unsubscribe) visit subscriptions_path click_on 'Subscribe to newsletter' + expect(mailing_list).to have_received(:subscribe) expect(page).to have_text('You have subscribed to codebar\'s newsletter') - expect(Services::MailingList).to receive(:new).and_return(mailing_list) - expect(mailing_list).to receive(:unsubscribe) - click_on 'Unsubscribe from newsletter' + expect(mailing_list).to have_received(:unsubscribe) expect(page).to have_text('You have unsubscribed from codebar\'s newsletter') end end diff --git a/spec/helpers/email_header_helper_spec.rb b/spec/helpers/email_header_helper_spec.rb index c9f07e07e..4fd135408 100644 --- a/spec/helpers/email_header_helper_spec.rb +++ b/spec/helpers/email_header_helper_spec.rb @@ -7,7 +7,7 @@ let(:member) { Struct.new(:id, :email).new(1, 'test@example.com') } it 'calls mail with correct arguments for valid email' do - expect(helper).to receive(:mail).with( + allow(helper).to receive(:mail).with( from: 'codebar.io ', to: 'test@example.com', cc: '', @@ -15,14 +15,24 @@ subject: 'Test Subject' ) helper.mail_to_member(member, 'Test Subject') + + expect(helper).to have_received(:mail).with( + from: 'codebar.io ', + to: 'test@example.com', + cc: '', + bcc: '', + subject: 'Test Subject' + ) end it 'forwards the block to mail' do - expect(helper).to receive(:mail) do |**kwargs, &block| + allow(helper).to receive(:mail) do |**kwargs, &block| expect(kwargs).to include(from: 'codebar.io ') expect(block).to be_a(Proc) end helper.mail_to_member(member, 'Test Subject', &:html) + + expect(helper).to have_received(:mail) end it 'returns SkippedEmail for nil email' do @@ -57,20 +67,28 @@ it 'logs the skip' do member = Struct.new(:id, :email).new(1, 'bad-email') - expect(Rails.logger).to receive(:info).with(/Skipped email to member 1/) + allow(Rails.logger).to receive(:info).with(/Skipped email to member 1/) helper.mail_to_member(member, 'Test Subject') + + expect(Rails.logger).to have_received(:info).with(/Skipped email to member 1/) end it 'includes custom from email when provided' do - expect(helper).to receive(:mail).with(hash_including(from: 'codebar.io ')) + allow(helper).to receive(:mail).with(hash_including(from: 'codebar.io ')) helper.mail_to_member(member, 'Test Subject', 'custom@codebar.io') + + expect(helper).to have_received(:mail).with(hash_including(from: 'codebar.io ')) end it 'includes cc and bcc when provided' do - expect(helper).to receive(:mail).with( + allow(helper).to receive(:mail).with( hash_including(cc: 'cc@codebar.io', bcc: 'bcc@codebar.io') ) helper.mail_to_member(member, 'Test Subject', 'from@codebar.io', 'cc@codebar.io', 'bcc@codebar.io') + + expect(helper).to have_received(:mail).with( + hash_including(cc: 'cc@codebar.io', bcc: 'bcc@codebar.io') + ) end end end diff --git a/spec/lib/services/mailing_list_spec.rb b/spec/lib/services/mailing_list_spec.rb index 894dfc05e..8f9c95758 100644 --- a/spec/lib/services/mailing_list_spec.rb +++ b/spec/lib/services/mailing_list_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Services::MailingList do let(:mailing_list) { described_class.new(:list_id) } - let(:client) { double(:flodesk) } + let(:client) { instance_double(Flodesk::Client) } before do allow(client).to receive(:disabled?).and_return(false) @@ -16,7 +16,7 @@ describe '#subscribe' do it 'adds a user to the mailing list' do - expect(client).to receive(:subscribe) + allow(client).to receive(:subscribe) .with({ email: :email, first_name: :first_name, @@ -25,24 +25,40 @@ }) mailing_list.subscribe(:email, :first_name, :last_name) + + expect(client).to have_received(:subscribe) + .with({ + email: :email, + first_name: :first_name, + last_name: :last_name, + segment_ids: [:list_id] + }) end end describe '#unsubscribe' do it 'removes a user from the mailing list' do - expect(client).to receive(:unsubscribe) + allow(client).to receive(:unsubscribe) .with({ email: :email, segment_ids: [:list_id] }) mailing_list.unsubscribe(:email) + + expect(client).to have_received(:unsubscribe) + .with({ email: :email, segment_ids: [:list_id] }) end end describe '#subscribed?' do it 'checks if a user is already subscribed to the mailing list' do - expect(client).to receive(:subscribed?) + allow(client).to receive(:subscribed?) .with({ email: :email, segment_ids: [:list_id] }) .and_return(true) - expect(mailing_list.subscribed?(:email)).to be true + + result = mailing_list.subscribed?(:email) + + expect(client).to have_received(:subscribed?) + .with({ email: :email, segment_ids: [:list_id] }) + expect(result).to be true end end end diff --git a/spec/lib/tasks/feedback_rake_spec.rb b/spec/lib/tasks/feedback_rake_spec.rb index f2b768bf8..93df21d1b 100644 --- a/spec/lib/tasks/feedback_rake_spec.rb +++ b/spec/lib/tasks/feedback_rake_spec.rb @@ -17,14 +17,17 @@ student = Fabricate(:member) Fabricate(:attending_workshop_invitation, role: 'Student', member: student, workshop: workshop) - expect(Workshop).to receive(:completed_since_yesterday).and_return([workshop]) - mailer = double(deliver_now: true) - expect(FeedbackRequestMailer).to receive(:request_feedback) + allow(Workshop).to receive(:completed_since_yesterday).and_return([workshop]) + allow(FeedbackRequestMailer).to receive(:request_feedback) .with(workshop, student, an_instance_of(FeedbackRequest)) .and_return(mailer) expect { task.execute }.to change(FeedbackRequest, :count).by(1) + + expect(Workshop).to have_received(:completed_since_yesterday) + expect(FeedbackRequestMailer).to have_received(:request_feedback) + .with(workshop, student, an_instance_of(FeedbackRequest)) end end diff --git a/spec/lib/tasks/mailing_list_rake_spec.rb b/spec/lib/tasks/mailing_list_rake_spec.rb index bd793ee91..367068c5c 100644 --- a/spec/lib/tasks/mailing_list_rake_spec.rb +++ b/spec/lib/tasks/mailing_list_rake_spec.rb @@ -18,21 +18,29 @@ subscribed[0...3].each { |member| Fabricate(:subscription, member: member) } newslettter = Services::MailingList.new(:id) - expect(Services::MailingList).to receive(:new).and_return(newslettter) + allow(Services::MailingList).to receive(:new).and_return(newslettter) subscribed.each do |subscriber| - expect(newslettter).to receive(:subscribe).with(subscriber.email, - subscriber.name, - subscriber.surname).once + allow(newslettter).to receive(:subscribe).with(subscriber.email, + subscriber.name, + subscriber.surname).once end - non_subscribed.each do |inactive_subscriber| - expect(newslettter).not_to receive(:subscribe).with(inactive_subscriber.email, - inactive_subscriber.name, - inactive_subscriber.surname) + task.execute + + expect(Services::MailingList).to have_received(:new) + + subscribed.each do |subscriber| + expect(newslettter).to have_received(:subscribe).with(subscriber.email, + subscriber.name, + subscriber.surname).once end - task.execute + non_subscribed.each do |inactive_subscriber| + expect(newslettter).not_to have_received(:subscribe).with(inactive_subscriber.email, + inactive_subscriber.name, + inactive_subscriber.surname) + end subscribed.each { |subscriber| expect(subscriber.reload.opt_in_newsletter_at).not_to be_nil } end diff --git a/spec/lib/tasks/reminders_meeting_rake_spec.rb b/spec/lib/tasks/reminders_meeting_rake_spec.rb index f890603de..be6974639 100644 --- a/spec/lib/tasks/reminders_meeting_rake_spec.rb +++ b/spec/lib/tasks/reminders_meeting_rake_spec.rb @@ -17,12 +17,15 @@ past_meeting = Fabricate(:meeting, date_and_time: 1.day.ago) invitation_manager = InvitationManager.new - expect(InvitationManager).to receive(:new).and_return(invitation_manager) - expect(invitation_manager).to receive(:send_monthly_attendance_reminder_emails).with(meeting) - expect(invitation_manager).not_to receive(:send_monthly_attendance_reminder_emails).with(past_meeting) - expect(invitation_manager).not_to receive(:send_monthly_attendance_reminder_emails).with(just_now_meeting) + allow(InvitationManager).to receive(:new).and_return(invitation_manager) + allow(invitation_manager).to receive(:send_monthly_attendance_reminder_emails).with(meeting) task.execute + + expect(InvitationManager).to have_received(:new) + expect(invitation_manager).to have_received(:send_monthly_attendance_reminder_emails).with(meeting) + expect(invitation_manager).not_to have_received(:send_monthly_attendance_reminder_emails).with(past_meeting) + expect(invitation_manager).not_to have_received(:send_monthly_attendance_reminder_emails).with(just_now_meeting) end end end diff --git a/spec/lib/tasks/reminders_workshop_rake_spec.rb b/spec/lib/tasks/reminders_workshop_rake_spec.rb index 730acb546..c989cbfcb 100644 --- a/spec/lib/tasks/reminders_workshop_rake_spec.rb +++ b/spec/lib/tasks/reminders_workshop_rake_spec.rb @@ -15,13 +15,15 @@ workshop = Fabricate(:workshop, date_and_time: 29.hours.from_now) invitation_manager = InvitationManager.new - expect(InvitationManager).to receive(:new).and_return(invitation_manager) - expect(invitation_manager).to receive(:send_workshop_attendance_reminders).with(workshop) - - expect(InvitationManager).to receive(:new).and_return(invitation_manager) - expect(invitation_manager).to receive(:send_workshop_waiting_list_reminders).with(workshop) + allow(InvitationManager).to receive(:new).and_return(invitation_manager) + allow(invitation_manager).to receive(:send_workshop_attendance_reminders).with(workshop) + allow(invitation_manager).to receive(:send_workshop_waiting_list_reminders).with(workshop) task.execute + + expect(InvitationManager).to have_received(:new).twice + expect(invitation_manager).to have_received(:send_workshop_attendance_reminders).with(workshop) + expect(invitation_manager).to have_received(:send_workshop_waiting_list_reminders).with(workshop) end end end diff --git a/spec/policies/application_policy_spec.rb b/spec/policies/application_policy_spec.rb index 281d23292..91d14897a 100644 --- a/spec/policies/application_policy_spec.rb +++ b/spec/policies/application_policy_spec.rb @@ -1,7 +1,7 @@ RSpec.describe ApplicationPolicy do subject(:policy) { described_class.new(user, record) } - let(:record) { double('record') } + let(:record) { instance_double(ApplicationRecord) } let(:admin) { Fabricate(:member).tap { |m| m.add_role(:admin) } } let(:regular_member) { Fabricate(:member) } diff --git a/spec/presenters/event_presenter_spec.rb b/spec/presenters/event_presenter_spec.rb index ffd129f19..0eab8265b 100644 --- a/spec/presenters/event_presenter_spec.rb +++ b/spec/presenters/event_presenter_spec.rb @@ -48,18 +48,16 @@ describe '#time' do it 'when no end_time is set it only returns the start_time' do - event = double(:event, date_and_time: Time.zone.now, - start_time: Time.zone.now, - ends_at: nil) + event = instance_double(Event, date_and_time: Time.zone.now, + ends_at: nil) presenter = described_class.new(event) expect(presenter.time).to eq(I18n.l(event.date_and_time, format: :time_with_zone)) end it 'when a start and an end_time are set it returns a formatted start and end_time' do - event = double(:event, date_and_time: Time.zone.now, - start_time: Time.zone.now, - ends_at: 1.hour.from_now) + event = instance_double(Event, date_and_time: Time.zone.now, + ends_at: 1.hour.from_now) presenter = described_class.new(event) expect(presenter.time) diff --git a/spec/presenters/meeting_presenter_spec.rb b/spec/presenters/meeting_presenter_spec.rb index 19c663bc3..fdb14ed22 100644 --- a/spec/presenters/meeting_presenter_spec.rb +++ b/spec/presenters/meeting_presenter_spec.rb @@ -23,9 +23,11 @@ end it '#time' do - expect(meeting).to receive(:date_and_time).and_return(Time.zone.now) + allow(meeting).to receive(:date_and_time).and_return(Time.zone.now) event.time + + expect(meeting).to have_received(:date_and_time) end it '#to_s' do diff --git a/spec/presenters/member_presenter_spec.rb b/spec/presenters/member_presenter_spec.rb index 4b572c070..4a34eb2ff 100644 --- a/spec/presenters/member_presenter_spec.rb +++ b/spec/presenters/member_presenter_spec.rb @@ -3,15 +3,19 @@ let(:member_presenter) { described_class.new(member) } it '#organiser?' do - expect(member).to receive(:has_role?).with(:organiser, :any) + allow(member).to receive(:has_role?).with(:organiser, :any) member_presenter.organiser? + + expect(member).to have_received(:has_role?).with(:organiser, :any) end it '#subscribed_to_newsletter?' do - expect(member).to receive(:opt_in_newsletter_at) + allow(member).to receive(:opt_in_newsletter_at) member_presenter.subscribed_to_newsletter? + + expect(member).to have_received(:opt_in_newsletter_at) end describe '#pairing_details_array' do @@ -95,11 +99,13 @@ it 'memoizes private methods across calls' do member.add_role(:admin) - expect(member).to receive(:has_role?).with(:admin).once.and_call_original + allow(member).to receive(:has_role?).with(:admin).once.and_call_original # First call loads and caches member_presenter.event_organiser?(workshop) # Second call uses cache member_presenter.event_organiser?(workshop) + + expect(member).to have_received(:has_role?).with(:admin).once end end end diff --git a/spec/presenters/sponsor_presenter_spec.rb b/spec/presenters/sponsor_presenter_spec.rb index ffb7d212a..c5eddcaae 100644 --- a/spec/presenters/sponsor_presenter_spec.rb +++ b/spec/presenters/sponsor_presenter_spec.rb @@ -6,25 +6,31 @@ describe '#decorate_collection' do it 'decorates a collection of Sponsors' do - expect(described_class).to receive(:new).with(sponsor) + allow(described_class).to receive(:new).with(sponsor) described_class.decorate_collection([sponsor]) + + expect(described_class).to have_received(:new).with(sponsor) end end describe '#address' do it 'decorates the sponsor address' do - expect(AddressPresenter).to receive(:new).with(sponsor.address) + allow(AddressPresenter).to receive(:new).with(sponsor.address) sponsor_presenter.address + + expect(AddressPresenter).to have_received(:new).with(sponsor.address) end end describe '#contacts' do it 'decorates the sponsor contacts' do - expect(ContactPresenter).to receive(:decorate_collection).with(contacts) + allow(ContactPresenter).to receive(:decorate_collection).with(contacts) sponsor_presenter.contacts + + expect(ContactPresenter).to have_received(:decorate_collection).with(contacts) end end diff --git a/spec/presenters/virtual_workshop_presenter_spec.rb b/spec/presenters/virtual_workshop_presenter_spec.rb index a25d88e88..b2cfe15dc 100644 --- a/spec/presenters/virtual_workshop_presenter_spec.rb +++ b/spec/presenters/virtual_workshop_presenter_spec.rb @@ -1,8 +1,8 @@ RSpec.describe VirtualWorkshopPresenter do def double_workshop(attending_coaches:, attending_students:) - double(:workshop, coach_spaces: 3, student_spaces: 5, chapter: chapter, - attending_coaches: double(:attending_coaches, length: attending_coaches), - attending_students: double(:attending_students, length: attending_students)) + instance_double(Workshop, coach_spaces: 3, student_spaces: 5, chapter: chapter, + attending_coaches: instance_double(Array, length: attending_coaches), + attending_students: instance_double(Array, length: attending_students)) end let(:chapter) { Fabricate(:chapter) } @@ -17,17 +17,21 @@ def double_workshop(attending_coaches:, attending_students:) describe '#coach_spaces' do it 'returns the workshop\'s coach_spaces' do - expect(workshop).to receive(:coach_spaces) + allow(workshop).to receive(:coach_spaces) presenter.coach_spaces + + expect(workshop).to have_received(:coach_spaces) end end describe '#student_spaces' do it 'returns the workshop\'s student spaces' do - expect(workshop).to receive(:student_spaces) + allow(workshop).to receive(:student_spaces) presenter.student_spaces + + expect(workshop).to have_received(:student_spaces) end end @@ -63,8 +67,8 @@ def double_workshop(attending_coaches:, attending_students:) describe '#send_attending_email' do it 'enqueues an attending email to the invitation user' do - invitation = double(:invitation, member: double(:member)) - mailer_double = double(:mailer) + invitation = instance_double(WorkshopInvitation, member: instance_double(Member)) + mailer_double = instance_double(ActionMailer::MessageDelivery) allow(VirtualWorkshopInvitationMailer) .to receive(:attending) .with(workshop, invitation.member, invitation, false) @@ -77,8 +81,8 @@ def double_workshop(attending_coaches:, attending_students:) end it 'enqueues a waiting list email to the invitation user' do - invitation = double(:invitation, member: double(:member)) - mailer_double = double(:mailer) + invitation = instance_double(WorkshopInvitation, member: instance_double(Member)) + mailer_double = instance_double(ActionMailer::MessageDelivery) allow(VirtualWorkshopInvitationMailer) .to receive(:attending) .with(workshop, invitation.member, invitation, true) diff --git a/spec/presenters/workshop_presenter_spec.rb b/spec/presenters/workshop_presenter_spec.rb index 60ce7f8ee..94366dcf2 100644 --- a/spec/presenters/workshop_presenter_spec.rb +++ b/spec/presenters/workshop_presenter_spec.rb @@ -1,24 +1,24 @@ RSpec.describe WorkshopPresenter do let(:chapter) { Fabricate(:chapter) } let(:host) { Fabricate(:sponsor, seats: 5, number_of_coaches: 15) } - let(:workshop) { double(:workshop, host: host, chapter: chapter) } + let(:workshop) { instance_double(Workshop, host: host, chapter: chapter) } let(:presenter) { described_class.new(workshop) } def double_workshop(attending_coaches:, attending_students:) - double(:workshop, host: host, chapter: chapter, - attending_coaches: double(:attending_coaches, count: attending_coaches), - attending_students: double(:attending_students, count: attending_students)) + instance_double(Workshop, host: host, chapter: chapter, + attending_coaches: instance_double(Array, count: attending_coaches), + attending_students: instance_double(Array, count: attending_students)) end describe '#decorate' do it 'returns a workshop decorated with the WorkshopPresenter' do - workshop = double(:workshop, virtual?: false) + workshop = instance_double(Workshop, virtual?: false) presenter = described_class.decorate(workshop) expect(presenter).to be_a(described_class) end it 'returns a virtual workshop decorated with the VirtualWorkshopPresenter' do - workshop = double(:workshop, virtual?: true) + workshop = instance_double(Workshop, virtual?: true) presenter = described_class.decorate(workshop) expect(presenter).to be_a(VirtualWorkshopPresenter) end @@ -53,9 +53,11 @@ def double_workshop(attending_coaches:, attending_students:) end it '#venue' do - expect(workshop).to receive(:host) + allow(workshop).to receive(:host) presenter.venue + + expect(workshop).to have_received(:host) end describe '#organisers' do @@ -82,7 +84,7 @@ def double_workshop(attending_coaches:, attending_students:) context 'with time formatting' do it '#start_time and #end_time' do travel_to(Time.current) do - workshop = double(:workshop, date_and_time: Time.current, ends_at: 1.hour.from_now) + workshop = instance_double(Workshop, date_and_time: Time.current, ends_at: 1.hour.from_now) presenter = described_class.new(workshop) expect(presenter.start_time).to eq(I18n.l(workshop.date_and_time, format: :time)) @@ -95,10 +97,10 @@ def double_workshop(attending_coaches:, attending_students:) let(:invitations) do [Fabricate.times(2, :student_workshop_invitation), Fabricate.times(2, :coach_workshop_invitation)].flatten end - let(:workshop) { double(:workshop, attendances: invitations) } + let(:workshop) { instance_double(Workshop, attendances: invitations) } it 'correctly returns the formatted list of workshop participants' do - expect(presenter).to receive(:organisers).at_least(:once).and_return [invitations.last.member] + allow(presenter).to receive(:organisers).and_return [invitations.last.member] expect(presenter.attendees_csv).not_to be_blank invitations.each do |invitation| @@ -108,11 +110,13 @@ def double_workshop(attending_coaches:, attending_students:) expect(presenter.attendees_csv).to include('ORGANISER') expect(presenter.attendees_csv).to include('STUDENT') expect(presenter.attendees_csv).to include('COACH') + + expect(presenter).to have_received(:organisers).at_least(:once) end end describe '#pairing_csv' do - let(:workshop) { double(:workshop, attendances: [invitation]) } + let(:workshop) { instance_double(Workshop, attendances: [invitation]) } let(:student) { Fabricate(:student) } let(:invitation) { Fabricate(:workshop_invitation, member: student, note: 'Note') } @@ -142,27 +146,31 @@ def double_workshop(attending_coaches:, attending_students:) describe '#coach_spaces' do it 'returns the available coach_spots' do - expect(host).to receive(:coach_spots) + allow(host).to receive(:coach_spots) presenter.coach_spaces + + expect(host).to have_received(:coach_spots) end end describe '#student_spaces' do it 'returns the available coach_spots' do - expect(host).to receive(:seats) + allow(host).to receive(:seats) presenter.student_spaces + + expect(host).to have_received(:seats) end end describe '#spaces?' do - let(:sponsor) { double(:sponsor, coach_spots: 3, seats: 5, chapter: chapter) } + let(:sponsor) { instance_double(Sponsor, coach_spots: 3, seats: 5) } def double_workshop(attending_coaches:, attending_students:) - double(:workshop, coach_spaces: 0, student_spaces: 0, host: sponsor, - attending_coaches: double(:attending_coaches, length: attending_coaches), - attending_students: double(:attending_students, length: attending_students)) + instance_double(Workshop, coach_spaces: 0, student_spaces: 0, host: sponsor, + attending_coaches: instance_double(Array, length: attending_coaches), + attending_students: instance_double(Array, length: attending_students)) end context 'when the host has more available spots' do @@ -184,8 +192,8 @@ def double_workshop(attending_coaches:, attending_students:) describe '#send_attending_email' do it 'enqueues an attending email to the invitation user' do - invitation = double(:invitation, member: double(:member)) - mailer_double = double(:mailer) + invitation = instance_double(WorkshopInvitation, member: instance_double(Member)) + mailer_double = instance_double(ActionMailer::MessageDelivery) allow(WorkshopInvitationMailer) .to receive(:attending) .with(workshop, invitation.member, invitation, false) @@ -198,8 +206,8 @@ def double_workshop(attending_coaches:, attending_students:) end it 'enqueues a waiting list email to the invitation user' do - invitation = double(:invitation, member: double(:member)) - mailer_double = double(:mailer) + invitation = instance_double(WorkshopInvitation, member: instance_double(Member)) + mailer_double = instance_double(ActionMailer::MessageDelivery) allow(WorkshopInvitationMailer) .to receive(:attending) .with(workshop, invitation.member, invitation, true) diff --git a/spec/services/invitation_manager_spec.rb b/spec/services/invitation_manager_spec.rb index 3cda1c746..418acc953 100644 --- a/spec/services/invitation_manager_spec.rb +++ b/spec/services/invitation_manager_spec.rb @@ -29,42 +29,58 @@ it 'can email only students' do event = Fabricate(:event, chapters: [chapter], audience: 'Students') students.each do |student| - expect(Invitation).to receive(:new).with(event: event, member: student, role: 'Student').and_call_original + allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Student').and_call_original end - coaches.each do |student| - expect(Invitation).not_to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original + manager.send_event_emails(event, chapter) + + students.each do |student| + expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Student') end - manager.send_event_emails(event, chapter) + coaches.each do |student| + expect(Invitation).not_to have_received(:new).with(event: event, member: student, role: 'Coach') + end end it 'can email only coaches' do event = Fabricate(:event, chapters: [chapter], audience: 'Coaches') + coaches.each do |student| + allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original + end + + manager.send_event_emails(event, chapter) + students.each do |student| - expect(Invitation).not_to receive(:new).with(event: event, member: student, role: 'Student').and_call_original + expect(Invitation).not_to have_received(:new).with(event: event, member: student, role: 'Student') end coaches.each do |student| - expect(Invitation).to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original + expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Coach') end - - manager.send_event_emails(event, chapter) end it 'can email both students and coaches' do event = Fabricate(:event, chapters: [chapter]) students.each do |student| - expect(Invitation).to receive(:new).with(event: event, member: student, role: 'Student').and_call_original + allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Student').and_call_original end coaches.each do |student| - expect(Invitation).to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original + allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original end manager.send_event_emails(event, chapter) + + students.each do |student| + expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Student') + end + + coaches.each do |student| + expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Coach') + end end it 'emails only students that accepted toc' do @@ -73,14 +89,8 @@ first_student, *other_students = students first_student.update(accepted_toc_at: nil) - expect(Invitation).not_to( - receive(:new) - .with(event: event, member: first_student, role: 'Student') - .and_call_original - ) - other_students.each do |other_student| - expect(Invitation).to( + allow(Invitation).to( receive(:new) .with(event: event, member: other_student, role: 'Student') .and_call_original @@ -88,6 +98,12 @@ end manager.send_event_emails(event, chapter) + + expect(Invitation).not_to have_received(:new).with(event: event, member: first_student, role: 'Student') + + other_students.each do |other_student| + expect(Invitation).to have_received(:new).with(event: event, member: other_student, role: 'Student') + end end it 'emails only coaches that accepted toc' do @@ -96,14 +112,8 @@ first_coach, *other_coaches = coaches first_coach.update(accepted_toc_at: nil) - expect(Invitation).not_to( - receive(:new) - .with(event: event, member: first_coach, role: 'Coach') - .and_call_original - ) - other_coaches.each do |other_coach| - expect(Invitation).to( + allow(Invitation).to( receive(:new) .with(event: event, member: other_coach, role: 'Coach') .and_call_original @@ -111,6 +121,12 @@ end manager.send_event_emails(event, chapter) + + expect(Invitation).not_to have_received(:new).with(event: event, member: first_coach, role: 'Coach') + + other_coaches.each do |other_coach| + expect(Invitation).to have_received(:new).with(event: event, member: other_coach, role: 'Coach') + end end it 'sends invitation emails for all eligible members' do @@ -125,13 +141,15 @@ event = Fabricate(:event, chapters: [chapter]) allow(event).to receive(:invitable?).and_raise(StandardError.new('DB connection error')) - expect(Rollbar).to receive(:error).with( + allow(Rollbar).to receive(:error) + + expect { manager.send_event_emails(event, chapter) }.to raise_error(StandardError, 'DB connection error') + + expect(Rollbar).to have_received(:error).with( an_instance_of(StandardError), event_id: event.id, chapter_id: chapter.id ) - - expect { manager.send_event_emails(event, chapter) }.to raise_error(StandardError, 'DB connection error') end end @@ -309,15 +327,17 @@ allow(WorkshopInvitation).to receive(:find_or_initialize_by) .and_raise(StandardError.new('database error')) - expect(Rails.logger).to receive(:error) do |message| - expect(message).to include("member_id=#{member.id}") - expect(message).to include("workshop_id=#{workshop.id}") - expect(message).to include('role=Student') - expect(message).not_to include(member.email) - expect(message).not_to match(/#{Regexp.escape(member.name)}/) - end + logged_message = nil + allow(Rails.logger).to receive(:error) { |message| logged_message = message } manager.send(:create_invitation, workshop, member, 'Student') + + expect(Rails.logger).to have_received(:error) + expect(logged_message).to include("member_id=#{member.id}") + expect(logged_message).to include("workshop_id=#{workshop.id}") + expect(logged_message).to include('role=Student') + expect(logged_message).not_to include(member.email) + expect(logged_message).not_to match(/#{Regexp.escape(member.name)}/) end it 'continues processing when invitation creation fails for one member' do diff --git a/spec/support/shared_examples/behaves_like_an_invitation.rb b/spec/support/shared_examples/behaves_like_an_invitation.rb index 464d4dccb..d3ae945fb 100644 --- a/spec/support/shared_examples/behaves_like_an_invitation.rb +++ b/spec/support/shared_examples/behaves_like_an_invitation.rb @@ -8,8 +8,10 @@ describe 'cache invalidation' do it 'clears member cache when attending changes' do - expect(invitation.member).to receive(:clear_attending_event_ids_cache!) + allow(invitation.member).to receive(:clear_attending_event_ids_cache!) invitation.update!(attending: !invitation.attending) + + expect(invitation.member).to have_received(:clear_attending_event_ids_cache!) end end diff --git a/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb b/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb index 7cd6a6a0e..24e319a56 100644 --- a/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb +++ b/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb @@ -3,7 +3,7 @@ Fabricate(:students, chapter: chapter, members: students) students.each do |student| - expect(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: student, role: 'Student').and_call_original + allow(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: student, role: 'Student').and_call_original end expect do @@ -11,6 +11,10 @@ end.to change { ActionMailer::Base.deliveries.count }.by(students.count) .and change { WorkshopInvitation.where(workshop: workshop, role: 'Student').count }.by(students.count) + students.each do |student| + expect(WorkshopInvitation).to have_received(:find_or_initialize_by).with(workshop: workshop, member: student, role: 'Student') + end + # Verify emails were sent to the right recipients emails = ActionMailer::Base.deliveries.last(students.count) student_emails = students.map(&:email) @@ -21,7 +25,7 @@ Fabricate(:coaches, chapter: chapter, members: coaches) coaches.each do |coach| - expect(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original + allow(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original end expect do @@ -29,6 +33,10 @@ end.to change { ActionMailer::Base.deliveries.count }.by(coaches.count) .and change { WorkshopInvitation.where(workshop: workshop, role: 'Coach').count }.by(coaches.count) + coaches.each do |coach| + expect(WorkshopInvitation).to have_received(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach') + end + # Verify emails were sent to the right recipients emails = ActionMailer::Base.deliveries.last(coaches.count) coach_emails = coaches.map(&:email) @@ -40,11 +48,15 @@ Fabricate(:coaches, chapter: chapter, members: coaches + [banned_coach]) coaches.each do |coach| - expect(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original + allow(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original end - expect(WorkshopInvitation).not_to receive(:find_or_initialize_by).with(workshop: workshop, member: banned_coach, role: 'Coach') manager.send(send_email, workshop, 'coaches') + + coaches.each do |coach| + expect(WorkshopInvitation).to have_received(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach') + end + expect(WorkshopInvitation).not_to have_received(:find_or_initialize_by).with(workshop: workshop, member: banned_coach, role: 'Coach') end it 'sends emails when a WorkshopInvitation is created' do @@ -59,11 +71,13 @@ it 'does not send emails when invitation creation returns nil' do Fabricate(:students, chapter: chapter, members: students) - expect(WorkshopInvitation).to receive(:find_or_initialize_by).and_return(nil).exactly(students.count) + allow(WorkshopInvitation).to receive(:find_or_initialize_by).and_return(nil).exactly(students.count) expect do manager.send(send_email, workshop, 'students') end.not_to(change { ActionMailer::Base.deliveries.count }) + + expect(WorkshopInvitation).to have_received(:find_or_initialize_by).exactly(students.count).times end it 'does not send duplicate emails when members are already invited' do