Don't crash on incomplete vocabulary configuration

[nichtich]

If a vocabulary configuration has no void:uriSpace, an error message should be shown instead of crashing. This closes #1408.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (a22b389) 69.55% compared to head (a789a77) 69.57%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1409      +/-   ##
============================================
+ Coverage     69.55%   69.57%   +0.01%     
- Complexity     1650     1651       +1     
============================================
  Files            32       32              
  Lines          4257     4259       +2     
============================================
+ Hits           2961     2963       +2     
  Misses         1296     1296              

Impacted Files	Coverage Δ
model/Vocabulary.php	85.66% <100.00%> (+0.10%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[osma]

Thanks for the PR!

I wonder what the implications are for omitting uriSpace. I mean, this PR obviously fixes the immediate crash, but the setting is used in several places in both PHP and JS code, so just returning a null value may have some consequences. Have you tried this in practice?

[nichtich]

Without uriSpace, basic vocabulary information is shown but not concepts, so it's better than a white screen. One may further think about how do handle incompletely configured vocabularies but for our instance this fix is enough.

[osma]

I tested this PR locally on a vocabulary with a commented out uriSpace setting. As expected it fixes the immediate PHP crash. But there are consequences on the JS side. When clicking a concept from the hierarchy, this error is shown on the JS console:

Uncaught SyntaxError: JSON.parse: unexpected end of data at line 1 column 1 of the JSON data
    makeCallbacks http://localhost/Skosmos/resource/js/scripts.js:363
    complete http://localhost/Skosmos/resource/js/scripts.js:555
    jQuery 6
    loadMappingProperties http://localhost/Skosmos/resource/js/scripts.js:497
    ajaxConceptMapping http://localhost/Skosmos/resource/js/docready.js:263
    success http://localhost/Skosmos/resource/js/docready.js:320
    jQuery 6
    <anonymous> http://localhost/Skosmos/resource/js/docready.js:308
    jQuery 8
    <anonymous> http://localhost/Skosmos/resource/js/docready.js:126
    jQuery 13

I traced this to some fragile JS code that is run when doing partial page loads. It can be fixed by this patch to view/scripts.twig that ensures that the var uriSpace declaration is always there:

diff --git a/view/scripts.twig b/view/scripts.twig
index 2b77ae9f..725444ec 100644
--- a/view/scripts.twig
+++ b/view/scripts.twig
@@ -31,6 +31,8 @@ var prefLabels = [{"lang": "{{ search_results|first.label.lang }}","label": "{{
 {% endif %}
 {% if request.vocab and request.vocab.uriSpace %}
 var uriSpace = "{{ request.vocab.uriSpace }}";
+{% else %}
+var uriSpace = null;
 {% endif %}
 var showNotation = {% if request.vocab and not request.vocab.config.showNotation %}false{% else %}true{% endif %};
 {% if request.vocab  %}

I will try to add this fix to this PR branch before merging it. It seems that the nichtich branch allows this. (BTW, it would be nice to have a more descriptive name for the PR branch - we generally use issueXXXX-some-short-description for branches that try to fix an already reported issue).

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[osma]

Thank you for the PR @nichtich ! This will go into the master branch and eventually the 2.17 release. But I will also cherry-pick the commits for the v2.16-maintenance branch.