Opened 7 years ago

Closed 6 years ago

#1557 closed bug (fixed)

Wrong number of members shown for people not logged in

Reported by: globetrotter_tt Owned by: jsfan
Priority: critical Milestone: 0.9
Component: BW Search Keywords:
Cc:

Description

from BeWelcome feedback:

Hello,

I have a friend who wanted to register , but got the idea to do a "searchmembers " for finland before doing it, then the website replied "4 members" which resulted in him giving up the idea to join BW...

I just did a search: this is replying 119 members.

So the problem seems to be that he wasn't loggued: Is it a privacy concerns, or some bug ? The count of hidden members should at least be listed. We may be losing a lot of potential new members here.

BW Rox Version: 419278c Browser: Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0 Languages: fr,fr-fr;q=0.8,en-us;q=0.5,en;q=0.3 Request URI: http://www.bewelcome.org/forums/s1885-_unlogged__searchmembers_gives_wrong_results/

Change History (31)

comment:1 Changed 7 years ago by planetcruiser

this ticket is about "map search". "browse by country" shows all members in a country - if you click on a member that has set their profile to non-public, you are asked to log in.

we should use a similar approach in the map search results: list all members, regardless if their profile is public or not, and ask to log in if a private profile is clicked.

comment:2 Changed 7 years ago by planetcruiser

  • Priority changed from major to critical

raising priority. it's easy to fix, and higher member counts will encourage more members to join. hopefully. ;)

comment:3 Changed 6 years ago by jsfan

  • Milestone changed from unassigned to 0.9
  • Owner set to jsfan
  • Status changed from new to assigned

comment:4 Changed 6 years ago by crumbking

Just add a related ticket: #1630

comment:5 Changed 6 years ago by mahouni

hi jsfan, sorry, to comment on that ticket only now. I don't understand exactly how the work on that ticket is meant.
Will you only show the member count of possible members or list them in the search result too? If the later is true, I disagree and would see it as a privacy concern and an unnecessary change. I think it is too much to show the information about profile summary, location, last login, member since, age, accommodation status in combination with a user who has chosen to set his profile visible to members only. In that case make sure that

  • in the result list no last login,age and member since is shown for non public profiles
  • in the map no profile summary is shown for non public members
  • the result list is sorted by public members first, and the non-public members later sorted by username only.

In my opinion it is enough to show the non-public members count only and add a note that by logging in or registering there is the posibility to see those members.

comment:6 Changed 6 years ago by jsfan

I am not showing any more than before. All I do is give two counts.

Except for the wording, I'm done already. Now the question is, what we want to output and if we want to add links of some description. I still don't know if the quick search is ever used at all, so I've only worked on the map search.

My suggestion would be a wording like

X members of Y shown (another Z members visible to [members only])

where [members only] is a link to signup.

comment:7 Changed 6 years ago by jsfan

Alternatively, we could make it

X members of Y shown (Z members visible to [BW members])

where Z would then be the full count instead of the difference.

comment:8 Changed 6 years ago by crumbking

Better we link to the login widget which shows the signup link anyway.

So X members of Y shown (Z members visible to members)

where members is a link to the login widget.

comment:9 Changed 6 years ago by mahouni

okay, thank you! it wasn't clear for me from the discussion here, which approach it would be.

I prefer the second wording too.

comment:10 Changed 6 years ago by crumbking

Would be great to come back to search members page after login with something like:

$login_url = 'login/'.implode('/', $request);

comment:11 Changed 6 years ago by jsfan

I've just pushed this patch and deployed alpha. The patch is ugly but even after thinking hard I could not think of a pretty version of it.

Also the link to the login widget is a hard-coded relative link. I suppose it could be done better particularly if crumbking's suggestion was taken into account. However, I suggest to create a new ticket for this if the patch works.

Let me know if there are performance issues. That's what I am most worried about.

This also fixes #1630.

The patch also introduces two new strings which need to be added to the words table.

Last edited 6 years ago by jsfan (previous) (diff)

comment:12 Changed 6 years ago by mahouni

It works fine on alpha. But on my local installation I get and error when I click on the link loggedInMembers.

"The requested URL /bewelcome/login/searchmembers was not found on this server." This worked for me:

<a href="login/searchmembers">' + loggedInMembers + '</a>

comment:13 Changed 6 years ago by jsfan

I've now changed the link to read

<a href="login/searchmembers#login-widget">' + loggedInMembers + '</a>

Not sure if we really need the anchor but I think it doesn't hurt. :)

Deployed on alpha.

comment:14 Changed 6 years ago by globetrotter_tt

This looks good. But what should be written in "285 membersVisibleTo loggedInMembers" ?

Christian, i just gave you rights to edit english and german text on the website. When you scroll down, you should see an "edit" and "translate" link below the languages in the footer

comment:15 Changed 6 years ago by jsfan

I added the strings. They are just the identifiers in proper strings. :)

