Merge pull request #1205 from NatLibFi/issue937-sort-by-notation-stra…

…tegy Add "lexical" and "natural" sort strategies for notation codes
NatLibFi · Sep 16, 2021 · 794bd18 · 794bd18
2 parents 33a2222 + e53e82b
commit 794bd18
Show file tree

Hide file tree

Showing 12 changed files with 149 additions and 77 deletions.
diff --git a/model/Concept.php b/model/Concept.php
@@ -558,7 +558,7 @@ public function getProperties()
                 if ($superprop) {
                     $superprop = EasyRdf\RdfNamespace::shorten($superprop) ? EasyRdf\RdfNamespace::shorten($superprop) : $superprop;
                 }
-                $sort_by_notation = $this->vocab->getConfig()->sortByNotation();
+                $sort_by_notation = $this->vocab->getConfig()->getSortByNotation();
                 $propobj = new ConceptProperty($prop, $proplabel, $prophelp, $superprop, $sort_by_notation);
 
                 if ($propobj->getLabel() !== null) {

diff --git a/model/ConceptProperty.php b/model/ConceptProperty.php
@@ -130,8 +130,12 @@ private function sortValues()
                         return -1;
                     }
                     else {
-                        // assume that notations are unique
-                        return strnatcasecmp($anot, $bnot);
+                        // assume that notations are unique, choose strategy
+                        if ($this->sort_by_notation == "lexical") {
+                            return strcoll($anot, $bnot);
+                        } else { // natural
+                            return strnatcasecmp($anot, $bnot);
+                        }
                     }
                 });
             }

diff --git a/model/ConceptPropertyValue.php b/model/ConceptPropertyValue.php
@@ -33,12 +33,7 @@ public function __construct($model, $vocab, $resource, $prop, $clang = '')
 
     public function __toString()
     {
-        $label = is_string($this->getLabel()) ? $this->getLabel() : $this->getLabel()->getValue();
-        if ($this->vocab->getConfig()->sortByNotation()) {
-            $label = ltrim($this->getNotation() . ' ') . $label;
-        }
-
-        return $label;
+        return is_string($this->getLabel()) ? $this->getLabel() : $this->getLabel()->getValue();
     }
 
     public function getLang()

diff --git a/model/VocabularyConfig.php b/model/VocabularyConfig.php
@@ -178,12 +178,22 @@ public function getTitle($lang = null)
     }
 
     /**
-     * Returns a boolean value set in the config.ttl config.
-     * @return boolean
+     * Returns the sorting strategy for notation codes set in the config.ttl
+     * config: either "lexical", "natural", or null if sorting by notations is 
+     * disabled. A "true" value in the configuration file is interpreted as
+     * "lexical".
+     * @return string|bool
      */
-    public function sortByNotation()
+    public function getSortByNotation(): ?string
     {
-        return $this->getBoolean('skosmos:sortByNotation');
+        $value = $this->getLiteral('skosmos:sortByNotation');
+        if ($value == "lexical" || $value == "natural") {
+            return $value;
+        }
+        // not a special value - interpret as boolean instead
+        $bvalue = $this->getBoolean('skosmos:sortByNotation');
+        // "true" is interpreted as "lexical"
+        return $bvalue ? "lexical" : null;
     }
 
     /**

diff --git a/resource/js/hierarchy.js b/resource/js/hierarchy.js
@@ -73,9 +73,9 @@ function getLabel(object) {
     labelProp = 'label';
   }
   if (window.showNotation && object.notation) {
-    return '<span class="tree-notation">' + object.notation + '</span> ' + object[labelProp];
+    return '<span class="tree-notation">' + object.notation + '</span> <span class="tree-label">' + escapeHtml(object[labelProp]) + '</span>';
   }
-  return escapeHtml(object[labelProp]);
+  return '<span class="tree-label">' + escapeHtml(object[labelProp]) + '</span>';
 }
 
 function createObjectsFromChildren(conceptData, conceptUri) {
@@ -423,6 +423,22 @@ function topConceptsToSchemes(topConcepts, schemes) {
   return childArray;
 }
 
+/*
+ * Return a sort key suitable for sorting hierarchy nodes mainly by label.
+ * Nodes with domain class will be sorted first, followed by non-domain nodes.
+ */
+function nodeLabelSortKey(node) {
+  // make sure the tree nodes with class 'domain' are sorted before the others
+  // domain will be "0" if the node has a domain class, else "1"
+  var domain = (node.original.a_attr['class'] == 'domain') ? "0" : "1";
+
+  // parse the HTML code in node.text and return just the label as a lower case value for sorting
+  // should look like '<span class="tree-notation">12.3</span> <span class="tree-label">Hello</span>'
+  var label = $(node.text.toLowerCase()).filter('.tree-label').text();
+
+  return domain + " " + label;
+}
+
 /* 
  * Gives you the Skosmos default jsTree configuration.
  */
