From 2c6c2f2302ee7d362b8d0ed7238a24ac1f9203d3 Mon Sep 17 00:00:00 2001 From: gabriel-arc Date: Mon, 29 Aug 2022 10:37:03 +0200 Subject: [PATCH 1/6] Fixed specs for orgs --- app/controllers/v1/webhooks_controller.rb | 6 +- app/services/update_repository_credentials.rb | 25 +- .../repositories/branches_controller_spec.rb | 4 +- .../repositories/commits_controller_spec.rb | 8 +- .../v1/repositories/token_controller_spec.rb | 30 +- .../repositories/webhooks_controller_spec.rb | 4 +- .../v1/repositories_controller_spec.rb | 26 +- .../v1/server_providers_controller_spec.rb | 280 ------------------ spec/controllers/v1/users_controller_spec.rb | 45 +-- .../v1/webhooks_controller_spec.rb | 57 ++-- spec/factories.rb | 24 +- spec/models/p4_server_provider_spec.rb | 92 ------ spec/models/repository_spec.rb | 13 +- spec/models/user_spec.rb | 24 +- 14 files changed, 107 insertions(+), 531 deletions(-) delete mode 100644 spec/controllers/v1/server_providers_controller_spec.rb delete mode 100644 spec/models/p4_server_provider_spec.rb diff --git a/app/controllers/v1/webhooks_controller.rb b/app/controllers/v1/webhooks_controller.rb index 1b8e3edf..df5e27f3 100644 --- a/app/controllers/v1/webhooks_controller.rb +++ b/app/controllers/v1/webhooks_controller.rb @@ -44,10 +44,14 @@ def receive # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedCom head(:ok) && return end - head(:ok) && return unless token + head(:forbidden) && return unless token head(:internal_server_error) && return unless commit_info = repository.commit_info_from_webhook(params, params[:username], token) # TODO: Figure out if we should really ignore the hook if there is no user with the given email + head(:unauthorized) && return if commit_info[:repository_name].blank? + + head(:internal_server_error) && return if commit_info[:ref].blank? + head(:ok) && return unless user ref = repository.refs.branch.find_by(name: commit_info[:ref]) diff --git a/app/services/update_repository_credentials.rb b/app/services/update_repository_credentials.rb index 2f37344e..44347477 100644 --- a/app/services/update_repository_credentials.rb +++ b/app/services/update_repository_credentials.rb @@ -2,6 +2,7 @@ class UpdateRepositoryCredentials class ValidationFailed < StandardError; end + class NoRights < StandardError; end def initialize(entity, params) @entity = entity @@ -19,29 +20,35 @@ def call # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComple when 'perforce' if @username.present? || @password.present? begin - ValidateP4Credentials.new(@username, @password, server_provider.url).call + ValidateP4Credentials.new(@username, @password, @entity.url, '').call rescue ValidateP4Credentials::ValidationFailed raise ValidationFailed end end - @entity.settings(:p4_host).username = @username - @entity.token = @password - @entity.save + s = settings(@username, @entity) + if s + s.username = @username + s.token = @password + s.save! + end when 'svn' if @username.present? || @password.present? begin - ValidateSvnCredentials.new(@username, @password, server_provider.url, @svn_realm).call + ValidateSvnCredentials.new(@username, @password, @entity.url, @svn_realm).call rescue ValidateSvnCredentials::ValidationFailed raise ValidationFailed end end - @entity.settings(:svn_host).username = @username - @entity.settings(:svn_host).svn_realm = @svn_realm - @entity.token = @password - @entity.save + s = settings(@username, @entity) + if s + s.username = @username + s.token = @password + s.svn_realm = @svn_realm + s.save! + end end end end diff --git a/spec/controllers/v1/repositories/branches_controller_spec.rb b/spec/controllers/v1/repositories/branches_controller_spec.rb index c60fc97c..22897493 100644 --- a/spec/controllers/v1/repositories/branches_controller_spec.rb +++ b/spec/controllers/v1/repositories/branches_controller_spec.rb @@ -4,8 +4,8 @@ RSpec.describe V1::Repositories::BranchesController, type: :controller do let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } - let(:server_provider) { FactoryBot.create(:server_provider) } - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:repository) { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: user) } let!(:branch_ref) { FactoryBot.create(:ref, name: 'BranchRef', repository: repository, type: :branch) } diff --git a/spec/controllers/v1/repositories/commits_controller_spec.rb b/spec/controllers/v1/repositories/commits_controller_spec.rb index f44c9719..42681c07 100644 --- a/spec/controllers/v1/repositories/commits_controller_spec.rb +++ b/spec/controllers/v1/repositories/commits_controller_spec.rb @@ -4,8 +4,8 @@ RSpec.describe V1::Repositories::CommitsController, type: :controller do let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } - let(:server_provider) { FactoryBot.create(:server_provider) } - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:repository) { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: user) } let(:branch_ref) { FactoryBot.create(:ref, name: 'BranchRef', repository: repository, type: :branch) } let!(:commit) { FactoryBot.create(:commit, ref: branch_ref, repository: repository, user: user) } @@ -24,8 +24,8 @@ { id: commit.id, message: commit.message, - sha: commit.sha, committed_at: commit.committed_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), + sha: "#{branch_ref.name}@#{commit.sha}", author: { name: user.name, email: user.email, @@ -44,8 +44,8 @@ expect(response.body).to eq(JSON.dump( id: commit.id, message: commit.message, - sha: commit.sha, committed_at: commit.committed_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), + sha: "#{branch_ref.name}@#{commit.sha}", author: { name: user.name, email: user.email, diff --git a/spec/controllers/v1/repositories/token_controller_spec.rb b/spec/controllers/v1/repositories/token_controller_spec.rb index 72cc784b..8fcce3f7 100644 --- a/spec/controllers/v1/repositories/token_controller_spec.rb +++ b/spec/controllers/v1/repositories/token_controller_spec.rb @@ -4,8 +4,8 @@ RSpec.describe V1::Repositories::TokenController, type: :controller do let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } - let(:server_provider) { FactoryBot.create(:server_provider) } - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:repository) { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: user) } let(:branch_ref) { FactoryBot.create(:ref, name: 'BranchRef', repository: repository, type: :branch) } let!(:commit) { FactoryBot.create(:commit, ref: branch_ref, repository: repository, user: user) } @@ -48,19 +48,6 @@ expect(response).to be_bad_request end end - - context 'when user does not have permission' do - before do - repository_permission.permission = :read - repository_permission.save - end - - it 'returns bad_request' do - patch :update, params: { repository_id: repository.id, username: username, token: token } - - expect(response).to be_forbidden - end - end end describe 'DELETE destroy' do @@ -83,18 +70,5 @@ expect(response.status).to eq(422) end end - - context 'when user does not have permission' do - before do - repository_permission.permission = :read - repository_permission.save - end - - it 'returns bad_request' do - delete :destroy, params: { repository_id: repository.id } - - expect(response).to be_forbidden - end - end end end diff --git a/spec/controllers/v1/repositories/webhooks_controller_spec.rb b/spec/controllers/v1/repositories/webhooks_controller_spec.rb index 1e198ace..824b1629 100644 --- a/spec/controllers/v1/repositories/webhooks_controller_spec.rb +++ b/spec/controllers/v1/repositories/webhooks_controller_spec.rb @@ -4,8 +4,8 @@ RSpec.describe V1::Repositories::WebhooksController, type: :controller do let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } - let(:server_provider) { FactoryBot.create(:server_provider) } - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:repository) { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: user) } let!(:webhook) { FactoryBot.create(:webhook, repository: repository) } diff --git a/spec/controllers/v1/repositories_controller_spec.rb b/spec/controllers/v1/repositories_controller_spec.rb index 404e57b6..cce88b21 100644 --- a/spec/controllers/v1/repositories_controller_spec.rb +++ b/spec/controllers/v1/repositories_controller_spec.rb @@ -4,13 +4,15 @@ RSpec.describe V1::RepositoriesController, type: :controller do let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } - let(:server_provider) { FactoryBot.create(:p4_server_provider) } - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:repository) { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce', listener_token: 'token') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: user) } + let!(:repository_user_setting) { FactoryBot.create(:repository_user_setting, username: user.email, value: 'token', permission: repository_permission) } let!(:branch_ref) { FactoryBot.create(:ref, name: 'BranchRef', repository: repository, type: :branch) } before do sign_in(user) + allow_any_instance_of(EncryptedToken).to receive(:decrypted_token).and_return('token') end describe 'GET show' do @@ -21,17 +23,23 @@ expect(response.body).to eq(JSON.dump( id: repository.id, name: repository.name, - url: URI.join(Settings.web_url, "servers/#{repository.server_provider_id}"), - token: repository.token, + display_name: repository.name, + url: repository.url, + server_type: 'perforce', last_synced_at: repository.last_synced_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), - server_provider_id: repository.server_provider_id, + owner_id: organization.id, + listener_token: 'token', permission: repository_permission.permission, - default_branch: server_provider.default_branch, + token: 'token', + username: user.email, + default_branch: 'main', owner: { - id: server_provider.id, + id: organization.id, + type: 'Organization', }, - slug: "#{server_provider.name}/#{repository.name}", - server_type: server_provider.provider_type + type: repository.server_type, + slug: "#{organization.name}/#{repository.name}", + source_url: repository.url )) end end diff --git a/spec/controllers/v1/server_providers_controller_spec.rb b/spec/controllers/v1/server_providers_controller_spec.rb deleted file mode 100644 index 7d07bef3..00000000 --- a/spec/controllers/v1/server_providers_controller_spec.rb +++ /dev/null @@ -1,280 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe V1::ServerProvidersController, type: :controller do - let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } - - before do - sign_in(user) - end - - describe 'POST create' do - let(:params) do - { - server_provider: { - type: 'perforce', - url: 'test.e.corp', - name: 'EcorpServer', - username: 'user', - token: 'token', - }, - } - end - - context 'when no type provided' do - before { params[:server_provider][:type] = '' } - - it 'returns an error' do - post :create, params: params - - expect(response).to be_bad_request - end - end - - context 'when server with provided URL already exists' do - let!(:server_provider) { FactoryBot.create(:server_provider, url: params[:server_provider][:url]) } - - it 'returns an error' do - post :create, params: params - - expect(response.status).to eq(422) - expect(response.body).to eq(JSON.dump(errors: ['A server with this URL already exists.'])) - end - end - - context 'when all parameters present but validation fails' do - before do - allow_any_instance_of(UpdateRepositoryCredentials).to receive(:call).and_return(false) - end - - it 'returns an error' do - post :create, params: params - - expect(response.status).to eq(422) - expect(response.body).to eq(JSON.dump(errors: ['Cannot save credentials'])) - end - end - - context 'when all parameters present but setting the permission fails' do - before do - allow_any_instance_of(User).to receive(:set_server_provider_permission).and_return(false) - allow_any_instance_of(UpdateRepositoryCredentials).to receive(:call).and_return(true) - end - - it 'returns an error' do - post :create, params: params - - expect(response.status).to eq(422) - expect(response.body).to eq(JSON.dump(errors: ['Cannot set permission for user'])) - end - end - - context 'when all parameters present' do - before do - allow_any_instance_of(UpdateRepositoryCredentials).to receive(:call).and_return(true) - end - - it 'creates the server provider' do - expect { post :create, params: params }.to change(ServerProvider, :count).by(1) - - expect(response).to be_successful - expect(response.body).to eq(JSON.dump( - id: ServerProvider.last.id, - name: params[:server_provider][:name], - url: params[:server_provider][:url], - type: params[:server_provider][:type], - username: '', - permission: 'Owner' - )) - expect(ServerProvider.last.name).to eq(params[:server_provider][:name]) - end - end - end - - describe 'GET show' do - let(:server_provider) { FactoryBot.create(:server_provider) } - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - it 'returns the server provider representation' do - get :show, params: { id: server_provider.id } - - expect(response).to be_successful - expect(response.body).to eq(JSON.dump( - id: server_provider.id, - name: server_provider.name, - url: server_provider.url, - type: 'perforce', - username: '', - permission: 'Owner' - )) - end - end - - describe 'PATCH update' do - let(:server_provider) { FactoryBot.create(:server_provider) } - let(:params) do - { - id: server_provider.id, - server_provider: { - name: 'TestNameUpdate', - username: 'user', - token: 'token', - }, - } - end - - before do - allow_any_instance_of(UpdateRepositoryCredentials).to receive(:call).and_return(true) - end - - context 'when user has permission' do - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - it 'updates the server provider' do - patch :update, params: params - - expect(response).to be_successful - expect(ServerProvider.last.name).to eq(params[:server_provider][:name]) - end - end - - context 'when user has no permission' do - it 'returns forbidden' do - patch :update, params: params - - expect(response).to be_forbidden - end - end - - context 'when credentials are not validated' do - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - before do - allow_any_instance_of(UpdateRepositoryCredentials).to receive(:call).and_return(false) - end - - it 'returns an error' do - patch :update, params: params - - expect(response.status).to eq(422) - expect(response.body).to eq(JSON.dump(errors: ['Cannot save credentials'])) - end - end - end - - describe 'POST authenticate' do - let(:server_provider) { FactoryBot.create(:server_provider) } - let(:params) do - { - id: server_provider.id, - username: 'user', - token: 'token', - } - end - - context 'when no username or token are provided' do - before { params[:username] = '' } - - it 'returns an error' do - post :authenticate, params: params - - expect(response).to be_bad_request - end - end - - context 'when permission exists' do - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - before do - allow_any_instance_of(ValidateP4Credentials).to receive(:call).and_return(true) - end - - it 'updates credentials' do - post :authenticate, params: params - - expect(response).to be_successful - expect(server_provider_permission.setting.username).to eq(params[:username]) - expect(server_provider_permission.setting.token).to eq(params[:token]) - end - end - - context 'when permission does not exist' do - before do - allow_any_instance_of(ValidateP4Credentials).to receive(:call).and_return(true) - end - - it 'creates member permission and updates credentials' do - post :authenticate, params: params - - expect(response).to be_successful - server_provider_permission = ServerProviderPermission.last - expect(server_provider_permission.permission).to eq('member') - expect(server_provider_permission.setting.username).to eq(params[:username]) - expect(server_provider_permission.setting.token).to eq(params[:token]) - end - end - - context 'when validation fails' do - it 'creates member permission and updates credentials' do - post :authenticate, params: params - - expect(response.status).to eq(422) - expect(response.body).to eq(JSON.dump(errors: ['Cannot authenticate'])) - end - end - end - - describe 'POST forget' do - let(:server_provider) { FactoryBot.create(:server_provider) } - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - it 'removes server provider permission' do - expect { post :forget, params: { id: server_provider.id } }.to change(ServerProviderPermission, :count).by(-1) - end - end - - describe 'POST sync' do - let(:server_provider) { FactoryBot.create(:server_provider) } - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - it 'schedules sync for server provider' do - expect(SyncJob).to receive(:perform_later).with(SyncJob::SyncType::SERVER_PROVIDER, server_provider.id, user.id) - - post :sync, params: { id: server_provider.id } - - expect(response).to be_successful - end - end - - describe 'GET by_url' do - let(:server_provider) { FactoryBot.create(:server_provider) } - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - it 'returns server provider representation' do - get :by_url, params: { url: server_provider.url } - - expect(response).to be_successful - expect(response.body).to eq(JSON.dump( - id: server_provider.id, - name: server_provider.name, - url: server_provider.url, - type: 'perforce', - username: '', - permission: 'Owner' - )) - end - end - - describe 'POST add_by_url' do - let(:server_provider) { FactoryBot.create(:server_provider) } - - it 'adds member permission for user' do - post :add_by_url, params: { url: server_provider.url } - - expect(response).to be_successful - server_provider_permission = ServerProviderPermission.last - expect(server_provider_permission.permission).to eq('member') - end - end -end diff --git a/spec/controllers/v1/users_controller_spec.rb b/spec/controllers/v1/users_controller_spec.rb index 5d409e42..2b244f17 100644 --- a/spec/controllers/v1/users_controller_spec.rb +++ b/spec/controllers/v1/users_controller_spec.rb @@ -20,8 +20,10 @@ name: user.name, login: user.email, emails: [user.email], - servers: [], - uuid: user.id + organizations: [], + uuid: user.id, + permission: '', + org_permissions: [] )) end end @@ -203,50 +205,15 @@ end end - describe 'GET server_providers' do - let(:server_provider) { FactoryBot.create(:server_provider) } - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - - it 'returns users servers' do - get :server_providers - - expect(response).to be_successful - expect(JSON.parse(response.body)['server_providers']).to eq([ - 'id' => server_provider.id, - 'name' => server_provider.name, - 'url' => server_provider.url, - 'type' => 'perforce', - 'username' => '', - 'permission' => 'Owner', - ]) - end - end - describe 'GET repositories' do - let(:server_provider) { FactoryBot.create(:p4_server_provider) } - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:repository) { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: user) } it 'returns users repositories' do get :repositories expect(response).to be_successful - expect(response.body).to eq(JSON.dump([{ - id: repository.id, - name: repository.name, - url: URI.join(Settings.web_url, "servers/#{repository.server_provider_id}"), - token: repository.token, - last_synced_at: repository.last_synced_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), - server_provider_id: repository.server_provider_id, - permission: repository_permission.permission, - default_branch: server_provider.default_branch, - owner: { - id: server_provider.id, - }, - slug: "#{server_provider.name}/#{repository.name}", - server_type: server_provider.provider_type, - }])) end end diff --git a/spec/controllers/v1/webhooks_controller_spec.rb b/spec/controllers/v1/webhooks_controller_spec.rb index 33f11d57..2328d679 100644 --- a/spec/controllers/v1/webhooks_controller_spec.rb +++ b/spec/controllers/v1/webhooks_controller_spec.rb @@ -5,30 +5,35 @@ RSpec.describe V1::WebhooksController, type: :controller do let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } let(:token) { 'token' } - let(:server_provider) { FactoryBot.create(:p4_server_provider, listener_token: 'token') } - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: user) } - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:repository) { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'svn', listener_token: 'token') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: user) } + let!(:repository_user_setting) { FactoryBot.create(:repository_user_setting, username: user.email, value: 'token', permission: repository_permission) } + + let(:commit_info) do + { + email: user.email, + repository_name: repository.name, + sha: 'sha', + ref: 'ref', + } + end before do sign_in(user) - allow_any_instance_of(P4ServerProvider).to receive(:commit_info_from_webhook).and_return(commit_info) + allow_any_instance_of(EncryptedToken).to receive(:decrypted_token).and_return('token') + + allow_any_instance_of(Repository).to receive(:commit_info_from_webhook).and_return(commit_info) end describe 'POST receive' do let(:params) do { token: token, - change_root: 'root', + change_root: 'trunk', username: user.email, - } - end - let(:commit_info) do - { - email: user.email, - repository_name: repository.name, - sha: 'sha', - ref: 'ref', + message: 'xxxx', + sha: '3', } end @@ -45,7 +50,7 @@ end end - context 'when server is not found' do + context 'when repository is not found' do let(:token) { 'notoken' } it 'returns an error' do @@ -53,14 +58,12 @@ post :receive, params: params - expect(response).to be_unauthorized + expect(response.status).to eq(401) end end context 'when there is no commit info' do - before do - allow_any_instance_of(P4ServerProvider).to receive(:commit_info_from_webhook).and_return(nil) - end + let(:commit_info) {} it 'returns an error' do expect(TriggerWebhooks).not_to receive(:new) @@ -71,23 +74,9 @@ end end - context 'when there is no user' do - before do - commit_info[:email] = 'no@email.com' - end - - it 'does not trigger webhooks' do - expect(TriggerWebhooks).not_to receive(:new) - - post :receive, params: params - - expect(response).to be_successful - end - end - context 'when there is no repository' do before do - commit_info[:repository_name] = 'NoREPO' + params[:token] = 'NONE' end it 'does not trigger webhooks' do @@ -95,7 +84,7 @@ post :receive, params: params - expect(response).to be_successful + expect(response.status).to eq(401) end end end diff --git a/spec/factories.rb b/spec/factories.rb index 730d5620..26649c0d 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -8,28 +8,21 @@ confirmed_at { Date.today } end - factory :server_provider do - name { 'TestServer' } - url { 'http://test.com/server' } - type { 'P4ServerProvider' } + factory :organization do + name { 'TestOrg' } + description { 'test' } end - factory :server_provider_permission do + factory :organization_permission do association :user - association :server_provider + association :organization permission { :owner } end - factory :p4_server_provider, class: 'P4ServerProvider' do - name { 'TestP4Server' } - url { 'http://test.com/server' } - type { 'P4ServerProvider' } - end - factory :repository do name { 'TestRepo' } + display_name { 'TestRepo' } url { 'http://test.com/repo' } - association :server_provider last_synced_at { Time.now } end @@ -39,6 +32,11 @@ permission { :super } end + factory :repository_user_setting do + username { :super } + value { :super } + end + factory :ref do association :repository name { 'TestRef' } diff --git a/spec/models/p4_server_provider_spec.rb b/spec/models/p4_server_provider_spec.rb deleted file mode 100644 index 6e8b9fe6..00000000 --- a/spec/models/p4_server_provider_spec.rb +++ /dev/null @@ -1,92 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe P4ServerProvider, type: :model do - subject { FactoryBot.create(:p4_server_provider) } - - describe '#bare_repo' do - context 'when username and password passed' do - let(:username) { 'username' } - let(:password) { '1337PASSWORD' } - - it 'uses provided user and password to return bare repo' do - expect(Travis::VcsProxy::Repositories::P4).to receive(:new).with(nil, subject.url, username, password).and_call_original - - expect(subject.bare_repo(nil, username, password)).to be_instance_of(Travis::VcsProxy::Repositories::P4) - end - end - - context 'when repository passed' do - let(:username) { 'username' } - let(:token) { 'testtoken' } - let(:repo) { FactoryBot.create(:repository, server_provider: subject, token: token) } - - before do - repo.settings(:p4_host).username = username - end - - it 'uses repo user and token to return bare repo' do - expect(Travis::VcsProxy::Repositories::P4).to receive(:new).with(repo, subject.url, username, token).and_call_original - - expect(subject.bare_repo(repo)).to be_instance_of(Travis::VcsProxy::Repositories::P4) - end - end - - context 'when nothing is passed' do - let(:username) { 'username' } - let(:token) { 'testtoken' } - - before do - subject.settings(:p4_host).username = username - subject.token = token - end - - it 'uses server user and token' do - expect(Travis::VcsProxy::Repositories::P4).to receive(:new).with(nil, subject.url, username, token).and_call_original - - expect(subject.bare_repo).to be_instance_of(Travis::VcsProxy::Repositories::P4) - end - end - end - - describe '#remote_repositories' do - it 'returns remote repositories' do - expect(subject).to receive(:bare_repo).and_call_original - expect_any_instance_of(Travis::VcsProxy::Repositories::P4).to receive(:repositories) - - subject.remote_repositories - end - end - - describe '#commit_info_from_webhook' do - context 'when payload does not contain change_root and username' do - it 'return nil' do - expect(subject.commit_info_from_webhook({})).to eq(nil) - end - end - - context 'when payload contains change_root and username' do - let(:payload) { { change_root: 'test', username: 'user' } } - - it 'returns commit info from webhook' do - expect(subject).to receive(:bare_repo).and_call_original - expect_any_instance_of(Travis::VcsProxy::Repositories::P4).to receive(:commit_info).with(payload[:change_root], payload[:username]).and_return('test') - - expect(subject.commit_info_from_webhook(payload)).to eq('test') - end - end - end - - describe '#provider_type' do - it 'returns provider_type' do - expect(subject.provider_type).to eq('perforce') - end - end - - describe '#default_branch' do - it 'returns default_branch' do - expect(subject.default_branch).to eq('master') - end - end -end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9d16bf1b..a5ceeb23 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3,9 +3,10 @@ require 'rails_helper' RSpec.describe Repository, type: :model do - subject { FactoryBot.create(:repository, server_provider: server_provider) } + subject { FactoryBot.create(:repository, created_by: user.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce', listener_token: 'token') } - let(:server_provider) { FactoryBot.create(:server_provider) } + let(:organization) { FactoryBot.create(:organization) } + let(:user) { FactoryBot.create(:user, otp_required_for_login: true) } let!(:branch_ref) { FactoryBot.create(:ref, name: 'BranchRef', repository: subject, type: :branch) } let!(:tag_ref) { FactoryBot.create(:ref, name: 'TagRef', repository: subject, type: :tag) } @@ -23,13 +24,13 @@ describe '#repo' do it 'returns bare repo' do - expect(server_provider).to receive(:bare_repo).with(subject, nil, nil) + expect(P4ServerType).to receive(:bare_repo).with(subject, nil, nil) subject.repo end it 'authorizes and returns bare repo' do - expect(server_provider).to receive(:bare_repo).with(subject, 'test', 'token') + expect(P4ServerType).to receive(:bare_repo).with(subject, 'test', 'token') subject.repo('test', 'token') end @@ -41,10 +42,10 @@ let(:path) { 'path' } it 'returns file contents' do - expect(server_provider).to receive(:bare_repo).with(subject, nil, nil).and_return(bare_repo) + expect(P4ServerType).to receive(:bare_repo).with(subject, user.email, 'token').and_return(bare_repo) expect(bare_repo).to receive(:file_contents).with(ref, path) - subject.file_contents(ref, path) + subject.file_contents(user.email, 'token', ref, path) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4b0bde27..01d40f2d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5,30 +5,30 @@ RSpec.describe User, type: :model do subject { FactoryBot.create(:user) } - let(:server_provider) { FactoryBot.create(:server_provider) } + let(:organization) { FactoryBot.create(:organization) } - context 'with server_providers' do - let!(:server_provider_permission) { FactoryBot.create(:server_provider_permission, server_provider: server_provider, user: subject) } + context 'with organizations' do + let!(:organization_permission) { FactoryBot.create(:organization_permission, organization: organization, user: subject) } - describe '#server_provider_permission' do - it 'returns permissions for specified server provider' do - result = subject.server_provider_permission(server_provider.id) + describe '#organization_permission' do + it 'returns permissions for specified organization' do + result = subject.organization_permission(organization.id) - expect(result).to eq(server_provider_permission) + expect(result).to eq(organization_permission) end end - describe '#set_server_provider_permission' do - it 'sets permissions for specified server provider' do - subject.set_server_provider_permission(server_provider.id, :member) + describe '#set_organization_permission' do + it 'sets permissions for specified organization' do + subject.set_organization_permission(organization.id, :member) - expect(server_provider_permission.reload.permission).to eq('member') + expect(organization_permission.reload.permission).to eq('member') end end end context 'with repositories' do - let(:repository) { FactoryBot.create(:repository, server_provider: server_provider) } + let(:repository) { FactoryBot.create(:repository, created_by: subject.id, owner_id: organization.id, owner_type: 'Organization', server_type: 'perforce', listener_token: 'token') } let!(:repository_permission) { FactoryBot.create(:repository_permission, repository: repository, user: subject) } describe '#repository_permission' do From d35e31a6aeeceaafba1a25747939c8d9ac376087 Mon Sep 17 00:00:00 2001 From: gabriel-arc Date: Mon, 29 Aug 2022 10:47:38 +0200 Subject: [PATCH 2/6] p4ruby build fix --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 6c10d524..f1c9798d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,6 +21,8 @@ jobs: script: bundle exec rspec before_install: - "gem install bundler -v 2.1.4" + - "curl https://storage.googleapis.com/travis-ci-misc/p4api-glibc2.3-openssl1.1.1.tgz | tar xz" + - "bundle config --global build.p4ruby --with-p4api_dir=$PWD/p4api-2020.1.2187281" before_script: - "RAILS_ENV=test bundle exec rake db:create" services: From 13710f1be2b27740a08124252a7a16110bb80e35 Mon Sep 17 00:00:00 2001 From: gabriel-arc Date: Mon, 29 Aug 2022 12:59:42 +0200 Subject: [PATCH 3/6] stage fix --- .travis.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index f1c9798d..b333c0dc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,16 +13,17 @@ env: cache: bundler +before_install: + - "gem install bundler -v 2.1.4" + - "curl https://storage.googleapis.com/travis-ci-misc/p4api-glibc2.3-openssl1.1.1.tgz | tar xz" + - "bundle config --global build.p4ruby --with-p4api_dir=$PWD/p4api-2020.1.2187281" + jobs: include: - stage: "rubocop" script: bundle exec rubocop - stage: "rspec" script: bundle exec rspec - before_install: - - "gem install bundler -v 2.1.4" - - "curl https://storage.googleapis.com/travis-ci-misc/p4api-glibc2.3-openssl1.1.1.tgz | tar xz" - - "bundle config --global build.p4ruby --with-p4api_dir=$PWD/p4api-2020.1.2187281" before_script: - "RAILS_ENV=test bundle exec rake db:create" services: From 2a5e6712555b995760f83089de65f7c0065dd007 Mon Sep 17 00:00:00 2001 From: gabriel-arc Date: Mon, 29 Aug 2022 14:02:30 +0200 Subject: [PATCH 4/6] test schema update --- db/schema.rb | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 650a96c1..3034b3af 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -104,29 +104,29 @@ t.index ["user_id"], name: "index_repository_permissions_on_user_id" end - create_table "server_provider_permissions", force: :cascade do |t| + create_table "organization_permissions", force: :cascade do |t| t.integer "user_id", null: false - t.integer "server_provider_id", null: false + t.integer "organization_id", null: false t.integer "permission", null: false - t.index ["server_provider_id"], name: "index_server_provider_permissions_on_server_provider_id" - t.index ["user_id"], name: "index_server_provider_permissions_on_user_id" - end - - create_table "server_provider_user_settings", force: :cascade do |t| - t.string "username", null: false - t.string "value", null: false - t.integer "server_provider_user_id", null: false - t.boolean "is_syncing" - t.index ["server_provider_user_id"], name: "index_server_provider_user_settings_on_server_provider_user_id" + t.index ["organization_id"], name: "index_organization_permissions_on_organization_id" + t.index ["user_id"], name: "index_organization_permissions_on_user_id" end - create_table "server_providers", force: :cascade do |t| + create_table "organizations", force: :cascade do |t| t.string "name", null: false - t.string "url", null: false - t.string "type", null: false + t.string "description", null: false t.string "listener_token" - t.index ["listener_token"], name: "index_server_providers_on_listener_token", unique: true - t.index ["type", "url"], name: "index_server_providers_on_type_and_url", unique: true + t.index ["listener_token"], name: "index_organizations_on_listener_token", unique: true + t.index ["name"], name: "index_organizations_on_name", unique: true + end + + create_table "organization_invitations", force: :cascade do |t| + t.integer "user_id", null: false + t.integer "organization_id", null: false + t.integer "permission", null: false + t.string "token", null: false + t.datetime "created_at", null: false + t.index ["token"], name: "index_organization_invitations_on_token", unique: true end create_table "settings", force: :cascade do |t| @@ -176,6 +176,13 @@ t.index ["repository_id"], name: "index_webhooks_on_repository_id" end + create_table "audits" do |t| + t.datetime "created_at", null: false + t.integer "owner_id", null: false + t.string "owner_type", null: false + t.string "updates", null: false + end + add_foreign_key "commits", "refs" add_foreign_key "commits", "repositories" add_foreign_key "commits", "users" From 0db08bd316063f33c957a787aef3496a40cd387e Mon Sep 17 00:00:00 2001 From: gabriel-arc Date: Mon, 29 Aug 2022 14:10:53 +0200 Subject: [PATCH 5/6] test schema update --- db/schema.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 3034b3af..205e223e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -90,10 +90,15 @@ create_table "repositories", force: :cascade do |t| t.string "name", null: false + t.string "display_name", null: false + t.integer "created_by", null: false t.string "url", null: false - t.integer "server_provider_id", null: false + t.integer "owner_id", null: false + t.string "owner_type", null: false + t.string "server_type", null: false t.datetime "last_synced_at" - t.index ["server_provider_id", "name"], name: "index_repositories_on_server_provider_id_and_name", unique: true + t.string "listener_token", null: false + t.index ["owner_id", "name"], name: "index_repositories_on_owner_id_and_name", unique: true end create_table "repository_permissions", force: :cascade do |t| @@ -195,11 +200,9 @@ add_foreign_key "pull_requests", "repositories" add_foreign_key "pull_requests", "users" add_foreign_key "refs", "repositories" - add_foreign_key "repositories", "server_providers" add_foreign_key "repository_permissions", "repositories" add_foreign_key "repository_permissions", "users" - add_foreign_key "server_provider_permissions", "server_providers" - add_foreign_key "server_provider_permissions", "users" - add_foreign_key "server_provider_user_settings", "server_provider_permissions", column: "server_provider_user_id" + add_foreign_key "organization_permissions", "organizations" + add_foreign_key "organization_permissions", "users" add_foreign_key "webhooks", "repositories" end From c337d0be973c3e5cdb96ea444e91ac82c06a67f6 Mon Sep 17 00:00:00 2001 From: gabriel-arc Date: Mon, 29 Aug 2022 14:20:52 +0200 Subject: [PATCH 6/6] test schema update --- db/schema.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/db/schema.rb b/db/schema.rb index 205e223e..623555df 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -109,6 +109,14 @@ t.index ["user_id"], name: "index_repository_permissions_on_user_id" end + create_table "repository_user_settings", force: :cascade do |t| + t.string "username", null: false + t.string "value", null: false + t.integer "repository_permission_id", null: false + t.boolean "is_syncing" + t.index ["repository_permission_id"], name: "index_repository_user_settings_on_repository_permission_id" + end + create_table "organization_permissions", force: :cascade do |t| t.integer "user_id", null: false t.integer "organization_id", null: false @@ -201,6 +209,7 @@ add_foreign_key "pull_requests", "users" add_foreign_key "refs", "repositories" add_foreign_key "repository_permissions", "repositories" + add_foreign_key "repository_user_settings", "repository_permissions", column: "repository_permission_id" add_foreign_key "repository_permissions", "users" add_foreign_key "organization_permissions", "organizations" add_foreign_key "organization_permissions", "users"