-
Notifications
You must be signed in to change notification settings - Fork 797
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
Tasks.IdentifyFeatures not returning GeoJson with ID #686
Comments
right now we have a little logic written into but since identify requests don't pass back metadata (because features from more than one layer can be retrieved), we're currently just hoping the fieldname is we could throw in a few more checks that interrogate the first feature for other common casing and fix the problem 80% of the time, but its just a hack because there's no guarantee that the next feature is even coming from the same layer. else if (response.results && response.results.length > 0) {
// dig into the first result to see if 'objectid' (all lowercase) is present
if (response.results[0].attributes.objectid) {
objectIdField = 'objectid';
// etc.
} else if (response.results[0].attributes.ObjectID) {
objectIdField = 'ObjectID';
}
} else {
objectIdField = 'OBJECTID';
} @patrickarlt may have a more clever idea, but it doesn't seem to me that its worth adding more checking than the above to this hot code. alternatively perhaps we could give developers an opportunity to tell us the fieldname they'd like to use when you set up the task (best revisited when looking more deeply at #633) edit: i'll also play around with trying to implement a case insensitive regex. |
@jgravois case insensitive regex sounds like a good bet. That should cover most known cases. Assigning to you. |
Hi, with the case insensitive regex in place the GeoJSON is now indeed returned together with an ID, but this ID is a String for Tasks.IdentifyFeatures compared to Tasks.Query that returns a GeoJSON with an ID that is an Integer. Could this be fixed to be consistent across Tasks? |
it can, and absolutely should. thanks for reporting. |
i finally got a chance to sit down and look at this today and was surprised to find that our developers made a conscious decision to pre-format the attributes returned by example query response: "features": [{
"attributes": {
"OBJECTID": 7,
"POP2000": 493782,
"POP2007": 523174
}
}], example identify response: "results": [{
"attributes": {
"OBJECTID": "7",
"Shape": "Polygon",
"POP2000": "493782",
"POP2007": "523174"
}
}] we could cast the // within Util.responseToFeatureCollection
if (count) {
if (typeof features[0].attributes[objectIdField] === 'string') {
var needsCast = true;
}
for (var i = features.length - 1; i >= 0; i--) {
if (needsCast) {
features[i].attributes[objectIdField] = +features[i].attributes[objectIdField];
}
var feature = arcgisToGeoJSON(features[i], objectIdField);
featureCollection.features.push(feature);
}
} is the discrepancy in the results really problematic for you? if so, how? |
@jgravois Ok, thanks for finding the time to look at this more closely. What I find odd is - why they decided to pre-format I see and understand that you don't want to mess with the raw response and can understand that. It is not really problematic, we already handle this in our code, but it would be nice if all the services that return the same kind of data, like in your example Probably you are right, that it is not worth adding this extra logic if it just creates extra complexity and does not give any real value to the library. Just for the record, in our case we take the properties of each feature in the feature collection returned from So at the end when I receive data from featureCollection.features[featureIndex].id = parseInt(featureCollection.features[featureIndex].id, 10);
delete featureCollection.features[featureIndex].layerId; |
thanks for taking the time to explain your workflow.
i'm sure they have their reasons, but i'm definitely in your camp. i would prefer consistency across operations too. i'm going to go ahead and re-close the issue here, but if anyone wants to discuss the issue further in the future, i'd be happy to. |
@jgravois No problem, :)
Maybe this can be raised to them and they can think about having a more consistent responses across operations? Is there a project where I can raise it as a bug? Yeah, no problem, thanks for your responses and time looking into this. |
unfortunately its already been raised as a bug (several times), and rejected as designed:
|
I see, sad to see that, but will have to live with it. Thanks, for the further information. |
looks like someone else felt the same way as @theashyster and i. in ArcGIS 10.5 a new parameter called |
@jgravois This is nice, they found a way for this to work both ways :) Now another fantastic option would be to be able to pass in outFields to Identify service the same way as we can do with Query service right now. |
The callback come GeoJSON without ID. But GeoJSON has all properties including objectid.
The text was updated successfully, but these errors were encountered: