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

viewer : restrict metadata searches to a specified extent, optionnally set that extent to the current map extent #964

Merged
merged 12 commits into from Apr 29, 2015

Conversation

Projects
None yet
2 participants
@fphg
Member

fphg commented Apr 23, 2015

No GUI for now, this is admin's choice

Show outdated Hide outdated mapfishapp/src/main/webapp/app/js/GEOR_cswquerier.js
if (GEOR.config.CSW_FILTER_SPATIAL.length!==4) {
finalFilters.push(new OpenLayers.Filter.Spatial({
type: OpenLayers.Filter.Spatial.BBOX,
value: this.map.getExtent().clone()

This comment has been minimized.

@fvanderbiest

fvanderbiest Apr 23, 2015

Member

no need to clone extent here IMO

@fvanderbiest

fvanderbiest Apr 23, 2015

Member

no need to clone extent here IMO

This comment has been minimized.

@fphg

fphg Apr 23, 2015

Member

dunno if we'll expand or alter this bbox later

@fphg

fphg Apr 23, 2015

Member

dunno if we'll expand or alter this bbox later

Show outdated Hide outdated mapfishapp/src/main/webapp/app/js/GEOR_config.js
* Defaults to [-180,-90,180,90] to cover the world.
* If the parameter is not an array of 4, the filter uses the current map extent.
*/
CSW_FILTER_SPATIAL: getCustomParameter("CSW_FILTER_SPATIAL", [-180,-90,180,90]),

This comment has been minimized.

@fvanderbiest

fvanderbiest Apr 23, 2015

Member

I would rather make this null by default.

@fvanderbiest

fvanderbiest Apr 23, 2015

Member

I would rather make this null by default.

This comment has been minimized.

@fphg

fphg Apr 23, 2015

Member

right

@fphg

fphg Apr 23, 2015

Member

right

Show outdated Hide outdated mapfishapp/src/main/webapp/app/js/GEOR_cswquerier.js
finalFilters.push(byTypes);
// spatial filter
if (GEOR.config.CSW_FILTER_SPATIAL.length!==4) {

This comment has been minimized.

@fvanderbiest

fvanderbiest Apr 23, 2015

Member

if null, then ...

@fvanderbiest

fvanderbiest Apr 23, 2015

Member

if null, then ...

@fvanderbiest

This comment has been minimized.

Show comment
Hide comment
@fvanderbiest

fvanderbiest Apr 23, 2015

Member

Some minor comments to be addressed.

Finally, I think we need to inform the user somehow that his search is/might be restricted on the current bbox.

Member

fvanderbiest commented Apr 23, 2015

Some minor comments to be addressed.

Finally, I think we need to inform the user somehow that his search is/might be restricted on the current bbox.

fphg added some commits Apr 23, 2015

better code design
checkbox to disable the auto extent filter
todo : better checkbox positioning
@fphg

This comment has been minimized.

Show comment
Hide comment
@fphg

fphg Apr 27, 2015

Member

fvanderbiest : now there's a warning in the search result report and a dedicated checkbox to enable/disable the auto bbox feature on the fly. this comes in handy at querying large external catalogs. Repeated queries trigger annoyingly the search tooltip, so PR makes it pop only once. Done for me for this PR unless other wishes & bugs.

Member

fphg commented Apr 27, 2015

fvanderbiest : now there's a warning in the search result report and a dedicated checkbox to enable/disable the auto bbox feature on the fly. this comes in handy at querying large external catalogs. Repeated queries trigger annoyingly the search tooltip, so PR makes it pop only once. Done for me for this PR unless other wishes & bugs.

@fphg fphg added the enhancement label Apr 27, 2015

@fphg fphg self-assigned this Apr 27, 2015

@fphg fphg added this to the 15.06 milestone Apr 27, 2015

Show outdated Hide outdated mapfishapp/src/main/webapp/app/js/GEOR_cswquerier.js
*/
getSpatialFilter: function() {
var f;
if (isLimitExtent) {

This comment has been minimized.

@fvanderbiest

fvanderbiest Apr 27, 2015

Member

I would factorize this contruct, in order to define the filter only once.
Its value should be set with eg:

var v = isLimitExtent ? 
    map.getExtent().clone().transform(
        map.getProjectionObject(), 
        new OpenLayers.Projection("EPSG:4326")
    ) :
    new OpenLayers.Bounds(
         GEOR.config.CSW_FILTER_SPATIAL
    )
@fvanderbiest

fvanderbiest Apr 27, 2015

Member

I would factorize this contruct, in order to define the filter only once.
Its value should be set with eg:

var v = isLimitExtent ? 
    map.getExtent().clone().transform(
        map.getProjectionObject(), 
        new OpenLayers.Projection("EPSG:4326")
    ) :
    new OpenLayers.Bounds(
         GEOR.config.CSW_FILTER_SPATIAL
    )

This comment has been minimized.

@fphg

fphg Apr 27, 2015

Member

done, thanks for the advice

@fphg

fphg Apr 27, 2015

Member

done, thanks for the advice

Show outdated Hide outdated mapfishapp/src/main/webapp/app/js/GEOR_cswquerier.js
* Property: isTooltip
* {Boolean} display search tooltip only once
*/
var isTooltip = true;

This comment has been minimized.

@fvanderbiest

fvanderbiest Apr 27, 2015

Member

rather tipDisplayed with a default value to false ?

@fvanderbiest

fvanderbiest Apr 27, 2015

Member

rather tipDisplayed with a default value to false ?

This comment has been minimized.

@fphg

fphg Apr 27, 2015

Member

thanks, renamed isTipDisplayed. don't we want the tip to be displayed on 1st search ?

@fphg

fphg Apr 27, 2015

Member

thanks, renamed isTipDisplayed. don't we want the tip to be displayed on 1st search ?

Show outdated Hide outdated mapfishapp/src/main/webapp/app/js/GEOR_cswquerier.js
*/
init: function(m) {
map = m;
isLimitExtent = (GEOR.config.CSW_FILTER_SPATIAL===null);

This comment has been minimized.

@fvanderbiest

fvanderbiest Apr 27, 2015

Member

I would rename it to dynamicSpatialFilter as well

@fvanderbiest

fvanderbiest Apr 27, 2015

Member

I would rename it to dynamicSpatialFilter as well

Show outdated Hide outdated mapfishapp/src/main/webapp/app/js/GEOR_cswquerier.js
* Property: isLimitExtent
* {Boolean} limit metadata results to map extent
*/
var isLimitExtent = false;

This comment has been minimized.

@fvanderbiest

fvanderbiest Apr 27, 2015

Member

In fact, no need to initialize it to anything, since this is done in init()

@fvanderbiest

fvanderbiest Apr 27, 2015

Member

In fact, no need to initialize it to anything, since this is done in init()

@fvanderbiest

This comment has been minimized.

Show comment
Hide comment
@fvanderbiest

fvanderbiest Apr 27, 2015

Member

Still some minor comments to be addressed.
Otherwise, looks good. Thanks for the contribution :-)

Member

fvanderbiest commented Apr 27, 2015

Still some minor comments to be addressed.
Otherwise, looks good. Thanks for the contribution :-)

@fphg

This comment has been minimized.

Show comment
Hide comment
@fphg

fphg Apr 29, 2015

Member

is it ok now ?

Member

fphg commented Apr 29, 2015

is it ok now ?

fvanderbiest added a commit that referenced this pull request Apr 29, 2015

Merge pull request #964 from georchestra/csw_filter_extent
viewer: restrict metadata searches to a specified extent

@fvanderbiest fvanderbiest merged commit b0dc986 into master Apr 29, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@fvanderbiest fvanderbiest deleted the csw_filter_extent branch Apr 29, 2015

@fvanderbiest

This comment has been minimized.

Show comment
Hide comment
@fvanderbiest

fvanderbiest Apr 29, 2015

Member

Yes, thanks again

Member

fvanderbiest commented Apr 29, 2015

Yes, thanks again

fphg added a commit to georchestra/template that referenced this pull request Apr 30, 2015

@fvanderbiest

This comment has been minimized.

Show comment
Hide comment
@fvanderbiest

fvanderbiest Jul 13, 2015

Member

I think this PR is incomplete.
When the user has checked the box, he expects the results to change on map extent changed => #1007

Member

fvanderbiest commented Jul 13, 2015

I think this PR is incomplete.
When the user has checked the box, he expects the results to change on map extent changed => #1007

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment