Skip to content

Commit

Permalink
align validations with validations in frontend. #334
Browse files Browse the repository at this point in the history
  • Loading branch information
Martin Fenner committed Aug 7, 2019
1 parent 291d31b commit c768210
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 42 deletions.
2 changes: 2 additions & 0 deletions app/models/client.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
15 changes: 4 additions & 11 deletions app/models/provider.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion spec/concerns/authenticable_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/factories/default.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,10 @@

factory :provider do
system_email { "[email protected]" }
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" }
Expand Down
7 changes: 6 additions & 1 deletion spec/models/provider_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/clients_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@
let(:params) do
{ "data" => { "type" => "clients",
"attributes" => {
"symbol" => client.symbol + "MegaCLient",
"symbol" => client.symbol + "M",
"email" => "[email protected]",
"name" => "Imperial College"}} }
end
Expand Down
6 changes: 3 additions & 3 deletions spec/requests/dois_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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",
Expand Down
53 changes: 30 additions & 23 deletions spec/requests/providers_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"name" => "British Library",
"displayName" => "British Library",
"systemEmail" => "[email protected]",
"website" => "https://www.bl.uk",
"country" => "GB" } } }
end
let(:headers) { {'HTTP_ACCEPT'=>'application/vnd.api+json', 'HTTP_AUTHORIZATION' => 'Bearer ' + token } }
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -103,6 +104,7 @@
"symbol" => "BL",
"name" => "British Library",
"displayName" => "British Library",
"website" => "https://www.bl.uk",
"salesforceId" => "abc012345678901234",
"region" => "EMEA",
"systemEmail" => "[email protected]",
Expand Down Expand Up @@ -147,6 +149,7 @@
"displayName" => "Figshare",
"region" => "EMEA",
"systemEmail" => "[email protected]",
"website" => "https://www.bl.uk",
"memberType" => "contractual_member",
"country" => "GB" } } }
end
Expand All @@ -171,6 +174,7 @@
"displayName" => "Figshare",
"region" => "EMEA",
"systemEmail" => "[email protected]",
"website" => "https://www.bl.uk",
"memberType" => "consortium_organization",
"country" => "GB" },
"relationships": {
Expand Down Expand Up @@ -225,6 +229,7 @@
"region" => "EMEA",
"systemEmail" => "[email protected]",
"memberType" => "provider",
"website" => "https://www.bl.uk",
"country" => "GB" },
"relationships": {
"consortium": {
Expand Down Expand Up @@ -257,6 +262,7 @@
"displayName" => "Figshare",
"region" => "EMEA",
"systemEmail" => "[email protected]",
"website" => "https://www.bl.uk",
"memberType" => "consortium_organization",
"country" => "GB" },
"relationships": {
Expand Down Expand Up @@ -293,6 +299,7 @@
"joined"=>"",
"keepPassword"=>"[FILTERED]",
"logoUrl"=>"",
"website" => "https://www.bl.uk",
"name"=>"Carnegie Mellon University",
"displayName"=>"Carnegie Mellon University",
"organizationType"=>"academicInstitution",
Expand All @@ -309,9 +316,8 @@
"postCode"=>"122dc"
},
"region"=>"",
"symbol"=>"CMfddff33333dd111d111113f4d",
"updated"=>"",
"website"=>""
"symbol"=>"CM",
"updated"=>""
}
}
}
Expand All @@ -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("[email protected]")
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
Expand Down Expand Up @@ -382,17 +383,18 @@
"familyName"=> "Dasler"
},
"region"=>"",
"symbol"=>"CMfddff33333dd111d111113f4d",
"symbol"=>"CM",
"updated"=>"",
"website"=>""
"website" => "https://www.bl.uk"
}
}
}
end

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("[email protected]")
expect(json.dig('data', 'attributes', 'technicalContact',"givenName")).to eq("Kristian")
expect(json.dig('data', 'attributes', 'technicalContact',"familyName")).to eq("Garza")
Expand All @@ -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
Expand All @@ -437,10 +433,14 @@
"organizationType" => "academicInstitution",
"focusArea" => "general", "logoUrl" => "",
"systemEmail" => "[email protected]",
"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"
}
}

Expand All @@ -467,6 +467,7 @@
"symbol" => "ADMIN",
"name" => "Admin",
"displayName" => "Admin",
"website" => "https://www.bl.uk",
"region" => "EMEA",
"systemEmail" => "[email protected]",
"country" => "GB" } } }
Expand All @@ -492,6 +493,7 @@
"symbol" => "BL",
"name" => "British Library",
"displayName" => "British Library",
"website" => "https://www.bl.uk",
"region" => "EMEA",
"systemEmail" => "[email protected]",
"country" => "GB" } } }
Expand Down Expand Up @@ -520,6 +522,7 @@
"symbol" => "BL",
"name" => "British Library",
"displayName" => "British Library",
"website" => "https://www.bl.uk",
"country" => "GB" } } }
end

Expand All @@ -539,6 +542,7 @@
"systemEmail" => "timAus",
"name" => "British Library",
"displayName" => "British Library",
"website" => "https://www.bl.uk",
"country" => "GB" } }
end

Expand All @@ -561,6 +565,7 @@
"attributes" => {
"name" => "British Library",
"displayName" => "British Library",
"website" => "https://www.bl.uk",
"region" => "Americas",
"systemEmail" => "[email protected]",
"country" => "GB" } } }
Expand Down Expand Up @@ -597,6 +602,7 @@
"displayName" => "British Library",
"region" => "Americas",
"systemEmail" => "[email protected]",
"website" => "https://www.bl.uk",
"country" => "GB" } } }
end
let(:admin) { create(:provider, symbol: "ADMIN", role_name: "ROLE_ADMIN", password_input: "12345") }
Expand All @@ -618,6 +624,7 @@
"name" => "British Library",
"displayName" => "British Library",
"region" => "Americas",
"website" => "https://www.bl.uk",
"systemEmail" => "[email protected]",
"country" => "GB" } } }
end
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/repositories_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@
let(:params) do
{ "data" => { "type" => "repositories",
"attributes" => {
"symbol" => client.symbol + "MegaCLient",
"symbol" => client.symbol + "M",
"email" => "[email protected]",
"name" => "Imperial College"}} }
end
Expand Down

0 comments on commit c768210

Please sign in to comment.