Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#1631 closed bug (fixed)

Location search by text not working for some cities

Reported by: mahouni Owned by: jsfan
Priority: blocker Milestone: 1.2
Component: BW Geo Keywords: location text search
Cc: planetcruiser

Description

The search is somehow buggy...

Location search by text (Field: "Enter a location") is not working for some cities. A search returns "No members found". Please add cities where this is true. Reported so far:
München,
Leipzig

It occurs on Mapview and on Textview and no matter if you search for "Munich" or "München".

"Search using map boundaries" works fine for these cities.

Change History (49)

comment:1 Changed 7 years ago by planetcruiser

  • Priority changed from major to critical

yep, this is definitively buggy. increasing priority. related forum post: http://www.bewelcome.org/forums/s2240

we should make sure that this is fixed when we switch to osm (#1635)

comment:2 Changed 6 years ago by crumbking

Also "Brazil" as a search string does not work.

comment:3 Changed 6 years ago by toub

There are 2 part in the OSM switch, what has been done is the usage of OSM map with Leaflet.

What has to be done is to replace the GMap geolocation search, and that's not so easy. Simple tried have been done with nominatim or geoname, but it is still buggy.

comment:4 Changed 6 years ago by toub

I did a test on my local version (using Leaflet/OSM) with brasil, and that works.

But the issue could be due to the user profile description, so let's try again when we have the updated version in production.

comment:5 Changed 6 years ago by mahouni

"New York": The map correctly zooms to New York. But the result list shows only one member, who lives in a New York Street somewhere at the Gulf of Mexico.

comment:6 Changed 6 years ago by mahouni

München and Leipzig are not found in the db tables geonames_cache and geonames_alternate_names:

in geo.entity.php

findLocationsByName($name)

$ids is empty...

comment:7 Changed 6 years ago by mahouni

text search not working for:

Cesano Boscone
Krakow -> (zooms to Krakow, but returns member from Krąków, that's a different place!)

comment:8 Changed 6 years ago by crumbking

It does not load any resultas at all if I enter France, Frankreich while NOT logged in! If I login everything is fine.

comment:9 Changed 6 years ago by toub

Yes, disconnected search is a disaster. We should open a new bug about that (if does not already exist).

We have 2 possibilities:

  • disable it
  • show all results points, without the pictures & names for private profiles (instead, display a message 'connect to see the profile details')

I also believe that search should be available directly from the Welcome page (but this is also an other topic).

comment:10 Changed 6 years ago by mahouni

I searched for France, there was no difference if I am logged in or not - no results are loaded in both cases. (Also the "search by map boundary" has troubles to load the results in some areas of France.

comment:11 follow-up: Changed 6 years ago by crumbking

Maybe this has something to do with the massive amount of members we have in France?? ;-)

comment:12 in reply to: ↑ 11 Changed 6 years ago by mahouni

Replying to crumbking:

Maybe this has something to do with the massive amount of members we have in France?? ;-)

I doubt it ;) There are a lot of members in Germany too, but it seems to work there. I guess on the one hand it has something to do with our geonames tables. On the other hand we are still using the deprecated webservice (ws.geonames.org).

comment:13 Changed 6 years ago by jsfan

There actually is a search result. If you check the XML that is returned in the background, it actually does list members. However, these are never properly copied into the page. I suspect that means that something goes wrong in the JavaScript even though I see no JS errors.

xmllint does not complain when fed the AJAX response. So, the XML still seems to be well-formed. :)

Over to the JS whizes... ;)

comment:14 Changed 6 years ago by abyssin

A user reports having had difficulties to set Montpellier (a major French city) as her location. She couldn’t give much information about the circumstances of the bug but it seems to be something recurrent. I wasn’t able to reproduce the bug. OTRS ticket (French): 2012082010000031.

comment:15 Changed 6 years ago by jsfan

The report http://alpha.bewelcome.org/forums/s2683-map_search sounds like it is the same issue. Searching for "zurzach" might actually be a great way of debugging it because it only seems to yield one member and exhibit the problem at the same time. The profile http://alpha.bewelcome.org/members/Olita contains characters from a number of alphabets (Cyrillic and German umlauts) but as the response is UTF-8 I can't see a problem with that in the response.

comment:16 Changed 6 years ago by jsfan

This is mysterious... I've copied the profile mentioned above to my test install (everything that is visible). I can reproduce the problem.

