diff --git a/app/controllers/providers_controller.rb b/app/controllers/providers_controller.rb index 3d63f3c2e..03bb1422b 100644 --- a/app/controllers/providers_controller.rb +++ b/app/controllers/providers_controller.rb @@ -84,7 +84,7 @@ def index def show options = {} - options[:meta] = { + options[:meta] = { providers: provider_count(provider_id: params[:id] == "admin" ? nil : params[:id]), clients: client_count(provider_id: params[:id] == "admin" ? nil : params[:id]), dois: doi_count(provider_id: params[:id] == "admin" ? nil : params[:id]) }.compact @@ -107,7 +107,7 @@ def create options[:is_collection] = false options[:params] = { :current_ability => current_ability, - } + } render json: ProviderSerializer.new(@provider, options).serialized_json, status: :ok else logger.warn @provider.errors.inspect @@ -120,7 +120,7 @@ def update # logger.debug safe_params.inspect if @provider.update_attributes(safe_params) options = {} - options[:meta] = { + options[:meta] = { providers: provider_count(provider_id: params[:id] == "admin" ? nil : params[:id]), clients: client_count(provider_id: params[:id] == "admin" ? nil : params[:id]), dois: doi_count(provider_id: params[:id] == "admin" ? nil : params[:id]) }.compact @@ -129,7 +129,7 @@ def update options[:params] = { :current_ability => current_ability, } - + render json: ProviderSerializer.new(@provider, options).serialized_json, status: :ok else logger.warn @provider.errors.inspect @@ -192,8 +192,21 @@ def safe_params # ] # params.require(:data).permit(:type, attributes: attributes) ActiveModelSerializers::Deserialization.jsonapi_parse!( - params, only: [:name, :symbol, :description, :website, :joined, "organizationType", "focusArea", :phone, "contactName", "contactEmail", "isActive", "passwordInput", :country, "billingInformation",{ "billingInformation": ["postCode", :state, :city, :address, :department, :organization, :country]}, "rorId", "twitterHandle" ], - keys: { "organizationType" => :organization_type, "focusArea" => :focus_area, "contactName" => :contact_name, "contactEmail" => :contact_email, :country => :country_code, "isActive" => :is_active, "passwordInput" => :password_input, "billingInformation" => :billing_information , "postCode" => :post_code, "rorId" => :ror_id, "twitterHandle" =>:twitter_handle } + params, + only: [ + :name, :symbol, :description, :website, :joined, "organizationType", "focusArea", :phone, "contactName", "contactEmail", "isActive", "passwordInput", :country, "billingInformation",{ "billingInformation": ["postCode", :state, :city, :address, :department, :organization, :country]}, "rorId", "twitterHandle", + "generalContact",{ "generalContact": [:email, "givenName", "familyName"]}, + "technicalContact",{ "technicalContact": [:email, "givenName", "familyName"]}, + "serviceContact",{ "serviceContact": [:email, "givenName", "familyName"]}, + "votingContact",{ "votingContact": [:email, "givenName", "familyName"]} + ], + keys: { + "organizationType" => :organization_type, "focusArea" => :focus_area, "contactName" => :contact_name, "contactEmail" => :contact_email, :country => :country_code, "isActive" => :is_active, "passwordInput" => :password_input, "billingInformation" => :billing_information , "postCode" => :post_code, "rorId" => :ror_id, "twitterHandle" =>:twitter_handle, + "generalContact" => :general_contact, + "technicalContact" => :technical_contact, + "serviceContact" => :service_contact, + "votingContact" => :voting_contact + } ) end end diff --git a/app/models/provider.rb b/app/models/provider.rb index 5b8a28904..19042eb39 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -41,6 +41,10 @@ class Provider < ActiveRecord::Base validate :freeze_symbol, :on => :update validates_format_of :ror_id, :with => /\A(?:(http|https):\/\/)?(?:ror\.org\/)?(0\w{6}\d{2})\z/, if: :ror_id? validates_format_of :twitter_handle, :with => /\A[a-zA-Z0-9_]{1,15}\z/, if: :twitter_handle? + validates :general_contact, contact: true + validates :technical_contact, contact: true + validates :service_contact, contact: true + validates :voting_contact, contact: true before_validation :set_region @@ -108,6 +112,26 @@ class Provider < ActiveRecord::Base city: { type: :text }, country: { type: :text }, address: { type: :text }} + indexes :general_contact, type: :object, properties: { + email: { type: :text }, + given_name: { type: :text}, + family_name: { type: :text } + } + indexes :technical_contact, type: :object, properties: { + email: { type: :text }, + given_name: { type: :text}, + family_name: { type: :text } + } + indexes :service_contact, type: :object, properties: { + email: { type: :text }, + given_name: { type: :text}, + family_name: { type: :text } + } + indexes :voting_contact, type: :object, properties: { + email: { type: :text }, + given_name: { type: :text}, + family_name: { type: :text } + } indexes :created, type: :date indexes :updated, type: :date indexes :deleted_at, type: :date @@ -152,6 +176,10 @@ def as_indexed_json(options={}) "country" => billing_country, "city" => billing_city }, + "general_contact" => general_contact, + "technical_contact" => technical_contact, + "service_contact" => service_contact, + "voting_contact" => voting_contact, "created" => created, "updated" => updated, "deleted_at" => deleted_at, @@ -162,7 +190,7 @@ def as_indexed_json(options={}) # def self.query_fields # ['symbol^10', 'name^10', 'contact_name^10', 'contact_email^10', '_all'] # end - + def self.query_aggregations { years: { date_histogram: { field: 'created', interval: 'year', min_doc_count: 1 } }, @@ -211,7 +239,7 @@ def csv def uid symbol.downcase end - + def cache_key "providers/#{uid}-#{updated.iso8601}" end diff --git a/app/serializers/provider_serializer.rb b/app/serializers/provider_serializer.rb index 015c9996c..c8740f31f 100644 --- a/app/serializers/provider_serializer.rb +++ b/app/serializers/provider_serializer.rb @@ -3,9 +3,9 @@ class ProviderSerializer set_key_transform :camel_lower set_type :providers set_id :uid - # cache_options enabled: true, cache_length: 24.hours ### we cannot filter if we cache - - attributes :name, :symbol, :website, :contact_name, :contact_email, :phone, :description, :region, :country, :logo_url, :organization_type, :focus_area, :is_active, :has_password, :joined, :twitter_handle, :billing_information, :ror_id, :created, :updated + # cache_options enabled: true, cache_length: 24.hours ### we cannot filter if we cache + + attributes :name, :symbol, :website, :contact_name, :contact_email, :phone, :description, :region, :country, :logo_url, :organization_type, :focus_area, :is_active, :has_password, :joined, :twitter_handle, :billing_information, :ror_id, :general_contact, :technical_contact, :service_contact, :voting_contact, :created, :updated has_many :clients, record_type: :clients has_many :prefixes, record_type: :prefixes @@ -29,4 +29,21 @@ class ProviderSerializer attribute :twitter_handle, if: Proc.new { |object, params| params[:current_ability] && params[:current_ability].can?(:read_billing_information, object) == true } do |object| object.twitter_handle end + + # Convert all contacts json models back to json style camelCase + attribute :general_contact do |object| + object.general_contact.present? ? object.general_contact.transform_keys!{ |key| key.to_s.camelcase(:lower) } : {} + end + + attribute :technical_contact do |object| + object.technical_contact.present? ? object.technical_contact.transform_keys!{ |key| key.to_s.camelcase(:lower) } : {} + end + + attribute :service_contact do |object| + object.service_contact.present? ? object.service_contact.transform_keys!{ |key| key.to_s.camelcase(:lower) } : {} + end + + attribute :voting_contact do |object| + object.voting_contact.present? ? object.voting_contact.transform_keys!{ |key| key.to_s.camelcase(:lower) } : {} + end end diff --git a/app/validators/contact_validator.rb b/app/validators/contact_validator.rb new file mode 100644 index 000000000..f563b2023 --- /dev/null +++ b/app/validators/contact_validator.rb @@ -0,0 +1,20 @@ +class ContactValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + # Don't try to validate if we have nothing + return unless value.present? + + # Email validation + unless value["email"].present? && value["email"] =~ /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i + record.errors[attribute] << "has an invalid email" + end + + # Name validation + unless value["given_name"].present? + record.errors[attribute] << "has no givenName specified" + end + + unless value["family_name"].present? + record.errors[attribute] << "has no familyName specified" + end + end +end \ No newline at end of file diff --git a/db/migrate/20190517154500_add_extra_provider_contacts.rb b/db/migrate/20190517154500_add_extra_provider_contacts.rb new file mode 100644 index 000000000..e6c1b1579 --- /dev/null +++ b/db/migrate/20190517154500_add_extra_provider_contacts.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddExtraProviderContacts < ActiveRecord::Migration[5.2] + def up + add_column :allocator, :general_contact, :json + add_column :allocator, :technical_contact, :json + add_column :allocator, :service_contact, :json + add_column :allocator, :voting_contact, :json + end + + def down + remove_column :allocator, :general_contact + remove_column :allocator, :technical_contact + remove_column :allocator, :service_contact + remove_column :allocator, :voting_contact + end + end diff --git a/db/schema.rb b/db/schema.rb index 40aa9887c..0291b06ac 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. @@ -12,9 +10,9 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_04_26_090046) do +ActiveRecord::Schema.define(version: 2019_05_17_154500) do - create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", force: :cascade do |t| + create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin", force: :cascade do |t| t.string "name", limit: 191, null: false t.string "record_type", null: false t.bigint "record_id", null: false @@ -24,7 +22,7 @@ t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true end - create_table "active_storage_blobs", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", force: :cascade do |t| + create_table "active_storage_blobs", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin", force: :cascade do |t| t.string "key", limit: 191, null: false t.string "filename", limit: 191, null: false t.string "content_type", limit: 191 @@ -35,7 +33,7 @@ t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true end - create_table "allocator", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "allocator", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.string "contact_email", null: false t.string "contact_name", limit: 80, null: false t.datetime "created" @@ -63,11 +61,15 @@ t.json "billing_information" t.string "twitter_handle", limit: 20 t.string "ror_id" + t.json "general_contact" + t.json "technical_contact" + t.json "service_contact" + t.json "voting_contact" t.index ["organization_type"], name: "index_allocator_organization_type" t.index ["symbol"], name: "symbol", unique: true end - create_table "allocator_prefixes", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "allocator_prefixes", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.bigint "allocator", null: false t.bigint "prefixes", null: false t.datetime "created_at" @@ -77,7 +79,7 @@ t.index ["prefixes"], name: "FKE7FBD674AF86A1C7" end - create_table "audits", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", force: :cascade do |t| + create_table "audits", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin", force: :cascade do |t| t.integer "auditable_id" t.string "auditable_type" t.integer "associated_id" @@ -99,7 +101,18 @@ t.index ["user_id", "user_type"], name: "user_index" end - create_table "datacentre", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "contacts", options: "ENGINE=InnoDB DEFAULT CHARSET=latin1", force: :cascade do |t| + t.bigint "allocator" + t.string "email" + t.string "given_name" + t.string "family_name" + t.string "role" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["allocator"], name: "fk_rails_5c598567a8" + end + + create_table "datacentre", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.text "comments", limit: 4294967295 t.string "contact_email", null: false t.string "contact_name", limit: 80, null: false @@ -127,7 +140,7 @@ t.index ["url"], name: "index_datacentre_on_url", length: 100 end - create_table "datacentre_prefixes", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "datacentre_prefixes", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.bigint "datacentre", null: false t.bigint "prefixes", null: false t.datetime "created_at" @@ -139,7 +152,7 @@ t.index ["prefixes"], name: "FK13A1B3BAAF86A1C7" end - create_table "dataset", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "dataset", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.datetime "created" t.string "doi", null: false t.binary "is_active", limit: 1, null: false @@ -194,7 +207,7 @@ t.index ["url"], name: "index_dataset_on_url", length: 100 end - create_table "media", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "media", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.datetime "created" t.string "media_type", limit: 80 t.datetime "updated" @@ -206,7 +219,7 @@ t.index ["url"], name: "index_media_on_url", length: 100 end - create_table "metadata", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "metadata", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.datetime "created" t.integer "metadata_version" t.integer "version" @@ -218,7 +231,7 @@ t.index ["dataset"], name: "FKE52D7B2F4D3D6B1B" end - create_table "prefix", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "prefix", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.datetime "created" t.string "prefix", limit: 80, null: false t.integer "version" @@ -228,6 +241,7 @@ add_foreign_key "allocator_prefixes", "allocator", column: "allocator", name: "FKE7FBD67446EBD781" add_foreign_key "allocator_prefixes", "prefix", column: "prefixes", name: "FKE7FBD674AF86A1C7" + add_foreign_key "contacts", "allocator", column: "allocator" add_foreign_key "datacentre", "allocator", column: "allocator", name: "FK6695D60546EBD781" add_foreign_key "datacentre_prefixes", "datacentre", column: "datacentre", name: "FK13A1B3BA47B5F5FF" add_foreign_key "datacentre_prefixes", "prefix", column: "prefixes", name: "FK13A1B3BAAF86A1C7" diff --git a/spec/factories/default.rb b/spec/factories/default.rb index 4cab66665..478df557a 100644 --- a/spec/factories/default.rb +++ b/spec/factories/default.rb @@ -214,6 +214,26 @@ "address": Faker::Address.street_address, "postCode": "10777" }} + general_contact {{ + "email": "richard@example.com", + "given_name": "Richard", + "family_name": "Hallett" + }} + technical_contact {{ + "email": "kristian@example.com", + "given_name": "Kristian", + "family_name": "Garza" + }} + service_contact {{ + "email": "martin@example.com", + "given_name": "Martin", + "family_name": "Fenner" + }} + voting_contact {{ + "email": "robin@example.com", + "given_name": "Robin", + "family_name": "Dasler" + }} is_active { true } initialize_with { Provider.where(symbol: symbol).first_or_initialize } diff --git a/spec/models/provider_spec.rb b/spec/models/provider_spec.rb index d32655be6..6caeefd88 100644 --- a/spec/models/provider_spec.rb +++ b/spec/models/provider_spec.rb @@ -9,6 +9,9 @@ it { should validate_presence_of(:contact_email) } it { should validate_presence_of(:contact_name) } it { is_expected.to strip_attribute(:name) } + it { + expect(provider).to be_valid + } end describe "admin" do diff --git a/spec/requests/providers_spec.rb b/spec/requests/providers_spec.rb index 0c9c8af71..144e8677d 100644 --- a/spec/requests/providers_spec.rb +++ b/spec/requests/providers_spec.rb @@ -34,7 +34,7 @@ describe 'GET /providers/:id' do before { get "/providers/#{provider.symbol}" , headers: headers} - + context 'when the record exists' do it 'returns the provider' do expect(json).not_to be_empty @@ -86,8 +86,8 @@ "country" => "GB" } } } end - before do - post '/providers', params: params.to_json, headers: headers + before do + post '/providers', params: params.to_json, headers: headers end it 'creates a provider' do @@ -111,7 +111,7 @@ sleep 1 get "/providers/#{providers.first.symbol}", headers: headers_last end - + it 'has no permission' do expect(json["data"].dig('attributes', 'symbol')).to eq(providers.first.symbol) @@ -119,7 +119,7 @@ expect(json["data"].dig( 'attributes', 'twitterHandle')).to eq(nil) end end - + context 'request is valid with billing information' do let(:params) do @@ -158,8 +158,8 @@ } end - before do - post '/providers', params: params.to_json, headers: headers + before do + post '/providers', params: params.to_json, headers: headers end it 'creates a provider' do @@ -176,6 +176,82 @@ end end + context 'request is valid with contact information' do + let(:params) do + { + "data"=>{ + "type"=>"providers", + "attributes"=>{ + "contactEmail"=>"jkiritha@andrew.cmu.edu", + "contactName"=>"Jonathan Kiritharan", + "country"=>"US", + "created"=>"", + "description"=>"", + "focusArea"=>"general", + "hasPassword"=>"[FILTERED]", + "isActive"=>true, + "joined"=>"", + "keepPassword"=>"[FILTERED]", + "logoUrl"=>"", + "name"=>"Carnegie Mellon University", + "organizationType"=>"academicInstitution", + "passwordInput"=>"[FILTERED]", + "phone"=>"", + "twitterHandle"=>"meekakitty", + "rorId"=>"https://ror.org/05njkjr15", + "generalContact":{ + "email"=>"richard@example.com", + "givenName"=>"Richard", + "familyName"=>"Hallett" + }, + "technicalContact": { + "email": "kristian@example.com", + "givenName": "Kristian", + "familyName": "Garza" + }, + "serviceContact": { + "email": "martin@example.com", + "givenName": "Martin", + "familyName": "Fenner" + }, + "votingContact": { + "email": "robin@example.com", + "givenName": "Robin", + "familyName": "Dasler" + }, + "region"=>"", + "symbol"=>"CMfddff33333dd111d111113f4d", + "updated"=>"", + "website"=>"" + } + } + } + end + + before do + post '/providers', params: params.to_json, headers: headers + end + + it 'creates a provider' do + expect(json.dig('data', 'attributes', 'generalContact',"email")).to eq("richard@example.com") + expect(json.dig('data', 'attributes', 'generalContact',"givenName")).to eq("Richard") + expect(json.dig('data', 'attributes', 'generalContact',"familyName")).to eq("Hallett") + 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") + expect(json.dig('data', 'attributes', 'serviceContact',"email")).to eq("martin@example.com") + expect(json.dig('data', 'attributes', 'serviceContact',"givenName")).to eq("Martin") + expect(json.dig('data', 'attributes', 'serviceContact',"familyName")).to eq("Fenner") + expect(json.dig('data', 'attributes', 'votingContact',"email")).to eq("robin@example.com") + 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 + expect(response).to have_http_status(200) + end + end + context 'request for admin provider with meta' do let(:params) do {