Skip to content

Modify code to always have an ld json element, may be empty#1188

Merged
kouralex merged 1 commit intoNatLibFi:masterfrom
kinow:always-have-a-jsonld-element
Aug 23, 2021
Merged

Modify code to always have an ld json element, may be empty#1188
kouralex merged 1 commit intoNatLibFi:masterfrom
kinow:always-have-a-jsonld-element

Conversation

@kinow
Copy link
Collaborator

@kinow kinow commented Aug 21, 2021

I think this issue happens due to a mix of Twig and JS code. When you visit a vocabulary, the value of request.page == 'page' is false, since request.page has the value "vocab".

But if you click on a concept, it will trigger the code to load concept mappings without changing the value of request.page. That code won't find the JSON-LD script element, and result in a JS error. If you reload the page, now the request.page will be page and now the concept mappings will be displayed correctly.

This pull request simply modifies the template to always have a JSON-LD script element, that may be empty ({}). This way the code should work no matter what page is trying to load concept mappings. Tested locally with YSO vocabulary.

Closes #1016

@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
2.3% 2.3% Duplication

@kouralex kouralex added this to the Next Tasks milestone Aug 23, 2021
@kouralex kouralex self-assigned this Aug 23, 2021
@kouralex
Copy link
Contributor

Perfect! Thank you @kinow for implementing this.

PS. I assume this also fixed #1186 ? 😃

@kouralex kouralex merged commit 3d05609 into NatLibFi:master Aug 23, 2021
@kouralex kouralex modified the milestones: Next Tasks, 2.12 Aug 23, 2021
@kinow kinow deleted the always-have-a-jsonld-element branch August 23, 2021 10:12
@kinow
Copy link
Collaborator Author

kinow commented Aug 23, 2021

Perfect! Thank you @kinow for implementing this.

PS. I assume this also fixed #1186 ? 😃

Yup, should have fixed both. Now after the release we can ask Henk to confirm it's working OK 🎉 thanks @kouralex !

@osma osma added the bug label Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unhandled JavaScript error causes mapping properties to be unable to load

3 participants