-
Notifications
You must be signed in to change notification settings - Fork 405
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
fix #1263 FilterUtils wrong usage of this removed most of the places #1285
fix #1263 FilterUtils wrong usage of this removed most of the places #1285
Conversation
… of the places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the proper way to fix this. You should simply avoid storing stuff in this or FilterUtils and pass all the needed values in function calls.
toOGCFilter: function(ftName, json, version, sortOptions = null, hits = false, format = null, propertyNames = null) { | ||
try { | ||
this.objFilter = (json instanceof Object) ? json : JSON.parse(json); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the JSON.parse has disappeared?
@@ -513,12 +771,12 @@ const FilterUtils = { | |||
* }} | |||
* } | |||
*/ | |||
processOGCCrossLayerFilter: function(crossLayerFilter) { | |||
let ogc = this.ogcSpatialOperator[crossLayerFilter.operation].startTag; | |||
processOGCCrossLayerFilter: function(crossLayerFilter, nsplaceholder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put a default value of "ogc" for nsplaceholder
@@ -486,7 +486,7 @@ describe('FilterUtils', () => { | |||
FilterUtils.nsplaceholder = "ogc"; | |||
FilterUtils.setOperatorsPlaceholders("{namespace}", "ogc"); | |||
|
|||
let filter = FilterUtils.processOGCCrossLayerFilter(crossLayerFilterObj); | |||
let filter = FilterUtils.processOGCCrossLayerFilter(crossLayerFilterObj, "ogc"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove "ogc" argument, we cannot change public methods behaviour
return ogc; | ||
}, | ||
getGmlPointElement: function(coordinates, srsName, version) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods were public, so they could be used elsewhere, removing them could break a lot of code
@@ -154,277 +641,30 @@ const FilterUtils = { | |||
} | |||
}); | |||
}, | |||
getGetFeatureBase: function(version, pagination, hits, format) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
|
||
return getFeature; | ||
}, | ||
processOGCFilterGroup: function(root) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
|
||
return ogc; | ||
}, | ||
processOGCFilterFields: function(group) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
|
||
return filter; | ||
}, | ||
processOGCSimpleFilterField: function(field) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
} | ||
return filter; | ||
}, | ||
ogcDateField: function(attribute, operator, value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
} | ||
return fieldFilter; | ||
}, | ||
cqlDateField: function(attribute, operator, value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
} | ||
return fieldFilter; | ||
}, | ||
processCQLSpatialFilter: function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
|
||
return cql + ")"; | ||
}, | ||
getCQLGeometryElement: function(coordinates, type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
geometry += ")"; | ||
return geometry; | ||
}, | ||
findSubGroups: function(root, groups) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one was public
@@ -484,7 +484,7 @@ describe('FilterUtils', () => { | |||
// this is a workarround for this issue : https://github.com/geosolutions-it/MapStore2/issues/1263 | |||
// please remove when fixed | |||
FilterUtils.nsplaceholder = "ogc"; | |||
FilterUtils.setOperatorsPlaceholders("{namespace}", "ogc"); | |||
FilterUtils.setOperatorsPlaceholders("{namespace}", FilterUtils.nsplaceholder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not this way: the FilterUtils.setOperatorsPlaceholders function should have a default for the second argument:
setOperatorsPlaceholders: function(placeholder, replacement = "ogc") {
...
}
No description provided.