diff --git a/app/components/request_metrics/request_metrics.rb b/app/components/request_metrics/request_metrics.rb index 829eaeb7a..484267e0f 100644 --- a/app/components/request_metrics/request_metrics.rb +++ b/app/components/request_metrics/request_metrics.rb @@ -21,7 +21,7 @@ def metrics [ { value: stats[:counts][:contributors], - total: stats[:counts][:recipients], + total: request_for_info.organization.contributors.active.with_tags(request_for_info.tag_list).count, label: I18n.t('components.request_metrics.contributors', count: stats[:counts][:contributors]), icon: 'single-03' }, diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 83225f0e0..5723c5fd8 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true # TODO: Refactor to remove the need to disable rubocop -# rubocop:disable Metrics/ClassLength class RequestsController < ApplicationController before_action :set_request, only: %i[show edit update notifications destroy messages_by_contributor stats] before_action :notifications_params, only: :notifications @@ -22,17 +21,7 @@ def create resize_image_files if request_params[:files].present? @request = @organization.requests.new(request_params.merge(user: current_user)) if @request.save - recipient_count = @request.organization.contributors.active.with_tags(@request.tag_list).count - if @request.planned? - redirect_to organization_requests_path(@organization, filter: :planned), flash: { - success: I18n.t('request.schedule_request_success', - count: recipient_count, - scheduled_datetime: I18n.l(@request.schedule_send_for, format: :long)) - } - else - redirect_to organization_request_path(@organization.id, @request), - flash: { success: I18n.t('request.success', count: recipient_count) } - end + trigger_broadcast_and_redirect(@request) else render :new, status: :unprocessable_entity end @@ -47,10 +36,7 @@ def edit; end def update @request.files.purge_later if @request.files.attached? && request_params[:files].blank? if @request.update(request_params) - success_message, filter = update_request_redirect_specifics - redirect_to organization_requests_path(@request.organization_id, filter: filter), flash: { - success: success_message - } + trigger_broadcast_and_redirect(@request) else render :edit, status: :unprocessable_entity end @@ -163,16 +149,18 @@ def filtered_requests end end - def update_request_redirect_specifics - if @request.planned? - [I18n.t('request.schedule_request_success', - count: @request.organization.contributors.active.with_tags(@request.tag_list).count, - scheduled_datetime: I18n.l(@request.schedule_send_for, format: :long)), - :planned] + def trigger_broadcast_and_redirect(request) + recipient_count = @request.organization.contributors.active.with_tags(request.tag_list).count + run_at = request.trigger_broadcast + if run_at + redirect_to organization_requests_path(@organization, filter: :planned), flash: { + success: I18n.t('request.schedule_request_success', + count: recipient_count, + scheduled_datetime: I18n.l(run_at, format: :long)) + } else - [I18n.t('request.success', count: @request.organization.contributors.active.with_tags(@request.tag_list).count), - nil] + redirect_to organization_request_path(@organization.id, request), + flash: { success: I18n.t('request.success', count: recipient_count) } end end end -# rubocop:enable Metrics/ClassLength diff --git a/app/jobs/broadcast_request_job.rb b/app/jobs/broadcast_request_job.rb index 57e373c30..3575981b7 100644 --- a/app/jobs/broadcast_request_job.rb +++ b/app/jobs/broadcast_request_job.rb @@ -7,11 +7,7 @@ def perform(request_id) request = Request.where(id: request_id).first return unless request return if request.broadcasted_at.present? - - if request.planned? # rescheduled for future after this job was created - BroadcastRequestJob.delay(run_at: request.schedule_send_for).perform_later(request.id) - return - end + return if request.planned? # rescheduled for future request.organization.contributors.active.with_tags(request.tag_list).each do |contributor| message = Message.new( @@ -22,7 +18,9 @@ def perform(request_id) broadcasted: true ) message.files = Request.attach_files(request.files) if request.files.attached? + message.save! + message.send! end request.update(broadcasted_at: Time.current) end diff --git a/app/models/message.rb b/app/models/message.rb index c175a4e6c..72617c77b 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -36,7 +36,14 @@ class Message < ApplicationRecord validates :raw_data, presence: true, if: -> { sent_from_contributor? } validates :unknown_content, inclusion: { in: [true, false] } - after_create_commit :send_if_outbound, :notify_recipient + after_create_commit :notify_recipient + + def send! + [PostmarkAdapter::Outbound, SignalAdapter::Outbound, TelegramAdapter::Outbound, ThreemaAdapter::Outbound, + WhatsAppAdapter::Delegator.new(organization)].each do |adapter| + adapter.send!(self) + end + end def reply? sent_from_contributor? @@ -86,13 +93,4 @@ def notify_recipient end end # rubocop:enable Metrics/AbcSize - - def send_if_outbound - return if manually_created? || reply? - - [PostmarkAdapter::Outbound, SignalAdapter::Outbound, TelegramAdapter::Outbound, ThreemaAdapter::Outbound, - WhatsAppAdapter::Delegator.new(organization)].each do |adapter| - adapter.send!(self) - end - end end diff --git a/app/models/request.rb b/app/models/request.rb index 93b8a1eb0..5f69476cc 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -27,9 +27,7 @@ class Request < ApplicationRecord acts_as_taggable_on :tags acts_as_taggable_tenant :organization_id - after_create :broadcast_request - - after_update_commit :broadcast_updated_request + after_create :notify_recipient delegate :replies, to: :messages delegate :outbound, to: :messages @@ -49,6 +47,16 @@ def stats } end + def trigger_broadcast + if planned? + BroadcastRequestJob.delay(run_at: schedule_send_for).perform_later(id) + schedule_send_for + else + BroadcastRequestJob.perform_later(id) + nil + end + end + def planned? schedule_send_for.present? && schedule_send_for > Time.current end @@ -68,29 +76,6 @@ def messages_by_contributor .transform_values { |messages| messages.sort_by(&:created_at) } end - # rubocop:disable Metrics/AbcSize - def self.broadcast!(request) - if request.planned? - BroadcastRequestJob.delay(run_at: request.schedule_send_for).perform_later(request.id) - RequestScheduled.with(request_id: request.id, - organization_id: request.organization.id).deliver_later(request.organization.users + User.admin.all) - else - request.organization.contributors.active.with_tags(request.tag_list).each do |contributor| - message = Message.new( - sender: request.user, - recipient: contributor, - text: request.personalized_text(contributor), - request: request, - broadcasted: true - ) - message.files = attach_files(request.files) if request.files.attached? - message.save! - end - request.update(broadcasted_at: Time.current) - end - end - # rubocop:enable Metrics/AbcSize - def self.attach_files(files) files.map do |file| message_file = Message::File.new @@ -101,13 +86,10 @@ def self.attach_files(files) private - def broadcast_request - Request.broadcast!(self) - end - - def broadcast_updated_request - return unless saved_change_to_schedule_send_for? + def notify_recipient + return unless planned? - Request.broadcast!(self) + RequestScheduled.with(request_id: id, + organization_id: organization.id).deliver_later(organization.users + User.admin.all) end end diff --git a/spec/jobs/broadcast_request_job_spec.rb b/spec/jobs/broadcast_request_job_spec.rb index 7137f7fbf..8ae543cab 100644 --- a/spec/jobs/broadcast_request_job_spec.rb +++ b/spec/jobs/broadcast_request_job_spec.rb @@ -35,16 +35,6 @@ context 'given a request has been rescheduled for the future' do before { request.update(schedule_send_for: 1.day.from_now) } - let(:expected_params) { { request_id: request.id } } - - it 'enqueues a job to broadcast the request, and broadcast it when called again' do - expect { subject.call }.to change(DelayedJob, :count).from(0).to(1) - expect(Delayed::Job.last.run_at).to be_within(1.second).of(request.schedule_send_for) - - Timecop.travel(1.day.from_now + 2.minutes) - - expect { subject.call }.to change { request.reload.broadcasted_at }.from(nil).to(kind_of(ActiveSupport::TimeWithZone)) - end it 'does not create a Message instance' do expect { subject.call }.not_to change(Message, :count) @@ -54,5 +44,98 @@ expect { subject.call }.not_to(change { request.reload.broadcasted_at }) end end + + context 'given a request that is to be sent out now' do + describe 'given contributors from multipile organizations' do + before(:each) do + create(:contributor, id: 1, email: 'somebody@example.org', organization: request.organization) + create(:contributor, id: 2, email: nil, telegram_id: 22, organization: request.organization) + create(:contributor, id: 3) + end + + it "schedules jobs to send out message with contributor's channel" do + subject.call + + expect(ActionMailer::MailDeliveryJob).to have_been_enqueued.with( + 'PostmarkAdapter::Outbound', + 'message_email', + 'deliver_now', # How ActionMailer works in test environment, even though in production we call deliver_later + { + params: { message: request.messages.where(recipient_id: 1).first, organization: request.organization }, + args: [] + } + ) + expect(TelegramAdapter::Outbound::Text).to have_been_enqueued.with({ organization_id: request.organization.id, + contributor_id: 2, + text: request.text, + message: request.messages.where(recipient_id: 2).first }) + end + + it 'only sends to contributors of the organization' do + expect { subject.call }.to change(Message, :count).from(0).to(2) + .and (change { Message.pluck(:recipient_id).sort }).from([]).to([1, 2]) + end + + it 'assigns the user of the request as the sender of the message' do + expect { subject.call }.to (change { Message.pluck(:sender_id) }).from([]).to([request.user.id, request.user.id]) + end + + it 'marks the messages as broadcasted' do + expect { subject.call }.to (change { Message.pluck(:broadcasted) }).from([]).to([true, true]) + end + + context 'and has files attached' do + before do + request.files.attach( + io: Rails.root.join('example-image.png').open, + filename: 'example-image.png' + ) + end + + it 'attaches the files to the messages' do + expect { subject.call }.to (change { Message::File.count }).from(0).to(2) + Message.find_each do |message| + message.files.each do |file| + expect(file.attachment).to be_attached + end + end + end + end + end + + describe 'given a request with a tag_list' do + let(:request) do + create(:request, + title: 'Hitchhiker’s Guide', + text: 'What is the answer to life, the universe, and everything?', + tag_list: 'programmer', + broadcasted_at: nil) + end + + before do + create(:contributor, id: 4, organization: request.organization, tag_list: ['programmer']) + create(:contributor, id: 5, organization: request.organization, tag_list: ['something_else']) + create(:contributor, id: 6, organization: request.organization) + end + + it 'only sends to contributors tagged with the tag' do + expect { subject.call }.to change(Message, :count).from(0).to(1) + .and (change { Message.pluck(:recipient_id) }).from([]).to([4]) + end + end + + describe 'given contributors who are deactivated' do + before(:each) do + create(:contributor, :inactive, id: 7, organization: request.organization) + create(:contributor, id: 8, organization: request.organization) + create(:contributor, :inactive, id: 9, telegram_id: 24, organization: request.organization) + end + + it 'only sends to active contributors' do + expect { subject.call }.to change(Message, :count).from(0).to(1) + .and (change { Message.pluck(:recipient_id) }).from([]).to([8]) + end + end + end end end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 8af424e96..8736f8326 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -90,10 +90,24 @@ end end + describe '#send!' do + subject { message.send! } + + let(:message) { create(:message, :outbound, recipient: create(:contributor, :whats_app_contributor)) } + + it 'enqueues a job to send the message' do + expect { subject }.to(have_enqueued_job(WhatsAppAdapter::TwilioOutbound::Text).on_queue('default').with do |params| + expect(params[:organization_id]).to eq(message.organization_id) + expect(params[:contributor_id]).to eq(message.contributor.id) + expect(params[:text]).to include(message.contributor.first_name) + expect(params[:text]).to include(message.request.title) + end) + end + end + describe '#after_commit(on: :commit)' do let!(:user) { create(:user) } - let!(:request) { create(:request, user: user, organization: organization) } - let(:message) { create(:message, sender: user, recipient: recipient, broadcasted: broadcasted, request: request) } + let!(:request) { create(:request, user: user, organization: organization, broadcasted_at: nil) } let(:organization) do create(:organization, name: '100eyes', telegram_bot_api_key: 'TELEGRAM_BOT_API_KEY', telegram_bot_username: 'USERNAME') end @@ -112,9 +126,8 @@ describe '#blocked' do subject do - perform_enqueued_jobs { message } - message.reload - message.blocked + perform_enqueued_jobs { BroadcastRequestJob.perform_later(request.id) } + request.messages.where(recipient_id: recipient.id).first.reload.blocked end it { should be(false) } @@ -128,6 +141,7 @@ describe '#notify_recipient' do subject { message } + let(:message) { create(:message, sender: user, recipient: recipient, broadcasted: broadcasted, request: request) } let!(:admin) { create(:user, admin: true) } before do diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index ccaad2046..062424c00 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -274,151 +274,50 @@ end end - describe '::after_create' do - subject { -> { request.save! } } + describe '#trigger_broadcast' do + context 'with a request not scheduled for' do + let!(:request) { create(:request, schedule_send_for: nil) } - before do - request.files.attach( - io: Rails.root.join('example-image.png').open, - filename: 'example-image.png' - ) - allow(Request).to receive(:broadcast!).and_call_original # is stubbed for every other test - end - - describe 'given some existing contributors in the moment of creation' do - before(:each) do - create(:contributor, id: 1, email: 'somebody@example.org', organization: request.organization) - create(:contributor, id: 2, email: nil, telegram_id: 22, organization: request.organization) - create(:contributor, id: 3) - end - - describe 'only sends to contributors of the organization' do - it { should change { Message.count }.from(0).to(2) } - it { should change { Message.pluck(:recipient_id).sort }.from([]).to([1, 2]) } - it { should change { Message.pluck(:sender_id) }.from([]).to([request.user.id, request.user.id]) } - it { should change { Message.pluck(:broadcasted) }.from([]).to([true, true]) } - it { should change { Message::File.count }.from(0).to(2) } + it 'schedules a job to broadcast the request and returns nil' do + expect(request.trigger_broadcast).to have_been_enqueued.with(request.id) end + end - describe 'given a planned request' do - before { request.schedule_send_for = 1.hour.from_now } - - let!(:admin) { create_list(:user, 2, admin: true) } - let!(:other_organization) { create(:organization, users_count: 2) } + context 'with a request scheduled to be sent out immediately' do + let!(:request) { create(:request, schedule_send_for: Time.current) } - it_behaves_like 'an ActivityNotification', 'RequestScheduled', 3 + it 'schedules a job to broadcast the request and returns nil' do + expect(request.trigger_broadcast).to have_been_enqueued.with(request.id) end end - describe 'creates message only for contributors tagged with tag_list' do - let(:request) do - Request.new( - title: 'Hitchhiker’s Guide', - text: 'What is the answer to life, the universe, and everything?', - tag_list: 'programmer', - user: user, - organization: organization - ) - end - before(:each) do - create(:contributor, id: 1, email: 'somebody@example.org', tag_list: ['programmer'], organization: organization) - create(:contributor, id: 2, email: nil, telegram_id: 22, organization: organization) - end - - it { should change { Message.count }.from(0).to(1) } - it { should change { Message.pluck(:recipient_id) }.from([]).to([1]) } - it { should change { Message.pluck(:sender_id) }.from([]).to([request.user.id]) } - it { should change { Message.pluck(:broadcasted) }.from([]).to([true]) } - end + context 'with a scheduled for request' do + let!(:request) { create(:request, schedule_send_for: 1.day.from_now) } - describe 'given contributors who are deactivated' do - before(:each) do - create(:contributor, :inactive, id: 3, email: 'deactivated@example.org', organization: organization) - create(:contributor, id: 4, email: 'activated@example.org', organization: organization) - create(:contributor, :inactive, id: 5, telegram_id: 24, organization: organization) + it 'delays the job for the future and returns the run time' do + expect { request.trigger_broadcast }.to change(DelayedJob, :count).from(0).to(1) + expect(Delayed::Job.last.run_at).to be_within(1.second).of(request.schedule_send_for) end - - it { should change { Message.count }.from(0).to(1) } - it { should change { Message.pluck(:recipient_id) }.from([]).to([4]) } - it { should change { Message.pluck(:sender_id) }.from([]).to([request.user.id]) } - it { should change { Message.pluck(:broadcasted) }.from([]).to([true]) } end end - describe '::after_update_commit' do + describe '::after_create' do + subject { -> { request.save! } } + before do - allow(Request).to receive(:broadcast!).and_call_original - create(:contributor, organization: request.organization) + request.files.attach( + io: Rails.root.join('example-image.png').open, + filename: 'example-image.png' + ) end - subject { request.update!(params) } - - describe '#broadcast_updated_request' do - context 'not planned request' do - before { request.save! } - let(:params) { { text: 'I have new text' } } + describe 'given a planned request' do + before { request.schedule_send_for = 1.hour.from_now } - it 'does not broadcast request' do - expect(Request).not_to receive(:broadcast!) + let!(:admin) { create_list(:user, 2, admin: true) } + let!(:other_organization) { create(:organization, users_count: 2) } - subject - end - - it 'does not create a notification' do - expect { subject }.not_to(change { ActivityNotification.where(type: RequestScheduled.name).count }) - end - end - - context 'planned request' do - let(:params) { { schedule_send_for: 1.day.from_now } } - - it 'calls broadcast! to schedule request' do - expect(Request).to receive(:broadcast!).with(request) - - subject - end - - it 'creates a notification' do - expect { subject }.to(change { ActivityNotification.where(type: RequestScheduled.name).count }.from(0).to(1)) - end - - context 'no change to scheduled time' do - before { request.save! } - let(:params) { { text: 'Fixed typo' } } - - it 'does not broadcast request' do - expect(Request).not_to receive(:broadcast!) - - subject - end - end - - context 'schedule_send_for set to nil' do - before { request.update(schedule_send_for: 1.day.from_now) } - let(:params) { { schedule_send_for: nil } } - - it 'does not create a notification' do - expect { subject }.not_to(change { ActivityNotification.where(type: RequestScheduled.name).count }) - end - - it 'broadcasts the messages' do - expect { subject }.to(change(Message, :count).from(0).to(1)) - end - end - - context 'schedule_send_for set to time in past' do - before { request.update(schedule_send_for: 1.day.from_now) } - let(:params) { { schedule_send_for: 1.day.ago } } - - it 'does not create a notification' do - expect { subject }.not_to(change { ActivityNotification.where(type: RequestScheduled.name).count }) - end - - it 'broadcasts the messages' do - expect { subject }.to(change(Message, :count).from(0).to(1)) - end - end - end + it_behaves_like 'an ActivityNotification', 'RequestScheduled', 3 end end end diff --git a/spec/requests/requests_spec.rb b/spec/requests/requests_spec.rb index 350d4ffbb..f4ccb68fb 100644 --- a/spec/requests/requests_spec.rb +++ b/spec/requests/requests_spec.rb @@ -140,10 +140,6 @@ let(:params) { { request: { title: 'Example Question', text: 'How do you do?', hints: ['confidential'] } } } let(:user) { create(:user, organizations: [organization]) } - before do - allow(Request).to receive(:broadcast!).and_call_original # is stubbed for every other test - end - context 'unauthenticated' do let(:user) { nil } @@ -208,11 +204,15 @@ expect(flash[:success]).to eq('Deine Frage wurde erfolgreich an 2 Mitglieder in der Community gesendet') end + it 'schedules a job to broadcast the request' do + expect(BroadcastRequestJob).to have_been_enqueued.with(organization.requests.first.id) + end + describe 'with no text' do before { params[:request][:text] = '' } it 'redirects to requests#show' do - request = Request.first + request = organization.requests.first expect(response).to redirect_to organization_request_path(organization, request) end @@ -222,28 +222,33 @@ end end end - end - context 'scheduled for future datetime' do - let(:scheduled_datetime) { Time.current.tomorrow.beginning_of_hour } - let(:params) do - { request: { title: 'Scheduled request', text: 'Did you get this scheduled request?', schedule_send_for: scheduled_datetime } } - end + context 'scheduled for future datetime' do + let(:scheduled_datetime) { Time.current.tomorrow.beginning_of_hour } + let(:params) do + { request: { title: 'Scheduled request', text: 'Did you get this scheduled request?', schedule_send_for: scheduled_datetime } } + end - it 'redirects to requests#show' do - response = subject.call - expect(response).to redirect_to organization_requests_path(organization, filter: :planned) - end + it 'redirects to requests#show' do + response = subject.call + expect(response).to redirect_to organization_requests_path(organization, filter: :planned) + end - it 'shows success notification' do - subject.call - request = Request.first - expect(flash[:success]).to include(I18n.l(request.schedule_send_for, format: :long)) + it 'shows success notification' do + subject.call + request = Request.first + expect(flash[:success]).to include(I18n.l(request.schedule_send_for, format: :long)) + end + + it 'delays the job for the future' do + expect { subject.call }.to change(DelayedJob, :count).from(0).to(1) + expect(Delayed::Job.last.run_at).to be_within(1.second).of(organization.requests.first.schedule_send_for) + end end end end - describe 'PATCH /:organization_id/requests/:id' do + describe 'PATCH /:organization_id/requests/:id' do subject { -> { patch organization_request_path(organization, request, as: user), params: params } } let(:request) { create(:request, title: 'Temp title', organization: organization) } @@ -313,6 +318,11 @@ ) end + it 'delays the job for the future' do + expect { subject.call }.to change(DelayedJob, :count).from(0).to(1) + expect(Delayed::Job.last.run_at).to be_within(1.second).of(request.schedule_send_for) + end + context 're-scheduled to be sent now' do let(:params) { { request: { schedule_send_for: Time.current } } } @@ -320,9 +330,15 @@ expect { subject.call }.to(change { request.reload.schedule_send_for }) end - it 'redirects to requests index page with success message' do + it 'schedules a job to broadcast the request' do + subject.call + + expect(BroadcastRequestJob).to have_been_enqueued.with(request.id) + end + + it "redirects to request's show page with success message" do subject.call - expect(response).to redirect_to(organization_requests_path(request.organization_id)) + expect(response).to redirect_to(organization_request_path(request.organization_id, request)) expect(flash[:success]).to eq('Deine Frage wurde erfolgreich an 2 Mitglieder in der Community gesendet') end end diff --git a/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb b/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb index cb6bdb6d9..b440c0102 100644 --- a/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb +++ b/spec/requests/whats_app/three_sixty_dialog_webhook_spec.rb @@ -28,7 +28,6 @@ describe '#messages' do before do allow(Sentry).to receive(:capture_exception) - allow(Request).to receive(:broadcast!).and_call_original end it 'should be successful' do diff --git a/spec/requests/whats_app/webhook_spec.rb b/spec/requests/whats_app/webhook_spec.rb index 178c046b1..a4667a738 100644 --- a/spec/requests/whats_app/webhook_spec.rb +++ b/spec/requests/whats_app/webhook_spec.rb @@ -43,7 +43,6 @@ before do allow(Sentry).to receive(:capture_exception) allow(Twilio::Security::RequestValidator).to receive(:new).and_return(mock_twilio_security_request_validator) - allow(Request).to receive(:broadcast!).and_call_original end describe 'fails Rack::TwilioWebhookAuthentication' do @@ -99,7 +98,7 @@ context 'no message template sent' do it 'creates a messsage' do - expect { subject.call }.to change(Message, :count).from(2).to(3) + expect { subject.call }.to change(Message, :count).from(1).to(2) end end @@ -143,12 +142,12 @@ end describe 'previous request' do - let(:message) { previous_request.messages.where(recipient_id: contributor.id).first } + let(:previous_message) { create(:message, request: previous_request, recipient_id: contributor.id) } let(:requested_message_job_args) do { organization_id: organization.id, contributor_id: contributor.id, - text: message.text, + text: previous_message.text, message: message } end @@ -164,7 +163,7 @@ end describe 'newer request' do - let(:message) { newer_request.messages.where(recipient_id: contributor.id).first } + let(:newer_message) { create(:message, request: newer_request, recipient_id: contributor.id) } let(:requested_message_job_args) do { organization_id: organization.id, diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5e0152ab2..01f7c9744 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -93,8 +93,4 @@ # # test failures related to randomization by passing the same `--seed` value # # as the one that triggered the failure. # Kernel.srand config.seed - - config.before(:each) do - allow(Request).to receive(:broadcast!) - end end diff --git a/spec/system/requests/deleting_requests_spec.rb b/spec/system/requests/deleting_requests_spec.rb index 294adfa78..acef25828 100644 --- a/spec/system/requests/deleting_requests_spec.rb +++ b/spec/system/requests/deleting_requests_spec.rb @@ -10,7 +10,6 @@ let!(:another_planned_request) { create(:request, schedule_send_for: 5.minutes.from_now, organization: organization, user: user) } before do - allow(Request).to receive(:broadcast!).and_call_original create(:contributor, organization: organization) end diff --git a/spec/system/requests/editing_requests_spec.rb b/spec/system/requests/editing_requests_spec.rb index b3eba4ee3..bf30631fc 100644 --- a/spec/system/requests/editing_requests_spec.rb +++ b/spec/system/requests/editing_requests_spec.rb @@ -9,8 +9,6 @@ let(:request_scheduled_in_future) { create(:request, schedule_send_for: 2.minutes.from_now, organization: organization) } before(:each) do - # `broadcast!` is stubbed in tests - allow(Request).to receive(:broadcast!).and_call_original create(:contributor, organization: organization) end diff --git a/spec/system/requests/personalization_spec.rb b/spec/system/requests/personalization_spec.rb deleted file mode 100644 index d4a8c863c..000000000 --- a/spec/system/requests/personalization_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe 'Request personalization' do - let(:user) { create(:user, organizations: [organization]) } - let(:organization) { create(:organization) } - - context 'given two contributors' - before(:each) do - # `broadcast!` is stubbed in tests - allow(Request).to receive(:broadcast!).and_call_original - - create(:contributor, first_name: 'Adam', last_name: 'Apfel', email: 'adam@example.org', organization: organization) - create(:contributor, first_name: 'Zora', last_name: 'Zimmermann', email: 'zora@example.org', organization: organization) - end - - it 'sending a request with placeholders' do - visit new_organization_request_path(organization, as: user) - - fill_in 'Titel', with: 'Personalizes request' - fill_in 'Was möchtest du wissen?', with: 'Hi {{VORNAME}}, how are you?' - - click_button 'Frage an die Community senden' - - # TODO: This isn't really what you'd do in a feature test. However, it's an easy to - # understand solution, given that we don't display broadcasted messages in the UI - expect(Message.pluck(:text)).to contain_exactly('Hi Adam, how are you?', 'Hi Zora, how are you?') - end -end diff --git a/spec/system/requests/scheduling_requests_spec.rb b/spec/system/requests/scheduling_requests_spec.rb index 31af5beda..5a0a42c42 100644 --- a/spec/system/requests/scheduling_requests_spec.rb +++ b/spec/system/requests/scheduling_requests_spec.rb @@ -8,8 +8,6 @@ context 'given contributors' do before(:each) do - # `broadcast!` is stubbed in tests - allow(Request).to receive(:broadcast!).and_call_original create(:contributor, organization: organization) end diff --git a/spec/system/requests/sending_images_spec.rb b/spec/system/requests/sending_images_spec.rb index ff6b6c948..db9cc17d3 100644 --- a/spec/system/requests/sending_images_spec.rb +++ b/spec/system/requests/sending_images_spec.rb @@ -7,10 +7,7 @@ let(:organization) { create(:organization) } context 'given contributors' do - before(:each) do - # `broadcast!` is stubbed in tests - allow(Request).to receive(:broadcast!).and_call_original - + before do create(:contributor, email: 'adam@example.org', organization: organization) create(:contributor, signal_phone_number: '+4912345678', organization: organization) create(:contributor, telegram_id: 125_689, organization: organization) @@ -18,91 +15,93 @@ end it 'sending a request with image files', flaky: true do - visit new_organization_request_path(organization, as: user) + perform_enqueued_jobs(only: BroadcastRequestJob) do + visit new_organization_request_path(organization, as: user) - # With no text, no file - fill_in 'Titel', with: 'No text, no files' + # With no text, no file + fill_in 'Titel', with: 'No text, no files' - click_button 'Frage an die Community senden' - message = page.find('textarea[name="request[text]"]').evaluate_script('this.validationMessage') - expect(message).to eq 'Please fill out this field.' - expect(page).to have_current_path(new_organization_request_path(organization), ignore_query: true) + click_button 'Frage an die Community senden' + message = page.find('textarea[name="request[text]"]').evaluate_script('this.validationMessage') + expect(message).to eq 'Please fill out this field.' + expect(page).to have_current_path(new_organization_request_path(organization), ignore_query: true) - # With no text, with file - visit new_organization_request_path(organization, as: user) + # With no text, with file + visit new_organization_request_path(organization, as: user) - fill_in 'Titel', with: 'Message with files, no text' + fill_in 'Titel', with: 'Message with files, no text' - find_button('Bilder anhängen').trigger('click') - image_file = File.expand_path('../../fixtures/files/example-image.png', __dir__) - find_field('request_files', visible: :all).attach_file(image_file) + find_button('Bilder anhängen').trigger('click') + image_file = File.expand_path('../../fixtures/files/example-image.png', __dir__) + find_field('request_files', visible: :all).attach_file(image_file) - click_button 'Frage an die Community senden' + click_button 'Frage an die Community senden' - expect(page).to have_content('Message with files, no text') - expect(page).to have_current_path(organization_request_path(organization, Request.first)) + expect(page).to have_content('Message with files, no text') + expect(page).to have_current_path(organization_request_path(organization, Request.first)) - # With text - visit new_organization_request_path(organization, as: user) + # With text + visit new_organization_request_path(organization, as: user) - fill_in 'Titel', with: 'Message with files' - fill_in 'Was möchtest du wissen?', with: 'Did you get my image?' + fill_in 'Titel', with: 'Message with files' + fill_in 'Was möchtest du wissen?', with: 'Did you get my image?' - # Non-image file - click_button 'Bilder anhängen' - image_file = File.expand_path('../../fixtures/files/invalid_profile_picture.pdf', __dir__) - find_field('request_files', visible: :all).attach_file(image_file) + # Non-image file + click_button 'Bilder anhängen' + image_file = File.expand_path('../../fixtures/files/invalid_profile_picture.pdf', __dir__) + find_field('request_files', visible: :all).attach_file(image_file) - expect(page).to have_content('Kein gültiges Bildformat. Bitte senden Sie Bilder als jpg, png oder gif.') + expect(page).to have_content('Kein gültiges Bildformat. Bitte senden Sie Bilder als jpg, png oder gif.') - click_button 'Frage an die Community senden' + click_button 'Frage an die Community senden' - expect(page).to have_current_path(new_organization_request_path(organization), ignore_query: true) - expect(page).to have_content('Kein gültiges Bildformat. Bitte senden Sie Bilder als jpg, png oder gif.') + expect(page).to have_current_path(new_organization_request_path(organization), ignore_query: true) + expect(page).to have_content('Kein gültiges Bildformat. Bitte senden Sie Bilder als jpg, png oder gif.') - # Image file - click_button 'Bilder anhängen' - image_file = File.expand_path('../../fixtures/files/example-image.png', __dir__) - find_field('request_files', visible: :all).attach_file(image_file) + # Image file + click_button 'Bilder anhängen' + image_file = File.expand_path('../../fixtures/files/example-image.png', __dir__) + find_field('request_files', visible: :all).attach_file(image_file) - within('#file-preview') do - expect(page).to have_css('img') - expect(page).to have_css('figcaption#caption', text: 'Did you get my image?') - end + within('#file-preview') do + expect(page).to have_css('img') + expect(page).to have_css('figcaption#caption', text: 'Did you get my image?') + end - expect(page).to have_content('Angehängte Bilder') - expect(page).to have_css('p.RequestForm-filename', text: 'example-image.png') - expect(page).to have_css( - 'button.RequestForm-removeListItemButton[data-action="request-form#removeImage"][data-request-form-image-index-value="0"]' - ) - click_button 'x' - - expect(page).not_to have_content('Angehängte Bilder') - expect(page).to have_no_css('p.RequestForm-filename', text: 'example-image.png') - expect(page).to have_no_css( - 'button.RequestForm-removeListItemButton[data-action="request-form#removeImage"][data-request-form-image-index-value="0"]' - ) - - within('figure.ChatPreview') do - expect(page).to have_no_css('img') - expect(page).to have_no_css('figcaption#caption', text: 'Did you get my image?') - end + expect(page).to have_content('Angehängte Bilder') + expect(page).to have_css('p.RequestForm-filename', text: 'example-image.png') + expect(page).to have_css( + 'button.RequestForm-removeListItemButton[data-action="request-form#removeImage"][data-request-form-image-index-value="0"]' + ) + click_button 'x' - click_button 'Bilder anhängen' - image_file = File.expand_path('../../fixtures/files/example-image.png', __dir__) - find_field('request_files', visible: :all).attach_file(image_file) + expect(page).not_to have_content('Angehängte Bilder') + expect(page).to have_no_css('p.RequestForm-filename', text: 'example-image.png') + expect(page).to have_no_css( + 'button.RequestForm-removeListItemButton[data-action="request-form#removeImage"][data-request-form-image-index-value="0"]' + ) - click_button 'Frage an die Community senden' + within('figure.ChatPreview') do + expect(page).to have_no_css('img') + expect(page).to have_no_css('figcaption#caption', text: 'Did you get my image?') + end - expect(page).to have_content('Did you get my image?') - expect(page).to have_current_path(organization_request_path(organization, Request.first)) + click_button 'Bilder anhängen' + image_file = File.expand_path('../../fixtures/files/example-image.png', __dir__) + find_field('request_files', visible: :all).attach_file(image_file) - within('.PageHeader') do - expect(page).to have_css("img[src*='example-image.png']") - end + click_button 'Frage an die Community senden' + + expect(page).to have_content('Did you get my image?') + expect(page).to have_current_path(organization_request_path(organization, Request.first)) + + within('.PageHeader') do + expect(page).to have_css("img[src*='example-image.png']") + end - within('.CardMetrics') do - expect(page).to have_content('0/4 haben geantwortet') + within('.CardMetrics') do + expect(page).to have_content('0/4 haben geantwortet') + end end end end