If I now start randomly removing parts of the description, the problem comes and goes following now apparent pattern. I haven't looked at the JS but maybe this has something to do with the description length being a multiple of some number? Just a guess at this stage...

comment:17 Changed 6 years ago by crumbking

  • Priority changed from critical to blocker

I would say this is really important as we can't find people in some places. I suggest to attach to the OSM milestone.

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

comment:18 Changed 6 years ago by jsfan

  • Owner set to jsfan
  • Status changed from new to assigned

I'll take this one. I have found the problem but haven't fixed it, yet... :)

comment:19 Changed 6 years ago by jsfan

  • Milestone changed from unassigned to 0.9

The ellipsis used for the profile summary in the AJAX response is not multibyte safe. If it happens to cut through a multibyte character, the results is a character which renders the XML invalid.

comment:20 Changed 6 years ago by mahouni

brilliant! Great work Christian!

So, "search using map boundaries" seems to work now. The search around Zurzach and Lyon is working fine. Also the "text search" seems to work for France, Brasil, New York and Krakow. Though, still no results when using text search for Munich, Leipzig and Cesano Boscone.

comment:21 Changed 6 years ago by mahouni

more mystery...

when I clear the cache and history of my firefox browser, it will only find one user for text search "New York", logged in, and zero when not logged in...

comment:22 Changed 6 years ago by jsfan

Oops, I forgot that there are really two issues on this ticket. One is no results where there should be some (displayed as "no members found") and one is no results with no display whatsoever. I suspect I only really fixed the latter. So, more work to do here...

That you see different numbers for logged in and not for New York is a result of the changes for ticket #1557.

comment:23 Changed 6 years ago by jsfan

I can confirm that Leipzig and Cesano Boscone still don't work without using map boundaries but Munich works for me (85 members). Is the radius too small for the search?

comment:24 Changed 6 years ago by jsfan

Melbourne has the same problem. If you wanted tind anyone that should probably gbe found, you'd need a radius of about 100km there. However, I know that most of the found members are no more than about 10-15km from the city centre. So, not sure the radius really is the problem

comment:25 Changed 6 years ago by jsfan

The bug discussed in the comments (loading spinner never disappearing) has been moved to ticket #1715.

comment:26 Changed 6 years ago by jsfan

  • Milestone changed from 0.9 to unassigned

comment:27 Changed 6 years ago by TimLoal

  • Component changed from unknown to BW Geo

comment:28 Changed 6 years ago by TimLoal

  • Milestone changed from Future to 1.1

comment:29 Changed 6 years ago by TimLoal

Move to milestone 1.3 if not suitable on 1.1

LnP

comment:30 Changed 6 years ago by crumbking

region/locations which does not work, too:

  • Florida
  • Sarasota

comment:31 Changed 6 years ago by mahouni

update: place names where the text search is not working:

  • Leipzig
  • Munich (works for jsfan, doesn't work for mahouni)
  • Florida
  • Sarasota
  • Cesano Boscone
  • New York (seems to depend on search history)
  • Melbourne
  • Nancy
  • Metz
  • Freiburg
Last edited 6 years ago by mahouni (previous) (diff)

comment:32 Changed 6 years ago by jsfan

  • Cc planetcruiser added
  • Milestone changed from 1.2 to unassigned

I am releasing this because I might not get to working on it in the next weeks. Depending on what the changes on OSM are like, this might be just a matter of testing and closing now, anyway. Anyone feel free to re-assign to milestone 1.2 and take ownership.

comment:33 Changed 6 years ago by beatnickgr

Hmm i think the workaround should also be the solution. It looks like Sarasota is not working, but when we "search using map boundaries" it does. And it also shows the places around.

Why not make "search using map boundaries" as the one and only search method? This would be better also because it would not search only for the city, but also it's suburbs, and if it's a village it shows hosts in nearby villages.

This would also increase the chances of people in the suburbs (like me, Nea Ionia) to have guests who had looked for Athens ;)

comment:34 Changed 6 years ago by jsfan

Looks like OSM has "shifted" this problem a bit. While e.g. Melbourne, Munich or New York work for me now, all the others I tried still seem broken. :(

comment:35 Changed 6 years ago by jsfan

  • Milestone changed from unassigned to 1.2

I'll have a look at this if I get to it in time. If anyone else wants to have a crack, feel free to take it on. I'll then go for OSM tickets, if this is already gone. :)

comment:36 Changed 6 years ago by shevek

