Add per-tenant Bulkrax field mappings #2384
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Test Results 3 files ±0 3 suites ±0 18m 18s ⏱️ +19s Results for commit 7802691. ± Comparison against base commit 2ea73b9. This pull request removes 42 and adds 49 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
K8Sewell
reviewed
Nov 21, 2024
K8Sewell
reviewed
Nov 21, 2024
K8Sewell
approved these changes
Nov 21, 2024
bkiahstroud
deleted the
i286-add-per-tenant-bulkrax-field-mapping-editor
branch
This was referenced Nov 21, 2024
laritakr
added a commit
to scientist-softserv/palni_palci_knapsack
that referenced
this pull request
Nov 21, 2024
PR samvera/hyku#2384 added a new account setting.
bkiahstroud
added a commit
that referenced
this pull request
Nov 22, 2024
A recent Hyku feature introduced the ability to configure Bulkrax field mappings on a per-tenant basis. It also introduced a bug that impacted existing Hyku applications who had custom field mappings. Since the feature fell back on Bulkrax's default field mappings if an Account hadn't explicitly set their own, the existing field mapping customizations in the Bulkrax initializer were ignored. To solve that issue, as well as the question of "what if I want all my tenants to have the same field mappings, but don't want to customize them all by hand?", this commit introduces the idea of a set of default field mappings at the Hyku application level. This means that when Bulkrax looks for field mappings, it will discover them in this order (depending on presence): Account setting? -> Hyku defaults? -> Bulkrax defaults There are a couple reasons why I decided to put this in the Hyku module instead of just using Hyku's Bulkrax initializer: 1. Since they're Hyku's defaults, it makes sense to denote them as such semantically and conceptually 2. Since the ultimate fallback is Bulkrax's default field mappings, it makes sense not to alter the field mappings that Bulkrax "manages" with this feature 3. When adding hyku_knapsack into the mix, their often ends up being three separate Bulkrax config files (the knapsack's config/initializers/bulkrax.rb, Hyku's config/initializers/bulkrax.rb, and Bulkrax's lib/bulkrax.rb). This gets very muddy very quickly with how knapsack works technically Ref: - #2384
laritakr
pushed a commit
that referenced
this pull request
Nov 26, 2024
* move Bulkrax field mappings to Hyku A recent Hyku feature introduced the ability to configure Bulkrax field mappings on a per-tenant basis. It also introduced a bug that impacted existing Hyku applications who had custom field mappings. Since the feature fell back on Bulkrax's default field mappings if an Account hadn't explicitly set their own, the existing field mapping customizations in the Bulkrax initializer were ignored. To solve that issue, as well as the question of "what if I want all my tenants to have the same field mappings, but don't want to customize them all by hand?", this commit introduces the idea of a set of default field mappings at the Hyku application level. This means that when Bulkrax looks for field mappings, it will discover them in this order (depending on presence): Account setting? -> Hyku defaults? -> Bulkrax defaults There are a couple reasons why I decided to put this in the Hyku module instead of just using Hyku's Bulkrax initializer: 1. Since they're Hyku's defaults, it makes sense to denote them as such semantically and conceptually 2. Since the ultimate fallback is Bulkrax's default field mappings, it makes sense not to alter the field mappings that Bulkrax "manages" with this feature 3. When adding hyku_knapsack into the mix, their often ends up being three separate Bulkrax config files (the knapsack's config/initializers/bulkrax.rb, Hyku's config/initializers/bulkrax.rb, and Bulkrax's lib/bulkrax.rb). This gets very muddy very quickly with how knapsack works technically Ref: - #2384 * add setter for Hyku.default_bulkrax_field_mappings This can be used by a hyku_knapsack app initializer to override the default Hyku application field mappings * add spec coverage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In multi-tenant Hyku applications, different tenants can have different requirements for field mappings. This is usually fine for the
:fromsetting; if one tenant has a column namedtitle_alphaand anothertitle_beta, both can be added to thetitle's field mapping:fromsetting.However, conflicts arise when tenants have different requirements for other settings, such as
:splitor deciding which field should be considered thesource_identifier. In those cases, having a single, static field mapping config doesn't work.This PR introduces a new
Accountsettings as well as an override to Bulkrax to allow for configuring field mappings completely independently on a per-tenant basis.Screenshots
New account default settings
Override possible by admins within tenant
JSON validation
Importer uses tenant field mappings
@samvera/hyku-code-reviewers