Merged
Conversation
…perties to show on the concept page (reducing useless SPARQL queries)
This reverts commit cc684fa.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1800 +/- ##
============================================
- Coverage 71.06% 70.73% -0.33%
- Complexity 1647 1650 +3
============================================
Files 33 33
Lines 4320 4326 +6
============================================
- Hits 3070 3060 -10
- Misses 1250 1266 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
joelit
approved these changes
Aug 26, 2025
Contributor
joelit
left a comment
There was a problem hiding this comment.
LGTM! In certain cases this really cuts down on the number of generated queries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reasons for creating this PR
Skosmos performs a number of useless SPARQL queries, see #1799.
This PR reduces the number of unnecessary SPARQL queries, thereby increasing performance.
Benchmark results
(Note: These have been updated following the merge of #1796, which eliminated duplicate SPARQL queries using a cache - before the query cache was added, the differences were even more dramatic.)
On my local Skosmos installation, a global search query for "aaron"
Showing the concept page for "Aaron (Raamatun henkilö)"
Link to relevant issue(s), if any
Description of the changes in this PR
$allowExternalparameter for many getLabel() methods that, when set to false, avoids performing a SPARQL query in situations where it isn't really necessary (in a few cases, this replaces a similar preexisting$queryExVocabsparameter that was used inconsistently)Known problems or uncertainties in this PR
Unit test coverage is not 100% for the modified code. In particular, I haven't written unit tests for the parts of Concept.php that were refactored. However, this is not a regression - we didn't have such tests earlier.
It's possible that some special cases related to how information is displayed on search results or concept pages have been broken by the changes in the PR, but it's pretty difficult to tell. I've tried to test for tricky cases but I may well have missed something.
Checklist
.sr-onlyclass, color contrast)