hi,

I ve coded an autocomplete widget on top of Search API and i ve found a little bug :

line 453 of node_reference.module should be :

        '#autocomplete_path' => $instance['widget']['settings']['autocomplete_path'] . '/' . $instance['bundle'] . '/' . $field['field_name'],

and so line 677 your @todo broken could be fixed

// $entity_type => $bundle
function node_reference_autocomplete($bundle, $field_name, $string = '') {
  //$field = field_info_field($field_name);
    $instance = field_info_instance('node', $field_name, $bundle);

  // @todo broken - this is in $instance['widget']['settings']['autocomplete_match'] - we'd need the $instance.

maybe you already knew it... was just trying to help !

PS :
what do you think about :
http://drupal.org/project/relation

CommentFileSizeAuthor
#16 993394-references-autocomplete-bundle.patch6.3 KBdave reid
#15 references_autocomplete-993394-15.patch5.64 KByched
#7 1004212-userref-dbtng.patch3.78 KBandypost
#6 1004212-userref-dbtng.patch3.75 KBandypost
#5 references_993394.patch1.98 KBfgm

Comments

danielb’s picture

danielb commented

A few questions, if I may ask.

- Does changing the $entity_type variable to $bundle have any other implications? $entity_type is used in other functions, should those be renamed to $bundle? Or was the name $entity_type used incorrectly in only node_reference_autocomplete()?

- It appears $field is still required to be passed to _node_reference_potential_references(). I notice you've commented $field out in yours - can you explain?

- Does the @todo, and the changes to the node_reference_autocomplete() function also apply to the user_reference module?

I think if these can be addressed, a patch could be made.

quazardous’s picture

quazardous commented

- Does changing the $entity_type variable to $bundle have any other implications? $entity_type is used in other functions, should those be renamed to $bundle? Or was the name $entity_type used incorrectly in only node_reference_autocomplete()?

>>> I think it s incorrect to use entity because fields are attached to bundle

- It appears $field is still required to be passed to _node_reference_potential_references(). I notice you've commented $field out in yours - can you explain?

>>> I rename $field in $instance for consistancy (instance is the "good" name) and it must be rename everywhere

- Does the @todo, and the changes to the node_reference_autocomplete() function also apply to the user_reference module?

>>> sry didn't pay attention to user_reference... but I think there is no different bundles for user entity. so the probleme is less important.

andypost’s picture

andypost
he/him
Primary language Russian
commented

@quazardous Can you provide a patch?

I think that using bundle is preferable. Field or Instance - this depends of context, if we talk about field that attached to bundle so we should use Instance

quazardous’s picture

quazardous commented

i'll try to provide a patch but last weeks of the year are hard :p

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Active » Needs review
StatusFileSize
new references_993394.patch1.98 KB

We cannot just replace $field with $instance everywhere: _node_reference_potential_references_standard(), invoked by _node_reference_potential_references() from node_reference_autocomplete(), makes use of $field['settings']['referenceable_types']: referenceable types are currently a per-field choice, not per-instance. It needn't remain so indefinitely, but this is a wider change.

Patch rolled from the original suggestion, nothing bigger.

Also, for the record, user_reference assumes a single bundle on the user entity, and does not make use of the instance to differentiate per-bundle widgets properties since there is only one.

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 1004212-userref-dbtng.patch3.75 KB

User autocomplete is totally broken now.

Patch fixes following:
- Fixes UID using more conditional logic
- Fixes arguments for filters: array_keys(array_filter($settings)) leaves only checked values for queries
- Used format_username() to allow modules a-la realname to alter user names?

Last one probably useless for this function so maybe it's different issue

Questions raised:
- Defaults for filters:
-- should we display autocomplete widget if nothing is checked in filters?
-- should we display Anonymous user? Currently using "%" as autocomplete value we could get Anonym into list

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 1004212-userref-dbtng.patch3.78 KB

First condition should be '&&' so if both settings empty

Added filter for anonymous user

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Sorry, my last 2 comments are related to #1004212: [release blocker] Add content crashes with User reference field

#5 seems reasonable but I think settings (referenceable types) should be stored per instance but this different issue

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Reviewed & tested by the community » Closed (duplicate)

See #1004212: [release blocker] Add content crashes with User reference field.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Closed (duplicate) » Needs work

Duplicate was obviously only for andypost's patch. I just rerolled the patch in #5 on today's version, but it needs

- to be reviewed
- a simpletest failing before the patch and passing after the patch

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Reviewed & tested by the community

Patch #5 works fine for a long time on one of my sites also this change is required

Possible test case: attach fields to different node types with different field settings and try to get right values from autocomplete. But I've no idea how to test results from autocomplete field because core's simpletest does not allow testing of JS

So it seems more reasonable to commit #5 without test coverage

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1004212-userref-dbtng.patch, failed testing.

johnv’s picture

johnv
Primary language Dutch
commented
Status: Needs work » Needs review

#5: references_993394.patch queued for re-testing.

calte’s picture

calte commented

Can anyone confirm that http://drupal.org/node/1053288 is related?

yched’s picture

yched commented
StatusFileSize
new references_autocomplete-993394-15.patch5.64 KB

Sorry for letting this slip by.

If we want to get the $instance, we need all three of [entity type, bundle, field name].

Attached patch seems to fix the thing for me - also I can't say that I really noticed any issue myself regarding autocomplete (apart from the 'starts with' / 'contains' widget setting which didn't work so far because we didn't get the instance).

I'd welcome a confirmation from the folks that had troubles with autocomplete. @andypost ?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 993394-references-autocomplete-bundle.patch6.3 KB

Re-rolled against current 7.x-2.x-git. Also abstracted the access callback out since it could be used for both node and user references.

yched’s picture

yched commented
Status: Needs review » Fixed

@Dave Reid : thks for the reroll, and good call about the access callback.

Committed roughly #16 (user_reference_autocomplete() still had the old signature). Thanks all !

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.