From a9684e13abe183028e254ca55e777e4f571dcc7f Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Fri, 15 Nov 2024 17:46:40 -0800
Subject: [PATCH 1/5] new account setting to override Bulkrax's field mappings
Add a new Account setting that will override Bulkrax's default field
mappings. This is laying out the groundwork that will allow each tenant
to have its own unique field mappings
---
app/assets/javascripts/application.js | 3 ++-
app/assets/stylesheets/application.css | 1 +
app/models/concerns/account_settings.rb | 23 ++++++++++++++++++---
app/views/proprietor/accounts/edit.html.erb | 2 +-
app/views/shared/_settings.html.erb | 18 +++++++++++++++-
config/initializers/assets.rb | 1 +
6 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index fb0f0e869..19d7834a8 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -49,8 +49,9 @@
//= require hyrax
//= require codemirror
-//= require codemirror-autorefresh
//= require codemirror/modes/css
+//= require codemirror/modes/javascript
+//= require codemirror-autorefresh
//= require iiif_print
diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css
index 3d8476fc6..1a530ae27 100644
--- a/app/assets/stylesheets/application.css
+++ b/app/assets/stylesheets/application.css
@@ -13,6 +13,7 @@
*= require bulkrax/application
*= require fontselect
*= require codemirror
+ *= require codemirror/themes/neat
*= require codemirror-theme
*= require hyrax
*= require dataTables.bootstrap4
diff --git a/app/models/concerns/account_settings.rb b/app/models/concerns/account_settings.rb
index a18c066a5..4546a7421 100644
--- a/app/models/concerns/account_settings.rb
+++ b/app/models/concerns/account_settings.rb
@@ -8,7 +8,7 @@ module AccountSettings
extend ActiveSupport::Concern
# rubocop:disable Metrics/BlockLength
included do
- cattr_accessor :array_settings, :boolean_settings, :hash_settings, :string_settings, :private_settings do
+ cattr_accessor :array_settings, :boolean_settings, :hash_settings, :json_editor_settings, :string_settings, :private_settings do
[]
end
cattr_accessor :all_settings do
@@ -23,6 +23,7 @@ module AccountSettings
setting :allow_downloads, type: 'boolean', default: true
setting :allow_signup, type: 'boolean', default: true
setting :analytics_provider, type: 'string'
+ setting :bulkrax_field_mappings, type: 'json_editor', default: Bulkrax.field_mappings.to_json
setting :bulkrax_validations, type: 'boolean', disabled: true
setting :cache_api, type: 'boolean', default: false
setting :contact_email, type: 'string', default: 'change-me-in-settings@example.com'
@@ -58,7 +59,7 @@ module AccountSettings
validates :contact_email, :oai_admin_email,
format: { with: URI::MailTo::EMAIL_REGEXP },
allow_blank: true
- validate :validate_email_format, :validate_contact_emails
+ validate :validate_email_format, :validate_contact_emails, :validate_json
validates :google_analytics_id,
format: { with: /((UA|YT|MO)-\d+-\d+|G-[A-Z0-9]{10})/i },
allow_blank: true
@@ -70,7 +71,7 @@ module AccountSettings
# rubocop:disable Metrics/BlockLength
class_methods do
def setting(name, args)
- known_type = ['array', 'boolean', 'hash', 'string'].include?(args[:type])
+ known_type = ['array', 'boolean', 'hash', 'string', 'json_editor'].include?(args[:type])
raise "Setting type #{args[:type]} is not supported. Can not laod." unless known_type
send("#{args[:type]}_settings") << name
@@ -136,6 +137,12 @@ def set_type(value, to_type)
value.is_a?(String) ? JSON.parse(value) : value
when 'string'
value.to_s
+ when 'json_editor'
+ begin
+ JSON.pretty_generate(JSON.parse(value))
+ rescue JSON::ParserError => e
+ value
+ end
end
end
@@ -155,6 +162,16 @@ def validate_contact_emails
end
end
+ def validate_json
+ json_editor_settings.each do |key|
+ begin
+ JSON.parse(settings[key])
+ rescue JSON::ParserError => e
+ errors.add(:"#{key}", e.message)
+ end
+ end
+ end
+
def initialize_settings
return true unless self.class.column_names.include?('settings')
set_smtp_settings
diff --git a/app/views/proprietor/accounts/edit.html.erb b/app/views/proprietor/accounts/edit.html.erb
index b835d66a1..64ddbd827 100644
--- a/app/views/proprietor/accounts/edit.html.erb
+++ b/app/views/proprietor/accounts/edit.html.erb
@@ -34,7 +34,7 @@
<%= t(".account_settings") %>
- <% current_account.live_settings.each do |key, value| %>
+ <% @account.live_settings.each do |key, value| %>
<%= render 'shared/settings', f: f, key: key, value: value %>
<% end %>
diff --git a/app/views/shared/_settings.html.erb b/app/views/shared/_settings.html.erb
index ebeeeaad6..8245476f6 100644
--- a/app/views/shared/_settings.html.erb
+++ b/app/views/shared/_settings.html.erb
@@ -1,8 +1,24 @@
<% if value[:type] == 'array' %>
- <% current_account.send(key).each do |sub_value| %>
+ <% @account.send(key).each do |sub_value| %>
<%= f.input key, value: sub_value %>
<% end %>
<% elsif value[:type] == 'hash' %>
+<% elsif value[:type] == 'json_editor' %>
+ <%= f.input key, as: :text, required: false, input_html: { value: @account.send(key) }, label: key.to_s.titleize %>
+
<% else %>
<%= f.input key, as: value[:type] %>
<% end %>
diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb
index 9287bcd33..edd17673c 100644
--- a/config/initializers/assets.rb
+++ b/config/initializers/assets.rb
@@ -10,3 +10,4 @@
# Precompile additional assets.
# application.js, application.css, and all non-JS/CSS in app/assets folder are already added.
# Rails.application.config.assets.precompile += %w( search.js )
+Rails.application.config.assets.precompile += ['codemirror*', 'codemirror/**/*']
From dab59524e6f04a181716f7772a843e099d9fdb90 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Tue, 19 Nov 2024 17:20:18 -0800
Subject: [PATCH 2/5] override Bulkrax with Account's field mapping setting
---
lib/bulkrax/bulkrax_decorator.rb | 12 ++++++++
spec/lib/bulkrax/bulkrax_decorator_spec.rb | 34 ++++++++++++++++++++++
2 files changed, 46 insertions(+)
create mode 100644 lib/bulkrax/bulkrax_decorator.rb
create mode 100644 spec/lib/bulkrax/bulkrax_decorator_spec.rb
diff --git a/lib/bulkrax/bulkrax_decorator.rb b/lib/bulkrax/bulkrax_decorator.rb
new file mode 100644
index 000000000..a16f6641e
--- /dev/null
+++ b/lib/bulkrax/bulkrax_decorator.rb
@@ -0,0 +1,12 @@
+module PerTenantFieldMappings
+ # OVERRIDE: [Bulkrax v8.2.0] Use tenant-specific field mappings if present
+ def field_mappings
+ if Site.account.present? && Site.account.bulkrax_field_mappings.present?
+ JSON.parse(Site.account.bulkrax_field_mappings).with_indifferent_access
+ else
+ super
+ end
+ end
+end
+
+Bulkrax.singleton_class.send(:prepend, PerTenantFieldMappings)
diff --git a/spec/lib/bulkrax/bulkrax_decorator_spec.rb b/spec/lib/bulkrax/bulkrax_decorator_spec.rb
new file mode 100644
index 000000000..2d7246015
--- /dev/null
+++ b/spec/lib/bulkrax/bulkrax_decorator_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+RSpec.describe PerTenantFieldMappings, type: :decorator do
+ before do
+ allow(Site).to receive(:account).and_return(account)
+ end
+
+ context 'when the current Account does not have any tenant-specific field mappings' do
+ let(:account) { build(:account) }
+
+ it "returns Bulkrax's default field mappings" do
+ default_bulkrax_mapping_keys = ['Bulkrax::OaiDcParser', 'Bulkrax::OaiQualifiedDcParser', 'Bulkrax::CsvParser', 'Bulkrax::BagitParser', 'Bulkrax::XmlParser']
+
+ expect(Site.account.settings['bulkrax_field_mappings']).to be_nil
+ expect(Bulkrax.field_mappings).to be_a(Hash)
+ expect(Bulkrax.field_mappings.keys.sort).to eq(default_bulkrax_mapping_keys.sort)
+ end
+ end
+
+ context 'when the current Account has tenant-specific field mappings' do
+ let(:account) { build(:account, settings: { bulkrax_field_mappings: field_mapping_json }) }
+ let(:field_mapping_json) do
+ {
+ 'Bulkrax::CsvParser' => {
+ 'fake_field' => { from: %w[fake_column], split: /\s*[|]\s*/ }
+ }
+ }.to_json
+ end
+
+ it "returns the tenant's custom field mappings" do
+ expect(Bulkrax.field_mappings).to eq(JSON.parse(Site.account.bulkrax_field_mappings))
+ end
+ end
+end
From 3bc0318975de695a35053754f3b8ae36be256a27 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Tue, 19 Nov 2024 17:22:17 -0800
Subject: [PATCH 3/5] add :bulkrax_field_mappings specs for AccountSettings
---
app/models/concerns/account_settings.rb | 2 +
spec/models/concerns/account_settings_spec.rb | 63 +++++++++++++++++++
2 files changed, 65 insertions(+)
diff --git a/app/models/concerns/account_settings.rb b/app/models/concerns/account_settings.rb
index 4546a7421..85e9196bb 100644
--- a/app/models/concerns/account_settings.rb
+++ b/app/models/concerns/account_settings.rb
@@ -164,6 +164,8 @@ def validate_contact_emails
def validate_json
json_editor_settings.each do |key|
+ next unless settings[key].present?
+
begin
JSON.parse(settings[key])
rescue JSON::ParserError => e
diff --git a/spec/models/concerns/account_settings_spec.rb b/spec/models/concerns/account_settings_spec.rb
index a41bce441..a63b14858 100644
--- a/spec/models/concerns/account_settings_spec.rb
+++ b/spec/models/concerns/account_settings_spec.rb
@@ -10,6 +10,7 @@
expect(account.public_settings(is_superadmin: true).keys.sort).to eq %i[allow_downloads
allow_signup
analytics_provider
+ bulkrax_field_mappings
cache_api
contact_email
contact_email_to
@@ -49,4 +50,66 @@
end
end
end
+
+ describe '#bulkrax_field_mappings' do
+ context 'when the setting is blank' do
+ it 'returns the default field mappings configured in Bulkrax' do
+ expect(account.settings['bulkrax_field_mappings']).to be_nil
+ # For parity, parse Bulkrax field mappings from JSON. #to_json will stringify keys as
+ # well as turn a regex like /\|/ into (?-mix:\\|)
+ default_bulkrax_mappings = JSON.parse(Bulkrax.field_mappings.to_json)
+ default_tenant_mappings = JSON.parse(account.bulkrax_field_mappings)
+
+ expect(default_tenant_mappings).to eq(default_bulkrax_mappings)
+ end
+ end
+
+ context 'when the setting is present' do
+ let(:account) { build(:account, settings: { bulkrax_field_mappings: setting_value }) }
+
+ context 'when the value is valid JSON' do
+ let(:setting_value) do
+ {
+ 'Bulkrax::CsvParser' => {
+ 'fake_field' => { from: %w[fake_column], split: /\s*[|]\s*/ }
+ }
+ }.to_json
+ end
+
+ it 'parses the JSON into a Hash and prints it as pretty JSON' do
+ expect(account.bulkrax_field_mappings)
+ .to eq(JSON.pretty_generate(JSON.parse(setting_value)))
+ end
+ end
+
+ context 'when the value is not valid JSON' do
+ let(:setting_value) { 'hello world' }
+
+ it 'returns the raw value' do
+ expect(account.bulkrax_field_mappings).to eq(setting_value)
+ end
+ end
+ end
+ end
+
+ describe '#validate_json' do
+ let(:account) { build(:account, settings: { bulkrax_field_mappings: setting_value }) }
+
+ context 'when a "json_editor" setting is valid JSON' do
+ let(:setting_value) { { a: 'b' }.to_json }
+
+ it 'does not error' do
+ expect(account.valid?).to eq(true)
+ end
+ end
+
+ context 'when a "json_editor" setting is not valid JSON' do
+ let(:setting_value) { 'hello world' }
+
+ it 'adds an error to the setting' do
+ expect(account.valid?).to eq(false)
+ expect(account.errors.messages[:bulkrax_field_mappings]).to eq(["unexpected token at 'hello world'"])
+ end
+ end
+ end
end
From f0efedfb1e14e5052caa4ec1bdcb0dcbfcc446c1 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Wed, 20 Nov 2024 09:54:04 -0800
Subject: [PATCH 4/5] resolve rubocop warnings
---
app/models/concerns/account_settings.rb | 4 ++--
lib/bulkrax/bulkrax_decorator.rb | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/app/models/concerns/account_settings.rb b/app/models/concerns/account_settings.rb
index 85e9196bb..1fb4fc198 100644
--- a/app/models/concerns/account_settings.rb
+++ b/app/models/concerns/account_settings.rb
@@ -140,7 +140,7 @@ def set_type(value, to_type)
when 'json_editor'
begin
JSON.pretty_generate(JSON.parse(value))
- rescue JSON::ParserError => e
+ rescue JSON::ParserError
value
end
end
@@ -164,7 +164,7 @@ def validate_contact_emails
def validate_json
json_editor_settings.each do |key|
- next unless settings[key].present?
+ next if settings[key].blank?
begin
JSON.parse(settings[key])
diff --git a/lib/bulkrax/bulkrax_decorator.rb b/lib/bulkrax/bulkrax_decorator.rb
index a16f6641e..5df4c161a 100644
--- a/lib/bulkrax/bulkrax_decorator.rb
+++ b/lib/bulkrax/bulkrax_decorator.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
module PerTenantFieldMappings
# OVERRIDE: [Bulkrax v8.2.0] Use tenant-specific field mappings if present
def field_mappings
From 780269133643af3a875eb2c135ddb1d2b6dbf488 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Wed, 20 Nov 2024 11:16:57 -0800
Subject: [PATCH 5/5] revert fix unrelated to this PR and add FIXME note
---
app/views/shared/_settings.html.erb | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/app/views/shared/_settings.html.erb b/app/views/shared/_settings.html.erb
index 8245476f6..f358c6dc4 100644
--- a/app/views/shared/_settings.html.erb
+++ b/app/views/shared/_settings.html.erb
@@ -1,5 +1,12 @@
<% if value[:type] == 'array' %>
- <% @account.send(key).each do |sub_value| %>
+ <%# FIXME:
+ I think current_account here should be @account. @account is present both within the tenant
+ as well as in the proprietor views, while current_account is only present within a tenant.
+ Changing it to @account causes a spec in spec/features/accounts_spec.rb to fail, I think due
+ to the form sending inputs that should be treated as arrays as strings.
+ @see AccountSettings#validate_email_format
+ %>
+ <% current_account.send(key).each do |sub_value| %>
<%= f.input key, value: sub_value %>
<% end %>
<% elsif value[:type] == 'hash' %>