mjordan / islandora_workbench

A command-line tool for managing content in an Islandora 2 repository
MIT License
24 stars 40 forks source link

Workbench fails when the Domain Access module is installed #796

Open g011um opened 3 months ago

g011um commented 3 months ago

The Domain module allows a Drupal site to serve content for multiple configured domains from the same Drupal instance; essentially a type of multi-site Drupal install. The Domain Access module allows individual nodes to be tagged as belonging to one of more configured domains. When installed, the Domain Access module adds a required entity reference field to all content types -- field_domain_access which is a reference to a Domain entity.

Workbench does not understand Domain entity references, and will throw an error when attempting a create or update task with records that contain data for this required field. The error occurs in workbench_fields.py in the create and update methods of the EntityReferenceField class. The variable target_type is set to the type of referenced entity. When the referenced entity type is unknown to workbench, this variable will have an empty/null value and will cause a "UnboundLocalError: local variable 'target_type' referenced before assignment" later in the method.

I have found a fix for this issue which simply requires the following 2 lines to be added to the create and update methods in EntityReferenceField to initialize the target_type variable to domain when a Domain entity is encountered.

        if field_definitions[field_name]['target_type'] == 'domain':
            target_type = 'domain'

I can submit a pull request for this is that's preferrable

mjordan commented 3 months ago

Thanks @g011um I'll run tests on this branch and if nothing breaks, I'll merge. We'll want to document this new feature as well, probably by adding a new section to https://mjordan.github.io/islandora_workbench_docs/fields/#drupal-field-types.

Question: is the fieldname field_domain_access hard coded in (or a hard expectation of) the Domain module?

g011um commented 3 months ago

@mjordan field_domain_access is hard coded into the Domain Access module. When the module is installed, it is added as a required field to all content types on the system.

dara2 commented 2 hours ago

Hi @mjordan - we have a client using Domain modules and I am running into this issue as well. It passes --check but when I try to run the create task, I get:

Born-Digitals-MacBook-Air-4:islandora_workbench Dara$ ./workbench --config create_sample_staging.yml
OK, connection to Drupal at https://i2-staging.digital.library.pitt.edu verified.
"Create" task started using config file create_sample_staging.yml.
Only node IDs for parents created during this session will be used (not using the CSV ID to node ID map).
╭───────────────────── Traceback (most recent call last) ──────────────────────╮
│                                                                              │
│ /Users/Dara_1/islandora_workbench/./workbench:3569 in <module>               │
│                                                                              │
│   3566                                                                       │
│   3567 try:                                                                  │
│   3568 │   if config["task"] == "create":                                    │
│ ❱ 3569 │   │   create()                                                      │
│   3570 │   if config["task"] == "update":                                    │
│   3571 │   │   update()                                                      │
│   3572 │   if config["task"] == "delete":                                    │
│ /Users/Dara_1/islandora_workbench/./workbench:283 in create                  │
│                                                                              │
│    280 │   │   │   # Entity reference fields (taxonomy_term and node).       │
│    281 │   │   │   if field_definitions[custom_field]["field_type"] == "enti │
│    282 │   │   │   │   entity_reference_field = workbench_fields.EntityRefer │
│ ❱  283 │   │   │   │   node = entity_reference_field.create(                 │
│    284 │   │   │   │   │   config, field_definitions, node, row, custom_fiel │
│    285 │   │   │   │   )                                                     │
│    286                                                                       │
│                                                                              │
│ /Users/Dara_1/islandora_workbench/workbench_fields.py:831 in create          │
│                                                                              │
│    828 │   │   subvalues = self.dedupe_values(subvalues)                     │
│    829 │   │   for subvalue in subvalues:                                    │
│    830 │   │   │   subvalue = str(subvalue)                                  │
│ ❱  831 │   │   │   field_values.append({"target_id": subvalue, "target_type" │
│    832 │   │                                                                 │
│    833 │   │   cardinality = int(field_definitions[field_name].get("cardinal │
│    834 │   │   if -1 < cardinality < len(field_values):                      │
╰──────────────────────────────────────────────────────────────────────────────╯
UnboundLocalError: local variable 'target_type' referenced before assignment

Is there a PR I could help test?

mjordan commented 2 hours ago

Yes, the one provided by @g011um , linked above.

dara2 commented 2 hours ago

I didn't see a PR - just the commit and you asking them if they were going to make a PR? I just tried to make one but I'm not a collaborator :) Sorry if I am missing something...

mjordan commented 2 hours ago

Sorry, over at https://github.com/usask-library/islandora_workbench/commit/137971b2629d3613c28e761203b4528f073cf566 I ask if there is a PR forthcoming... since that branch is now 64 commits behind main I'll add these changes to a new branch in https://github.com/mjordan/islandora_workbench this evening and then ask you to test that. That OK?

dara2 commented 2 hours ago

That's awesome - thanks!

dara2 commented 2 hours ago

By the way - I just manually made those edits to my local workbench_fields.py file and it seems to be working!

mjordan commented 2 hours ago

That is awesome! Thanks for testing and thanks @g011um for the code contribution.

I'd love to get some basic docs to explain under what situation you'd use this (I know absolutely nothing about domain access) and some sample CSV to include in the docs. Can one of you provide that?

dara2 commented 2 hours ago

Sure! It may not be til next week but I can definitely provide those.

mjordan commented 2 hours ago

No problem at all.

mjordan commented 2 hours ago

Thanks! I just changed "same CSV" above to "sample CSV" (typo). No need for a lot, just enough to illustrate how to populate the CSV will suffice.