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

#9589: Notify data projection not compatible / available for COG #9690

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

dsuren1
Copy link
Contributor

@dsuren1 dsuren1 commented Nov 10, 2023

Description

This PR check for projection compatibility and notifies the user with meaningful feedback

Please check if the PR fulfills these requirements

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

  • Enhancement

Issue

What is the current behavior?

What is the new behavior?

CATALOG
Screenshot 2023-11-10 at 6 02 16 PM

TOC
Screenshot 2023-11-10 at 5 44 33 PM

NOTIFICATION
Screenshot 2023-11-10 at 5 44 44 PM

Breaking change

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

  • Yes, and I documented them in migration notes
  • No

Other useful information

@dsuren1 dsuren1 added enhancement BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch labels Nov 10, 2023
@dsuren1 dsuren1 added this to the 2023.02.01 milestone Nov 10, 2023
@dsuren1 dsuren1 self-assigned this Nov 10, 2023
@dsuren1 dsuren1 linked an issue Nov 10, 2023 that may be closed by this pull request
6 tasks
@offtherailz
Copy link
Member

offtherailz commented Nov 16, 2023

@dsuren1 this PR after our recent PRs needs to be upated with master, some conflicts present.
I can not test it because I think it needs the integration with the other functionalites. Can you provide a valid test case for the COG context https://dev-mapstore.geosolutionsgroup.com/mapstore/#/context/COG ?

@dsuren1
Copy link
Contributor Author

dsuren1 commented Nov 17, 2023

@dsuren1

Can you provide a valid test case for the COG context https://dev-mapstore.geosolutionsgroup.com/mapstore/#/context/COG?

For the cog test service urls, we have projection defs already defined in Dev so, maybe it's because of that, i'm not able to create a test data that satisfies all cases. However, you can test it locally by removing projection defs and adding this service url to test catalog error and import this map map_cog_notify.json to test warning and TOC

@offtherailz offtherailz merged commit 76207ab into geosolutions-it:master Nov 20, 2023
6 checks passed
@offtherailz
Copy link
Member

@ElenaGallo, could you please test this on DEV ? Thank you

@ElenaGallo
Copy link
Contributor

Test passed @dsuren1 please backport. Thanks

dsuren1 added a commit to dsuren1/MapStore2 that referenced this pull request Nov 21, 2023
offtherailz pushed a commit that referenced this pull request Nov 21, 2023
@ElenaGallo ElenaGallo removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Nov 21, 2023
@tdipisa
Copy link
Member

tdipisa commented Nov 22, 2023

@dsuren1 (FYI @ElenaGallo @offtherailz)

I've tested this as well the other issues. In this case I've reverted projDefs definitions from DEV data dir
and added COGs previosuly tested before releasing 2023.02.00 to this test map.

The notification works fine and the layer is not added to the map. Probably it would be better to let the user know why somehow (e.g possibly a link to the related user guide section? I'm open to suggestions).
image

There is anyway what I think is a wrong behavior. Every time the map is loaded the following notification appears independently of what is the COG layer visible in TOC (I've tried visualizing them one by one).

image

This is not correct because the projections are supported for all COG layers in TOC (once needed projectionDefs have been restored in DEV data dir).

@dsuren1
Copy link
Contributor Author

dsuren1 commented Nov 23, 2023

@tdipisa

There is anyway what I think is a wrong behavior. Every time the map is loaded the following notification appears independently of what is the COG layer visible in TOC (I've tried visualizing them one by one).

This is not correct because the projections are supported for all COG layers in TOC (once needed projectionDefs have been restored in DEV data dir).

It's because the layer's sourceMetadata info is not saved on export and map save. I have fixed this in the PR

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.

Notify data projection not compatible / available for COG
4 participants