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

Map / External map viewer should not redirect to map app #6548

Closed
wants to merge 2 commits into from

Conversation

fxprunayre
Copy link
Member

When an external mapviewer is configured, location URL is changed. Only change location if buildAddToMapConfig return a config. If not, then it has redirected to the external viewer.

When an external mapviewer is configured, location URL is changed. 
Only change location if buildAddToMapConfig return a config. If not, then it has redirected to the external viewer.
@fxprunayre fxprunayre added this to the 4.2.2 milestone Sep 16, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@fxprunayre fxprunayre modified the milestones: 4.2.2, 4.2.3 Dec 12, 2022
$location.path('map').search({
add: encodeURIComponent(angular.toJson(
[config]))});
}
return;
},
addAllMdLayersToMap: function (layers, md) {
Copy link
Member

@josegar74 josegar74 Jan 11, 2023

Choose a reason for hiding this comment

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

buildAddToMapConfig is also used in this function and the logic doesn't seem it will work fine when using an external viewer.

Not a problem of this pull request, but the logic buildAddToMapConfig it's not very consistent in my opinion: returns a configuration value, when using the internal viewer, otherwise when using an external viewer executes an action, opening the viewer.

Also when adding several layers and using a external viewer, the logic in addAllMdLayersToMap would try to open the viewer multiple times, if it would work with the current logic, that I doubt it.


@fxprunayre, @jahow I was checking externalviewer.js to update the pull request code to work fine with the external viewer and noticed that this change seems intended to handle several chained view requests when calling the viewService method.

5444d79

I'm not a Javascript expert, but checking the following code seems the purpose is to be able to call viewService several times and when finished open the viewer with all the metadata in _toView. But viewService calls every time the _commit method to open the viewer. So probably opens the viewer multiple times for each metadata?

As I said, probably I'm missing something, but can you explain how is this working to open the external viewer with multiple metadata?

viewService: function (md, service) {
if (!(this.isEnabled() || this.isEnabledViewAction())) {
return;
}
md.url = md.uuid ? baseMdUrl + md.uuid : "";
this._toView.push({
md: md,
service: service
});
// ask for a commit (will be executed once all services are requested
var commitFunction = this._commit.bind(this);
setTimeout(commitFunction);
},
_toView: [],

Copy link
Contributor

Choose a reason for hiding this comment

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

No the external viewer will be opened once and then the _toView array is emptied, so every subsequent call to _commit will have no effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

The _toView array contains all the layers that should be opened in the external viewer, that's it! every time viewService is called, a layer is added in _toView and on the first call to _commit the external viewer is opened with all the layers.

add: encodeURIComponent(angular.toJson([buildAddToMapConfig(link, md)]))
});
var config = buildAddToMapConfig(link, md);
if (angular.isObject(config)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a question: why use angular.isObject? config will be undefined right? why not just config === undefined??

@landryb
Copy link
Contributor

landryb commented Jun 6, 2023

not 100% sure, but is it the same issue as /geonetwork/srv/fre/catalog.search#/metadata/a1b0f173-769a-479e-be2f-de8472d2ea96 in the url bar being rewritten to /geonetwork/srv/fre/catalog.search#/map?add=%255Bnull%255D when the external viewer is opened in a new tab ?

@fxprunayre fxprunayre modified the milestones: 4.2.5, 4.2.6 Jul 5, 2023
@tkohr
Copy link
Contributor

tkohr commented Aug 1, 2023

Could this PR be merged?

@jahow
Copy link
Contributor

jahow commented Oct 13, 2023

Superseded by changes in #7387

@jahow jahow closed this Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants