Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple magic keys for provider L.esri.Geocoding.mapServiceProvider not geocoding correctly #250

Closed
jwasilgeo opened this issue May 22, 2020 · 4 comments · Fixed by #253
Assignees
Labels

Comments

@jwasilgeo
Copy link
Contributor

In our /debug/sample.html and in the public docs example that uses a mapServiceProvider with the geocoder, an error is encountered when trying to query for the suggested result a user has chosen.

TypeError: features is undefined.

Somehow invalid numbers are getting added into the query URL structure, thus the query request doesn't happen and it is just returning an info json response.

http://sampleserver6.arcgisonline.com/arcgis/rest/services/Census/MapServer/2,2932/query?returnGeometry=true&where=1%3D1&outSR=4326&outFields=*&objectIds=2346&f=json

Note specifically this part: ...MapServer/2,2932/query....

Here is a screenshot for some stack tracing.

image

@jwasilgeo jwasilgeo added the Bug label May 22, 2020
@jwasilgeo
Copy link
Contributor Author

jwasilgeo commented May 29, 2020

Reproduction steps:

  1. Go to https://esri.github.io/esri-leaflet/examples/search-map-service.html

  2. Type in "fay" to get a few suggestion results, and focus on the "States and Counties" list of suggestions coming back from the MapServer's find REST call.

    image

  3. Look at the DOM structure of those 2 suggestions.

    image

    The ["data-magic-key"] attribute for each <li> turns out to be:

    • "2268:2,2450:2,2596:2,3001:2"
    • "3079:2"

    (Which is OBJECTID:LAYERID)

  4. The "Fayette Detailed Counties" suggestion result has more than 1 magic key combo separated by commas because of how new DOM nodes are created and appended here.

    if (suggestionTextArray.indexOf(suggestion.text) === -1) {
    var suggestionItem = DomUtil.create('li', 'geocoder-control-suggestion', list);
    suggestionItem.innerHTML = suggestion.text;
    suggestionItem.provider = suggestion.provider;
    suggestionItem['data-magic-key'] = suggestion.magicKey;
    suggestionItem.unformattedText = suggestion.unformattedText;
    } else {
    for (var j = 0; j < list.childNodes.length; j++) {
    // if the same text already appears in the list of suggestions, append an additional ObjectID to its magicKey instead
    if (list.childNodes[j].innerHTML === suggestion.text) {
    list.childNodes[j]['data-magic-key'] += ',' + suggestion.magicKey;
    }
    }
    }

    Look specifically at lines 74 and 85. Because the MapServer's find REST response included more than 1 unique feature result with name "Fayette", the else block is appending more magicKey values to the ["data-magic-key"] attribute for that single <li> for the "Fayette Detailed Counties" suggestion result.

  5. Here comes the breakdown. Choose that suggestion result to arrive at the error where the geocoding query REST call's URL is malformed with ...MapServer/2,2450/query...:

    https://sampleserver6.arcgisonline.com/arcgis/rest/services/Census/MapServer/2,2450/query?returnGeometry=true&where=1%3D1&outSR=4326&outFields=*&objectIds=2268&f=json
    
  6. The 2,2450 value comes from here, which only works when there is one magic key, but not when there are multiple ones with comma separation.

    if (key) {
    var featureId = key.split(':')[0];
    var layer = key.split(':')[1];
    request = this.query().layer(layer).featureIds(featureId);
    } else {

I am unsure at this time what the next steps are to go about fixing this situation. Ideas and comments welcome. For example, do we list "Fayette Detailed Counties" multiple times in the DOM with only 1 magic key each? Other ideas?

@jgravois
Copy link
Contributor

jgravois commented Jun 2, 2020

this looks to be a bug i introduced in #151 when the geocoding control aggregates more than one match behind a single suggestion.

do we list "Fayette Detailed Counties" multiple times in the DOM with only 1 magic key each?

no. IMHO a single result should appear in the list of suggestions and the control should return all matching features in the event it emits when a geocode is executed.

to accomplish this, you could:

  1. use the commas to create an array of keys
  2. gather all the unique featureIds and group them by layer
  3. make a call to this.query for each layer that contains a match
  4. try and resolve/process more than one parallel response when necessary instead of just one.

f$%k all that noise though, it'd be a lot easier and probably work just fine to just trigger the else block here anytime you have more than one match.

// if more than one result is present, use find()
if (key && !key.includes(',')) {
} else {
  request = this.find().text(text).fields(this.options.searchFields).layers(this.options.layers);
}

you'll have to test with featureLayerProvider too to make sure though.

@jwasilgeo
Copy link
Contributor Author

Thanks for the background info and ideas, @jgravois! I had a moment to give the latter idea a spin and it checks out nicely. Here's the mapped result without any errors for "Fayette".

image

I'll try to give this more time and thoroughly test it out (and also double-check with the FL provider) in the coming days.

@jwasilgeo jwasilgeo self-assigned this Jun 2, 2020
@jwasilgeo
Copy link
Contributor Author

FL providers queries correctly when there are more than 1 keys available. Added comments and tests.

@jwasilgeo jwasilgeo changed the title Broken example with L.esri.Geocoding.mapServiceProvider Multiple magic keys for provider L.esri.Geocoding.mapServiceProvider not geocoding correctly Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants