Fix sorting of concepts in hierarchy that broke in PR 1133

[osma]

In PR #1133, the sorting of concepts in the hierarchy view was modified to take into account the domain (if any) of concept schemes. However, as a side effect, it broke the sorting in a frequently occurring situation: when sortByNotation is enabled (which is the default) but no notation codes are actually defined in the vocabulary. This happens e.g. in YSO.

Original situation (good), concepts are alphabetically ordered:

Situation after PR #1133 when sortByNotation is enabled (bad), not ordered:

The problem was that PR #1133 introduced an if...else structure but it didn't take into account the possibility of having sortByNotation enabled but not having any notations in the vocabulary data.

The solution is simply to remove the else block, so that the code that used to be in the else block is also executed if execution falls through the other if clauses. I've also added comments to highlight this somewhat unusual execution flow.

Thanks to @kouralex for originally reporting this!

[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

Merging #1140 (110bac8) into master (e213096) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1140   +/-   ##
=========================================
  Coverage     67.96%   67.96%           
  Complexity     1584     1584           
=========================================
  Files            32       32           
  Lines          3890     3890           
=========================================
  Hits           2644     2644           
  Misses         1246     1246           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e213096...110bac8. Read the comment docs.

[kouralex]

This seems to do the trick! Good work, @osma !

For fun, one can check how this is done (except for domains) in PHP code here:

Skosmos/model/ConceptProperty.php

Lines 113 to 146 in e213096

	private function sortValues()
	{
	# TODO: sort by URI as last resort
	# Note that getLabel() returns URIs in case of no label and may return a prefixed value which affects sorting
	if (!empty($this->values)) {
	if ($this->sort_by_notation) {
	uasort($this->values, function($a, $b) {
	$anot = $a->getNotation();
	$bnot = $b->getNotation();
	if ($anot == null) {
	if ($bnot == null) {
	// assume that labels are unique
	return strcoll(strtolower($a->getLabel()), strtolower($b->getLabel()));
	}
	return 1;
	}
	else if ($bnot == null) {
	return -1;
	}
	else {
	// assume that notations are unique
	return strnatcasecmp($anot, $bnot);
	}
	});
	}
	else {
	uasort($this->values, function($a, $b) {
	// assume that labels are unique
	return strcoll(strtolower($a->getLabel()), strtolower($b->getLabel()));
	});
	}
	}
	$this->is_sorted = true;
	}

As it is standing, there is no handling of similar labels but with different URIs but that is the case for above PHP code as well (see TODO comment above). This may result in different order in cases that are common in e.g., special ontologies that extend the more general one as in the original image. This can be seen below where the order of black-blue pairs are in inconsistent state. But this is how it already was so no harm done. Also, it seems to be happening in quite rare conditions now that I just tested (usually the black one is first, black meaning the more general ontology).

That said, this is an improvement and very well meets Skosmos quality measures and can be merged as-is.

Edit: it seems like the above case only happens when the more specific ontology concept has children but the other one does not and even then not always (= 50/50 chance). Another requirement is that it only happens on the initially opened hierarchical view of the defined concept - further openings via hierarchy (clicking triangle icons or concept labels) seem to be behave in a consistent manner.

[osma]

Thanks for the review @kouralex ! I'm merging this as is. If you want you can open a separate issue for the "similar labels but with different URIs" case.