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

#9466: handling GetFeatureInfo exceptions parameter format configuration #9471

Merged
merged 6 commits into from
Sep 28, 2023

Conversation

mahmoudadel54
Copy link
Contributor

Description

  • Handling GFI exceptions parameter format by removing "application/json" and putting the geoserver default format for exceptions 'application/vnd.ogc.se_xml' that will work with all WMS servcies including arcgis services.
  • Add getIdentifyWorkflow to catch the GFI errors and exceptions.

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix

Issue

#9466

What is the current behavior?
Some services has no "application/json" into their exception format list. So an exception of "Parameter 'exceptions' contains unacceptable value" shown for the GFI and prevent showing the identify results.

#
#9466

What is the new behavior?
There is no exception related to wrong exception format in GFI and identify works well with all WMS servcies including arcgis services and the identify results shown to user normally.

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • No

Other useful information

…iguration

Desription: - put a default format for GFI exceptions - add getIdentifyFlow to catch GFI errors and exceptions
Copy link
Member

@tdipisa tdipisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mahmoudadel54 it seems the unit test check is failing. Can you please check?

Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please:

  • getIdentifyFlow should have a unit test that checks the effective output for exception.
  • the failing unit tests must be fixed

Description: fix unit test failing results, create new unit test for getIdentifyFlow for wms
@tdipisa tdipisa removed their request for review September 22, 2023 16:03
offtherailz

This comment was marked as off-topic.

@offtherailz offtherailz requested review from tdipisa, offtherailz and MV88 and removed request for MV88 September 22, 2023 16:34
@offtherailz
Copy link
Member

sorry I sent my review to this PR in error, but the comment was for another pull request.

🤦

Copy link
Contributor

@MV88 MV88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few things
the rest lgtm

web/client/utils/mapinfo/__tests__/wms-test.js Outdated Show resolved Hide resolved
web/client/utils/mapinfo/__tests__/wms-test.js Outdated Show resolved Hide resolved
add test cases for getIdentifyFlow in wms-test
@mahmoudadel54 mahmoudadel54 requested a review from MV88 September 26, 2023 10:31
web/client/utils/mapinfo/__tests__/wms-test.js Outdated Show resolved Hide resolved
web/client/utils/mapinfo/__tests__/wms-test.js Outdated Show resolved Hide resolved
edit in wms-test of mapInfo file by adding a unit test for exception
@mahmoudadel54 mahmoudadel54 requested a review from MV88 September 26, 2023 14:02
Copy link
Contributor

@MV88 MV88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MV88 MV88 merged commit 5305d09 into geosolutions-it:master Sep 28, 2023
@MV88
Copy link
Contributor

MV88 commented Sep 28, 2023

@ElenaGallo Please test it in dev

@ElenaGallo
Copy link
Contributor

Test passe, @mahmoudadel54 please backport this to 2023.02.xx. Thanks

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

Successfully merging this pull request may close these issues.

GetFeatureInfo exceptions parameter format configuration
5 participants