[release blocker] Add content crashes with User reference field

Collision happens between the uid columns in users and users_roles.

Aliasing that field seems to fix the issue.

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 (last patch fixes this with continue)

subscribe

Subscribing and escalating to critical, because with this bug the whole user reference field is not working at all. I will apply the patch later today and report back :)

XiaN, since you are testing, can you check both patches separately ? andypost's is more significant, but both should work.

This does not crash sites, so downgrading from critical.

I will, however i won't have the time to test them on my site until next Thursday, so it might be faster if you test them :)

subscribe

subscribe

The patch in #3 works for me.

Thank you andypost!

Drupal 7.0
References 7.x-2.x-dev, date: 2011-jan-15

#2: references_1004212.patch queued for re-testing.

Tested manually, patch looks good to me.

Side note: #1038102: user ref settings : 'blocked' checkbox does not save

@fgm : if you don't commit this, I will ;-)

The patch in #3 is indeed mostly good. Some nitpicking, and a more basic question:

+++ user_reference/user_reference.module	26 Dec 2010 08:28:51 -0000
@@ -435,65 +435,67 @@ function _user_reference_potential_refer
+        $query->condition('u.name', $string, '=');

'=' is the default on condition(), so not needed

+++ user_reference/user_reference.module	26 Dec 2010 08:28:51 -0000
@@ -435,65 +435,67 @@ function _user_reference_potential_refer
+    if ($account->uid == 0) {

This is probably for another issue, but do we really want to forbid user_referencing anonymous ?

Powered by Dreditor.

In addition, it would be good to have a simpletest that fails prior to patch and succeeds with it, to prevent regressions later on.

Forgot to set status.

Also, if nothing is set in filters, it should mean no filters are applied, as elsewhere in Drupal, so the query should return all users, just as if all filters had been checked individually.

- "if nothing is set in filters, it should mean no filters are applied"

I probably agree - although that's not how current D6 fields work, and not how noderef's 'referenceable node types' checkboxes work either.

But I guess we can assume that there are little chances of actual D6 fields out there with empty referenceable types or roles, since they would be basically useless - aside from noderef fields defined using a view, but for those we overlook the 'referenceable types' setting altogether anyway.

So yes, I think that change would be fine, and a usability win (catch all, including node types / user roles added subsequently), but that's probably outside the scope of this 'unbreak userref' issue, since this would imply some noderef changes as well for consistency.

- "do we really want to forbid user_referencing anonymous ?"
Hm. Different question indeed. For now let's say we do. Currently uid 0 is considered an empty value (see user_reference_field_is_empty()).

However, this uid != 0 check should probably be included in the query rather than in the loop over the result set.

created #1047534: make "no referenceable types" = "no filter" (all types)

#3: 1004212-userref-dbtng.patch queued for re-testing.

Thank you for the patch #3. It is working perfect thanks for Real Name support!

Patch in #3 works great!

+++ user_reference/user_reference.module	26 Dec 2010 08:28:51 -0000
@@ -435,65 +435,67 @@ function _user_reference_potential_refer
+        $query->condition('u.name', $string, '=');

Using '=' brings more readability to code

+++ user_reference/user_reference.module	26 Dec 2010 08:28:51 -0000
@@ -435,65 +435,67 @@ function _user_reference_potential_refer
+    if ($account->uid == 0) {
+      // Skip Anonymous user.
+      continue;
+    }

This place need special processing because UID=0 has no name. So if we really need Anonymous we should care about it's display

I think we need to commit this patch and fix UID0 case in different issue. I have no ideas about writing a tests for this code because it has no UI...

Powered by Dreditor.

+        $query->condition('u.name', $string, '=');

I usually advocate leaving defaults out, but I agree that in the specific code sequence and in the middle of the other "conditional conditions", leaving the explicit '=' operator is clearer.

Added a 'uid <> 0' condition to the query (rather than checking on the PHP side), and some minor code reordering, and committed the attached patch.
Interdiff with #3 attached.

Great step to module release!

Agreed that Anonymous user usage should be discussed but I see not real examples of usage...

Also module planing to support displaying UID0 so it needs own implementation of user_load to assign a name to UID0

Thanks #3 worked for me too.