Encode concept URIs in API calls with URLSearchParams (fix hash namespace issues) by osma · Pull Request #1835 · NatLibFi/Skosmos

osma · 2025-11-26T11:59:06Z

Reasons for creating this PR

It was reported in #1828 that concept mappings are not shown on concept pages where the concept URI contains a # character. This turned out to be an URL encoding problem that affects many API calls and also breaks e.g. the hierarchy display.

This PR changes the way REST API URLs are constructed to ensure that all arguments are properly URL encoded. String concatenation is replaced with URLSearchParams which handles encoding automatically.

The PR also includes a small test vocabulary which uses a hash-style namespace, and a Cypress test that verifies that mappings are loaded for these concepts (and indirectly also that the hierarchy is working).

Link to relevant issue(s), if any

Closes Mapping endpoint call fails with 500 #1828

Description of the changes in this PR

change all URL construction expressions in JS front-end code to use URLSearchParams
add hash test vocabulary
add Cypress test that verifies that the mappings and hiearchy are working for the hash vocabulary concepts

Known problems or uncertainties in this PR

I tried to dig through all our JS front-end code looking for problematic ways to construct URLs and replaced them all with URLSearchParams. I don't think I've missed any, but it's possible. I'm a bit unsure about get-concept-url.js ; it's quite ugly in its current form and doesn't use URLSearchParams, but it seems to me that it is still encoding URL parameters properly using encodeURIComponent calls, so I didn't touch it.

Checklist

phpUnit tests pass locally with my changes
I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

… performing REST API calls

miguelvaara

Is there possibly a need to use URLSearchParams here in the same way as in the other corresponding cases?

osma · 2025-11-26T13:27:37Z

@miguelvaara it is used there (one line below the one you linked)

miguelvaara

Here the toString() function is used even though it is inside backticks (as I understand, the function should not be needed in that case).

Byt, for the sake of consistency, I think it would be good to either use the toString() function in all cases or leave it out everywhere. Regardless if it is actually needed or not, if it is used it would be good to use it in all cases of remove the one case with the function.

Here is the list of my quick glance:

https://github.com/NatLibFi/Skosmos/blob/dace6a80082fb4f21d7863005ff26d22d57b675f/resource/js/concept-mappings.js#L32C6-L32C82

https://github.com/NatLibFi/Skosmos/blob/dace6a80082fb4f21d7863005ff26d22d57b675f/resource/js/tab-alpha.js#L78C9-L78C80

Skosmos/resource/js/tab-alpha.js

Line 108 in dace6a8

    
           const url = `rest/v1/${window.SKOSMOS.vocab}/index/${this.selectedLetter}?${params}`

Skosmos/resource/js/tab-changes.js

Line 57 in dace6a8

fetch(`rest/v1/${window.SKOSMOS.vocab}/new?${params}`)

Skosmos/resource/js/tab-changes.js

Line 97 in dace6a8

fetch(`rest/v1/${window.SKOSMOS.vocab}/new?${params}`)

Skosmos/resource/js/tab-groups.js

Line 97 in dace6a8

fetch(`rest/v1/${window.SKOSMOS.vocab}/groupMembers/?${params}`)

Skosmos/resource/js/tab-groups.js

Line 176 in dace6a8

fetch(`rest/v1/${window.SKOSMOS.vocab}/groupMembers/?${params}`)

Skosmos/resource/js/tab-hierarchy.js

Line 127 in dace6a8

fetch(`rest/v1/${window.SKOSMOS.vocab}/topConcepts?${params}`)

Skosmos/resource/js/tab-hierarchy.js

Line 145 in dace6a8

fetch(`rest/v1/${window.SKOSMOS.vocab}/children?${params}`)

Skosmos/resource/js/tab-hierarchy.js

Line 175 in dace6a8

    
           const res = await fetch(`rest/v1/${window.SKOSMOS.vocab}/topConcepts?${params}`)

Skosmos/resource/js/tab-hierarchy.js

Line 190 in dace6a8

const res = await fetch(`rest/v1/${window.SKOSMOS.vocab}/hierarchy/?${params}`)

miguelvaara

After many checks and a fairly extensive review, I can see that everything works as it should. In my view, it can be merged once the toString cases have been considered and the related fixes have been made (either removed everywhere or included everywhere)

Great job!

miguelvaara · 2025-11-26T14:01:48Z

@miguelvaara it is used there (one line below the one you linked)

You right - my bad - sorry!

sonarqubecloud · 2025-11-26T14:17:41Z

Quality Gate passed

Issues
35 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

osma · 2025-11-26T14:18:59Z

I dropped the extra toString() from one of the expressions.

Note that since the search URL construction functionality is implemented in a different style (string concatenation, not string interpolation expressions), toString() is still needed there (I think) and I didn't touch that code in the PR as it already handled URL encoding properly.

miguelvaara

Skosmos/tests/testconfig.ttl

