diff --git a/app/models/client.rb b/app/models/client.rb index 15bfa55e7..d3424d397 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -36,6 +36,8 @@ class Client < ActiveRecord::Base validates_presence_of :symbol, :name, :system_email validates_uniqueness_of :symbol, message: "This Client ID has already been taken" + validates_format_of :symbol, :with => /\A([A-Z]+\.[A-Z0-9]+(-[A-Z0-9]+)?)\Z/, message: "should only contain capital letters, numbers, and at most one hyphen" + validates_length_of :symbol, minimum: 5, maximum: 18 validates_format_of :system_email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i validates_format_of :salesforce_id, :with => /[a-zA-Z0-9]{18}/, message: "wrong format for salesforce id", if: :salesforce_id? validates_inclusion_of :role_name, :in => %w( ROLE_DATACENTRE ), :message => "Role %s is not included in the list" diff --git a/app/models/provider.rb b/app/models/provider.rb index ad6603de1..54605faac 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -32,15 +32,17 @@ class Provider < ActiveRecord::Base attr_readonly :symbol attr_accessor :password_input - validates_presence_of :symbol, :name, :display_name, :system_email + validates_presence_of :symbol, :name, :display_name, :system_email, :website validates_uniqueness_of :symbol, message: "This name has already been taken" + validates_format_of :symbol, :with => /\A([A-Z]+)\Z/, message: "should only contain capital letters" + validates_length_of :symbol, minimum: 2, maximum: 8 validates_format_of :system_email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i, message: "system_email should be an email" validates_format_of :group_email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i, if: :group_email?, message: "group_email should be an email" validates_format_of :website, :with => /https?:\/\/[\S]+/ , if: :website?, message: "Website should be a url" validates_format_of :salesforce_id, :with => /[a-zA-Z0-9]{18}/, message: "wrong format for salesforce id", if: :salesforce_id? validates_inclusion_of :role_name, :in => %w( ROLE_FOR_PROFIT_PROVIDER ROLE_CONTRACTUAL_PROVIDER ROLE_CONSORTIUM ROLE_CONSORTIUM_ORGANIZATION ROLE_ALLOCATOR ROLE_MEMBER ROLE_REGISTRATION_AGENCY ROLE_ADMIN ROLE_DEV ), :message => "Role %s is not included in the list" validates_inclusion_of :organization_type, :in => %w(researchInstitution academicInstitution governmentAgency nationalInstitution professionalSociety publisher serviceProvider other), message: "organization type %s is not included in the list", if: :organization_type? - validates_inclusion_of :non_profit_status, :in => %w(non-profit for-profit), message: "non-profit status %s is not included in the list" + validates_inclusion_of :non_profit_status, :in => %w(non-profit for-profit), message: "non-profit status '%s' is not included in the list" validates_inclusion_of :focus_area, :in => %w(naturalSciences engineeringAndTechnology medicalAndHealthSciences agriculturalSciences socialSciences humanities general), message: "focus area %s is not included in the list", if: :focus_area? validate :freeze_symbol, :on => :update validate :can_be_in_consortium @@ -569,15 +571,6 @@ def set_region write_attribute(:region, r) end - # def set_provider_type - # if doi_quota_allowed != 0 - # r = "allocating" - # else - # r = "non_allocating" - # end - # write_attribute(:provider_type, r) - # end - def set_defaults self.symbol = symbol.upcase if symbol.present? self.is_active = is_active ? "\x01" : "\x00" diff --git a/spec/concerns/authenticable_spec.rb b/spec/concerns/authenticable_spec.rb index 826bbb07f..b39e3f963 100644 --- a/spec/concerns/authenticable_spec.rb +++ b/spec/concerns/authenticable_spec.rb @@ -47,7 +47,7 @@ describe 'encode_auth_param' do it "works" do credentials = subject.encode_auth_param(username: subject.symbol, password: 12345) - expect(credentials).to start_with("VEVTVD") + expect(credentials).to start_with("VEVTVE") end it "no password" do diff --git a/spec/factories/default.rb b/spec/factories/default.rb index 6f0fccffc..f9371e118 100644 --- a/spec/factories/default.rb +++ b/spec/factories/default.rb @@ -205,9 +205,10 @@ factory :provider do system_email { "josiah@example.org" } - sequence(:symbol) { |n| "TEST#{n}" } + sequence(:symbol, 'A') { |n| "TEST#{n}" } name { "My provider" } display_name { "My provider" } + website { Faker::Internet.url } country_code { "DE" } password_input { "12345" } twitter_handle { "@egaTwitterlac" } diff --git a/spec/models/provider_spec.rb b/spec/models/provider_spec.rb index a2dca7bef..f0363c709 100644 --- a/spec/models/provider_spec.rb +++ b/spec/models/provider_spec.rb @@ -8,7 +8,12 @@ it { should validate_presence_of(:name) } it { should validate_presence_of(:display_name) } it { should validate_presence_of(:system_email) } + it { should validate_presence_of(:website) } it { is_expected.to strip_attribute(:name) } + it { should allow_value("AB").for(:symbol) } + it { should_not allow_value("A").for(:symbol) } + it { should_not allow_value("A9").for(:symbol) } + it { should_not allow_value("AAAAAAAAAA").for(:symbol) } it { expect(provider).to be_valid } end @@ -21,7 +26,7 @@ end describe "provider with ROLE_CONTRACTUAL_PROVIDER" do - subject { create(:provider, role_name: "ROLE_CONTRACTUAL_PROVIDER", name: "Contractor", symbol: "CONTRACT_SLASH") } + subject { create(:provider, role_name: "ROLE_CONTRACTUAL_PROVIDER", name: "Contractor", symbol: "CONTRCTR") } it "works" do expect(subject.role_name).to eq("ROLE_CONTRACTUAL_PROVIDER") diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index 5858a16d4..cc7eaa070 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -201,7 +201,7 @@ let(:params) do { "data" => { "type" => "clients", "attributes" => { - "symbol" => client.symbol + "MegaCLient", + "symbol" => client.symbol + "M", "email" => "bob@example.com", "name" => "Imperial College"}} } end diff --git a/spec/requests/dois_spec.rb b/spec/requests/dois_spec.rb index c3853a225..92870d77e 100644 --- a/spec/requests/dois_spec.rb +++ b/spec/requests/dois_spec.rb @@ -772,7 +772,7 @@ let(:provider_headers) { {'HTTP_ACCEPT'=>'application/vnd.api+json', 'CONTENT_TYPE'=>'application/vnd.api+json', 'HTTP_AUTHORIZATION' => 'Bearer ' + provider_bearer}} let(:doi) { create(:doi, client: client) } - let(:new_client) { create(:client, symbol: "#{provider.symbol}.magic", provider: provider, password: ENV['MDS_PASSWORD']) } + let(:new_client) { create(:client, symbol: "#{provider.symbol}.M", provider: provider, password: ENV['MDS_PASSWORD']) } # attributes MUST be empty let(:valid_attributes) do @@ -806,7 +806,7 @@ context 'when we transfer a DOI as staff' do let(:doi) { create(:doi, doi: "10.14454/119495", url: "http://www.bl.uk/pdf/pat.pdf", client: client, aasm_state: "registered") } - let(:new_client) { create(:client, symbol: "#{provider.symbol}.magic", provider: provider, password: ENV['MDS_PASSWORD']) } + let(:new_client) { create(:client, symbol: "#{provider.symbol}.M", provider: provider, password: ENV['MDS_PASSWORD']) } let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) } let(:valid_attributes) do { @@ -2605,7 +2605,7 @@ # Create a different dummy client and a doi with entry associated # This is so we can test clients accessing others information - let(:other_client) { create(:client, provider: provider, symbol: 'DATACITE.DOESNTEXIST', password: 'notarealpassword') } + let(:other_client) { create(:client, provider: provider, symbol: 'DATACITE.DNE', password: 'notarealpassword') } let(:other_doi) { create(:doi, doi: "10.24425/2210181332", client: other_client, state: "findable", diff --git a/spec/requests/providers_spec.rb b/spec/requests/providers_spec.rb index 7bb519710..b2441e185 100644 --- a/spec/requests/providers_spec.rb +++ b/spec/requests/providers_spec.rb @@ -10,6 +10,7 @@ "name" => "British Library", "displayName" => "British Library", "systemEmail" => "bob@example.com", + "website" => "https://www.bl.uk", "country" => "GB" } } } end let(:headers) { {'HTTP_ACCEPT'=>'application/vnd.api+json', 'HTTP_AUTHORIZATION' => 'Bearer ' + token } } @@ -56,7 +57,7 @@ end context 'get provider type ROLE_CONTRACTUAL_PROVIDER and check it works ' do - let(:provider) { create(:provider, role_name: "ROLE_CONTRACTUAL_PROVIDER", name: "Contractor", symbol: "CONTRACT_SLASH") } + let(:provider) { create(:provider, role_name: "ROLE_CONTRACTUAL_PROVIDER", name: "Contractor", symbol: "CONTRCTR") } it 'get provider' do get "/providers/#{provider.symbol.downcase}", nil, headers @@ -103,6 +104,7 @@ "symbol" => "BL", "name" => "British Library", "displayName" => "British Library", + "website" => "https://www.bl.uk", "salesforceId" => "abc012345678901234", "region" => "EMEA", "systemEmail" => "doe@joe.joe", @@ -147,6 +149,7 @@ "displayName" => "Figshare", "region" => "EMEA", "systemEmail" => "doe@joe.joe", + "website" => "https://www.bl.uk", "memberType" => "contractual_member", "country" => "GB" } } } end @@ -171,6 +174,7 @@ "displayName" => "Figshare", "region" => "EMEA", "systemEmail" => "doe@joe.joe", + "website" => "https://www.bl.uk", "memberType" => "consortium_organization", "country" => "GB" }, "relationships": { @@ -225,6 +229,7 @@ "region" => "EMEA", "systemEmail" => "doe@joe.joe", "memberType" => "provider", + "website" => "https://www.bl.uk", "country" => "GB" }, "relationships": { "consortium": { @@ -257,6 +262,7 @@ "displayName" => "Figshare", "region" => "EMEA", "systemEmail" => "doe@joe.joe", + "website" => "https://www.bl.uk", "memberType" => "consortium_organization", "country" => "GB" }, "relationships": { @@ -293,6 +299,7 @@ "joined"=>"", "keepPassword"=>"[FILTERED]", "logoUrl"=>"", + "website" => "https://www.bl.uk", "name"=>"Carnegie Mellon University", "displayName"=>"Carnegie Mellon University", "organizationType"=>"academicInstitution", @@ -309,9 +316,8 @@ "postCode"=>"122dc" }, "region"=>"", - "symbol"=>"CMfddff33333dd111d111113f4d", - "updated"=>"", - "website"=>"" + "symbol"=>"CM", + "updated"=>"" } } } @@ -320,18 +326,13 @@ it 'creates a provider' do post '/providers', params, headers + expect(last_response.status).to eq(200) expect(json.dig('data', 'attributes', 'systemEmail')).to eq("jkiritha@andrew.cmu.edu") expect(json.dig('data', 'attributes', 'billingInformation',"state")).to eq("Rennes") expect(json.dig('data', 'attributes', 'billingInformation',"postCode")).to eq("122dc") expect(json.dig('data', 'attributes', 'twitterHandle')).to eq("@meekakitty") expect(json.dig('data', 'attributes', 'rorId')).to eq("https://ror.org/05njkjr15") end - - it 'returns status code 201' do - post '/providers', params, headers - - expect(last_response.status).to eq(200) - end end context 'request is valid with contact information' do @@ -382,9 +383,9 @@ "familyName"=> "Dasler" }, "region"=>"", - "symbol"=>"CMfddff33333dd111d111113f4d", + "symbol"=>"CM", "updated"=>"", - "website"=>"" + "website" => "https://www.bl.uk" } } } @@ -392,7 +393,8 @@ it 'creates a provider' do post '/providers', params, headers - + + expect(last_response.status).to eq(200) expect(json.dig('data', 'attributes', 'technicalContact',"email")).to eq("kristian@example.com") expect(json.dig('data', 'attributes', 'technicalContact',"givenName")).to eq("Kristian") expect(json.dig('data', 'attributes', 'technicalContact',"familyName")).to eq("Garza") @@ -409,12 +411,6 @@ expect(json.dig('data', 'attributes', 'votingContact',"givenName")).to eq("Robin") expect(json.dig('data', 'attributes', 'votingContact',"familyName")).to eq("Dasler") end - - it 'returns status code 201' do - post '/providers', params, headers - - expect(last_response.status).to eq(200) - end end context 'request for admin provider with meta' do @@ -437,10 +433,14 @@ "organizationType" => "academicInstitution", "focusArea" => "general", "logoUrl" => "", "systemEmail" => "jkiritha@andrew.cmu.edu", - "website" => "", "isActive" => true, - "passwordInput" => "@change", "hasPassword" => false, - "keepPassword" => false, "joined" => "" - }, "type" => "providers" + "website" => "https://www.bl.uk", + "isActive" => true, + "passwordInput" => "@change", + "hasPassword" => false, + "keepPassword" => false, + "joined" => "" + }, + "type" => "providers" } } @@ -467,6 +467,7 @@ "symbol" => "ADMIN", "name" => "Admin", "displayName" => "Admin", + "website" => "https://www.bl.uk", "region" => "EMEA", "systemEmail" => "doe@joe.joe", "country" => "GB" } } } @@ -492,6 +493,7 @@ "symbol" => "BL", "name" => "British Library", "displayName" => "British Library", + "website" => "https://www.bl.uk", "region" => "EMEA", "systemEmail" => "doe@joe.joe", "country" => "GB" } } } @@ -520,6 +522,7 @@ "symbol" => "BL", "name" => "British Library", "displayName" => "British Library", + "website" => "https://www.bl.uk", "country" => "GB" } } } end @@ -539,6 +542,7 @@ "systemEmail" => "timAus", "name" => "British Library", "displayName" => "British Library", + "website" => "https://www.bl.uk", "country" => "GB" } } end @@ -561,6 +565,7 @@ "attributes" => { "name" => "British Library", "displayName" => "British Library", + "website" => "https://www.bl.uk", "region" => "Americas", "systemEmail" => "Pepe@mdm.cod", "country" => "GB" } } } @@ -597,6 +602,7 @@ "displayName" => "British Library", "region" => "Americas", "systemEmail" => "Pepe@mdm.cod", + "website" => "https://www.bl.uk", "country" => "GB" } } } end let(:admin) { create(:provider, symbol: "ADMIN", role_name: "ROLE_ADMIN", password_input: "12345") } @@ -618,6 +624,7 @@ "name" => "British Library", "displayName" => "British Library", "region" => "Americas", + "website" => "https://www.bl.uk", "systemEmail" => "Pepe@mdm.cod", "country" => "GB" } } } end diff --git a/spec/requests/repositories_spec.rb b/spec/requests/repositories_spec.rb index 642a6b947..870db490a 100644 --- a/spec/requests/repositories_spec.rb +++ b/spec/requests/repositories_spec.rb @@ -178,7 +178,7 @@ let(:params) do { "data" => { "type" => "repositories", "attributes" => { - "symbol" => client.symbol + "MegaCLient", + "symbol" => client.symbol + "M", "email" => "bob@example.com", "name" => "Imperial College"}} } end