@Nancy: Nancy has an fcode of 'PPLA2' (as several of the other examples). The method placeType doesn't know that and returns "Unknown" instead of type city, region or country. (And logs it nicely with $this->logWrite("Database Bug: geonames_cache ({$this->getPKValue()}) fcode={$this->fcode} which is unknown", "Bug");)

@Munich: If the search somehow settle on the region of Munich instead of Munich the city something similar happens. The fcode of Munich region is ADM3 which leads to unknown as region should have ADM1.

(By the way München and Munich have rather different coordinates in geonames_cache.)

So the mitigation seems to be to that everything that has an fcode that starts with 'PPL' is a city, 'ADM' is a region and 'PCL' is a country.

Opinions?

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

comment:37 Changed 6 years ago by crumbking

Not a city but maybe related: Netherlands/ Niederlande/ Holland no results

comment:38 Changed 6 years ago by beatnickgr

i was chatting with jsfan on IRC about the issue, and we agreed that if we "search within map boundaries" by default, and not having the choice of looking just the specific name:

1) the initial problem of not finding results is solved (we already new this)...

2) the potential hosts who live in the suburbs will have more chances to get guests, without having to lie about their location (and say they're in the center), because they will appear in their correct location without having searched for them.

3) I also think it will be easier to devellop (without knowing much about develloping...) instead of looking all problematic locations one by one.

comment:39 Changed 6 years ago by jsfan

I don't think "agreement" is the right term... :P But, yes, I think we should take the suburb problem into consideration somehow.

1) It is not really. As shevek has determined the code is badly broken and just doesn't really do a proper search at all. It's almost the other way around that you're lucky to find anything because it only works reliably if the location you searched for is unique.

2) This is a fair point as I stated earlier but independent of the real problem.

3) Actually, using the map search will not save us any work at all because it does not address the actual problem. :(

comment:40 Changed 6 years ago by jsfan

Checked in a refactored search at https://gitorious.org/bewelcome/rox/commit/154cdf79a2c4367bfcf078d8df8ea93027d15bf6/diffs/c31336fc34817b44060cee868accf047d113a412

As this is a BIG patch, I'd appreciate someone looking at the code and making sure I didn't overlook anything.

No changes (except for a few security related things to avoid SQL injection) so far. Just refactored the method into a number of different ones.

comment:41 Changed 6 years ago by mahouni

That's great. I haven't looked at the code in details yet.

Just tested a few things. It seems to work, only that somewhere the check if the member logged in got lost.

     if (!$this->getLoggedInMember())
	{
	$where .= " AND memberspublicprofiles.IdMember=members.id";
	$tablelist=$tablelist.", memberspublicprofiles" ;

comment:42 Changed 6 years ago by jsfan

Fixed. Mainly caused by a misnomer. Two or three other problems which weren't triggered as a result fixed as well.

https://gitorious.org/bewelcome/rox/commit/c31336fc34817b44060cee868accf047d113a412/diffs/5a297909fcff26f8dc6917e33554eb83ca9e9e56

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

comment:43 Changed 6 years ago by jsfan

Can we close this and create a new ticket for the frontend code instead? The backend code should be reasonably clean now but that probably fixes very little (if anything). :(

comment:44 Changed 6 years ago by crumbking

Yeah let's close this one. But let's move the missing/not working examples also to the new ticket. Just I really worry that the new ticket won't be touched again for a couple of month...

comment:45 Changed 6 years ago by jsfan

What I fear much more is that this new ticket will actually be a whole release and when worked on will split up into half a dozen separate tickets. And that's just going by what shevek has already found out so far... :(

comment:46 Changed 6 years ago by shevek

Let's open a new ticket.

I agree that the search should be a release of its own but don't fear that the new ticket will be split into many. (I expect to get rid of one JS and the rest can be done correctly in the backend).

comment:47 Changed 6 years ago by jsfan

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

I have now moved this to a new ticket #1807. The new ticket makes no reference to specific locations but links back to this ticket.

I'm closing this ticket now.

comment:48 Changed 6 years ago by planetcruiser

comment:49 Changed 6 years ago by planetcruiser

being more specific about city districts, but still returning users that registered with PPLX feature code (e.g. 5 people in manhattan) in keyword search: https://gitorious.org/bewelcome/rox/commit/3d1e5e3806c0d6f8458a7ff3b7333cd82c99ce97

hat tip to shevek for the hint. :)

Note: See TracTickets for help on using tickets.