Lines 457 to 461 in fb0e839

    
           dc:title "Hash URI namespace"@en ; 
        
           skosmos:language "en"; 
        
           skosmos:showTopConcepts "true"; 
        
           skosmos:sparqlGraph <http://www.skosmos.skos/hash/>; 
        
           void:uriSpace "http://www.skosmos.skos/hash#" .

I think you should add the following:
skosmos:loadExternalResources "true";

After that tests pass and the issue disappears ...

miguelvaara · 2025-12-02T09:49:09Z

tests/cypress/template/concept-full-vs-partial.cy.js

+      // check the first mapping property name
+      cy.get('.prop-mapping h2').eq(0).contains('Exactly matching concepts')
+      // check the first mapping property values
+      cy.get('.prop-mapping').eq(0).find('.prop-mapping-label').eq(0).contains('number sign')


cy.get('.prop-mapping').eq(0).find('.prop-mapping-label').eq(0).contains('number sign')

I cannot get it to pass. It fails with the following error because it expects the string ‘number sign’.

containsnumber sign / AssertionError

Timed out retrying after 4000ms: Expected to find content: 'number sign' within the element: <div.col-5.prop-mapping-label> but never did.

tests/cypress/template/concept-full-vs-partial.cy.js:164:71

cy.get('.prop-mapping').eq(0).find('.prop-mapping-label').eq(0).contains('number sign')

skosmos:loadExternalResources defaults to true, so I don't think that can be the problem.

miguelvaara

The cause of the Wikidata label issue remains unknown. Nevertheless all tests now pass, so the merge can proceed.

Great work!

osma added 2 commits November 26, 2025 13:24

ensure concept URI is URL encoded (e.g. in hash namespace cases) when…

3d5b10f

… performing REST API calls

rewrite remaining URL construction expressions to use URLSearchParams

dace6a8

osma added this to the 3.0 milestone Nov 26, 2025

osma self-assigned this Nov 26, 2025

osma added the bug label Nov 26, 2025

osma added this to Skosmos 3.x Backlog Nov 26, 2025

osma requested a review from miguelvaara November 26, 2025 11:59

osma moved this to Needs review in Skosmos 3.x Backlog Nov 26, 2025

miguelvaara reviewed Nov 26, 2025

View reviewed changes

miguelvaara requested changes Nov 26, 2025

View reviewed changes

drop useless toString()

fb0e839

miguelvaara reviewed Dec 2, 2025

View reviewed changes

miguelvaara approved these changes Dec 2, 2025

View reviewed changes

osma merged commit 274e0dc into main Dec 2, 2025
30 checks passed

osma deleted the issue1828-mapping-hash-namespace branch December 2, 2025 12:56

github-project-automation bot moved this from Needs review to Issue/PR closed in Skosmos 3.x Backlog Dec 2, 2025

osma removed this from Skosmos 3.x Backlog Jan 20, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Encode concept URIs in API calls with URLSearchParams (fix hash namespace issues)#1835

Encode concept URIs in API calls with URLSearchParams (fix hash namespace issues)#1835
osma merged 3 commits intomainfrom
issue1828-mapping-hash-namespace

osma commented Nov 26, 2025 •

edited

Loading

Uh oh!

miguelvaara left a comment

Uh oh!

osma commented Nov 26, 2025

Uh oh!

miguelvaara left a comment

Uh oh!

miguelvaara left a comment

Uh oh!

miguelvaara commented Nov 26, 2025

Uh oh!

sonarqubecloud bot commented Nov 26, 2025

Uh oh!

osma commented Nov 26, 2025

Uh oh!

miguelvaara left a comment

Uh oh!

miguelvaara Dec 2, 2025

Uh oh!

osma Dec 2, 2025

Uh oh!

miguelvaara left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	dc:title "Hash URI namespace"@en ;
	skosmos:language "en";
	skosmos:showTopConcepts "true";
	skosmos:sparqlGraph <http://www.skosmos.skos/hash/>;
	void:uriSpace "http://www.skosmos.skos/hash#" .

Conversation

osma commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reasons for creating this PR

Link to relevant issue(s), if any

Description of the changes in this PR

Known problems or uncertainties in this PR

Checklist

Uh oh!

miguelvaara left a comment

Choose a reason for hiding this comment

Uh oh!

osma commented Nov 26, 2025

Uh oh!

miguelvaara left a comment

Choose a reason for hiding this comment

Uh oh!

miguelvaara left a comment

Choose a reason for hiding this comment

Uh oh!

miguelvaara commented Nov 26, 2025

Uh oh!

sonarqubecloud bot commented Nov 26, 2025

Quality Gate passed

Uh oh!

osma commented Nov 26, 2025

Uh oh!

miguelvaara left a comment

Choose a reason for hiding this comment

Uh oh!

miguelvaara Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

osma Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

miguelvaara left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

osma commented Nov 26, 2025 •

edited

Loading