From 77c7cfa8543de1dd9476cb4be1b665501ac2caaa Mon Sep 17 00:00:00 2001 From: "Marcus.Sjoqvist" Date: Fri, 26 Jun 2020 21:23:35 +0200 Subject: [PATCH 1/6] refactors messages namespace into offers --- config/routes.rb | 5 +-- .../users_can_send_messages_spec.rb | 35 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 89118d7..0420e14 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,8 +3,9 @@ Rails.application.routes.draw do mount_devise_token_auth_for 'User', at: 'api/auth' namespace :api do - resources :messages, only: [:create] - resources :offers, only: %i[create show update] + resources :offers, only: %i[create show update] do + resources :messages, only: [:create] + end resources :karma_points, only: [:index], constraints: { format: 'json' } resources :requests, only: %i[index], constraints: { format: 'json' } namespace :my_request do diff --git a/spec/requests/api/conversions/users_can_send_messages_spec.rb b/spec/requests/api/conversions/users_can_send_messages_spec.rb index e11f791..a5e8210 100644 --- a/spec/requests/api/conversions/users_can_send_messages_spec.rb +++ b/spec/requests/api/conversions/users_can_send_messages_spec.rb @@ -1,4 +1,4 @@ -RSpec.describe 'POST /message users can post messages' do +RSpec.describe 'POST /offers/:offer_id/message users can post messages' do let(:requester) { create(:user, email: 'requester@mail.com') } let(:req_creds) { requester.create_new_auth_token } let(:req_headers) { { HTTP_ACCEPT: 'application/json' }.merge!(req_creds) } @@ -18,7 +18,7 @@ describe 'successfully as the requester' do before do - post '/api/messages', headers: req_headers, params: { offer_id: offer.id, content: "message content" } + post "/api/offers/#{offer.id}/messages", headers: req_headers, params: { content: "message content" } end it 'gives a success status' do @@ -33,7 +33,7 @@ describe 'successfully as the helper' do before do - post '/api/messages', headers: helper_headers, params: { offer_id: offer.id, content: "message content" } + post "/api/offers/#{offer.id}/messages", headers: helper_headers, params: { content: "message content" } end it 'gives a success status' do @@ -49,7 +49,7 @@ describe 'unsuccessfully' do describe 'without content' do before do - post '/api/messages', headers: helper_headers, params: { offer_id: offer.id } + post "/api/offers/#{offer.id}/messages", headers: helper_headers end it 'gives an error status' do @@ -61,23 +61,23 @@ end end - describe 'without offer_id' do - before do - post '/api/messages', headers: helper_headers, params: { content: 'message content' } - end + # describe 'without offer_id' do + # before do + # post '/api/messages', headers: helper_headers, params: { content: 'message content' } + # end - it 'gives an error status' do - expect(response).to have_http_status 422 - end + # it 'gives an error status' do + # expect(response).to have_http_status 422 + # end - it 'gives an error message' do - expect(response_json['message']).to eq "Couldn't find Offer without an ID" - end - end + # it 'gives an error message' do + # expect(response_json['message']).to eq "Couldn't find Offer without an ID" + # end + # end describe 'unauthorized' do before do - post '/api/messages', headers: third_user_headers, params: { offer_id: offer.id, content: 'message content' } + post "/api/offers/#{offer.id}/messages", headers: third_user_headers, params: { content: 'message content' } end it 'gives an error status' do @@ -91,10 +91,9 @@ describe 'unauthenticated' do before do - post '/api/messages', params: { offer_id: offer.id, content: 'message content' } + post "/api/offers/#{offer.id}/messages", params: { content: 'message content' } end - it 'gives an error status' do expect(response).to have_http_status 401 end From c80a3a749150ab137638afcdc00357ea5a03b662 Mon Sep 17 00:00:00 2001 From: "Marcus.Sjoqvist" Date: Fri, 26 Jun 2020 22:24:09 +0200 Subject: [PATCH 2/6] sets up actioncable and attempts testing it --- Gemfile | 1 + Gemfile.lock | 2 ++ app/channels/offer_conversation_channel.rb | 16 ++++++++++++++++ app/controllers/api/messages_controller.rb | 1 + app/models/message.rb | 5 +++++ config/cable.yml | 15 ++++++--------- config/routes.rb | 1 + spec/channels/offer_conversation_channel_spec.rb | 5 +++++ spec/models/message_spec.rb | 9 +++++++++ spec/rails_helper.rb | 1 + .../conversions/users_can_send_messages_spec.rb | 6 ++++++ 11 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 app/channels/offer_conversation_channel.rb create mode 100644 spec/channels/offer_conversation_channel_spec.rb diff --git a/Gemfile b/Gemfile index 69160a9..e49253c 100644 --- a/Gemfile +++ b/Gemfile @@ -13,6 +13,7 @@ gem 'rack-cors', require: 'rack/cors' gem 'devise_token_auth' gem 'active_model_serializers' gem 'haversine' +gem 'redis' group :development, :test do gem 'pry-byebug' diff --git a/Gemfile.lock b/Gemfile.lock index eb44e1d..f4d0c20 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -171,6 +171,7 @@ GEM rb-fsevent (0.10.4) rb-inotify (0.10.1) ffi (~> 1.0) + redis (4.2.1) responders (3.0.0) actionpack (>= 5.0) railties (>= 5.0) @@ -243,6 +244,7 @@ DEPENDENCIES puma (~> 4.1) rack-cors rails (~> 6.0.3, >= 6.0.3.1) + redis rspec-rails shoulda-matchers spring diff --git a/app/channels/offer_conversation_channel.rb b/app/channels/offer_conversation_channel.rb new file mode 100644 index 0000000..ae9edbb --- /dev/null +++ b/app/channels/offer_conversation_channel.rb @@ -0,0 +1,16 @@ +class OfferConversationChannel < ApplicationCable::Channel + def subscribed + stream_from offer_identifier + end + + def unsubscribed + # Any cleanup needed when channel is unsubscribed + end + + private + + def offer_identifier + identifier = params[:data][:offer_id] + "offer_conversation_#{identifier}" + end +end diff --git a/app/controllers/api/messages_controller.rb b/app/controllers/api/messages_controller.rb index ff0feb1..3d5f2f7 100644 --- a/app/controllers/api/messages_controller.rb +++ b/app/controllers/api/messages_controller.rb @@ -5,6 +5,7 @@ def create message = Offer.find(params[:offer_id]).conversation .messages.create(content: params[:content], sender: current_user) if message.persisted? + # ActionCable.server.broadcast("offer_conversation_#{params[:offer_id]}", message: message.content ) render status: :created else render_error_message(message.errors) diff --git a/app/models/message.rb b/app/models/message.rb index 6504966..8fd89f9 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -3,6 +3,7 @@ class Message < ApplicationRecord belongs_to :sender, class_name: 'User' validates_presence_of :content before_create :validate_user_is_authorized + after_create :broadcast private @@ -10,4 +11,8 @@ def validate_user_is_authorized is_valid_user = sender == conversation.offer.helper || sender == conversation.offer.request.requester raise StandardError, 'You are not authorized to do this!' unless is_valid_user end + + def broadcast + ActionCable.server.broadcast("offer_conversation_#{conversation.offer.id}", message: content ) + end end diff --git a/config/cable.yml b/config/cable.yml index 07da6f3..d5eac3c 100644 --- a/config/cable.yml +++ b/config/cable.yml @@ -1,10 +1,7 @@ -development: - adapter: async - -test: - adapter: test - -production: +redis: &redis adapter: redis - url: <%= ENV.fetch("REDIS_URL") { "redis://localhost:6379/1" } %> - channel_prefix: reQuest_api_production + url: redis://localhost:6379/1 +production: *redis +development: *redis +test: + adapter: test \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 0420e14..c711cb1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true Rails.application.routes.draw do + mount ActionCable.server => '/cable' mount_devise_token_auth_for 'User', at: 'api/auth' namespace :api do resources :offers, only: %i[create show update] do diff --git a/spec/channels/offer_conversation_channel_spec.rb b/spec/channels/offer_conversation_channel_spec.rb new file mode 100644 index 0000000..47a431d --- /dev/null +++ b/spec/channels/offer_conversation_channel_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe OfferConversationChannel, type: :channel do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index f2cd23c..11cfacc 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -21,4 +21,13 @@ expect(create(:message)).to be_valid end end + + describe 'broadcast' do + it 'should broadcast after creation' do + expect { + @message = create(:message) + binding.pry + }.to have_broadcasted_to("offer_conversation_#{@message.conversation.offer.id}") + end + end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 522ddc0..709eea2 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -8,6 +8,7 @@ abort('The Rails environment is running in production mode!') if Rails.env.production? require 'spec_helper' + require 'rspec/rails' ActiveRecord::Migration.maintain_test_schema! diff --git a/spec/requests/api/conversions/users_can_send_messages_spec.rb b/spec/requests/api/conversions/users_can_send_messages_spec.rb index a5e8210..c1686ab 100644 --- a/spec/requests/api/conversions/users_can_send_messages_spec.rb +++ b/spec/requests/api/conversions/users_can_send_messages_spec.rb @@ -29,6 +29,12 @@ offer.reload expect(offer.conversation.messages.last['content']).to eq "message content" end + + # it 'dispatches a websocket message' do + # expect { + # 3.times { ActionCable.server.broadcast "offer_conversation_#{offer.id}", text: 'Hi!' } + # }.to have_broadcasted_to("offer_conversation_#{offer.id}").at_least(2).times + # end end describe 'successfully as the helper' do From 8c03572f1e59c73683eff9ceeeae4ccc7237ee4c Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 27 Jun 2020 11:07:58 +0200 Subject: [PATCH 3/6] fixes request, model and channel specs for OfferConversationChannel --- app/channels/offer_conversation_channel.rb | 10 +++++++-- app/models/message.rb | 2 +- .../offer_conversation_channel_spec.rb | 22 ++++++++++++++++++- spec/models/message_spec.rb | 8 +++---- .../users_can_send_messages_spec.rb | 12 +++++----- 5 files changed, 40 insertions(+), 14 deletions(-) diff --git a/app/channels/offer_conversation_channel.rb b/app/channels/offer_conversation_channel.rb index ae9edbb..0725424 100644 --- a/app/channels/offer_conversation_channel.rb +++ b/app/channels/offer_conversation_channel.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class OfferConversationChannel < ApplicationCable::Channel def subscribed stream_from offer_identifier @@ -10,7 +12,11 @@ def unsubscribed private def offer_identifier - identifier = params[:data][:offer_id] - "offer_conversation_#{identifier}" + if params[:data][:offer_id] + "offer_conversation_#{params[:data][:offer_id]}" + else + connection.transmit identifier: params, message: 'No params specified.' + reject + end end end diff --git a/app/models/message.rb b/app/models/message.rb index 8fd89f9..45a385c 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -3,7 +3,7 @@ class Message < ApplicationRecord belongs_to :sender, class_name: 'User' validates_presence_of :content before_create :validate_user_is_authorized - after_create :broadcast + after_save :broadcast private diff --git a/spec/channels/offer_conversation_channel_spec.rb b/spec/channels/offer_conversation_channel_spec.rb index 47a431d..6f17a04 100644 --- a/spec/channels/offer_conversation_channel_spec.rb +++ b/spec/channels/offer_conversation_channel_spec.rb @@ -1,5 +1,25 @@ +# frozen_string_literal: true + require 'rails_helper' RSpec.describe OfferConversationChannel, type: :channel do - pending "add some examples to (or delete) #{__FILE__}" + describe 'subscription' do + describe 'valid parameters' do + it { + subscribe(data: { offer_id: 234 }) + expect(subscription).to be_confirmed + } + end + + describe 'without valid params' do + before do + subscribe(data: {}) + end + it { expect(subscription).to be_rejected } + + it { + expect(transmissions.last).to eq('No params specified.') + } + end + end end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 11cfacc..88928fd 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -24,10 +24,10 @@ describe 'broadcast' do it 'should broadcast after creation' do - expect { - @message = create(:message) - binding.pry - }.to have_broadcasted_to("offer_conversation_#{@message.conversation.offer.id}") + message = create(:message) + expect { + message.save + }.to have_broadcasted_to("offer_conversation_#{Message.last.conversation.offer.id}") end end end diff --git a/spec/requests/api/conversions/users_can_send_messages_spec.rb b/spec/requests/api/conversions/users_can_send_messages_spec.rb index c1686ab..a87f1e4 100644 --- a/spec/requests/api/conversions/users_can_send_messages_spec.rb +++ b/spec/requests/api/conversions/users_can_send_messages_spec.rb @@ -10,7 +10,7 @@ let(:third_user) { create(:user) } let(:third_user_credentials) { third_user.create_new_auth_token } let(:third_user_headers) { { HTTP_ACCEPT: 'application/json' }.merge!(third_user_credentials) } - + let(:request) { create(:request, requester: requester) } let(:offer) { create(:offer, request: request, helper: helper) } @@ -30,11 +30,11 @@ expect(offer.conversation.messages.last['content']).to eq "message content" end - # it 'dispatches a websocket message' do - # expect { - # 3.times { ActionCable.server.broadcast "offer_conversation_#{offer.id}", text: 'Hi!' } - # }.to have_broadcasted_to("offer_conversation_#{offer.id}").at_least(2).times - # end + it 'dispatches a websocket message' do + expect( + ActionCable.server.worker_pool.executor.worker_task_completed + ).to eq 1 + end end describe 'successfully as the helper' do From fda27b4646849683b14545ec86d3035c237b3851 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 27 Jun 2020 11:55:13 +0200 Subject: [PATCH 4/6] adds specs for connection and channel adds current user identifier to connection --- app/channels/application_cable/connection.rb | 17 +++++++++++++++++ spec/channels/connection_spec.rb | 13 +++++++++++++ .../channels/offer_conversation_channel_spec.rb | 13 ++++++++++++- 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 spec/channels/connection_spec.rb diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb index 0ff5442..e357d5b 100644 --- a/app/channels/application_cable/connection.rb +++ b/app/channels/application_cable/connection.rb @@ -1,4 +1,21 @@ +# frozen_string_literal: true + module ApplicationCable class Connection < ActionCable::Connection::Base + identified_by :current_user + + def connect + self.current_user = find_verified_user + end + + private + + def find_verified_user + if verified_user = User.find_by(id: request.params[:uid]) + verified_user + else + reject_unauthorized_connection + end + end end end diff --git a/spec/channels/connection_spec.rb b/spec/channels/connection_spec.rb new file mode 100644 index 0000000..d4d7638 --- /dev/null +++ b/spec/channels/connection_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +RSpec.describe ApplicationCable::Connection, type: :channel do + let(:user) { create(:user) } + it 'successfully connects' do + connect "/cable?uid=#{user.id}" + expect(connection.current_user).to eq user + end + + it 'rejects connection' do + expect { connect '/cable' }.to have_rejected_connection + end +end diff --git a/spec/channels/offer_conversation_channel_spec.rb b/spec/channels/offer_conversation_channel_spec.rb index 6f17a04..2d7af07 100644 --- a/spec/channels/offer_conversation_channel_spec.rb +++ b/spec/channels/offer_conversation_channel_spec.rb @@ -3,18 +3,29 @@ require 'rails_helper' RSpec.describe OfferConversationChannel, type: :channel do + let(:user) { create(:user) } describe 'subscription' do + before { stub_connection current_user: user } describe 'valid parameters' do - it { + before do subscribe(data: { offer_id: 234 }) + end + + it { expect(subscription).to be_confirmed } + + it { + expect(subscription).to have_stream_from("offer_conversation_234") + } end describe 'without valid params' do + before do subscribe(data: {}) end + it { expect(subscription).to be_rejected } it { From d68688337aa94b4bfad663b991358e461bc03ed6 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 27 Jun 2020 12:41:55 +0200 Subject: [PATCH 5/6] refactors params key due to weeird implemeentation of clientt side package --- app/channels/offer_conversation_channel.rb | 7 ++++--- spec/channels/offer_conversation_channel_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/channels/offer_conversation_channel.rb b/app/channels/offer_conversation_channel.rb index 0725424..0b964e6 100644 --- a/app/channels/offer_conversation_channel.rb +++ b/app/channels/offer_conversation_channel.rb @@ -12,11 +12,12 @@ def unsubscribed private def offer_identifier - if params[:data][:offer_id] - "offer_conversation_#{params[:data][:offer_id]}" + if params[:room][:offer_id] + channel = "offer_conversation_#{params[:room][:offer_id]}" else connection.transmit identifier: params, message: 'No params specified.' - reject + reject && return end + channel end end diff --git a/spec/channels/offer_conversation_channel_spec.rb b/spec/channels/offer_conversation_channel_spec.rb index 2d7af07..41c9313 100644 --- a/spec/channels/offer_conversation_channel_spec.rb +++ b/spec/channels/offer_conversation_channel_spec.rb @@ -8,7 +8,7 @@ before { stub_connection current_user: user } describe 'valid parameters' do before do - subscribe(data: { offer_id: 234 }) + subscribe(room: { offer_id: 234 }) end it { @@ -23,7 +23,7 @@ describe 'without valid params' do before do - subscribe(data: {}) + subscribe(room: {}) end it { expect(subscription).to be_rejected } From 8759b07c2e0d454052b4e7352055f5fe692f25da Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 27 Jun 2020 13:58:31 +0200 Subject: [PATCH 6/6] further refactorings of channel and connection as well as message structure --- app/channels/application_cable/connection.rb | 2 +- app/models/message.rb | 8 +++++++- spec/channels/connection_spec.rb | 2 +- .../api/conversions/users_can_send_messages_spec.rb | 10 ++++++---- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb index e357d5b..2e57bba 100644 --- a/app/channels/application_cable/connection.rb +++ b/app/channels/application_cable/connection.rb @@ -11,7 +11,7 @@ def connect private def find_verified_user - if verified_user = User.find_by(id: request.params[:uid]) + if verified_user = User.find_by(email: request.params[:uid]) verified_user else reject_unauthorized_connection diff --git a/app/models/message.rb b/app/models/message.rb index 45a385c..bb8739a 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Message < ApplicationRecord belongs_to :conversation belongs_to :sender, class_name: 'User' @@ -13,6 +15,10 @@ def validate_user_is_authorized end def broadcast - ActionCable.server.broadcast("offer_conversation_#{conversation.offer.id}", message: content ) + data = { content: content, sender_id: sender.id } + ActionCable.server.broadcast( + "offer_conversation_#{conversation.offer.id}", + message: data + ) end end diff --git a/spec/channels/connection_spec.rb b/spec/channels/connection_spec.rb index d4d7638..4a81c0f 100644 --- a/spec/channels/connection_spec.rb +++ b/spec/channels/connection_spec.rb @@ -3,7 +3,7 @@ RSpec.describe ApplicationCable::Connection, type: :channel do let(:user) { create(:user) } it 'successfully connects' do - connect "/cable?uid=#{user.id}" + connect "/cable?uid=#{user.email}" expect(connection.current_user).to eq user end diff --git a/spec/requests/api/conversions/users_can_send_messages_spec.rb b/spec/requests/api/conversions/users_can_send_messages_spec.rb index a87f1e4..3cd9201 100644 --- a/spec/requests/api/conversions/users_can_send_messages_spec.rb +++ b/spec/requests/api/conversions/users_can_send_messages_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + RSpec.describe 'POST /offers/:offer_id/message users can post messages' do let(:requester) { create(:user, email: 'requester@mail.com') } let(:req_creds) { requester.create_new_auth_token } @@ -18,7 +20,7 @@ describe 'successfully as the requester' do before do - post "/api/offers/#{offer.id}/messages", headers: req_headers, params: { content: "message content" } + post "/api/offers/#{offer.id}/messages", headers: req_headers, params: { content: 'message content' } end it 'gives a success status' do @@ -27,7 +29,7 @@ it 'creates a message based on the params' do offer.reload - expect(offer.conversation.messages.last['content']).to eq "message content" + expect(offer.conversation.messages.last['content']).to eq 'message content' end it 'dispatches a websocket message' do @@ -39,7 +41,7 @@ describe 'successfully as the helper' do before do - post "/api/offers/#{offer.id}/messages", headers: helper_headers, params: { content: "message content" } + post "/api/offers/#{offer.id}/messages", headers: helper_headers, params: { content: 'message content' } end it 'gives a success status' do @@ -48,7 +50,7 @@ it 'creates a message based on the params' do offer.reload - expect(offer.conversation.messages.last['content']).to eq "message content" + expect(offer.conversation.messages.last['content']).to eq 'message content' end end