Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New account setting to override Bulkrax's default split pattern #2380

Conversation

bkiahstroud
Copy link
Collaborator

@bkiahstroud bkiahstroud commented Nov 12, 2024

Ref:

Summary

Add a new Account setting that will override Bulkrax's default multi-value split pattern. This effectively allows each tenant to have its own default split pattern

This change relies on this Bulkrax commit to work:

image

Considerations

The default split pattern is only used when a field mapping specifies split: true:

config.field_mappings['Bulkrax::CsvParser'] = {
  'contributor' => { from: %w[contributor], split: true },

If a field declares a specific split pattern, it will be used for all tenants:

config.field_mappings['Bulkrax::CsvParser'] = {
  'contributor' => { from: %w[contributor], split: /\s*[|]\s*/ },

I think this is okay; in my experience, most installations use a single pattern for the vast majority of their fields. If I'm reading Bulkrax's #process_parse method correctly, that could be used in conjunction with split: false to handle one-off fields with unique split patterns:

config.field_mappings['Bulkrax::CsvParser'] = {
  'contributor' => { from: %w[contributor], split: false },

---

def parse_contributor(src)
  src.split(/custom pattern/)
end

That being said, if you can think of a better way to accomplish this, I'm open to suggestions

@samvera/hyku-code-reviewers

Add a new Account setting that will override Bulkrax's default
multi-value split pattern. This effectively allows each tenant to have
its own default split pattern

This change relies on this Bulkrax commit to work:
- samvera/bulkrax@9268bbb
Copy link

Test Results

    3 files  ±0      3 suites  ±0   18m 21s ⏱️ +39s
2 040 tests ±0  1 988 ✅  - 1  50 💤 ±0  2 ❌ +1 
2 067 runs  ±0  2 013 ✅  - 1  52 💤 ±0  2 ❌ +1 

For more details on these failures, see this check.

Results for commit 0f27e3b. ± Comparison against base commit 8cf3a6f.

This pull request removes 42 and adds 42 tests. Note that renamed tests count towards both.
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 6df95f38-2284-48dc-90da-74f8e2d534e7
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit f5234842-fc4a-4cc0-b259-fa5c45f4a0d0
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read cf13596d-3498-4852-b704-7008dd755a41
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update cdc04d97-794b-47cd-8f67-e2d1262a4206
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 0ee06ccb-f816-41ac-8eb7-c330279d37a5
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit a1b32a54-2dc6-4205-836a-8300338b178e
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read eb9e8faf-1b6c-4789-8fee-10c337dec923
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 8fec72d5-e094-404f-8a10-9843653b508a
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy a28e5006-8694-4f47-b5bb-84afd1597eba
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit 716c1da2-cb5e-4345-a87a-beac86b5f8ca
…
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 154ddb13-f0d6-450c-a95d-fd61ec27611d
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit 04b732f7-494a-44f1-b48f-ba4f35491d86
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read 369c441a-58b1-408e-a76a-167674b31404
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update 73b77517-3f53-4387-9801-9c50c688008d
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 1ca08ef0-6a22-4892-85e0-26c941954740
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit c48ab258-8531-40c6-8994-67a4262760b9
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read 036498a4-eeda-44f9-af43-857cdc01e7a1
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update c42cb286-6046-4b04-9aae-778c08a6f8db
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy 37ae4675-f202-4215-bebd-b9f53da72da9
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit e37cd23f-e550-4b62-8f62-a5e565d42218
…

Copy link
Member

@orangewolf orangewolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of the time I feel like the default split pattern isn't as useful. its better to list the split pattern per field. And if we do that, then this wont help much. lets touch base tomorrow and see if this really solves the problem its pointed at

@bkiahstroud bkiahstroud marked this pull request as draft November 14, 2024 22:55
@bkiahstroud
Copy link
Collaborator Author

This PR has been superseded by:

@bkiahstroud bkiahstroud deleted the new-account-setting-to-override-default-bulkrax-split-pattern branch November 21, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants