Synopsis

This module provides a hybrid field type that accepts either text or entity reference as its value, allowing fields to optionally reference arbitrary entities via their label.

The project page is currently located at:
https://www.drupal.org/sandbox/pascalanimateur/2499833

Git clone command:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/PascalAnimateur/2499833.git text_or_entity

Features

  • Allowed entity type / bundles and maximum length of text value are configured in field settings.
  • Autocomplete widget combines results based on previously entered text values and labels of allowed entity references. Limit on the number of suggestions and match operator (starts with / contains) are configured in widget settings.
  • Field formatter optionally displays label as a link to the referenced entity if one exists, or as text otherwise.
  • Inserting, updating or deleting referenced entities also updates field values where they are referenced, so changing the node title also changes the text value on a field referencing that node as would be expected.
  • Support for the Real Name module is transparent to the user.
  • Views integration exposes direct and reverse relationships just like regular entity references.

Current release state

The module is fully functional and being actively tested in some of my actual projects. Integration with the Feeds module is something I'll be looking into shortly, mostly allowing text_or_entity fields to accept both text values and entity references.

Similar modules

The code is largely based on the Text or Nodereference module (last commit 2013-Oct-19) but overcomes its limitations of being only able to reference nodes by fully integrating with Entity API. I have contacted the author of text_noderef to ask for his collaboration but haven't heard back from him yet.

Other project applications I did

This is my second third attempt at proposing a full project, my first attempt for a video conversion wrapper (here) was deemed too similar to other modules and I decided to use mine internally for my projects. My second attempt was for a previous version of this module (I used the same proposal issue for clarity's sake) using an approach that wasn't useful for views integration (see my comments below).

CommentFileSizeAuthor
#9 d7_text_or_entity-2488942-9.patch729 bytesayesh

Comments

PA robot’s picture

PA robot commented
Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxPascalAnimateur248754...

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

PascalAnimateur’s picture

PascalAnimateur commented
Status: Needs work » Postponed

Marking as "Postponed" since I'm rewriting this module so that it integrates more closely with entityreference (using entityreference_autocomplete) by defining a custom field type instead of just using a text widget+formatter. Besides, having the target entity type settings at the field instance level makes views relationship a nightmare to implement, while a custom field type solves this.

PascalAnimateur’s picture

PascalAnimateur commented
Status: Postponed » Closed (won't fix)
PascalAnimateur’s picture

PascalAnimateur commented

I've completely rewritten the module to implement the ideas outlined in comment #2 above... will update the original issue as soon as everything is ready for promotion to full-project.

PascalAnimateur’s picture

PascalAnimateur commented
Issue summary: View changes
PascalAnimateur’s picture

PascalAnimateur commented
Status: Closed (won't fix) » Needs work
PascalAnimateur’s picture

PascalAnimateur commented
Status: Needs work » Needs review
PascalAnimateur’s picture

PascalAnimateur commented

No errors reported by the automated review tools except for missing tests!!

ayesh’s picture

ayesh commented
Issue tags: +PAreview: security
StatusFileSize
new d7_text_or_entity-2488942-9.patch729 bytes

Hi Pascal,
Thank you for your contribution. I'm sorry that your other two applications didn't went through. Your contributions are appreciated nonetheless.

I have taken a real good look at the module, and found a few major changes that needs to be fixed. Some are just minor concerns though.

- Pareview is clean. Awesome work!

- Module uses PHP 5.5-style array operators. Unless it's a strict requirement, modules should run in PHP 5.2 at least (the Drupal core requirement). There's only one issue of this ($target_entity = $target_entity[array_keys($target_entity)[0]];)

- Most importantly, module should call entity_access to ensure the current user has access to the target entities.
You have added an access callback to the autocomplete path, which is great. However, we need to make sure that even if the user has access to edit that particular node, he has access to refer the target nodes as well. entity_access will take care of access callback defined for each entity, including nodes. Check the attached patch for a fix. Without the access check, anyone who is permitted to access to edit the field can look up all target entity labels regardless of the access level. This includes node titles of unpublished nodes, nodes protected by other node access modules, custom entities, etc.

Note that entity_access returns NULL if the entity has no access call, hence the strict FALSE comparison.
I added the PAReview: Security tag solely to help others and for statistical purposes. Please do not remove it. We can of course proceed with the applications once this is fixed.

Please note that the entity_access needs to go into text_or_entity_field_validate() and whenever it is validated/rendered too.

- Is there any specific reason the module queries the field data tables directly? You can use EntityFieldQuery::fieldCondition() to query on field conditions.

- In text_or_eneity_field_validate() the entity_extract_ids($entity_type, $target_entity) call uses wrong entity type. It has to be entity_extract_ids($target_type, $target_entity);

- In text_or_entity_field_formatter_view(), the [entity-type]/[entity-id] URL pattern is always expected, which is not the case (taxonomy terms do follow this pattern for example). A better way would be to load all the entities first (you have the entity type in field settings) all of them, at once, run entity_access, and then entity_uri() to build the URLs.

ayesh’s picture

ayesh commented
Status: Needs review » Needs work
PascalAnimateur’s picture

PascalAnimateur commented

Wow! Thanks for taking the time to review with such careful attention to details! I will implement the suggested changes shortly, plus I've made great progress with feeds integration (importing text value also sets the target_id if it exists, while importing target_id sets the text value). While working on this, I had to use entity_base_table as a dependency, which made me realize I should call the source_type column in the schema base_type instead.. Plus i'll double check my variable names to make the whole thing really coherent.. Just so you know next time you look at the code.

As for why I'm directly querying the field data tables, in the case of realname this is needed I think (does realname register a property in the user entity?). Would using EFQ be faster/safer/better practice?

ayesh’s picture

ayesh commented

Thanks.
Real name does not alter the user entity, so it won't work with EFQ you are right.

Feeds integration would be really nice. Anyone who imported content via Feeds would have realized entity references are difficult to implement. You are free to add it before or after approving this application.

PascalAnimateur’s picture

PascalAnimateur commented

Feeds support will definitely be there as I need it for at least two of my current projects and I'm really close to getting both feeds targets working (value and target_id). So by this weekend I should have everything ready for full project release.

The only problem I can see with supporting realname is not being able to propagate an updated text value back to the entity label when updating the field as it is token based. Updating text values currently flows the other way (changing entity label updates field text value only), so it's not a problem for now, but configurable two-way data binding would be a nice setting to add in the field / instance settings. I'll document this behavior on the project page and README.

PascalAnimateur’s picture

PascalAnimateur commented
Status: Needs work » Needs review

I think I've addressed all of your concerns:

  • Converted PHP 5.5 style array operators to use reset() instead of [0]
  • I've committed your patch for checking entity view access rights in field validation, plus I've added the same check in the display formatter.
  • Fixed the wrong entity_type / target_type and refactored variable names to be more descriptive (base_* / target_* insteat of entity_*). The code is way clearer now!
  • Use entity_uri to return path to referenced entity instead of hand-crafting the link.

Plus I've added Feeds integration as described in my last comment, it works for both value and target_id importing (using both together gives abcolute precedence to the last one in the mapping list). Entity type / bundle filtering seems to be working too.

I'll be doing some more testing until monday, but I'm confident the code is already in beta stage, what do you think?

Thanks again for your time reviewing this, I've learned a lot in the process!

PascalAnimateur’s picture

PascalAnimateur commented

I've added support for user entity selection based on target user's role(s), plus I've fixed a bug with duplicate entries in the field_map.

Will be looking at field_collection integration soon...

vgriffin’s picture

vgriffin commented

I tested the module and found a very serious problem, something I consider a blocker. There is no way to tell it how to deal with duplicate values. The entity label field is used for node matches, but that field is not guaranteed to be unique. There is no way for the user to tell that the entity being referenced is not unique. If the wrong entity is chosen by the module, there is no way for the user to change the selection. If a text match is turned into an invalid entity reference, there is no way to undo that reference.

The described use case is to match authors' names in an academic setting. I graduated from a university with about 3,000 students, graduate and undergraduate. At this university, I knew of four distinct sets of duplicate names. It would be impossible to use this module for the desired purpose, even at a university this small.

vgriffin’s picture

vgriffin commented
Status: Needs review » Needs work
PascalAnimateur’s picture

PascalAnimateur commented

Thanks for your time and the feedback, I'll look into these issues when I get the time to sort this out.

What do you think would be the best way of resolving duplicate conflicts? I can imagine using something similar to entityreference, i.e. showing the entity id in parentheses after the label in the autocomplete suggestions, but this wouldn't be practical for newly created entities with existing id. Perhaps a warning containing links to referencing host entities would be sufficient to let the user know some text_or_entity fields have to be manually verified? And maybe the default behaviour could be configured in the field settings, i.e. keep text value / keep existing referenced entity / update to latest entity ?

I would like to know if this approach would be sufficient to get the module officially released, and if you think such a module is relevant to the drupal community in general...

vgriffin’s picture

vgriffin commented

This is not an easy problem, and there cannot be a perfect solution.

I like the idea of using a text match to do a first-level match test, including autocomplete. I also like the idea of keeping the entity reference even if the name no longer matches, although there should be a way to remember the previous name(s). Otherwise, it might be confusing to users if associated publications do not list the name of the person they're looking up. You would want to offer an "upstream update" when a user changes a referenced field or adds a new referenceable entity.

Perhaps, the autocomplete could use only text values of instances of this particular field. You might want to default to a larger number of values and allow a count of zero to include all unique matching values. Then, when the autocomplete is chosen or created, appropriate fields in the candidate entities could be shown for selection (possibly name, department, title, and photo), along with a "text-only" choice. Updates would need to offer the same choices.

Have you used the Entity Reference View Widget? It allows users to select an entity reference based on values in one or more fields. Your module is taking a sidestep from that module, but you should look a it for ideas.

Thanks for tackling this problem.

PascalAnimateur’s picture

PascalAnimateur commented

Concerning the "upstream updates", this is what I described as "default behaviour setting".. i.e. What happens to an existing text value/reference in a text_or_entity field when it's referencing something that has changed its label.

I see three cases:
- Always keep the existing text value, breaking the reference (display message informing the use of broken references and possible entities with the same label, if any)
- Update the text value to reflect the new entity label (could also display a message to inform the user of which referencing entities are affected, and which other entities match this new value)
- Leave the text value and entity reference intact, even if the label doesn't match anymore (and deal with the conflict manually, again through links in a warning message)

I havent 't thought of offering the option of a text-only choice even when matching entities exist. We might also have the option of explicitely referencing an entity through a text value that doesn't match the label (a way of validating case 3 above). This would be very powerful for the users when choosing option 3. Again, these features could be turned on/off on a per-field basis to allow fine grained control over the module's behavior.

As for entity reference view widget, I haven't used it yet but like the idea. Actually, it would be very beneficial for text_or_entity to reuse components from entityreference, in order to offer better integration with the entityreference ecosystem of modules. But this might prove to be very difficult..

Thanks again for your feedback!

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.