diff --git a/app/services/chat_names/authorize_user_service.rb b/app/services/chat_names/authorize_user_service.rb index 78b53cb3637068ccaa01752a9927d9a14a1e7220..f77804889232843e77dad22bcb3c3dc80c22b708 100644 --- a/app/services/chat_names/authorize_user_service.rb +++ b/app/services/chat_names/authorize_user_service.rb @@ -24,16 +24,16 @@ module ChatNames end def chat_name_token - Gitlab::ChatNameToken.new + @chat_name_token ||= Gitlab::ChatNameToken.new end def chat_name_params { - service_id: @service.id, - team_id: @params[:team_id], + service_id: @service.id, + team_id: @params[:team_id], team_domain: @params[:team_domain], - chat_id: @params[:user_id], - chat_name: @params[:user_name] + chat_id: @params[:user_id], + chat_name: @params[:user_name] } end end diff --git a/spec/services/chat_names/authorize_user_service_spec.rb b/spec/services/chat_names/authorize_user_service_spec.rb index 41cbac4e8e9bf5b96b27a444e3fa40794c1c10df..7f32948daad21d6525d85f1f289b155abc58d08c 100644 --- a/spec/services/chat_names/authorize_user_service_spec.rb +++ b/spec/services/chat_names/authorize_user_service_spec.rb @@ -4,23 +4,36 @@ require 'spec_helper' describe ChatNames::AuthorizeUserService do describe '#execute' do - let(:service) { create(:service) } + subject { described_class.new(service, params) } - subject { described_class.new(service, params).execute } + let(:result) { subject.execute } + let(:service) { create(:service) } context 'when all parameters are valid' do let(:params) { { team_id: 'T0001', team_domain: 'myteam', user_id: 'U0001', user_name: 'user' } } + it 'produces a valid HTTP URL' do + expect(result).to be_http_url + end + it 'requests a new token' do - is_expected.to be_url + expect(subject).to receive(:request_token).once.and_call_original + + subject.execute end end context 'when there are missing parameters' do let(:params) { {} } + it 'does not produce a URL' do + expect(result).to be_nil + end + it 'does not request a new token' do - is_expected.to be_nil + expect(subject).not_to receive(:request_token) + + subject.execute end end end diff --git a/spec/support/matchers/be_url.rb b/spec/support/matchers/be_url.rb index 69171f53891fd71f8dbd5d509d84299362e94810..388c1b384c7555ac6c1f87abdfc5b70f2b333967 100644 --- a/spec/support/matchers/be_url.rb +++ b/spec/support/matchers/be_url.rb @@ -1,11 +1,29 @@ # frozen_string_literal: true -RSpec::Matchers.define :be_url do |_| +# Assert that this value is a valid URL of at least one type. +# +# By default, this checks that the URL is either a HTTP or HTTPS URI, +# but you can check other URI schemes by passing the type, eg: +# +# ``` +# expect(value).to be_url(URI::FTP) +# ``` +# +# Pass an empty array of types if you want to match any URI scheme (be +# aware that this might not do what you think it does! `foo` is a valid +# URI, for instance). +RSpec::Matchers.define :be_url do |types = [URI::HTTP, URI::HTTPS]| match do |actual| - URI.parse(actual) rescue false + next false unless actual.present? + + uri = URI.parse(actual) + Array.wrap(types).any? { |t| uri.is_a?(t) } + rescue URI::InvalidURIError + false end end # looks better when used like: # expect(thing).to receive(:method).with(a_valid_url) RSpec::Matchers.alias_matcher :a_valid_url, :be_url +RSpec::Matchers.alias_matcher :be_http_url, :be_url