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

Add term support for entity_reference_view_endpoints #646

Merged
merged 6 commits into from
Apr 12, 2024
Merged

Add term support for entity_reference_view_endpoints #646

merged 6 commits into from
Apr 12, 2024

Conversation

seth-shaw-asu
Copy link
Contributor

Link to Github issue or other discussion

#642

What does this PR do?

Uses the configured endpoints for term lookups.

What changes were made?

  • Adds a check if the entity_reference_view_endpoints config is set up
  • Uses the configured endpoint for a given field to perform the lookup
  • If the term is not found and a vocabulary is provided, create the term
  • Additional check provides a warning and omits the field value if nothing is found or created

How to test / verify this PR?

Set up entity_reference_view_endpoints in your config file, e.g.

entity_reference_view_endpoints:
  field_linked_agent: /taxonomy/linked_agents
  field_language: /language/lookup
  field_subject: /taxonomy/subjects

See #452 (comment)

Perform a create or update action with fields with and without the term vocabulary that do and don't exist and see the appropriate log messages appear.

E.g.

node_id,field_linked_agent
118116,relators:pbl:Arizona State University
118117,relators:pbl:corporate_body:Arizona State University
118118,relators:pbl:DOES NOT EXIST
118119,relators:pbl:corporate_body:DOES NOT EXIST

Interested Parties

@mjordan


Checklist

  • [:heavy_check_mark:] Before opening this PR, have you opened an issue explaining what you want to to do?
  • [:heavy_check_mark:] Have you run pycodestyle --show-source --show-pep8 --ignore=E402,W504 --max-line-length=200 yourfile.py?
  • [:heavy_check_mark:] Have you included some configuration and/or CSV files useful for testing this PR?
  • [ No ] Have you written unit or integration tests if applicable?
  • [ No ] Does the code added in this PR require a version of Python that is higher than the current minimum version?
  • [ N/A ] If the changes in this PR require an additional Python library, have you included it in setup.py?
  • [ N/A ] If the changes in this PR add a new configuration option, have you provided a default for when the option is not present in the .yml file?

@seth-shaw-asu
Copy link
Contributor Author

Any sense of when we could get this merged, @mjordan? I'd like to get our staff off my fork so they can get the newest updates.

@mjordan
Copy link
Owner

mjordan commented Apr 10, 2024

@seth-shaw-asu thanks for resurrecting this, I'm sorry to let it drop. I'll review ASAP.

@mjordan
Copy link
Owner

mjordan commented Apr 10, 2024

Got this all configured and it works (only create with no adding terms tested so far) but I have a question about the formatting of the config block:

entity_reference_view_endpoints:
  field_linked_agent: /taxonomy/linked_agents
  field_language: /language/lookup
  field_subject: /taxonomy/subjects

This is valid YAML but it other workbench config settings like this (e.g. CSV field templates) use a different format:

entity_reference_view_endpoints:
 - field_linked_agent: /taxonomy/linked_agents
 - field_language: /language/lookup
 - field_subject: /taxonomy/subjects

Any reason you chose to use the if config.get("entity_reference_view_endpoints", {}) syntax instead of calling the get_entity_reference_view_endpoints(config) function, which returns an ordinary dict which would have a .get method?

@seth-shaw-asu
Copy link
Contributor Author

For the config, it made getting the endpoint quicker. Right now I can access the URI by simply doing entity_reference_view_endpoints.get(field_name, False) whereas the formatting you suggest means I have to iterate through an array of configurations to find one with the field_name->endpoint I need.

As for get_entity_reference_view_endpoints(config); I probably didn't realize it existed.

@mjordan
Copy link
Owner

mjordan commented Apr 10, 2024

I'm concerned that the difference in formatting of the config block is going to cause confustion, but I guess some clear docs will mitigate against that.

@seth-shaw-asu
Copy link
Contributor Author

@mjordan , I updated the code to use the get_entity_reference_view_endpoints(config) call. This call actually uses the config structure you recommended but turns it into the structure I need for easy reference. Win-win.

@mjordan
Copy link
Owner

mjordan commented Apr 10, 2024

create and update tasks that use terms that exist within the View are working as expected. I'll move on to testing non-existent terms/terms outside the View next, probably this evening.

As usual, this is awesome and relieves a major UX glitch in Workbench. Thanks!

@mjordan
Copy link
Owner

mjordan commented Apr 11, 2024

Using terms that are not in the linked vocabulary, with allow_adding_terms: true, doesn't work the same as it does for ordinary linked vocabs:

In create tasks:

  • Term in CSV is not in vocab and would not be in View: term is not added, node is not created
  • Term in CSV is not in vocab but would be in View: term is added, node is not created
  • Term in CSV is in vocab but not in View: term is not added, node is not created

In update tasks:

  • Term in CSV is not in vocab in View, term is added (as long as it has the correct vocabulary namespace), node is not updated

One thing that might be problematic is that unless there is a way to programatically check which vocabulary/vocabularies are used in the View, we may need to require the vocab ID in the config. Otherwise, a user could include the wrong namespace in the CSV value and Drupal would not create the node.

I'm OK with merging as is so people can at least use term names or URIs in their CSV and get the rest working later. Any objections? I can then update the docs.

@seth-shaw-asu
Copy link
Contributor Author

I would appreciate it.

Yes, it doesn't work exactly like the ordinary linked vocabs, but I think you can agree that once a site decides to use "Filter by an entity reference View" they need to account for the realities of that option.

@mjordan
Copy link
Owner

mjordan commented Apr 11, 2024

I think you can agree that once a site decides to use "Filter by an entity reference View" they need to account for the realities of that option.

I totally agree, they are a royal PITA. I'll merge this evening and document. Thanks again for working on this.

@mjordan mjordan merged commit 13f3618 into mjordan:main Apr 12, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants