Needs review
Project:
Text or Nodereference
Version:
7.x-1.x-dev
Component:
Text or Nodereference
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
28 Aug 2012 at 00:35 UTC
Updated:
16 Nov 2012 at 00:32 UTC
Jump to comment: Most recent file
Comments
Comment #1
boobaaCould you please provide a _minimal_ step-by-step howto on reproducing this? I don't really get why it should behave like that.
Comment #2
adam1 commentedIt’s straight forward to reproduce the issue:
Install Drupal via Commerce Kickstart Installation-Profile . Add a Textfield/Text-or-Nodereference-Widget to the Producttype under admin/commerce/products/types/product/fields and configure on which Nodetypes it should autocomplete. Add a new Product under admin/commerce/products/add/product. When trying to fill out the field in Question, no existing Node/Nodetitle will be recognized as Reference. The circling animation which normaly indicates the lookup in database is not displayed at any time.
Thanks for your fast Response, i like your Module a lot!
Comment #3
boobaaIt took me a while to pinpoint what's going on here, even after I could reproduce the bug smoothly following your advice. Actually it turned out that text_noderef.module only supports _node_ references at the time being. I think it's not impossible to make it work with other entities as well, but will took more than those some minutes I could afford today working on this issue.
If somebody wants to start working on this, grep for
'node', and make it an argument (in this particular case it should be'product', but actually it could be any other entity as well; so making it an argument seems to be the best way to go). Beware, there are quite many of those occurences…Comment #4
tmsimont commentedThe problem isn't what can be referenced, it's that the field totally fails and acts like a normal text field when the field is placed on a non-node entity type, like a commerce product.
I'm having the same issue.
Comment #5
tmsimont commentedI altered the json callback to work with non-nodes, but the field is mysteriously still just a regular old text field. I wonder if commerce isn't tweaking it...
Comment #6
tmsimont commentedhmm i take my last comment back, even though I managed to pass entity type to
text_noderef_json(), it looks likefield_info_instance()returns nothing for commerce fields.Comment #7
tmsimont commentedok so now i know text_noderef_json() is returning valid data, but somehow the field itself is still just a plain old textfield. Is there anything converting it to an autocomplete other than the setting of
$element['value']['#autocomplete_path']inhook_field_widget_form()?Comment #8
tmsimont commentedOK I think I got it!
Patch
Comment #9
boobaaFirstly, if you want to make the JSON callback work with other entities than node, then it should be pretty straightforward that the
user_access('access content')check should be removed as it has nothing to do with other entities than nodes.Secondly, you seem to be patching only the JSON callback, which only takes care of the autocompletion part. The display part is totally missing from your patch, which would result in a text_noderef.module that helps entering user/product/whatever entities, but would never display them as links.
Marking this needs work because of these.
Comment #10
tmsimont commentedOk I addressed the
user_access("access content")part, but as far as I can tell the second issue you mentioned is not an issue...I see links when I view the field even on a commerce product entity. All of the "node" references in the implementation of
hook_field_formatter_view()are only related to the actual reference value, not the referring field.I think there is some confusion here about the original issue. The problem is not referencing other entity types. The original problem is that the field acts like a normal text input when is placed onto a non-node entity. This is because of checks in the JSON callback that pull field info for only node entity types. My patch addresses this. My patch does not address references to non-nodes. Addressing that would require a new field settings form and the alterations you reference above. That would be a lot more work than just making the field work when used on non-nodes.
Does that make sense?
Comment #11
tmsimont commentedComment #12
boobaaOkay, I finally seem to understand your problem: A. you want to add a nodereference field to a non-node entity (such as user, commerce product, comment, taxonomy term, anything), B. with an autocomplete widget, C. which gets formatted simply as text if there's no node with the given title. While A and B could be simply addressed with the nodereference.module (and I was tempted to simply get you off towards that), the C is the one where text_noderef.module kicks in.
Your patch seem to solve this (though I'd like actually checking it before committing), but why don't you take this a bit more forward and do the other part of the dance? It would be much more useful to have a text_noderef field which is actually a text_entityreference: A. an entity reference field for any entity (node, user, commerce product, comment, taxonomy term, anything), B. with an autocomplete widget (where one can select any entities and any bundles for the autocompletion), C. which gets formatted simply as text if there's no entity with the given title (in the selected entities and bundles).
On the other hand, I can understand why you don't do the other part of the dance: the functionality described above seems to be pretty much difficult to implement. Besides that, I don't know:
- if the entity-related A and B could be handled by entityreference.module;
- if the entity-related C could be added to entityreference.module as a patch if A and B already exist there.
Finally, if all these gets implemented, this text_noderef.module should be renamed to text_entityref.module. ;)
To sum up: I'm glad to commit this as-is after checking it myself.
Comment #13
adam1 commented@tmsimont:
Your patch is working superb – Thanks!
Comment #14
tmsimont commentedthanks adam1
@Boobaa -- I like the idea of making this a more robust entity reference module, but I don't have the need for that right now nor do I have the time to develop that at the moment. Maybe at some point in the future I will have a need and make time for development of that. In the mean time though, all I really have to offer is the patch to address the original issue.