#9585: fix zoom to 3d layer to the correct extent in csw catalog #9610

mahmoudadel54 · 2023-10-17T09:48:52Z

Description

Fix zoom to any 3D layer in CSW Catalog to the correct extent.
I tried to handle bbox in metadata and parse the extent to the correct one but there is a missing point - I think it's related to axis order with different CSW versions and it needs a lot of conditions in code- so I go with the 2nd option fixing by getting layer extent from tileset json.

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/geosolutions-it/MapStore2/blob/master/CONTRIBUTING.md
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

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

Issue

#9585

What is the current behavior?
#9585
What is the new behavior?

when user adds a 3D layer from CSW catalog, it gets the extent of 3D layer from the tileset json then it zooms to the correct extent of the layer.

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

- fixing by getting layer extent from tileset json

description: fix test case in CSW-test

web/client/api/CSW.js

MV88

please isolate the behaviour you have edited by removing async await
you can chaining it with a function similar to addCapabilitiesToRecords
possibly I would like to have a dedicated test for 3dtiles use case

description: write unit test for add recordsfor 3d tile layers

MV88

please checkout inline comments,

MV88 · 2023-10-23T10:06:13Z

web/client/api/CSW.js

+    if (!result) return result;
+    let { records } = result;
+    if (records?.length) {
+        let records3D = records.filter(rec=>rec?.dc?.format === THREE_D_TILES);


hi, there is a mistake here that could have been fetched from adding proper unit testing

you cannot filter records here and then update them layer on because you can end updating the wrong one:
examples that you must have covered during tests

you have 4 records, the first one is a THREE_D_TILES

you have 4 records, the last one is a THREE_D_TILES

you have 3 records, the middle one is a THREE_D_TILES

in second and third scenario you are going to update the extent to the wrong layer

avoid filtering and return the getCapabilities only if you match the config otherwise you can return a a promise that resolves the original record for those that are not THREE_D_TILES

MV88 · 2023-10-23T10:16:14Z

web/client/api/CSW.js

-                    }));
+                    }
+                    return null;
+                }).then(results=>getBboxFor3DLayersToRecords(results)));         // handle getting bbox from capabilities in case of 3D tile layer


Suggested change

}).then(results=>getBboxFor3DLayersToRecords(results))); // handle getting bbox from capabilities in case of 3D tile layer

}).then(results => getBboxFor3DLayersToRecords(results))); // handle getting bbox from capabilities in case of 3D tile layer

MV88 · 2023-10-23T10:17:38Z

web/client/api/CSW.js