@@ -508,17 +524,21 @@ function getTreeConfiguration() {
         var bNode = this.get_node(b);
 
         // sort on notation if requested, and notations exist
-        if (window.showNotation) {
+        if (window.sortByNotation) {
             var aNotation = aNode.original.notation;
             var bNotation = bNode.original.notation;
 
             if (aNotation) {
                 if (bNotation) {
-                    if (aNotation < bNotation) {
-                        return -1;
-                    }
-                    else if (aNotation > bNotation) {
-                        return 1;
+                    if (window.sortByNotation == "lexical") {
+                        if (aNotation < bNotation) {
+                            return -1;
+                        }
+                        else if (aNotation > bNotation) {
+                            return 1;
+                        }
+                    } else { // natural
+                        return naturalCompare(aNotation, bNotation);
                     }
                 }
                 else return -1;
@@ -527,12 +547,7 @@ function getTreeConfiguration() {
             // NOTE: if no notations found, fall back on label comparison below
         }
         // no sorting on notation requested, or notations don't exist
-        // make sure the tree nodes with class 'domain' are sorted before the others
-        // aDomain/bDomain will be "0" if a/b has a domain class, else "1"
-        var aDomain = (aNode.original.a_attr['class'] == 'domain') ? "0" : "1";
-        var bDomain = (bNode.original.a_attr['class'] == 'domain') ? "0" : "1";
-        return naturalCompare(aDomain + " " + aNode.text.toLowerCase(),
-                              bDomain + " " + bNode.text.toLowerCase());
+        return naturalCompare(nodeLabelSortKey(aNode), nodeLabelSortKey(bNode));
     }
   });
 }

diff --git a/resource/js/scripts.js b/resource/js/scripts.js
@@ -326,7 +326,14 @@ function countAndSetOffset() {
   }
 }
 
+// return -1 if the value is negative, 1 otherwise
+// used to coerce sort values so they are compatible with the jsTree sort plugin
+function negVsPos(val) {
+  return (val < 0) ? -1 : 1;
+}
+
 // Natural sort from: http://stackoverflow.com/a/15479354/3894569
