From ac23a85f1bad146e66dc397e30c00754d2824c69 Mon Sep 17 00:00:00 2001 From: Cody Ross Date: Thu, 9 Feb 2023 16:28:05 -0500 Subject: [PATCH 1/8] Added igsnCatalog to client_type validation --- app/models/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/client.rb b/app/models/client.rb index c0d12d1b9..5e1452e8a 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -76,7 +76,7 @@ class Client < ApplicationRecord in: %w[ROLE_DATACENTRE], message: "Role %s is not included in the list" validates_inclusion_of :client_type, - in: %w[repository periodical], + in: %w[repository periodical igsnCatalog], message: "Client type %s is not included in the list" validates_associated :provider validate :check_id, on: :create From e8948848c6ca258b68c6ccd3d2f3cafc4cd8efc1 Mon Sep 17 00:00:00 2001 From: Cody Ross Date: Wed, 15 Feb 2023 12:18:27 -0500 Subject: [PATCH 2/8] Adds support for custom clientType titles in serializer aggregations. --- app/controllers/clients_controller.rb | 2 +- app/controllers/concerns/facetable.rb | 16 ++++++++++++++++ app/controllers/repositories_controller.rb | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index 73bb5e23d..60ca43eb5 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -67,7 +67,7 @@ def index end client_types = if total.positive? - facet_by_key(response.aggregations.client_types.buckets) + facet_by_client_type(response.aggregations.client_types.buckets) end certificates = if total.positive? diff --git a/app/controllers/concerns/facetable.rb b/app/controllers/concerns/facetable.rb index 990f71629..ae61a55ce 100644 --- a/app/controllers/concerns/facetable.rb +++ b/app/controllers/concerns/facetable.rb @@ -73,6 +73,12 @@ module Facetable LOWER_BOUND_YEAR = 2_010 + CLIENT_TYPES = { + "repository" => "Repository", + "periodical" => "Periodical", + "igsnCatalog" => "IGSN ID Catalog" + }.freeze + included do def facet_by_key_as_string(arr) arr.map do |hsh| @@ -201,6 +207,16 @@ def facet_by_bool(arr) end end + def facet_by_client_type(arr) + arr.map do |hsh| + { + "id" => hsh["key"], + "title" => CLIENT_TYPES[hsh["key"]] || hsh["key"], + "count" => hsh["doc_count"], + } + end + end + def facet_by_software(arr) arr.map do |hsh| { diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index 3f53b7329..35b8309dc 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -74,7 +74,7 @@ def index end client_types = if total.positive? - facet_by_key(response.aggregations.client_types.buckets) + facet_by_client_type(response.aggregations.client_types.buckets) end repository_types = if total.positive? From 8cffdf92df345d2183a7c7135c854cd60799cef6 Mon Sep 17 00:00:00 2001 From: Cody Ross Date: Wed, 15 Feb 2023 14:49:49 -0500 Subject: [PATCH 3/8] Adds support for IGSN reporting aggregations and client-type parameter. --- app/controllers/datacite_dois_controller.rb | 1 + app/models/doi.rb | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/app/controllers/datacite_dois_controller.rb b/app/controllers/datacite_dois_controller.rb index 3d09dc1ce..853198ff3 100644 --- a/app/controllers/datacite_dois_controller.rb +++ b/app/controllers/datacite_dois_controller.rb @@ -133,6 +133,7 @@ def index page: page, sort: sort, random: params[:random], + client_type: params[:client_type], ) end diff --git a/app/models/doi.rb b/app/models/doi.rb index e5c0cb729..96363b804 100644 --- a/app/models/doi.rb +++ b/app/models/doi.rb @@ -769,6 +769,13 @@ def self.export_sub_aggregations } end + def self.igsn_id_catalog_aggregations + { + created: { date_histogram: { field: "created", interval: "month", format: "yyyy-MM", order: { _key: "desc" }, min_doc_count: 1 }, + aggs: { bucket_truncate: { bucket_sort: { size: 10 } } } } + } + end + def self.query_fields ["uid^50", "related_identifiers.relatedIdentifier^3", "titles.title^3", "creator_names^3", "creators.id^3", "publisher^3", "descriptions.description^3", "subjects.subject^3"] end @@ -1028,6 +1035,8 @@ def self.query(query, options = {}) client_export_aggregations elsif options[:totals_agg] == "prefix" prefix_aggregations + elsif options[:client_type] == "igsnCatalog" + query_aggregations(disable_facets: options[:disable_facets]).merge(self.igsn_id_catalog_aggregations) else query_aggregations(disable_facets: options[:disable_facets]) end @@ -1133,6 +1142,8 @@ def self.query(query, options = {}) filter << { terms: { "client.certificate" => options[:certificate].split(",") } } if options[:certificate].present? filter << { term: { "creators.nameIdentifiers.nameIdentifier" => "https://orcid.org/#{orcid_from_url(options[:user_id])}" } } if options[:user_id].present? filter << { term: { "creators.nameIdentifiers.nameIdentifierScheme" => "ORCID" } } if options[:has_person].present? + filter << { term: { "client.client_type" => options[:client_type] } } if options[:client_type] + filter << { term: { "types.resourceTypeGeneral" => "PhysicalObject" } } if options[:client_type] == "igsnCatalog" # match either one of has_affiliation, has_organization, or has_funder if options[:has_organization].present? From fa3be8825cb633a7ca01753384a702190ed3535a Mon Sep 17 00:00:00 2001 From: Cody Ross Date: Wed, 22 Feb 2023 11:46:12 -0500 Subject: [PATCH 4/8] Adds tests for igsnCatalog clients and client-type filters --- spec/models/client_spec.rb | 6 +++++ spec/requests/clients_spec.rb | 37 +++++++++++++++++++++++++++++ spec/requests/datacite_dois_spec.rb | 28 ++++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index 4f26310ee..921389037 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -337,6 +337,12 @@ expect(client.errors.details).to be_empty end + it "igsnCatalog" do + client.client_type = "igsnCatalog" + expect(client.save).to be true + expect(client.errors.details).to be_empty + end + it "unsupported" do client.client_type = "conference" expect(client.save).to be false diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index d500c2649..b4c27c57f 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -137,6 +137,25 @@ describe "POST /clients" do context "when the request is valid" do + let(:params_igsn_catalog) do + { + "data" => { + "type" => "clients", + "attributes" => { + "symbol" => provider.symbol + ".IGSN", + "name" => "Imperial College", + "contactEmail" => "bob@example.com", + "clientType" => "igsnCatalog", + }, + "relationships": { + "provider": { + "data": { "type": "providers", "id": provider.uid }, + }, + }, + }, + } + end + it "creates a client" do post "/clients", params, headers @@ -161,6 +180,24 @@ [{ "count" => 2, "id" => "repository", "title" => "Repository" }], ) end + + it "creates a client with igsnCatalog client_type" do + post "/clients", params_igsn_catalog, headers + + expect(last_response.status).to eq(201) + attributes = json.dig("data", "attributes") + expect(attributes["clientType"]).to eq("igsnCatalog") + + Client.import + sleep 2 + + get "/clients", nil, headers + + expect(json["data"].size).to eq(2) + expect(json.dig("meta", "clientTypes").find { |clientTypeAgg| clientTypeAgg["id"] == "igsnCatalog" }).to eq( + { "count" => 1, "id" => "igsnCatalog", "title" => "IGSN ID Catalog" }, + ) + end end context "when the request is invalid" do diff --git a/spec/requests/datacite_dois_spec.rb b/spec/requests/datacite_dois_spec.rb index de19e45c0..6bf64c7b8 100755 --- a/spec/requests/datacite_dois_spec.rb +++ b/spec/requests/datacite_dois_spec.rb @@ -225,6 +225,34 @@ end end + describe "GET /dois with filter", elasticsearch: true do + let!(:dois) { create_list(:doi, 10, client: client, aasm_state: "findable", version_info: "testtag") } + let(:client_igsn_id_catalog) { create(:client, provider: provider, client_type: "igsnCatalog") } + let!(:doi_igsn_id) { create(:doi, client: client_igsn_id_catalog, aasm_state: "findable", types: { "resourceTypeGeneral": "PhysicalObject" } ) } + let!(:dois_other) { create_list(:doi, 5, client: client_igsn_id_catalog, aasm_state: "findable", types: { "resourceTypeGeneral": "Dataset" } ) } + + before do + DataciteDoi.import + sleep 2 + end + + it "filters by client_type when client-type is set", vcr: true do + get "/dois?client-type=repository", nil, headers + + expect(last_response.status).to eq(200) + expect(json["data"].size).to eq(10) + end + + it "returns custom created attribute and only DOIs with resourceTypeGeneral=PhysicalObject and client_type=igsnCatalog when client-type is set to igsnCatalog", vcr: true do + get "/dois?client-type=igsnCatalog", nil, headers + + expect(last_response.status).to eq(200) + expect(json["data"].size).to eq(1) + expect(json.dig("data", 0, "id")).to eq(doi_igsn_id.uid) + expect(json.dig("meta", "created", 0, "title")).to eq(doi_igsn_id.created.to_time.strftime('%Y-%m')) + end + end + describe "GET /dois with query", elasticsearch: true do let!(:doi) do create(:doi, client: client, aasm_state: "findable", creators: From 6bd6bd1db4b1a93a68db4edb4cbabb54404e54fd Mon Sep 17 00:00:00 2001 From: Cody Ross Date: Wed, 22 Feb 2023 12:17:29 -0500 Subject: [PATCH 5/8] Linting --- spec/requests/clients_spec.rb | 2 +- spec/requests/datacite_dois_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index b4c27c57f..c5e69a340 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -155,7 +155,7 @@ }, } end - + it "creates a client" do post "/clients", params, headers diff --git a/spec/requests/datacite_dois_spec.rb b/spec/requests/datacite_dois_spec.rb index 6bf64c7b8..201829ef5 100755 --- a/spec/requests/datacite_dois_spec.rb +++ b/spec/requests/datacite_dois_spec.rb @@ -228,8 +228,8 @@ describe "GET /dois with filter", elasticsearch: true do let!(:dois) { create_list(:doi, 10, client: client, aasm_state: "findable", version_info: "testtag") } let(:client_igsn_id_catalog) { create(:client, provider: provider, client_type: "igsnCatalog") } - let!(:doi_igsn_id) { create(:doi, client: client_igsn_id_catalog, aasm_state: "findable", types: { "resourceTypeGeneral": "PhysicalObject" } ) } - let!(:dois_other) { create_list(:doi, 5, client: client_igsn_id_catalog, aasm_state: "findable", types: { "resourceTypeGeneral": "Dataset" } ) } + let!(:doi_igsn_id) { create(:doi, client: client_igsn_id_catalog, aasm_state: "findable", types: { "resourceTypeGeneral": "PhysicalObject" }) } + let!(:dois_other) { create_list(:doi, 5, client: client_igsn_id_catalog, aasm_state: "findable", types: { "resourceTypeGeneral": "Dataset" }) } before do DataciteDoi.import @@ -249,7 +249,7 @@ expect(last_response.status).to eq(200) expect(json["data"].size).to eq(1) expect(json.dig("data", 0, "id")).to eq(doi_igsn_id.uid) - expect(json.dig("meta", "created", 0, "title")).to eq(doi_igsn_id.created.to_time.strftime('%Y-%m')) + expect(json.dig("meta", "created", 0, "title")).to eq(doi_igsn_id.created.to_time.strftime("%Y-%m")) end end From d2b40eaa7eb878819647d2aa256f81524ac71b74 Mon Sep 17 00:00:00 2001 From: Cody Ross Date: Wed, 22 Feb 2023 14:30:33 -0500 Subject: [PATCH 6/8] Changes to prefix tests to accommodate the addition of clients in other tests --- spec/models/prefix_spec.rb | 4 ++++ spec/rails_helper.rb | 5 +++-- spec/requests/provider_prefixes_spec.rb | 23 +++++++++++++++++------ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/spec/models/prefix_spec.rb b/spec/models/prefix_spec.rb index e3123d5f3..a34918333 100644 --- a/spec/models/prefix_spec.rb +++ b/spec/models/prefix_spec.rb @@ -6,6 +6,10 @@ let!(:prefixes) { create_list(:prefix, 10) } let!(:prefix) { prefixes.first } + after(:each) do + Prefix.destroy_all + end + describe "Validations" do it { should validate_presence_of(:uid) } end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index e22f9865f..75d669571 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -76,12 +76,13 @@ end config.before(:each) do |example| - prefix_pool_size = example.metadata[:prefix_pool_size].present? ? example.metadata[:prefix_pool_size] : ENV["PREFIX_POOL_SIZE"].to_i - if prefix_pool_size < 0 + prefix_pool_size = example.metadata[:prefix_pool_size].present? ? example.metadata[:prefix_pool_size].to_i : ENV["PREFIX_POOL_SIZE"].to_i + if prefix_pool_size <= 0 @prefix_pool = [] else @prefix_pool = create_list(:prefix, prefix_pool_size) end + Prefix.import end end diff --git a/spec/requests/provider_prefixes_spec.rb b/spec/requests/provider_prefixes_spec.rb index db6a9fad5..a3e46ccac 100644 --- a/spec/requests/provider_prefixes_spec.rb +++ b/spec/requests/provider_prefixes_spec.rb @@ -14,10 +14,19 @@ end let(:prefix) { create(:prefix) } let!(:provider_prefixes) do - create_list(:provider_prefix, 3, provider: provider) + [ + create(:provider_prefix, provider: provider, prefix: @prefix_pool[0]), + create(:provider_prefix, provider: provider, prefix: @prefix_pool[1]), + create(:provider_prefix, provider: provider, prefix: @prefix_pool[2]) + ] end - let!(:provider_prefixes2) { create_list(:provider_prefix, 2) } - let(:provider_prefix) { create(:provider_prefix) } + let!(:provider_prefixes2) do + [ + create(:provider_prefix, prefix: @prefix_pool[3]), + create(:provider_prefix, prefix: @prefix_pool[4]) + ] + end + let(:provider_prefix) { create(:provider_prefix, prefix: @prefix_pool[5]) } let(:bearer) { User.generate_token(role_id: "staff_admin") } let(:headers) do { @@ -26,7 +35,7 @@ } end - before do + before(:each) do ProviderPrefix.import Prefix.import Provider.import @@ -191,19 +200,21 @@ get "/prefixes?state=unassigned", nil, headers expect(last_response.status).to eq(200) - expect(json.dig("meta", "total")).to eq(@prefix_pool.length) + expect(json.dig("meta", "total")).to eq(@prefix_pool.length - 5) delete "/provider-prefixes/#{provider_prefix.uid}", nil, headers expect(last_response.status).to eq(204) + Prefix.import + ProviderPrefix.import sleep 2 get "/prefixes?state=unassigned", nil, headers expect(last_response.status).to eq(200) - expect(json.dig("meta", "total")).to eq(@prefix_pool.length + 1) + expect(json.dig("meta", "total")).to eq(@prefix_pool.length - 5) end end From ed51c88e4a4ea62c872d7c9363bea977205b7572 Mon Sep 17 00:00:00 2001 From: Cody Ross Date: Thu, 23 Feb 2023 11:32:45 -0500 Subject: [PATCH 7/8] Makes createdByMonth aggregation an additional attribute instead of replacing the created attribute when querying client-type=igsnCatalog --- app/controllers/datacite_dois_controller.rb | 2 ++ app/models/doi.rb | 2 +- spec/requests/datacite_dois_spec.rb | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/datacite_dois_controller.rb b/app/controllers/datacite_dois_controller.rb index 853198ff3..69fd2822c 100644 --- a/app/controllers/datacite_dois_controller.rb +++ b/app/controllers/datacite_dois_controller.rb @@ -220,6 +220,7 @@ def index resource_types = facet_by_combined_key(response.aggregations.resource_types.buckets) published = facet_by_range(response.aggregations.published.buckets) created = facet_by_key_as_string(response.aggregations.created.buckets) + created_by_month = response.aggregations.created_by_month ? facet_by_key_as_string(response.aggregations.created_by_month.buckets) : nil registered = facet_by_key_as_string(response.aggregations.registered.buckets) providers = facet_by_combined_key(response.aggregations.providers.buckets) clients = facet_by_combined_key(response.aggregations.clients.buckets) @@ -279,6 +280,7 @@ def index states: states, "resourceTypes" => resource_types, created: created, + createdByMonth: created_by_month, published: published, registered: registered, providers: providers, diff --git a/app/models/doi.rb b/app/models/doi.rb index 96363b804..60c136feb 100644 --- a/app/models/doi.rb +++ b/app/models/doi.rb @@ -771,7 +771,7 @@ def self.export_sub_aggregations def self.igsn_id_catalog_aggregations { - created: { date_histogram: { field: "created", interval: "month", format: "yyyy-MM", order: { _key: "desc" }, min_doc_count: 1 }, + created_by_month: { date_histogram: { field: "created", interval: "month", format: "yyyy-MM", order: { _key: "desc" }, min_doc_count: 1 }, aggs: { bucket_truncate: { bucket_sort: { size: 10 } } } } } end diff --git a/spec/requests/datacite_dois_spec.rb b/spec/requests/datacite_dois_spec.rb index 201829ef5..4e71d4bed 100755 --- a/spec/requests/datacite_dois_spec.rb +++ b/spec/requests/datacite_dois_spec.rb @@ -243,13 +243,13 @@ expect(json["data"].size).to eq(10) end - it "returns custom created attribute and only DOIs with resourceTypeGeneral=PhysicalObject and client_type=igsnCatalog when client-type is set to igsnCatalog", vcr: true do + it "returns additional createdByMonth meta attribute and only DOIs with resourceTypeGeneral=PhysicalObject and client_type=igsnCatalog when client-type is set to igsnCatalog", vcr: true do get "/dois?client-type=igsnCatalog", nil, headers expect(last_response.status).to eq(200) expect(json["data"].size).to eq(1) expect(json.dig("data", 0, "id")).to eq(doi_igsn_id.uid) - expect(json.dig("meta", "created", 0, "title")).to eq(doi_igsn_id.created.to_time.strftime("%Y-%m")) + expect(json.dig("meta", "createdByMonth", 0, "title")).to eq(doi_igsn_id.created.to_time.strftime("%Y-%m")) end end From 78c7897027fbb79642934b9a9e5c58bae42d8bf8 Mon Sep 17 00:00:00 2001 From: Suzanne Vogt Date: Fri, 3 Mar 2023 11:07:52 -0500 Subject: [PATCH 8/8] Try to remove flakiness. --- spec/requests/provider_prefixes_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/requests/provider_prefixes_spec.rb b/spec/requests/provider_prefixes_spec.rb index a3e46ccac..cb1cb266e 100644 --- a/spec/requests/provider_prefixes_spec.rb +++ b/spec/requests/provider_prefixes_spec.rb @@ -3,8 +3,8 @@ require "rails_helper" describe ProviderPrefixesController, type: :request, elasticsearch: true do - let(:consortium) { create(:provider, role_name: "ROLE_CONSORTIUM") } - let(:provider) do + let!(:consortium) { create(:provider, role_name: "ROLE_CONSORTIUM") } + let!(:provider) do create( :provider, consortium: consortium, @@ -12,7 +12,7 @@ password_input: "12345", ) end - let(:prefix) { create(:prefix) } + let!(:prefix) { create(:prefix) } let!(:provider_prefixes) do [ create(:provider_prefix, provider: provider, prefix: @prefix_pool[0]), @@ -26,9 +26,9 @@ create(:provider_prefix, prefix: @prefix_pool[4]) ] end - let(:provider_prefix) { create(:provider_prefix, prefix: @prefix_pool[5]) } - let(:bearer) { User.generate_token(role_id: "staff_admin") } - let(:headers) do + let!(:provider_prefix) { create(:provider_prefix, prefix: @prefix_pool[5]) } + let!(:bearer) { User.generate_token(role_id: "staff_admin") } + let!(:headers) do { "HTTP_ACCEPT" => "application/vnd.api+json", "HTTP_AUTHORIZATION" => "Bearer " + bearer, @@ -111,7 +111,7 @@ get "/provider-prefixes?query=10.508", nil, headers expect(last_response.status).to eq(200) - expect(json["data"].size).to eq(5) + expect(json["data"].size).to eq(6) end end @@ -120,7 +120,7 @@ get "/provider-prefixes", nil, headers expect(last_response.status).to eq(200) - expect(json["data"].size).to eq(5) + expect(json["data"].size).to eq(6) end it "returns correct paging links" do @@ -200,7 +200,7 @@ get "/prefixes?state=unassigned", nil, headers expect(last_response.status).to eq(200) - expect(json.dig("meta", "total")).to eq(@prefix_pool.length - 5) + expect(json.dig("meta", "total")).to eq(@prefix_pool.length - 6) delete "/provider-prefixes/#{provider_prefix.uid}", nil, headers