From 7c48a35071f3c1b85eda561e79c843ec9804fef8 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 22 Apr 2020 14:50:57 +0200 Subject: [PATCH 01/28] data generation for new type of Ids --- .../development/consortium_transfer.seeds.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 db/seeds/development/consortium_transfer.seeds.rb diff --git a/db/seeds/development/consortium_transfer.seeds.rb b/db/seeds/development/consortium_transfer.seeds.rb new file mode 100644 index 000000000..d091a585d --- /dev/null +++ b/db/seeds/development/consortium_transfer.seeds.rb @@ -0,0 +1,31 @@ +require "factory_bot_rails" + +fail "Seed tasks can only be used in the development enviroment" if Rails.env.production? + +after "development:base" do + +## ranks +# Lieutenant of Supplies +# Master of Ships +# Commander of the Law +# General of Warlords +# Supreme Master +# Supreme General +# Wing Commander +# Lawbringer +# Lord General +# Paladin + + provider = Provider.where(symbol: "GENERAL").first || FactoryBot.create(:provider, symbol: "GENERAL") + client = Client.where(symbol: "PALADIN").first || FactoryBot.create(:client, provider: provider, symbol: "PALADIN", password: ENV["MDS_PASSWORD"]) + if Prefix.where(uid: "10.14456").blank? + prefix = FactoryBot.create(:prefix, uid: "10.14456") + # FactoryBot.create(:client_prefix, client_id: client.id, prefix_id: prefix.id) + end + dois = FactoryBot.create_list(:doi, 10, client: client, state: "findable") + FactoryBot.create_list(:event_for_datacite_related, 3, obj_id: dois.first.doi) + FactoryBot.create_list(:event_for_datacite_usage, 2, obj_id: dois.first.doi) +end + + + From abf78853f44188281b0483640b6e2e2a1b245a6a Mon Sep 17 00:00:00 2001 From: kjgarza Date: Mon, 27 Apr 2020 12:05:50 +0200 Subject: [PATCH 02/28] linitng --- app/controllers/clients_controller.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index 5f9cff3c3..8577f044c 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -23,7 +23,8 @@ def index elsif params[:ids].present? response = Client.find_by_id(params[:ids], page: page, sort: sort) else - response = Client.query(params[:query], + response = Client.query( + params[:query], year: params[:year], from_date: params[:from_date], until_date: params[:until_date], @@ -33,9 +34,10 @@ def index software: params[:software], certificate: params[:certificate], repository_type: params[:repository_type], - client_type: params[:client_type], - page: page, - sort: sort) + client_type: params[:client_type], + page: page, + sort: sort, + ) end begin From 379d77f046ba1c09e988a2c6c546915d05bf1563 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Mon, 27 Apr 2020 15:20:24 +0200 Subject: [PATCH 03/28] do not consider provider_id is part user_id addresses https://github.com/datacite/lupo/issues/487 --- app/models/concerns/authenticable.rb | 14 +++++++------- spec/concerns/authenticable_spec.rb | 8 +++++++- spec/models/user_spec.rb | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/models/concerns/authenticable.rb b/app/models/concerns/authenticable.rb index 2b99b4af9..298925910 100644 --- a/app/models/concerns/authenticable.rb +++ b/app/models/concerns/authenticable.rb @@ -159,7 +159,7 @@ def get_payload(uid: nil, user: nil, password: nil) # we only need password for clients registering DOIs in the handle system if uid.include? "." payload.merge!( - "provider_id" => uid.split(".", 2).first, + "provider_id" => user.provider_id, "client_id" => uid, "password" => password, ) @@ -297,15 +297,15 @@ def get_payload(uid: nil, user: nil, password: nil) # we only need password for clients registering DOIs in the handle system if uid.include? "." - payload.merge!({ - "provider_id" => uid.split(".", 2).first, + payload.merge!( + "provider_id" => user.provider_id, "client_id" => uid, - "password" => password - }) + "password" => password, + ) elsif uid != "admin" - payload.merge!({ + payload.merge!( "provider_id" => uid - }) + ) end payload diff --git a/spec/concerns/authenticable_spec.rb b/spec/concerns/authenticable_spec.rb index af60723f0..0b0ec1b36 100644 --- a/spec/concerns/authenticable_spec.rb +++ b/spec/concerns/authenticable_spec.rb @@ -286,7 +286,13 @@ describe 'decode_auth_param' do it "works" do - expect(subject.decode_auth_param(username: subject.symbol, password: 12345)).to eq("uid"=>subject.symbol.downcase, "name"=>subject.name, "email"=>subject.system_email, "password" => "12345", "role_id"=>"client_admin", "provider_id"=>subject.symbol.downcase.split(".").first, "client_id"=>subject.symbol.downcase) + expect(subject.decode_auth_param(username: subject.symbol, password: 12345)).to eq("uid"=>subject.symbol.downcase, "name"=>subject.name, "email"=>subject.system_email, "password" => "12345", "role_id"=>"client_admin", "provider_id"=>subject.provider_id, "client_id"=>subject.symbol.downcase) + end + end + + describe 'get_payload' do + it "works" do + expect(subject.get_payload(uid: subject.symbol.downcase, user: subject, password: 12345)).to eq("uid"=>subject.symbol.downcase, "name"=>subject.name, "email"=>subject.system_email, "password" => 12345, "role_id"=>"client_admin", "provider_id"=>subject.provider_id, "client_id"=>subject.symbol.downcase) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index aecb26786..9a439f10c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -89,7 +89,7 @@ end it "has provider_id" do - expect(user.provider_id).to eq(client.symbol.downcase.split(".").first) + expect(user.provider_id).to eq(client.provider_id) end it "has client" do From 69ff80f5d9a7bb6ceefe700fe84ce1d081c633e4 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 29 Apr 2020 14:55:40 +0200 Subject: [PATCH 04/28] abilitites for transfer --- app/models/ability.rb | 4 ++-- spec/models/ability_spec.rb | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index c7d33f631..8441eab58 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -27,7 +27,7 @@ def initialize(user) can [:manage], ProviderPrefix do |provider_prefix| provider_prefix.provider && user.provider_id.casecmp(provider_prefix.provider.consortium_id) end - can [:manage], Client do |client| + can [:read, :create, :update, :destroy], Client do |client| client.provider && user.provider_id.casecmp(client.provider.consortium_id) end can [:manage], ClientPrefix #, :client_id => user.provider_id @@ -50,7 +50,7 @@ def initialize(user) elsif user.role_id == "provider_admin" && user.provider_id.present? can [:update, :read, :read_billing_information], Provider, symbol: user.provider_id.upcase can [:manage], ProviderPrefix, provider_id: user.provider_id - can [:manage], Client, provider_id: user.provider_id + can [:read, :create, :update, :destroy], Client, provider_id: user.provider_id can [:manage], ClientPrefix #, :client_id => user.provider_id # if Flipper[:delete_doi].enabled?(user) diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 7a928d087..72d13bcce 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -38,6 +38,7 @@ it { is_expected.not_to be_able_to(:create, client) } it { is_expected.not_to be_able_to(:update, client) } it { is_expected.not_to be_able_to(:destroy, client) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.not_to be_able_to(:read, prefix) } it { is_expected.not_to be_able_to(:create, prefix) } @@ -65,6 +66,7 @@ it { is_expected.not_to be_able_to(:create, client) } it { is_expected.to be_able_to(:update, client) } it { is_expected.not_to be_able_to(:destroy, client) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.not_to be_able_to(:read, prefix) } it { is_expected.not_to be_able_to(:create, prefix) } @@ -97,6 +99,7 @@ it { is_expected.not_to be_able_to(:create, client) } it { is_expected.not_to be_able_to(:update, client) } it { is_expected.not_to be_able_to(:destroy, client) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.not_to be_able_to(:read, prefix) } it { is_expected.not_to be_able_to(:create, prefix) } @@ -129,6 +132,7 @@ it { is_expected.to be_able_to(:create, client) } it { is_expected.to be_able_to(:update, client) } it { is_expected.to be_able_to(:destroy, client) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.not_to be_able_to(:read, prefix) } it { is_expected.not_to be_able_to(:create, prefix) } @@ -161,6 +165,7 @@ it { is_expected.to be_able_to(:create, provider) } it { is_expected.to be_able_to(:update, provider) } it { is_expected.to be_able_to(:destroy, provider) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.to be_able_to(:read, client) } it { is_expected.to be_able_to(:create, client) } @@ -198,6 +203,7 @@ it { is_expected.not_to be_able_to(:create, client) } it { is_expected.not_to be_able_to(:update, client) } it { is_expected.not_to be_able_to(:destroy, client) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.not_to be_able_to(:read, prefix) } it { is_expected.not_to be_able_to(:create, prefix) } @@ -223,6 +229,7 @@ it { is_expected.to be_able_to(:create, provider) } it { is_expected.to be_able_to(:update, provider) } it { is_expected.to be_able_to(:destroy, provider) } + it { is_expected.to be_able_to(:transfer, client) } it { is_expected.to be_able_to(:read, client) } it { is_expected.to be_able_to(:create, client) } @@ -250,6 +257,7 @@ it { is_expected.not_to be_able_to(:create, client) } it { is_expected.not_to be_able_to(:update, client) } it { is_expected.not_to be_able_to(:destroy, client) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.to be_able_to(:read, doi) } it { is_expected.not_to be_able_to(:transfer, doi) } @@ -269,6 +277,7 @@ it { is_expected.not_to be_able_to(:create, client) } it { is_expected.not_to be_able_to(:update, client) } it { is_expected.not_to be_able_to(:destroy, client) } + it { is_expected.not_to be_able_to(:transfer, client) } it { is_expected.to be_able_to(:read, doi) } it { is_expected.not_to be_able_to(:transfer, doi) } From f468c76944407b2bd4ef3ecd3d51fe675aeb5189 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 29 Apr 2020 14:56:49 +0200 Subject: [PATCH 05/28] transfer API call --- app/controllers/clients_controller.rb | 28 ++++++++++++++++----- spec/requests/clients_spec.rb | 35 +++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index 8577f044c..35257ae1b 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -124,15 +124,31 @@ def create end def update - if @client.update(safe_params) + @client = Client.where(symbol: params[:id]).first + exists = @client.present? + + if exists options = {} options[:is_collection] = false options[:params] = { current_ability: current_ability } - - render json: ClientSerializer.new(@client, options).serialized_json, status: :ok - else - Rails.logger.error @client.errors.inspect - render json: serialize_errors(@client.errors), status: :unprocessable_entity + + if params.dig(:data, :attributes, :mode) == "transfer" + + # only update provider_id + authorize! :transfer, @client + + @client.transfer(target_id: safe_params.slice(:provider).dig(:provider)) + render json: ClientSerializer.new(@client, options).serialized_json, status: :ok + else + authorize! :update, @client + if @client.update(safe_params) + + render json: ClientSerializer.new(@client, options).serialized_json, status: :ok + else + Rails.logger.error @client.errors.inspect + render json: serialize_errors(@client.errors), status: :unprocessable_entity + end + end end end diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index 0f66312ed..43d959236 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -218,6 +218,41 @@ end end + context "transfer repository" do + let(:new_provider) { create(:provider, symbol: "QUECHUA", password_input: "12345") } + let!(:prefix) { create(:prefix) } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } + let(:doi) { create_list(:doi,10, client: client) } + + + let(:params) do + { "data" => { "type" => "clients", + "attributes" => { + "mode" => "transfer", + "targetId" => new_provider.symbol, + "provider" => new_provider.symbol, + } } } + end + + it "updates the record" do + put "/clients/#{client.symbol}", params, headers + + expect(last_response.status).to eq(200) + expect(json.dig("data", "attributes", "name")).to eq("My data center") + expect(json.dig("data", "relationships", "provider", "data", "id")).to eq("quechua") + expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq("10.5081") + + get "/providers/#{provider.symbol}" + + expect(json.dig("data", "relationships", "prefixes", "data")).to be_empty + + get "/providers/#{new_provider.symbol}" + + expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq("10.5081") + end + end + context 'invalid globus_uuid' do let(:params) do { "data" => { "type" => "clients", From f3359b73979a27a9dd105a92c642b48762b99aa7 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 29 Apr 2020 16:49:59 +0200 Subject: [PATCH 06/28] fix seeding keys --- db/seeds/development/base.seeds.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/seeds/development/base.seeds.rb b/db/seeds/development/base.seeds.rb index dd1c92b03..35ade98b8 100644 --- a/db/seeds/development/base.seeds.rb +++ b/db/seeds/development/base.seeds.rb @@ -8,7 +8,8 @@ client = Client.where(symbol: "DATACITE.TEST").first || FactoryBot.create(:client, provider: provider, symbol: ENV["MDS_USERNAME"], password: ENV["MDS_PASSWORD"]) if Prefix.where(uid: "10.14454").blank? prefix = FactoryBot.create(:prefix, uid: "10.14454") - FactoryBot.create(:client_prefix, client_id: client.id, prefix_id: prefix.id) + FactoryBot.create(:provider_prefix, provider_id: provider.symbol, prefix_id: prefix.uid) + FactoryBot.create(:client_prefix, client_id: client.symbol, prefix_id: prefix.uid) end dois = FactoryBot.create_list(:doi, 10, client: client, state: "findable") FactoryBot.create_list(:event_for_datacite_related, 3, obj_id: dois.first.doi) From 539c7321bbd38ff42f234831f210426566729b78 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 29 Apr 2020 16:51:33 +0200 Subject: [PATCH 07/28] request to transfer repository addresses: https://github.com/datacite/lupo/issues/486 --- app/jobs/transfer_client_job.rb | 25 +++++++++++++++++ app/jobs/update_provider_id_job.rb | 22 +++++++++++++++ app/models/client.rb | 44 ++++++++++++++++++++++++++++++ spec/models/client_spec.rb | 24 ++++++++++++++++ 4 files changed, 115 insertions(+) create mode 100644 app/jobs/transfer_client_job.rb create mode 100644 app/jobs/update_provider_id_job.rb diff --git a/app/jobs/transfer_client_job.rb b/app/jobs/transfer_client_job.rb new file mode 100644 index 000000000..d6e6a7eec --- /dev/null +++ b/app/jobs/transfer_client_job.rb @@ -0,0 +1,25 @@ +class TransferClientJob < ActiveJob::Base + queue_as :lupo_background + + def perform(symbol, options = {}) + client = Client.where(symbol: symbol).first + + if client.present? && options[:target_id].present? + options = { + from_id: Doi.minimum(:id).to_i, + until_id: Doi.maximum(:id).to_i, + filter: { client_id: symbol.downcase }, + label: "[ClientTransfer]", + job_name: "UpdateProviderIdJob", + target_id: options[:target_id], + } + puts "fgdkjlfdgjklgfdjklgfd" + puts options + Doi.loop_through_dois(options) + + Rails.logger.info "[Transfer] DOIs transfer has started for #{client.symbol} to #{options[:target_id]}." + else + Rails.logger.error "[Transfer] Error transferring DOIs " + symbol + ": not found" + end + end +end diff --git a/app/jobs/update_provider_id_job.rb b/app/jobs/update_provider_id_job.rb new file mode 100644 index 000000000..79878d0ad --- /dev/null +++ b/app/jobs/update_provider_id_job.rb @@ -0,0 +1,22 @@ +class UpdateProviderIdJob < ActiveJob::Base + queue_as :lupo_transfer + + # retry_on ActiveRecord::RecordNotFound, wait: 10.seconds, attempts: 3 + # retry_on Faraday::TimeoutError, wait: 10.minutes, attempts: 3 + + # discard_on ActiveJob::DeserializationError + + def perform(doi_id, options = {}) + doi = Doi.where(doi: doi_id).first + + if doi.present? && options[:target_id].present? + doi.__elasticsearch__.index_document + + Rails.logger.info "[Transfer] Transferred DOI #{doi.doi}." + elsif doi.present? + Rails.logger.error "[Transfer] Error transferring DOI " + doi_id + ": no target client" + else + Rails.logger.error "[Transfer] Error transferring DOI " + doi_id + ": not found" + end + end +end diff --git a/app/models/client.rb b/app/models/client.rb index cd9ff0a4a..cb1f85b27 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -323,6 +323,50 @@ def target_id=(value) Doi.transfer(client_id: symbol.downcase, target_id: target.id) end + def transfer(options = {}) + + if options[:target_id].blank? + Rails.logger.error "[Transfer] No target provider provided." + return nil + end + + target_provider = Provider.where(symbol: options[:target_id]).first + + if target_provider.blank? + Rails.logger.error "[Transfer] Provider doesn't exist." + return nil + end + + original_provider = Provider.where(symbol: provider_id).first + ## Transfer client + update_attribute(:allocator, target_provider.id) + + # These prefixes are used by multiple clients + prefixes_to_keep = ["10.4124", "10.4225", "10.4226", "10.4227"] + + # delete all associated prefixes + prefix_ids = original_provider.prefixes.reject{ |prefix| prefixes_to_keep.include?(prefix)}.pluck(:id) + prefixes = original_provider.prefix_ids.reject{ |prefix| prefixes_to_keep.include?(prefix)} + + if prefix_ids.present? + response = ProviderPrefix.where("prefix_id IN (?)", prefix_ids).destroy_all + puts "#{response.count} provider prefixes deleted." + end + + # # update dois + # Doi.transfer(from_date: "2011-01-01", client_id: client.symbol, target_id: target.symbol) + + # Transfer DOIs + TransferClientJob.perform_later(symbol, target_id: options[:target_id]) + + # # assign prefixes to new client + prefixes.each do |prefix| + ProviderPrefix.create(provider_id: target_provider.symbol, prefix_id: prefix) + puts "Provider prefix for provider #{target_provider.symbol} and prefix #{prefix} created." + end + + end + def service_contact_email service_contact.fetch("email",nil) if service_contact.present? end diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index 2805364ec..fb370b692 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -23,6 +23,30 @@ end end + describe "Client transfer" do + let!(:prefix) { create(:prefix) } + let!(:prefixes) { create(:client_prefix, client: client, prefix: prefix) } + let!(:prefixes_dos) { create(:provider_prefix, provider: provider, prefix: prefix) } + let(:new_provider) { create(:provider, symbol: "QUECHUA") } + let(:options) { { target_id: new_provider.symbol } } + let(:bad_options) { { target_id: "SALS" } } + + it "works" do + client.transfer(options) + + expect(client.provider_id).to eq(new_provider.symbol.downcase) + expect(new_provider.prefix_ids).to include(prefix.uid) + expect(provider.prefix_ids).not_to include(prefix.uid) + end + + it "it fails" do + client.transfer(bad_options) + + expect(client.provider_id).to eq(provider.symbol.downcase) + expect(provider.prefix_ids).to include(prefix.uid) + end + end + describe "methods" do it "should not update the symbol" do client.update_attributes :symbol => client.symbol+'foo.bar' From 47e998bf63e1b61e2daaf9b881a00ad96db5c304 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 29 Apr 2020 17:37:56 +0200 Subject: [PATCH 08/28] not allow changes to consortiums addresses: https://github.com/datacite/lupo/issues/486 --- app/models/client.rb | 8 +++++--- app/models/doi.rb | 37 +++++++++++++++++++++++++++++++++++++ spec/models/client_spec.rb | 16 ++++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/app/models/client.rb b/app/models/client.rb index cb1f85b27..609bb68d9 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -336,6 +336,11 @@ def transfer(options = {}) Rails.logger.error "[Transfer] Provider doesn't exist." return nil end + + if target_provider.member_type == "consortium" + Rails.logger.error "[Transfer] Consortiums cannot have repositories." + return nil + end original_provider = Provider.where(symbol: provider_id).first ## Transfer client @@ -353,9 +358,6 @@ def transfer(options = {}) puts "#{response.count} provider prefixes deleted." end - # # update dois - # Doi.transfer(from_date: "2011-01-01", client_id: client.symbol, target_id: target.symbol) - # Transfer DOIs TransferClientJob.perform_later(symbol, target_id: options[:target_id]) diff --git a/app/models/doi.rb b/app/models/doi.rb index a3226a8dc..af5ac3ec1 100644 --- a/app/models/doi.rb +++ b/app/models/doi.rb @@ -1765,6 +1765,43 @@ def self.transfer(options={}) response.results.total end + ## Transverses the index in batches and using the cursor pagination and executes a Job that matches the query and filer + # Options: + # +filter+:: paramaters to filter the index + # +label+:: String to output in the logs printout + # +query+:: ES query to filter the index + # +job_name+:: Acive Job class name of the Job that would be executed on every matched results + def self.loop_through_dois(options) + size = (options[:size] || 1000).to_i + cursor = [options[:from_id], options[:until_id]] + filter = options[:filter] || {} + label = options[:label] || "" + job_name = options[:job_name] || "" + query = options[:query] || nil + + + response = Doi.query(query, filter.merge(page: { size: 1, cursor: [] })) + Rails.logger.info "#{label} #{response.results.total} Dois with #{label}." + + # walk through results using cursor + if response.results.total.positive? + while response.results.results.length.positive? + response = Doi.query(query, filter.merge(page: { size: size, cursor: cursor })) + break unless response.results.results.length.positive? + + Rails.logger.info "#{label} #{response.results.results.length} Dois starting with _id #{response.results.to_a.first[:_id]}." + cursor = response.results.to_a.last[:sort] + Rails.logger.info "#{label} Cursor: #{cursor} " + + ids = response.results.results.map(&:uid) + ids.each do |id| + Object.const_get(job_name).perform_later(id, options) + end + end + end + end + + # save to metadata table when xml has changed def save_metadata metadata.build(doi: self, xml: xml, namespace: schema_version) if xml.present? && xml_changed? diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index fb370b692..4c6f9a243 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -46,6 +46,22 @@ expect(provider.prefix_ids).to include(prefix.uid) end end + + describe "Client transfer to consortium organisation" do + let!(:prefix) { create(:prefix) } + let!(:prefixes) { create(:client_prefix, client: client, prefix: prefix) } + let!(:prefixes_dos) { create(:provider_prefix, provider: provider, prefix: prefix) } + let(:new_provider) { create(:provider, symbol: "QUECHUA", role_name: "ROLE_CONSORTIUM") } + let(:options) { { target_id: new_provider.symbol } } + + it "it fails" do + client.transfer(options) + + expect(client.provider_id).to eq(provider.symbol.downcase) + expect(provider.prefix_ids).to include(prefix.uid) + end + end + describe "methods" do it "should not update the symbol" do From 422f2db3e5d8bb0b492cd47e99197fb252e00d9d Mon Sep 17 00:00:00 2001 From: kjgarza Date: Thu, 30 Apr 2020 15:01:38 +0200 Subject: [PATCH 09/28] fix specs --- spec/requests/clients_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index 43d959236..9e0ce3ee5 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -241,7 +241,7 @@ expect(last_response.status).to eq(200) expect(json.dig("data", "attributes", "name")).to eq("My data center") expect(json.dig("data", "relationships", "provider", "data", "id")).to eq("quechua") - expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq("10.5081") + expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq(prefix.uid) get "/providers/#{provider.symbol}" @@ -249,7 +249,7 @@ get "/providers/#{new_provider.symbol}" - expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq("10.5081") + expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq(prefix.uid) end end From 9ab93ed1555da87de82cb4528c931a1b910bdfac Mon Sep 17 00:00:00 2001 From: kjgarza Date: Mon, 4 May 2020 12:20:48 +0200 Subject: [PATCH 10/28] fix abilities avoid modifying current abilities --- app/models/ability.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 8441eab58..58f6967a7 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -27,7 +27,10 @@ def initialize(user) can [:manage], ProviderPrefix do |provider_prefix| provider_prefix.provider && user.provider_id.casecmp(provider_prefix.provider.consortium_id) end - can [:read, :create, :update, :destroy], Client do |client| + can [:manage], Client do |client| + client.provider && user.provider_id.casecmp(client.provider.consortium_id) + end + cannot [:transfer], Client do |client| client.provider && user.provider_id.casecmp(client.provider.consortium_id) end can [:manage], ClientPrefix #, :client_id => user.provider_id @@ -50,7 +53,8 @@ def initialize(user) elsif user.role_id == "provider_admin" && user.provider_id.present? can [:update, :read, :read_billing_information], Provider, symbol: user.provider_id.upcase can [:manage], ProviderPrefix, provider_id: user.provider_id - can [:read, :create, :update, :destroy], Client, provider_id: user.provider_id + can [:manage], Client, provider_id: user.provider_id + cannot [:transfer], Client, provider_id: user.provider_id can [:manage], ClientPrefix #, :client_id => user.provider_id # if Flipper[:delete_doi].enabled?(user) From 0b60240d642df082d7691bc663fa6d5bf1a9beca Mon Sep 17 00:00:00 2001 From: kjgarza Date: Mon, 4 May 2020 13:01:30 +0200 Subject: [PATCH 11/28] enable transfer also in repository endpoint --- app/controllers/repositories_controller.rb | 26 ++++++++++++--- spec/requests/repositories_spec.rb | 38 ++++++++++++++++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index be265ef6c..762843e1c 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -155,15 +155,31 @@ def create end def update - if @client.update_attributes(safe_params) + @client = Client.where(symbol: params[:id]).first + exists = @client.present? + + if exists options = {} options[:is_collection] = false options[:params] = { current_ability: current_ability } - render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok - else - Rails.logger.error @client.errors.inspect - render json: serialize_errors(@client.errors), status: :unprocessable_entity + if params.dig(:data, :attributes, :mode) == "transfer" + + # only update provider_id + authorize! :transfer, @client + + @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) + render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok + else + authorize! :update, @client + if @client.update(safe_params) + + render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok + else + Rails.logger.error @client.errors.inspect + render json: serialize_errors(@client.errors), status: :unprocessable_entity + end + end end end diff --git a/spec/requests/repositories_spec.rb b/spec/requests/repositories_spec.rb index 896928006..c1a220d27 100644 --- a/spec/requests/repositories_spec.rb +++ b/spec/requests/repositories_spec.rb @@ -279,6 +279,44 @@ end end + context "transfer repository" do + let(:bearer) { User.generate_token } + let(:staff_headers) { {'HTTP_ACCEPT'=>'application/vnd.api+json', 'HTTP_AUTHORIZATION' => 'Bearer ' + bearer}} + + let(:new_provider) { create(:provider, symbol: "QUECHUA", password_input: "12345") } + let!(:prefix) { create(:prefix) } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } + let(:doi) { create_list(:doi,10, client: client) } + + + let(:params) do + { "data" => { "type" => "clients", + "attributes" => { + "mode" => "transfer", + "targetId" => new_provider.symbol, + + } } } + end + + it "updates the record" do + put "/repositories/#{client.symbol}", params, staff_headers + puts params + expect(last_response.status).to eq(200) + expect(json.dig("data", "attributes", "name")).to eq("My data center") + expect(json.dig("data", "relationships", "provider", "data", "id")).to eq("quechua") + expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq(prefix.uid) + + get "/providers/#{provider.symbol}" + + expect(json.dig("data", "relationships", "prefixes", "data")).to be_empty + + get "/providers/#{new_provider.symbol}" + + expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq(prefix.uid) + end + end + context 'invalid globus_uuid' do let(:params) do { "data" => { "type" => "repositories", From fa58034b9921bdacee9eafc3e1c5e4c4f3c1da4b Mon Sep 17 00:00:00 2001 From: kjgarza Date: Mon, 4 May 2020 13:01:46 +0200 Subject: [PATCH 12/28] reduce number of arguments needed for a trasnfer --- app/controllers/clients_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index 9d1c94981..802a00936 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -138,7 +138,7 @@ def update # only update provider_id authorize! :transfer, @client - @client.transfer(target_id: safe_params.slice(:provider).dig(:provider)) + @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) render json: ClientSerializer.new(@client, options).serialized_json, status: :ok else authorize! :update, @client From f2aac360a23441e2b04e13563d6554dd2d7790ce Mon Sep 17 00:00:00 2001 From: kjgarza Date: Mon, 4 May 2020 13:28:59 +0200 Subject: [PATCH 13/28] remove puts --- app/jobs/transfer_client_job.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/jobs/transfer_client_job.rb b/app/jobs/transfer_client_job.rb index d6e6a7eec..26f02e1e8 100644 --- a/app/jobs/transfer_client_job.rb +++ b/app/jobs/transfer_client_job.rb @@ -13,8 +13,7 @@ def perform(symbol, options = {}) job_name: "UpdateProviderIdJob", target_id: options[:target_id], } - puts "fgdkjlfdgjklgfdjklgfd" - puts options + Doi.loop_through_dois(options) Rails.logger.info "[Transfer] DOIs transfer has started for #{client.symbol} to #{options[:target_id]}." From b1a2c4a19d87471de998d6542923489933946ac4 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Mon, 4 May 2020 13:30:00 +0200 Subject: [PATCH 14/28] remove comments --- .../development/consortium_transfer.seeds.rb | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/db/seeds/development/consortium_transfer.seeds.rb b/db/seeds/development/consortium_transfer.seeds.rb index d091a585d..ce1ec1ad1 100644 --- a/db/seeds/development/consortium_transfer.seeds.rb +++ b/db/seeds/development/consortium_transfer.seeds.rb @@ -4,28 +4,14 @@ after "development:base" do -## ranks -# Lieutenant of Supplies -# Master of Ships -# Commander of the Law -# General of Warlords -# Supreme Master -# Supreme General -# Wing Commander -# Lawbringer -# Lord General -# Paladin - - provider = Provider.where(symbol: "GENERAL").first || FactoryBot.create(:provider, symbol: "GENERAL") - client = Client.where(symbol: "PALADIN").first || FactoryBot.create(:client, provider: provider, symbol: "PALADIN", password: ENV["MDS_PASSWORD"]) - if Prefix.where(uid: "10.14456").blank? - prefix = FactoryBot.create(:prefix, uid: "10.14456") - # FactoryBot.create(:client_prefix, client_id: client.id, prefix_id: prefix.id) + provider = Provider.where(symbol: "QUECHUA").first || FactoryBot.create(:provider, symbol: "QUECHUA") + client = Client.where(symbol: "QUECHUA.TEXT").first || FactoryBot.create(:client, provider: provider, symbol: "QUECHUA.TEXT", password: ENV["MDS_PASSWORD"]) + if Prefix.where(uid: "10.14459").blank? + prefix = FactoryBot.create(:prefix, uid: "10.14459") + FactoryBot.create(:provider_prefix, provider_id: provider.symbol, prefix_id: prefix.uid) + FactoryBot.create(:client_prefix, client_id: client.symbol, prefix_id: prefix.uid) end dois = FactoryBot.create_list(:doi, 10, client: client, state: "findable") FactoryBot.create_list(:event_for_datacite_related, 3, obj_id: dois.first.doi) FactoryBot.create_list(:event_for_datacite_usage, 2, obj_id: dois.first.doi) end - - - From 12313c78ec50e5fb79e30da3d7b7074e1585dfc0 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 5 May 2020 14:59:20 +0200 Subject: [PATCH 15/28] lint fixes --- spec/requests/clients_spec.rb | 23 +++++++++++++---------- spec/requests/repositories_spec.rb | 26 ++++++++++++++------------ 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index 9e0ce3ee5..279d23189 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -220,19 +220,22 @@ context "transfer repository" do let(:new_provider) { create(:provider, symbol: "QUECHUA", password_input: "12345") } - let!(:prefix) { create(:prefix) } - let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } - let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } - let(:doi) { create_list(:doi,10, client: client) } + let!(:prefix) { create(:prefix) } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } + let(:doi) { create_list(:doi, 10, client: client) } let(:params) do - { "data" => { "type" => "clients", - "attributes" => { - "mode" => "transfer", - "targetId" => new_provider.symbol, - "provider" => new_provider.symbol, - } } } + { + "data" => { + "type" => "clients", + "attributes" => { + "mode" => "transfer", + "targetId" => new_provider.symbol, + }, + }, + } end it "updates the record" do diff --git a/spec/requests/repositories_spec.rb b/spec/requests/repositories_spec.rb index c1a220d27..4333b56e4 100644 --- a/spec/requests/repositories_spec.rb +++ b/spec/requests/repositories_spec.rb @@ -284,24 +284,26 @@ let(:staff_headers) { {'HTTP_ACCEPT'=>'application/vnd.api+json', 'HTTP_AUTHORIZATION' => 'Bearer ' + bearer}} let(:new_provider) { create(:provider, symbol: "QUECHUA", password_input: "12345") } - let!(:prefix) { create(:prefix) } - let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } - let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } - let(:doi) { create_list(:doi,10, client: client) } - + let!(:prefix) { create(:prefix) } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } + let(:doi) { create_list(:doi, 10, client: client) } let(:params) do - { "data" => { "type" => "clients", - "attributes" => { - "mode" => "transfer", - "targetId" => new_provider.symbol, - - } } } + { + "data" => { + "type" => "clients", + "attributes" => { + "mode" => "transfer", + "targetId" => new_provider.symbol, + }, + }, + } end it "updates the record" do put "/repositories/#{client.symbol}", params, staff_headers - puts params + expect(last_response.status).to eq(200) expect(json.dig("data", "attributes", "name")).to eq("My data center") expect(json.dig("data", "relationships", "provider", "data", "id")).to eq("quechua") From 06e00b918f2562703636948a5506794773df6ff3 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 5 May 2020 18:06:23 +0200 Subject: [PATCH 16/28] avoid deleting other prefixes --- app/models/client.rb | 16 +++++++--------- spec/models/client_spec.rb | 14 ++++++++++---- spec/requests/clients_spec.rb | 7 +++++-- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/app/models/client.rb b/app/models/client.rb index e9eba926e..d566b615b 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -334,37 +334,35 @@ def transfer(options = {}) Rails.logger.error "[Transfer] Provider doesn't exist." return nil end - + if target_provider.member_type == "consortium" Rails.logger.error "[Transfer] Consortiums cannot have repositories." return nil end - original_provider = Provider.where(symbol: provider_id).first ## Transfer client update_attribute(:allocator, target_provider.id) - # These prefixes are used by multiple clients prefixes_to_keep = ["10.4124", "10.4225", "10.4226", "10.4227"] # delete all associated prefixes - prefix_ids = original_provider.prefixes.reject{ |prefix| prefixes_to_keep.include?(prefix)}.pluck(:id) - prefixes = original_provider.prefix_ids.reject{ |prefix| prefixes_to_keep.include?(prefix)} + associated_prefixes = prefixes.reject{ |prefix| prefixes_to_keep.include?(prefix.uid)} + prefix_ids = associated_prefixes.pluck(:id) + prefixes_names = associated_prefixes.pluck(:uid) if prefix_ids.present? response = ProviderPrefix.where("prefix_id IN (?)", prefix_ids).destroy_all puts "#{response.count} provider prefixes deleted." end - # Transfer DOIs + # Update DOIs TransferClientJob.perform_later(symbol, target_id: options[:target_id]) - # # assign prefixes to new client - prefixes.each do |prefix| + # Assign prefixes to provider + prefixes_names.each do |prefix| ProviderPrefix.create(provider_id: target_provider.symbol, prefix_id: prefix) puts "Provider prefix for provider #{target_provider.symbol} and prefix #{prefix} created." end - end def service_contact_email diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index 4c6f9a243..f27063f17 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -24,9 +24,12 @@ end describe "Client transfer" do - let!(:prefix) { create(:prefix) } - let!(:prefixes) { create(:client_prefix, client: client, prefix: prefix) } - let!(:prefixes_dos) { create(:provider_prefix, provider: provider, prefix: prefix) } + let!(:prefixes) { create_list(:prefix, 3) } + let!(:prefix) { prefixes.first } + + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } + let!(:provider_prefix_more) { create(:provider_prefix, provider: provider, prefix: prefixes.last) } let(:new_provider) { create(:provider, symbol: "QUECHUA") } let(:options) { { target_id: new_provider.symbol } } let(:bad_options) { { target_id: "SALS" } } @@ -35,11 +38,14 @@ client.transfer(options) expect(client.provider_id).to eq(new_provider.symbol.downcase) + expect(new_provider.prefixes.length).to eq(1) + expect(provider.prefixes.length).to eq(1) + expect(new_provider.prefix_ids).to include(prefix.uid) expect(provider.prefix_ids).not_to include(prefix.uid) end - it "it fails" do + it "it doesn't transfer" do client.transfer(bad_options) expect(client.provider_id).to eq(provider.symbol.downcase) diff --git a/spec/requests/clients_spec.rb b/spec/requests/clients_spec.rb index 279d23189..154d40961 100644 --- a/spec/requests/clients_spec.rb +++ b/spec/requests/clients_spec.rb @@ -220,9 +220,11 @@ context "transfer repository" do let(:new_provider) { create(:provider, symbol: "QUECHUA", password_input: "12345") } - let!(:prefix) { create(:prefix) } + let!(:prefixes) { create_list(:prefix, 3) } + let!(:prefix) { prefixes.first } let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } + let!(:provider_prefix_more) { create(:provider_prefix, provider: provider, prefix: prefixes.last) } let(:doi) { create_list(:doi, 10, client: client) } @@ -248,7 +250,8 @@ get "/providers/#{provider.symbol}" - expect(json.dig("data", "relationships", "prefixes", "data")).to be_empty + expect(json.dig("data", "relationships", "prefixes", "data").length).to eq(1) + expect(json.dig("data", "relationships", "prefixes", "data").first.dig("id")).to eq(prefixes.last.uid) get "/providers/#{new_provider.symbol}" From 832c95246f62e62cf812d2d41eeff8654e4eebae Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 5 May 2020 18:30:43 +0200 Subject: [PATCH 17/28] updating language --- app/jobs/transfer_client_job.rb | 6 ++---- app/models/doi.rb | 4 ++-- app/models/event.rb | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/jobs/transfer_client_job.rb b/app/jobs/transfer_client_job.rb index 26f02e1e8..d0e80c0ba 100644 --- a/app/jobs/transfer_client_job.rb +++ b/app/jobs/transfer_client_job.rb @@ -6,8 +6,6 @@ def perform(symbol, options = {}) if client.present? && options[:target_id].present? options = { - from_id: Doi.minimum(:id).to_i, - until_id: Doi.maximum(:id).to_i, filter: { client_id: symbol.downcase }, label: "[ClientTransfer]", job_name: "UpdateProviderIdJob", @@ -16,9 +14,9 @@ def perform(symbol, options = {}) Doi.loop_through_dois(options) - Rails.logger.info "[Transfer] DOIs transfer has started for #{client.symbol} to #{options[:target_id]}." + Rails.logger.info "[Transfer] DOIs updating has started for #{client.symbol} to #{options[:target_id]}." else - Rails.logger.error "[Transfer] Error transferring DOIs " + symbol + ": not found" + Rails.logger.error "[Transfer] Error updating DOIs " + symbol + ": not found" end end end diff --git a/app/models/doi.rb b/app/models/doi.rb index 7d8afe3f1..72d363ed6 100644 --- a/app/models/doi.rb +++ b/app/models/doi.rb @@ -1765,7 +1765,7 @@ def self.transfer(options={}) response.results.total end - ## Transverses the index in batches and using the cursor pagination and executes a Job that matches the query and filer + # Transverses the index in batches and using the cursor pagination and executes a Job that matches the query and filer # Options: # +filter+:: paramaters to filter the index # +label+:: String to output in the logs printout @@ -1773,7 +1773,7 @@ def self.transfer(options={}) # +job_name+:: Acive Job class name of the Job that would be executed on every matched results def self.loop_through_dois(options) size = (options[:size] || 1000).to_i - cursor = [options[:from_id], options[:until_id]] + cursor = [options[:from_id] || Doi.minimum(:id).to_i, options[:until_id] || Doi.maximum(:id).to_i] filter = options[:filter] || {} label = options[:label] || "" job_name = options[:job_name] || "" diff --git a/app/models/event.rb b/app/models/event.rb index c2d507504..1c2194858 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -519,7 +519,7 @@ def self.label_state_event(event) # +job_name+:: Acive Job class name of the Job that would be executed on every matched results def self.loop_through_events(options) size = (options[:size] || 1000).to_i - cursor = [options[:from_id], options[:until_id]] + cursor = [options[:from_id] || Doi.minimum(:id).to_i, options[:until_id] || Doi.maximum(:id).to_i] filter = options[:filter] || {} label = options[:label] || "" job_name = options[:job_name] || "" From a5154d90bc3390ae43470a3f54a1fbb554653d54 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 5 May 2020 18:32:19 +0200 Subject: [PATCH 18/28] abstracting prefix transfer out --- app/models/client.rb | 23 +++++++++++++++-------- spec/models/client_spec.rb | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/app/models/client.rb b/app/models/client.rb index d566b615b..c30d51a80 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -335,13 +335,23 @@ def transfer(options = {}) return nil end - if target_provider.member_type == "consortium" - Rails.logger.error "[Transfer] Consortiums cannot have repositories." + if ["member_only", "consortium"].include?(target_provider.member_type) + Rails.logger.error "[Transfer] Consortiums and Members-only cannot have repositories." return nil end ## Transfer client update_attribute(:allocator, target_provider.id) + + # transfer prefixes + transfer_prefixes(target_provider.symbol) + + # Update DOIs + TransferClientJob.perform_later(symbol, target_id: options[:target_id]) + end + + + def transfer_prefixes(target_id) # These prefixes are used by multiple clients prefixes_to_keep = ["10.4124", "10.4225", "10.4226", "10.4227"] @@ -355,13 +365,10 @@ def transfer(options = {}) puts "#{response.count} provider prefixes deleted." end - # Update DOIs - TransferClientJob.perform_later(symbol, target_id: options[:target_id]) - - # Assign prefixes to provider + # Assign prefix(es) to provider prefixes_names.each do |prefix| - ProviderPrefix.create(provider_id: target_provider.symbol, prefix_id: prefix) - puts "Provider prefix for provider #{target_provider.symbol} and prefix #{prefix} created." + ProviderPrefix.create(provider_id: target_id, prefix_id: prefix) + puts "Provider prefix for provider #{target_id} and prefix #{prefix} created." end end diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index f27063f17..c39e8fb02 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -68,6 +68,25 @@ end end + describe "Client prefixes transfer" do + let!(:prefixes) { create_list(:prefix, 3) } + let!(:prefix) { prefixes.first } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } + let!(:provider_prefix_more) { create(:provider_prefix, provider: provider, prefix: prefixes.last) } + let(:new_provider) { create(:provider, symbol: "QUECHUA") } + + it "works" do + client.transfer_prefixes(new_provider.symbol) + + expect(new_provider.prefixes.length).to eq(1) + expect(provider.prefixes.length).to eq(1) + + expect(new_provider.prefix_ids).to include(prefix.uid) + expect(provider.prefix_ids).not_to include(prefix.uid) + end + end + describe "methods" do it "should not update the symbol" do From 0c1381026ac4c6b00577788e733c29347da6d867 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 5 May 2020 18:43:08 +0200 Subject: [PATCH 19/28] refactor test to accomodate more test cases --- spec/models/client_spec.rb | 60 +++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index c39e8fb02..a486c47ec 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -34,37 +34,51 @@ let(:options) { { target_id: new_provider.symbol } } let(:bad_options) { { target_id: "SALS" } } - it "works" do - client.transfer(options) + context "to direct_member" do + it "works" do + client.transfer(options) - expect(client.provider_id).to eq(new_provider.symbol.downcase) - expect(new_provider.prefixes.length).to eq(1) - expect(provider.prefixes.length).to eq(1) + expect(client.provider_id).to eq(new_provider.symbol.downcase) + expect(new_provider.prefixes.length).to eq(1) + expect(provider.prefixes.length).to eq(1) - expect(new_provider.prefix_ids).to include(prefix.uid) - expect(provider.prefix_ids).not_to include(prefix.uid) + expect(new_provider.prefix_ids).to include(prefix.uid) + expect(provider.prefix_ids).not_to include(prefix.uid) + end + + it "it doesn't transfer" do + client.transfer(bad_options) + + expect(client.provider_id).to eq(provider.symbol.downcase) + expect(provider.prefixes.length).to eq(2) + expect(provider.prefix_ids).to include(prefix.uid) + end end - it "it doesn't transfer" do - client.transfer(bad_options) + context "to member_only" do + let(:new_provider) { create(:provider, symbol: "QUECHUA", member_type: "member_only") } + let(:options) { { target_id: new_provider.symbol } } + + it "it doesn't transfer" do + client.transfer(options) - expect(client.provider_id).to eq(provider.symbol.downcase) - expect(provider.prefix_ids).to include(prefix.uid) + expect(client.provider_id).to eq(provider.symbol.downcase) + expect(provider.prefixes.length).to eq(2) + expect(provider.prefix_ids).to include(prefix.uid) + end end - end - - describe "Client transfer to consortium organisation" do - let!(:prefix) { create(:prefix) } - let!(:prefixes) { create(:client_prefix, client: client, prefix: prefix) } - let!(:prefixes_dos) { create(:provider_prefix, provider: provider, prefix: prefix) } - let(:new_provider) { create(:provider, symbol: "QUECHUA", role_name: "ROLE_CONSORTIUM") } - let(:options) { { target_id: new_provider.symbol } } - it "it fails" do - client.transfer(options) + context "to consortium" do + let(:new_provider) { create(:provider, symbol: "QUECHUA", role_name: "ROLE_CONSORTIUM") } + let(:options) { { target_id: new_provider.symbol } } + + it "it doesn't transfer" do + client.transfer(options) - expect(client.provider_id).to eq(provider.symbol.downcase) - expect(provider.prefix_ids).to include(prefix.uid) + expect(client.provider_id).to eq(provider.symbol.downcase) + expect(provider.prefixes.length).to eq(2) + expect(provider.prefix_ids).to include(prefix.uid) + end end end From 4cb634d3a4c8b6f8d2fa21f9c96c957ad3e0e663 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Wed, 6 May 2020 08:56:20 +0200 Subject: [PATCH 20/28] filter roles --- app/models/client.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/client.rb b/app/models/client.rb index c30d51a80..80af554ca 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -322,7 +322,6 @@ def target_id=(value) end def transfer(options = {}) - if options[:target_id].blank? Rails.logger.error "[Transfer] No target provider provided." return nil @@ -335,14 +334,14 @@ def transfer(options = {}) return nil end - if ["member_only", "consortium"].include?(target_provider.member_type) + unless ["direct_member", "consortium_organization"].include?(target_provider.member_type) Rails.logger.error "[Transfer] Consortiums and Members-only cannot have repositories." return nil end - ## Transfer client + # Transfer client update_attribute(:allocator, target_provider.id) - + # transfer prefixes transfer_prefixes(target_provider.symbol) @@ -350,7 +349,6 @@ def transfer(options = {}) TransferClientJob.perform_later(symbol, target_id: options[:target_id]) end - def transfer_prefixes(target_id) # These prefixes are used by multiple clients prefixes_to_keep = ["10.4124", "10.4225", "10.4226", "10.4227"] From 2978bb70add27828dd9e128bab8b206c79d4fe1e Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 6 May 2020 11:26:39 +0200 Subject: [PATCH 21/28] set explicity membership --- spec/models/client_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index a486c47ec..65a7733af 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -30,7 +30,7 @@ let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } let!(:provider_prefix) { create(:provider_prefix, provider: provider, prefix: prefix) } let!(:provider_prefix_more) { create(:provider_prefix, provider: provider, prefix: prefixes.last) } - let(:new_provider) { create(:provider, symbol: "QUECHUA") } + let(:new_provider) { create(:provider, symbol: "QUECHUA", member_type: "direct_member") } let(:options) { { target_id: new_provider.symbol } } let(:bad_options) { { target_id: "SALS" } } From 592b8ac0429a8d86cf273eb94eed149bea92b417 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 6 May 2020 11:27:00 +0200 Subject: [PATCH 22/28] remove extra steps on update --- app/controllers/clients_controller.rb | 36 +++++++++------------- app/controllers/repositories_controller.rb | 35 ++++++++------------- 2 files changed, 27 insertions(+), 44 deletions(-) diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index 802a00936..c738280da 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -124,32 +124,24 @@ def create end end - def update - @client = Client.where(symbol: params[:id]).first - exists = @client.present? - if exists - options = {} - options[:is_collection] = false - options[:params] = { current_ability: current_ability } + def update + options = {} + options[:is_collection] = false + options[:params] = { current_ability: current_ability } - if params.dig(:data, :attributes, :mode) == "transfer" + if params.dig(:data, :attributes, :mode) == "transfer" + # only update provider_id + authorize! :transfer, @client - # only update provider_id - authorize! :transfer, @client + @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) + render json: ClientSerializer.new(@client, options).serialized_json, status: :ok + elsif @client.update(safe_params) - @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) - render json: ClientSerializer.new(@client, options).serialized_json, status: :ok - else - authorize! :update, @client - if @client.update(safe_params) - - render json: ClientSerializer.new(@client, options).serialized_json, status: :ok - else - Rails.logger.error @client.errors.inspect - render json: serialize_errors(@client.errors), status: :unprocessable_entity - end - end + render json: ClientSerializer.new(@client, options).serialized_json, status: :ok + else + Rails.logger.error @client.errors.inspect + render json: serialize_errors(@client.errors), status: :unprocessable_entity end end diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index 762843e1c..2afa4dcf0 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -155,31 +155,22 @@ def create end def update - @client = Client.where(symbol: params[:id]).first - exists = @client.present? - - if exists - options = {} - options[:is_collection] = false - options[:params] = { current_ability: current_ability } - - if params.dig(:data, :attributes, :mode) == "transfer" + options = {} + options[:is_collection] = false + options[:params] = { current_ability: current_ability } - # only update provider_id - authorize! :transfer, @client + if params.dig(:data, :attributes, :mode) == "transfer" + # only update provider_id + authorize! :transfer, @client - @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) - render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok - else - authorize! :update, @client - if @client.update(safe_params) + @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) + render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok + elsif @client.update(safe_params) - render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok - else - Rails.logger.error @client.errors.inspect - render json: serialize_errors(@client.errors), status: :unprocessable_entity - end - end + render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok + else + Rails.logger.error @client.errors.inspect + render json: serialize_errors(@client.errors), status: :unprocessable_entity end end From f253be3690e930c3c46a8de19f01872aed77d6c9 Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 6 May 2020 11:46:16 +0200 Subject: [PATCH 23/28] change language --- app/jobs/update_provider_id_job.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/jobs/update_provider_id_job.rb b/app/jobs/update_provider_id_job.rb index 79878d0ad..50ceed98f 100644 --- a/app/jobs/update_provider_id_job.rb +++ b/app/jobs/update_provider_id_job.rb @@ -12,11 +12,11 @@ def perform(doi_id, options = {}) if doi.present? && options[:target_id].present? doi.__elasticsearch__.index_document - Rails.logger.info "[Transfer] Transferred DOI #{doi.doi}." + Rails.logger.info "[Transfer] updated DOI #{doi.doi}." elsif doi.present? - Rails.logger.error "[Transfer] Error transferring DOI " + doi_id + ": no target client" + Rails.logger.error "[Transfer] Error updateding DOI " + doi_id + ": no target client" else - Rails.logger.error "[Transfer] Error transferring DOI " + doi_id + ": not found" + Rails.logger.error "[Transfer] Error updateding DOI " + doi_id + ": not found" end end end From 5800f730a1724b05cbca58692011cfc00c5552ab Mon Sep 17 00:00:00 2001 From: kjgarza Date: Wed, 6 May 2020 14:19:05 +0200 Subject: [PATCH 24/28] remove conditions for transfer --- app/models/ability.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 58f6967a7..bb6bab8fe 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -30,9 +30,7 @@ def initialize(user) can [:manage], Client do |client| client.provider && user.provider_id.casecmp(client.provider.consortium_id) end - cannot [:transfer], Client do |client| - client.provider && user.provider_id.casecmp(client.provider.consortium_id) - end + cannot [:transfer], Client can [:manage], ClientPrefix #, :client_id => user.provider_id # if Flipper[:delete_doi].enabled?(user) @@ -54,7 +52,7 @@ def initialize(user) can [:update, :read, :read_billing_information], Provider, symbol: user.provider_id.upcase can [:manage], ProviderPrefix, provider_id: user.provider_id can [:manage], Client, provider_id: user.provider_id - cannot [:transfer], Client, provider_id: user.provider_id + cannot [:transfer], Client can [:manage], ClientPrefix #, :client_id => user.provider_id # if Flipper[:delete_doi].enabled?(user) From 30066a969b9f889c4e7cdb6e75b35586c438b579 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Mon, 11 May 2020 19:14:57 +0200 Subject: [PATCH 25/28] style: lint --- app/controllers/clients_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index c738280da..d94895fd2 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -124,7 +124,6 @@ def create end end - def update options = {} options[:is_collection] = false From 6cf9eb2469bcfd9839d4f74bdd0aa1932e95b187 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 12 May 2020 09:05:22 +0200 Subject: [PATCH 26/28] refactor: :art: simplifies extracting target --- app/controllers/clients_controller.rb | 2 +- app/controllers/repositories_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/clients_controller.rb b/app/controllers/clients_controller.rb index d94895fd2..1e1792cb2 100644 --- a/app/controllers/clients_controller.rb +++ b/app/controllers/clients_controller.rb @@ -133,7 +133,7 @@ def update # only update provider_id authorize! :transfer, @client - @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) + @client.transfer(safe_params.slice(:target_id)) render json: ClientSerializer.new(@client, options).serialized_json, status: :ok elsif @client.update(safe_params) diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index 2afa4dcf0..01d16451d 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -163,7 +163,7 @@ def update # only update provider_id authorize! :transfer, @client - @client.transfer(target_id: safe_params.slice(:target_id).dig(:target_id)) + @client.transfer(safe_params.slice(:target_id)) render json: RepositorySerializer.new(@client, options).serialized_json, status: :ok elsif @client.update(safe_params) From 56216fa655cdabbb361efdb3d70b5fad16a99526 Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 12 May 2020 09:06:24 +0200 Subject: [PATCH 27/28] refactor: :art: send object rather than id --- app/jobs/transfer_client_job.rb | 6 +++--- app/models/client.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/jobs/transfer_client_job.rb b/app/jobs/transfer_client_job.rb index d0e80c0ba..f0a7c35fd 100644 --- a/app/jobs/transfer_client_job.rb +++ b/app/jobs/transfer_client_job.rb @@ -1,8 +1,8 @@ class TransferClientJob < ActiveJob::Base queue_as :lupo_background - def perform(symbol, options = {}) - client = Client.where(symbol: symbol).first + def perform(client, options = {}) + symbol = client.symbol if client.present? && options[:target_id].present? options = { @@ -14,7 +14,7 @@ def perform(symbol, options = {}) Doi.loop_through_dois(options) - Rails.logger.info "[Transfer] DOIs updating has started for #{client.symbol} to #{options[:target_id]}." + Rails.logger.info "[Transfer] DOIs updating has started for #{symbol} to #{options[:target_id]}." else Rails.logger.error "[Transfer] Error updating DOIs " + symbol + ": not found" end diff --git a/app/models/client.rb b/app/models/client.rb index 80af554ca..24157eede 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -346,7 +346,7 @@ def transfer(options = {}) transfer_prefixes(target_provider.symbol) # Update DOIs - TransferClientJob.perform_later(symbol, target_id: options[:target_id]) + TransferClientJob.perform_later(self, target_id: options[:target_id]) end def transfer_prefixes(target_id) From 2087ae545e5cd0d53e48609694cd6a4421bbc5ae Mon Sep 17 00:00:00 2001 From: kjgarza Date: Tue, 12 May 2020 13:53:03 +0200 Subject: [PATCH 28/28] test: additional tests --- spec/models/client_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb index 65a7733af..69bc481fb 100644 --- a/spec/models/client_spec.rb +++ b/spec/models/client_spec.rb @@ -68,6 +68,22 @@ end end + context "to consortium_organization" do + let(:new_provider) { create(:provider, symbol: "QUECHUA", member_type: "consortium_organization") } + let(:options) { { target_id: new_provider.symbol } } + + it "works" do + client.transfer(options) + + expect(client.provider_id).to eq(new_provider.symbol.downcase) + expect(new_provider.prefixes.length).to eq(1) + expect(provider.prefixes.length).to eq(1) + + expect(new_provider.prefix_ids).to include(prefix.uid) + expect(provider.prefix_ids).not_to include(prefix.uid) + end + end + context "to consortium" do let(:new_provider) { create(:provider, symbol: "QUECHUA", role_name: "ROLE_CONSORTIUM") } let(:options) { { target_id: new_provider.symbol } }