+// adapted to return only -1 or 1 using negVsPos function above
 function naturalCompare(a, b) {
   var ax = [], bx = [];
 
@@ -337,10 +344,10 @@ function naturalCompare(a, b) {
     var an = ax.shift();
     var bn = bx.shift();
     var nn = (an[0] - bn[0]) || an[1].localeCompare(bn[1], lang);
-    if(nn) return nn;
+    if(nn) return negVsPos(nn);
   }
 
-  return ax.length - bx.length;
+  return negVsPos(ax.length - bx.length);
 }
 
 function makeCallbacks(data, pageType) {

diff --git a/tests/ConceptPropertyTest.php b/tests/ConceptPropertyTest.php
@@ -123,12 +123,52 @@ public function testAddValue() {
    * @covers ConceptProperty::addValue
    * @covers ConceptProperty::sortValues
    */
-  public function testSortNotatedValues() {
+  public function testSortNotatedValuesLexical() {
+    # the vocabulary is configured to use lexical sorting
     $vocab = $this->model->getVocabulary('test-notation-sort');
     $concepts = $vocab->getConceptInfo('http://www.skosmos.skos/test/ta01', 'en');
     $concept = $concepts[0];
     $props = $concept->getProperties();
-    $expected = array("test:ta0112", "test:ta0119", "test:ta0117", "test:ta0116", "test:ta0114","test:ta0115","test:ta0113", "test:ta0120", "test:ta0111", );
+    $expected = array(
+      "test:ta0111", # 33.01
+      "test:ta0116", # 33.02
+      "test:ta0112", # 33.1
+      "test:ta0114", # 33.10
+      "test:ta0115", # 33.2
+      "test:ta0117", # 33.9
+      "test:ta0119", # 33.90
+      "test:ta0120", # K2
+      "test:ta0113"  # concept not defined, no notation code
+    );
+    $ret = array();
+
+    foreach($props['skos:narrower']->getValues() as $val) {
+        $ret[] = EasyRdf\RdfNamespace::shorten($val->getUri());
+    }
+    $this->assertEquals($expected, $ret);
+  }
+
+  /**
+   * @covers ConceptProperty::addValue
+   * @covers ConceptProperty::sortValues
+   */
+  public function testSortNotatedValuesNatural() {
+    # the vocabulary is configured to use natural sorting
+    $vocab = $this->model->getVocabulary('testNotation');
+    $concepts = $vocab->getConceptInfo('http://www.skosmos.skos/test/ta01', 'en');
+    $concept = $concepts[0];
+    $props = $concept->getProperties();
+    $expected = array(
+      "test:ta0111", # 33.01
+      "test:ta0116", # 33.02
+      "test:ta0112", # 33.1
+      "test:ta0115", # 33.2
+      "test:ta0117", # 33.9
+      "test:ta0114", # 33.10
+      "test:ta0119", # 33.90
+      "test:ta0120", # K2
+      "test:ta0113"  # concept not defined, no notation code
+    );
     $ret = array();
 
     foreach($props['skos:narrower']->getValues() as $val) {

diff --git a/tests/ConceptPropertyValueTest.php b/tests/ConceptPropertyValueTest.php
@@ -130,44 +130,6 @@ public function testToStringWhenSortByNotationNotSet() {
     $this->assertEquals('Carp', (string)$propvals['665 Carp http://www.skosmos.skos/test/ta112']);
   }
 
-  /**
-   * @covers ConceptPropertyValue::__toString
-   */
-  public function testToStringWithNotation() {
-    $mockres = $this->getMockBuilder('EasyRdf\\Resource')->disableOriginalConstructor()->getMock();
-    $mockvoc = $this->getMockBuilder('Vocabulary')->disableOriginalConstructor()->getMock();
-    $mockconf = $this->getMockBuilder('VocabularyConfig')->disableOriginalConstructor()->getMock();
-    $mocklit = $this->getMockBuilder('EasyRdf\\Literal')->disableOriginalConstructor()->getMock();
-    $mocklit->method('getValue')->will($this->returnValue('T3ST'));
-    $mockconf->method('sortByNotation')->will($this->returnValue(true));
-    $mockconf->method('showNotation')->will($this->returnValue(true));
-    $mockvoc->method('getConfig')->will($this->returnValue($mockconf));
-    $mockres->method('label')->will($this->returnValue('Term label'));
-    $mockres->method('get')->will($this->returnValue($mocklit));
-    $mockres->method('getUri')->will($this->returnValue('http://thisdoesntexistatalland.sefsf/2j2h4/'));
-    $propval = new ConceptPropertyValue($this->model, $mockvoc, $mockres, null);
-    $this->assertEquals('T3ST Term label', (string)$propval);
-  }
-
-  /**
-   * @covers ConceptPropertyValue::__toString
-   */
-  public function testToStringWhenNotationExistsButIsConfiguredOff() {
-    $mockres = $this->getMockBuilder('EasyRdf\\Resource')->disableOriginalConstructor()->getMock();
-    $mockvoc = $this->getMockBuilder('Vocabulary')->disableOriginalConstructor()->getMock();
-    $mockconf = $this->getMockBuilder('VocabularyConfig')->disableOriginalConstructor()->getMock();
-    $mocklit = $this->getMockBuilder('EasyRdf\\Literal')->disableOriginalConstructor()->getMock();
-    $mocklit->method('getValue')->will($this->returnValue('T3ST'));
-    $mockconf->method('sortByNotation')->will($this->returnValue(true));
-    $mockconf->method('showNotation')->will($this->returnValue(false));
-    $mockvoc->method('getConfig')->will($this->returnValue($mockconf));
-    $mockres->method('label')->will($this->returnValue('Term label'));
-    $mockres->method('get')->will($this->returnValue($mocklit));
-    $mockres->method('getUri')->will($this->returnValue('http://thisdoesntexistatalland.sefsf/2j2h4/'));
-    $propval = new ConceptPropertyValue($this->model, $mockvoc, $mockres, null);
-    $this->assertEquals('Term label', (string)$propval);
-  }
-
   /**
    * @covers ConceptPropertyValue::addSubMember
    * @covers ConceptPropertyValue::sortSubMembers

diff --git a/tests/VocabularyConfigTest.php b/tests/VocabularyConfigTest.php
@@ -318,12 +318,43 @@ public function testShowChangeListDefaultValue() {
   }
 
   /**
-   * @covers VocabularyConfig::sortByNotation
+   * @covers VocabularyConfig::getSortByNotation
+   * @covers VocabularyConfig::getLiteral
    * @covers VocabularyConfig::getBoolean
    */
   public function testShowSortByNotationDefaultValue() {
     $vocab = $this->model->getVocabulary('test');
-    $this->assertEquals(false, $vocab->getConfig()->sortByNotation());
+    $this->assertNull($vocab->getConfig()->getSortByNotation());
+  }
+
+  /**
+   * @covers VocabularyConfig::getSortByNotation
+   * @covers VocabularyConfig::getLiteral
+   * @covers VocabularyConfig::getBoolean
+   */
+  public function testShowSortByNotationTrueIsLexical() {
+    $vocab = $this->model->getVocabulary('test-notation-sort');
+    $this->assertEquals("lexical", $vocab->getConfig()->getSortByNotation());
+  }
+
+  /**
+   * @covers VocabularyConfig::getSortByNotation
+   * @covers VocabularyConfig::getLiteral
+   * @covers VocabularyConfig::getBoolean
+   */
+  public function testShowSortByNotationLexical() {
+    $vocab = $this->model->getVocabulary('test-qualified-notation');
+    $this->assertEquals("lexical", $vocab->getConfig()->getSortByNotation());
+  }
+
+  /**
+   * @covers VocabularyConfig::getSortByNotation
+   * @covers VocabularyConfig::getLiteral
+   * @covers VocabularyConfig::getBoolean
+   */
+  public function testShowSortByNotationNatural() {
+    $vocab = $this->model->getVocabulary('testNotation');
+    $this->assertEquals("natural", $vocab->getConfig()->getSortByNotation());
   }
 
   /**

diff --git a/tests/test-vocab-data/test-notation-sort.ttl b/tests/test-vocab-data/test-notation-sort.ttl
@@ -38,11 +38,12 @@ test:ta0111 a skos:Concept, meta:TestClass ;
     skos:broader test:ta01 ;
     skos:inScheme test:conceptscheme ;
     owl:deprecated true ;
+    skos:notation "33.01" ;
     skos:prefLabel "Tuna"@en .
 
 test:ta0112 a skos:Concept, meta:TestClass ;
     skosmos:testprop "Test property value" ;
-    skos:notation "665" ;
+    skos:notation "33.1" ;
     skos:broader test:ta01 ;
     skos:narrower test:ta0121 ;
     skos:exactMatch test:ta0118 ;
@@ -56,6 +57,7 @@ test:ta0112 a skos:Concept, meta:TestClass ;
 test:ta0114 a skos:Concept, meta:TestClass ;
     skos:broader test:ta01 ;
     dct:modified "1986-21-00"^^xsd:date ; # date invalid on purpose
+    skos:notation "33.10" ;
     skos:inScheme test:conceptscheme ;
     skos:prefLabel "Buri"@en .
 
@@ -64,16 +66,19 @@ test:ta0115 a skos:Concept, meta:TestClass ;
     skos:inScheme test:conceptscheme ;
     skos:definition "any fish belonging to the order Anguilliformes"@en, 
       "Iljettävä limanuljaska"@fi ;
+    skos:notation "33.2" ;
     skos:prefLabel "Eel"@en .
 
 test:ta0116 a skos:Concept, meta:TestClass ;
     skos:broader test:ta01 ;
     skos:inScheme test:conceptscheme ;
+    skos:notation "33.02" ;
     skos:prefLabel "Barracuda"@en .
 
 test:ta0117 a skos:Concept, meta:TestClass ;
     skos:broader test:ta01 ;
     skos:inScheme test:conceptscheme ;
+    skos:notation "33.9" ;
     skos:relatedMatch test:ta0115 ;
     skos:prefLabel "3D Barracuda"@en .
 
@@ -85,11 +90,12 @@ test:ta0118 a skos:Concept, meta:TestClass ;
 test:ta0119 a skos:Concept, meta:TestClass ;
     skos:broader test:ta01 ;
     skos:inScheme test:conceptscheme ;
-    skos:notation "690" ;
+    skos:notation "33.90" ;
     skos:prefLabel "Hauki"@fi .
 
 test:ta0120 a skos:Concept, meta:TestClass ;
     skos:broader test:ta01 ;
+    skos:notation "K2" ;
     skos:inScheme test:conceptscheme .
 
 test:ta0121 a skos:Concept, meta:TestClass ;

diff --git a/tests/testconfig.ttl b/tests/testconfig.ttl
@@ -117,6 +117,7 @@
     void:uriSpace "http://www.skosmos.skos/test-qualified-notation/" ;
     skosmos:defaultLanguage "en" ;
     skosmos:groupClass skos:Collection ;
+    skosmos:sortByNotation "lexical" ;
     skosmos:language "en" ;
     skosmos:alphabeticalListQualifier skos:notation ;
     skosmos:sparqlGraph <http://www.skosmos.skos/test-qualified-notation/> .
@@ -335,13 +336,13 @@
 :testNotation a skosmos:Vocabulary, void:Dataset ;
     dc11:title "A vocabulary for testing vocabularies with notation features"@en ;
     dc:subject :cat_general ;
-    void:uriSpace "http://www.skosmos.skos/notationFeatures/";
+    void:uriSpace "http://www.skosmos.skos/test-notation-sort/";
     skosmos:language "fi";
-    skosmos:sortByNotation true ;
+    skosmos:sortByNotation "natural" ;
     skosmos:searchByNotation true ;
     skosmos:showNotation true ;
     skosmos:useModifiedDate "true";
-    skosmos:sparqlGraph <http://www.skosmos.skos/notation/>;
+    skosmos:sparqlGraph <http://www.skosmos.skos/test-notation-sort/>;
     skosmos:mainConceptScheme test:notationMainConceptScheme .
 
 :paramPluginTest a skosmos:Vocabulary, void:Dataset ;