comment:16 Changed 6 years ago by mahouni

One more thing:
maybe add a line break and display the text in parentheses on the next line?

25 members displayed of 817 found
(18289 members visible to logged in members)

comment:17 Changed 6 years ago by jsfan

I think it is better the way it is. I added a line break in my browser and it looks crap. Have you tried modifying it in your browser?

Any other opinions on this?

comment:18 Changed 6 years ago by globetrotter_tt

I just tried in my browser and without line break it looks much better.

Another thing: I tried to search for "France" not logged in, the correct section of the map is shown, but i don't get any results. It worked with all other countries so far. Maybe it's because we have "too many" members in France? Can anyone reproduce this? (Tested on Linux 3.2.0.3 with Iceweasel 15 and WinXP IE8)

comment:19 Changed 6 years ago by jsfan

I can confirm that the search for France does not work (Gentoo Linux/Kernel? 3.5.0/Firefox 14.0.1).

EDIT: I just had a look at the AJAX response and it contains members. So, I suspect something is going wrong in the JavaScript?. Firebug does not show any errors, however.

However, I have observed other problems which might be related... When I search for "Melbourne, Australia" I get no hits but there will be 70 members within the map boundaries. Is the radius for the search just too small? But then again, Paris covers not too small an area, either, and there it works ok (2/3 of members within map boundaries shown straight away).

Oh, and searching Antarctica is unreliable as well. While the map zooms in on Antarctica, all members anywhere on the globe are shown. Now, that's a bug we really need to fix... ;)

Last edited 6 years ago by jsfan (previous) (diff)

comment:20 Changed 6 years ago by jsfan

Actually, I just realised myself that my patch still has a weakness. I don't handle a zero result properly. Because of how my query is organised, I will not know the number of members whose profiles are private, either. :(

I've just changed my patch and redeployed alpha. :)

EDIT: "No public profiles but members exist" can be tested e.g. with The Gambia, Togo, Tibet, Malta, Laos or Cambodia. ;)

Last edited 6 years ago by jsfan (previous) (diff)

comment:21 Changed 6 years ago by globetrotter_tt

Should we set this ticket to fixed as the number of members is now shown correctly?

I noticed that the "France-problem" exists also on production. Maybe we should leave this for milestone 1.0 as Toub has already done many changes in the search with the switch to OSM.

comment:22 Changed 6 years ago by mahouni

  • Resolution set to fixed
  • Status changed from assigned to closed

There already is a ticket regarding the "France-problem" :)
http://trac.bewelcome.org/ticket/1631

I tested the new changes, everything okay!

An you are right, one line looks much better.

I am closing the ticket now.

comment:23 Changed 6 years ago by mahouni

  • Resolution fixed deleted
  • Status changed from closed to reopened

hmm, found a minor issue:

searching for "Bellinzona" on alpha.bewelcome.org shows "no members found" when logged out (no additional link to log in). But when logged in there is 1 member (using text search too). This happened when I was using Safari, with Firefox it worked.

Don't know if that's really an issue. But I thought it's better to reopen the ticket, so that someone else can check too.

Last edited 6 years ago by mahouni (previous) (diff)

comment:24 Changed 6 years ago by jsfan

I get the member you're talking about when searching on alpha.

comment:25 Changed 6 years ago by jsfan

  • Resolution set to fixed
  • Status changed from reopened to closed

Cannot confirm with neither FF nor Opera. globi also could not confirm on FF. I suspect this was a caching issue of some description.

I'm closing this for now. If issues arise, they should go onto a new ticket.

comment:26 Changed 6 years ago by planetcruiser

hm, i am not sure if i like the wording of this. i think it sounds a little confusing (too many numbers). how about "31 members found (log in to see 78 more)" (link on "log in"). also i would maybe move this above the pagination buttons. the number of results is not really important, and can be seen in the advanced search options if really needed. so i wouldn't have a problem with removing it from underneath the pagination buttons to have less clutter.

another thing: the problem with recycling word codes is that most likely they will not be translated at the exact time of the release for all languages. that's why i highly recommend to create and use a new word code each time the meaning of a string is changed. this avoids translation inconsistencies.

reopen?

comment:27 Changed 6 years ago by jsfan

I like the idea of shortening the whole thing and moving it above the page browser.

There are no existing words used in the new label. I've only put it in already.

comment:28 Changed 6 years ago by planetcruiser

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:29 Changed 6 years ago by planetcruiser

  • Status changed from reopened to assigned

:)

comment:30 Changed 6 years ago by jsfan

Wording changed... Recycling "more" but I suspect that's not too risky...

Last edited 6 years ago by jsfan (previous) (diff)

comment:31 Changed 6 years ago by planetcruiser

  • Resolution set to fixed
  • Status changed from assigned to closed

ok, just "x found" (instead of "x members found") is a little short, but it will do

Note: See TracTickets for help on using tickets.