@@ -433,7 +461,7 @@ const Api = {
    workspaceSearch: function(url, startPosition, maxRecords, text, workspace) {
        return new Promise((resolve) => {
            require.ensure(['../utils/ogc/CSW', '../utils/ogc/Filter'], () => {
-                const {Filter} = require('../utils/ogc/Filter');
+                const { Filter } = require('../utils/ogc/Filter');


why have you started to add all of this formatting?
is there a tool you are using or what?

let's avoid introducing custom syntax formation different from what setup with vscode or the eslint rules of this project, so please let me know if you can disable this

MV88 · 2023-10-23T10:19:21Z

web/client/api/__tests__/CSW-test.js

+
+});
+
+describe("getLayerReferenceFromDc", () => {


wrong title?

MV88 · 2023-10-23T10:20:27Z

web/client/api/__tests__/CSW-test.js

+        mockAxios.restore();
+        setTimeout(done);
+    });
+    it('test getBboxFor3DLayersToRecords function that handles getting bbox for 3D tile layers from ', (done) => {


improve this title, it's too generic, which case are you testing exactly?
4 records the first is 3d_tile or what else?

MV88 · 2023-10-23T10:20:45Z

web/client/api/__tests__/CSW-test.js

+        mockAxios.onGet().reply(()=>{
+            return [200, tileSetResponse];
+        });
+        mockAxios.onGet().reply(()=>{
+            return [200, tileSetResponse];
+        });


duplication

description: add unit tests for csw that handle 3d tile layers

MV88

I approve, but make sure to not send other format syntax change in the future

MV88 · 2023-10-23T15:46:22Z

@ElenaGallo please test it in DEV

ElenaGallo · 2023-10-24T08:25:43Z

Test passed, @mahmoudadel54 please backport to 2023.02.xx. Thanks

…sw catalog (geosolutions-it#9610) * geosolutions-it#9585: fix zoom to 3d layer from csw catalog

mahmoudadel54 · 2023-10-24T09:08:49Z

Test passed, @mahmoudadel54 please backport to 2023.02.xx. Thanks

Backport is done ---> #9649

…) (#9649) * #9585: fix zoom to 3d layer from csw catalog

geosolutions-it#9585: fix zoom to 3d layer from csw catalog

bfaf2b8

- fixing by getting layer extent from tileset json

mahmoudadel54 added bug BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch C027-COMUNE_FI-2023-SUPPORT labels Oct 17, 2023

mahmoudadel54 added this to the 2023.02.01 milestone Oct 17, 2023

mahmoudadel54 requested a review from MV88 October 17, 2023 09:48

mahmoudadel54 self-assigned this Oct 17, 2023

mahmoudadel54 linked an issue Oct 17, 2023 that may be closed by this pull request

Zoom to layer is not correct for the 3D tiles resources in CSW #9585

Closed

1 task

geosolutions-it#9585: resolve test case failing

446ad51

description: fix test case in CSW-test

MV88 requested changes Oct 17, 2023

View reviewed changes

web/client/api/CSW.js Outdated Show resolved Hide resolved

mahmoudadel54 requested a review from MV88 October 19, 2023 09:05

MV88 requested changes Oct 19, 2023

View reviewed changes

resolve review comments

1902e96

description: write unit test for add recordsfor 3d tile layers

mahmoudadel54 requested a review from MV88 October 23, 2023 07:14

MV88 requested changes Oct 23, 2023

View reviewed changes

mahmoudadel54 added 2 commits October 23, 2023 17:06

resolve review comments

a70432e

description: add unit tests for csw that handle 3d tile layers

resolve review comments

15ff317

MV88 approved these changes Oct 23, 2023

View reviewed changes

MV88 merged commit eef0cee into geosolutions-it:master Oct 23, 2023
4 checks passed

mahmoudadel54 added a commit to mahmoudadel54/MapStore2 that referenced this pull request Oct 24, 2023

geosolutions-it#9585: fix zoom to 3d layer to the correct extent in c…

ce02c2a

…sw catalog (geosolutions-it#9610) * geosolutions-it#9585: fix zoom to 3d layer from csw catalog

mahmoudadel54 mentioned this pull request Oct 24, 2023

[Backport 2023.02.xx] Fix zoom to 3d layer to the correct extent in csw catalog (#9585) #9649

Merged

MV88 pushed a commit that referenced this pull request Oct 24, 2023

#9585: fix zoom to 3d layer to the correct extent in csw catalog (#9610…

61bc556

…) (#9649) * #9585: fix zoom to 3d layer from csw catalog

ElenaGallo removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Oct 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#9585: fix zoom to 3d layer to the correct extent in csw catalog #9610

#9585: fix zoom to 3d layer to the correct extent in csw catalog #9610

mahmoudadel54 commented Oct 17, 2023 •

edited

Loading

MV88 left a comment •

edited by mahmoudadel54

Loading

MV88 left a comment

MV88 Oct 23, 2023

MV88 Oct 23, 2023

MV88 Oct 23, 2023

MV88 Oct 23, 2023

MV88 Oct 23, 2023

MV88 Oct 23, 2023

MV88 left a comment

MV88 commented Oct 23, 2023

ElenaGallo commented Oct 24, 2023 •

edited

Loading

mahmoudadel54 commented Oct 24, 2023

	}).then(results=>getBboxFor3DLayersToRecords(results))); // handle getting bbox from capabilities in case of 3D tile layer
	}).then(results => getBboxFor3DLayersToRecords(results))); // handle getting bbox from capabilities in case of 3D tile layer


		});

		describe("getLayerReferenceFromDc", () => {

#9585: fix zoom to 3d layer to the correct extent in csw catalog #9610

#9585: fix zoom to 3d layer to the correct extent in csw catalog #9610

Conversation

mahmoudadel54 commented Oct 17, 2023 • edited Loading

Description

Issue

Breaking change

Other useful information

MV88 left a comment • edited by mahmoudadel54 Loading

Choose a reason for hiding this comment

MV88 left a comment

Choose a reason for hiding this comment

MV88 Oct 23, 2023

Choose a reason for hiding this comment

MV88 Oct 23, 2023

Choose a reason for hiding this comment

MV88 Oct 23, 2023

Choose a reason for hiding this comment

MV88 Oct 23, 2023

Choose a reason for hiding this comment

MV88 Oct 23, 2023

Choose a reason for hiding this comment

MV88 Oct 23, 2023

Choose a reason for hiding this comment

MV88 left a comment

Choose a reason for hiding this comment

MV88 commented Oct 23, 2023

ElenaGallo commented Oct 24, 2023 • edited Loading

mahmoudadel54 commented Oct 24, 2023

mahmoudadel54 commented Oct 17, 2023 •

edited

Loading

MV88 left a comment •

edited by mahmoudadel54

Loading

ElenaGallo commented Oct 24, 2023 •

edited

Loading