From 29e4622eba4166f6f437fc526a98f17e463c10c1 Mon Sep 17 00:00:00 2001 From: Martin Fenner Date: Sat, 27 Jul 2019 06:17:31 +0200 Subject: [PATCH] categorize client types. datacite/datacite#796 --- app/controllers/clients_controller.rb | 11 +++++---- app/models/client.rb | 6 ++++- app/models/concerns/indexable.rb | 1 + app/serializers/client_serializer.rb | 2 +- .../20190727035047_add_client_type_column.rb | 5 ++++ db/schema.rb | 3 ++- spec/requests/clients_spec.rb | 23 ++++--------------- 7 files changed, 26 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20190727035047_add_client_type_column.rb diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index 951439357..f8bd3e493 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -30,6 +30,7 @@ def index provider_id: params[:provider_id], repository_id: params[:repository_id], software: params[:software], + client_type: params[:client_type], page: page, sort: sort) end @@ -40,7 +41,8 @@ def index years = total > 0 ? facet_by_year(response.response.aggregations.years.buckets) : nil providers = total > 0 ? facet_by_provider(response.response.aggregations.providers.buckets) : nil software = total > 0 ? facet_by_software(response.response.aggregations.software.buckets) : nil - + client_types = total > 0 ? facet_by_key(response.response.aggregations.client_types.buckets) : nil + @clients = response.results options = {} @@ -50,7 +52,8 @@ def index page: page[:number], years: years, providers: providers, - software: software + software: software, + "clientTypes" => client_types }.compact options[:links] = { @@ -196,8 +199,8 @@ def set_client def safe_params fail JSON::ParserError, "You need to provide a payload following the JSONAPI spec" unless params[:data].present? ActiveModelSerializers::Deserialization.jsonapi_parse!( - params, only: [:symbol, :name, "contactName", "contactEmail", :domains, :provider, :url, :repository, :description, :software, "targetId", "isActive", "passwordInput"], - keys: { "contactName" => :contact_name, "contactEmail" => :contact_email, "targetId" => :target_id, "isActive" => :is_active, "passwordInput" => :password_input } + params, only: [:symbol, :name, "contactName", "contactEmail", :domains, :provider, :url, :repository, :description, :software, "targetId", "isActive", "passwordInput", "clientType"], + keys: { "contactName" => :contact_name, "contactEmail" => :contact_email, "targetId" => :target_id, "isActive" => :is_active, "passwordInput" => :password_input, "clientType" => :client_type } ) end end diff --git a/app/models/client.rb b/app/models/client.rb index a2d2e7f23..6d36e4efc 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -35,6 +35,7 @@ class Client < ActiveRecord::Base validates_uniqueness_of :symbol, message: "This Client ID has already been taken" validates_format_of :contact_email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i validates_inclusion_of :role_name, :in => %w( ROLE_DATACENTRE ), :message => "Role %s is not included in the list" + validates_inclusion_of :client_type, :in => %w( repository serial other ), :message => "Client type %s is not included in the list", if: :client_type? validates_associated :provider validate :check_id, :on => :create validate :freeze_symbol, :on => :update @@ -87,6 +88,7 @@ class Client < ActiveRecord::Base indexes :url, type: :text, fields: { keyword: { type: "keyword" }} indexes :software, type: :text, fields: { keyword: { type: "keyword" }, raw: { type: "text", analyzer: "string_lowercase", "fielddata": true }} indexes :cache_key, type: :keyword + indexes :client_type, type: :keyword indexes :created, type: :date indexes :updated, type: :date indexes :deleted_at, type: :date @@ -117,6 +119,7 @@ def as_indexed_json(options={}) "is_active" => is_active, "password" => password, "cache_key" => cache_key, + "client_type" => client_type, "created" => created, "updated" => updated, "deleted_at" => deleted_at, @@ -135,7 +138,8 @@ def self.query_aggregations years: { date_histogram: { field: 'created', interval: 'year', min_doc_count: 1 } }, cumulative_years: { terms: { field: 'cumulative_years', min_doc_count: 1, order: { _count: "asc" } } }, providers: { terms: { field: 'provider_id', size: 15, min_doc_count: 1 } }, - software: { terms: { field: 'software.keyword', size: 15, min_doc_count: 1 } } + software: { terms: { field: 'software.keyword', size: 15, min_doc_count: 1 } }, + client_types: { terms: { field: 'client_type', size: 15, min_doc_count: 1 } } } end diff --git a/app/models/concerns/indexable.rb b/app/models/concerns/indexable.rb index e77cc3e63..d858af55c 100644 --- a/app/models/concerns/indexable.rb +++ b/app/models/concerns/indexable.rb @@ -182,6 +182,7 @@ def query(query, options={}) must << { range: { created: { gte: "#{options[:year].split(",").min}||/y", lte: "#{options[:year].split(",").max}||/y", format: "yyyy" }}} if options[:year].present? must << { terms: { "software.raw" => options[:software].split(",") }} if options[:software].present? must << { term: { repository_id: options[:repository_id].gsub("/", '\/') }} if options[:repository_id].present? + must << { term: { client_type: options[:client_type] }} if options[:client_type].present? must_not << { exists: { field: "deleted_at" }} unless options[:include_deleted] must_not << { terms: { provider_id: ["crossref", "medra", "op"] }} if options[:exclude_registration_agencies] elsif self.name == "Doi" diff --git a/app/serializers/client_serializer.rb b/app/serializers/client_serializer.rb index 47e32759e..ee057b499 100644 --- a/app/serializers/client_serializer.rb +++ b/app/serializers/client_serializer.rb @@ -5,7 +5,7 @@ class ClientSerializer set_id :uid cache_options enabled: true, cache_length: 24.hours - attributes :name, :symbol, :year, :contact_name, :contact_email, :description, :domains, :url, :created, :updated + attributes :name, :symbol, :year, :contact_name, :contact_email, :description, :client_type, :domains, :url, :created, :updated belongs_to :provider, record_type: :providers belongs_to :repository, record_type: :repositories, if: Proc.new { |client| client.repository_id } diff --git a/db/migrate/20190727035047_add_client_type_column.rb b/db/migrate/20190727035047_add_client_type_column.rb new file mode 100644 index 000000000..6769151fd --- /dev/null +++ b/db/migrate/20190727035047_add_client_type_column.rb @@ -0,0 +1,5 @@ +class AddClientTypeColumn < ActiveRecord::Migration[5.2] + def change + add_column :datacentre, :client_type, :string, limit: 191 + end +end diff --git a/db/schema.rb b/db/schema.rb index 81d20db7c..79da5c8f2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_07_23_114539) do +ActiveRecord::Schema.define(version: 2019_07_27_035047) do create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=latin1", force: :cascade do |t| t.string "name", limit: 191, null: false @@ -127,6 +127,7 @@ t.text "url" t.string "software", limit: 191 t.text "description" + t.string "client_type", limit: 191 t.index ["allocator"], name: "FK6695D60546EBD781" t.index ["re3data"], name: "index_datacentre_on_re3data" t.index ["symbol"], name: "symbol", unique: true diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index ab21a7763..9726b51e4 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -11,7 +11,8 @@ "symbol" => provider.symbol + ".IMPERIAL", "name" => "Imperial College", "contactName" => "Madonna", - "contactEmail" => "bob@example.com" + "contactEmail" => "bob@example.com", + "clientType" => "repository" }, "relationships": { "provider": { @@ -82,13 +83,8 @@ it 'returns the client' do get "/clients/#{client.uid}", nil, headers - expect(json.dig('data', 'attributes', 'name')).to eq(client.name) - end - - it 'returns status code 200' do - get "/clients/#{client.uid}", nil, headers - expect(last_response.status).to eq(200) + expect(json.dig('data', 'attributes', 'name')).to eq(client.name) end end @@ -97,11 +93,6 @@ get "/clients/xxx", nil, headers expect(last_response.status).to eq(404) - end - - it 'returns a not found message' do - get "/clients/xxx", nil, headers - expect(json["errors"].first).to eq("status"=>"404", "title"=>"The resource you are looking for doesn't exist.") end end @@ -112,19 +103,15 @@ it 'creates a client' do post '/clients', params, headers + expect(last_response.status).to eq(201) attributes = json.dig('data', 'attributes') expect(attributes["name"]).to eq("Imperial College") expect(attributes["contactName"]).to eq("Madonna") + expect(attributes["clientType"]).to eq("repository") relationships = json.dig('data', 'relationships') expect(relationships.dig("provider", "data", "id")).to eq(provider.symbol.downcase) end - - it 'returns status code 201' do - post '/clients', params, headers - - expect(last_response.status).to eq(201) - end end context 'when the request is invalid' do