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

#10040 ArcGIS Interoperability - ArcGIS MapServer Catalog and Layer support #10330

Merged
merged 11 commits into from
May 21, 2024

Conversation

allyoucanmap
Copy link
Contributor

@allyoucanmap allyoucanmap commented May 16, 2024

Description

This PR introduces a new arcgis layer type to support ArcGIS Rest MapServer services

Please check if the PR fulfills these requirements

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

  • Feature

Issue

What is the current behavior?

#10040

What is the new behavior?

A new arcgis layer type has been added to support ArcGIS Rest MapServer services

Breaking change

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

  • No

Other useful information

Endpoints for testing https://sampleserver6.arcgisonline.com/arcgis/rest/services

@allyoucanmap allyoucanmap added this to the 2024.02.00 milestone May 16, 2024
@allyoucanmap allyoucanmap self-assigned this May 16, 2024
@tdipisa tdipisa requested a review from offtherailz May 16, 2024 09:53
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.

I only found an issue testing the services below to check. The code is super-clear. I see also some good improvements
Test done:

Other things / questions

  • Legend support is missing, but not mentioned
  • What about the auth key suggested in the original issue. Is there a separate issue or you don't want to integrate it?
  • How I test for leaflet right now?
  • I found that the annotations icon print is not working, but I think it is not related to this. Just mention my finding

image

@allyoucanmap
Copy link
Contributor Author

allyoucanmap commented May 17, 2024

@offtherailz @tdipisa

https://sampleserver6.arcgisonline.com/arcgis/rest/services/911CallsHotspot/MapServer Failing <--

About this I tried and I'm able to see it, the only issue I'm seeing is the missing supported projection that is ESRI:102726.

Note: we are assuming EPSG in the message and maybe we should review the authorities support in the core

image

I found other two issue releted to bounding box and identify using https://sampleserver6.arcgisonline.com/arcgis/rest/services/SampleWorldCities/MapServer

  1. selecting a feature near the IDL select all the features on the same latitude

image

  1. Using the identify Zoom to feature action does not work if the bounding box of the selected features exceed the maximum extent of the map projection

image

Both above issues has been solved by this commit 78f88d7

Note: I think a similar problem to the first one could happen also for WMS layer. In that case we could apply the same function implemented to solve this

It has been introduced a minimal support for ImageServer service to make the layer works on geonode istances with this commit 35a528f

Other things / questions

Legend support is missing, but not mentioned

Legend was not part of the requirement at the moment but we could investigate on the possible implementation. Probably this should take into account that a layer could be composed by sub layers (cc @tdipisa )

What about the auth key suggested in the original issue. Is there a separate issue or you don't want to integrate it?

I would prefer to merge this initial implementation and discuss the token in a second step because we should ensure this will work both on mapstore product and geonode (cc @tdipisa )

How I test for leaflet right now?

I think the only way is to configure it manually

I found that the annotations icon print is not working, but I think it is not related to this. Just mention my finding

I did a quick test and the client send the image as base64, I think we should investigate this in a separate issue

@allyoucanmap allyoucanmap linked an issue May 17, 2024 that may be closed by this pull request
3 tasks
@allyoucanmap allyoucanmap merged commit 9baf447 into master May 21, 2024
6 checks passed
@allyoucanmap
Copy link
Contributor Author

@ElenaGallo please test this enhancement on dev, thanks

@ElenaGallo
Copy link
Contributor

@allyoucanmap

1_ If I set the Resolution as the Limits type, when I reopen the settings panel the Limits type changes to Scale

1.mp4

2_If I set EPSG:4326 like map projection than the layer is not visible on Print preview

2.mp4

@ElenaGallo
Copy link
Contributor

@tdipisa point one of the comment above also applies to other services and is also present on prod. Do I have to open an issue or is this correct?

@tdipisa
Copy link
Member

tdipisa commented May 22, 2024

@ElenaGallo

@tdipisa point one of the comment above also applies to other services and is also present on prod. Do I have to open an issue or is this correct?

a dedicated issue should be opened for this, of course. Thank you.

@ElenaGallo
Copy link
Contributor

@tdipisa new issue #10355

@allyoucanmap
Copy link
Contributor Author

allyoucanmap commented May 22, 2024

@ElenaGallo @tdipisa the issue number 2 #10330 (comment) has been fixed here #10354

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C130-TOTAL-2024-GeoNode-DEV New Feature used for new functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArcGIS Interoperability
5 participants