From b5674223e59b6245317f61568d4952c184c86f50 Mon Sep 17 00:00:00 2001
From: zadam <zadam.apps@gmail.com>
Date: Thu, 22 Apr 2021 20:28:26 +0200
Subject: [PATCH] fixed some incorrect order by behavior, #1881

---
 src/public/app/services/note_list_renderer.js |  4 +++-
 src/services/search/expressions/and.js        |  3 +++
 src/services/search/expressions/or.js         |  4 ++++
 .../search/expressions/order_by_and_limit.js  | 24 +++++++++++++++----
 src/services/search/expressions/true.js       | 11 +++++++++
 src/services/search/services/parse.js         |  6 +++--
 6 files changed, 45 insertions(+), 7 deletions(-)
 create mode 100644 src/services/search/expressions/true.js

diff --git a/src/public/app/services/note_list_renderer.js b/src/public/app/services/note_list_renderer.js
index 10c0b8fc6..4537419f7 100644
--- a/src/public/app/services/note_list_renderer.js
+++ b/src/public/app/services/note_list_renderer.js
@@ -317,7 +317,9 @@ class NoteListRenderer {
         const $expander = $('<span class="note-expander bx bx-chevron-right"></span>');
 
         const {$renderedAttributes} = await attributeRenderer.renderNormalAttributes(note);
-        const notePath = this.parentNote.noteId + '/' + note.noteId;
+        const notePath = this.parentNote.type === 'search'
+            ? note.noteId // for search note parent we want to display non-search path
+            : this.parentNote.noteId + '/' + note.noteId;
 
         const $card = $('<div class="note-book-card">')
             .attr('data-note-id', note.noteId)
diff --git a/src/services/search/expressions/and.js b/src/services/search/expressions/and.js
index 4ac216939..8708cfada 100644
--- a/src/services/search/expressions/and.js
+++ b/src/services/search/expressions/and.js
@@ -1,6 +1,7 @@
 "use strict";
 
 const Expression = require('./expression');
+const TrueExp = require("./true.js");
 
 class AndExp extends Expression {
     static of(subExpressions) {
@@ -10,6 +11,8 @@ class AndExp extends Expression {
             return subExpressions[0];
         } else if (subExpressions.length > 0) {
             return new AndExp(subExpressions);
+        } else {
+            return new TrueExp();
         }
     }
 
diff --git a/src/services/search/expressions/or.js b/src/services/search/expressions/or.js
index d04725670..4aabb7a65 100644
--- a/src/services/search/expressions/or.js
+++ b/src/services/search/expressions/or.js
@@ -2,6 +2,7 @@
 
 const Expression = require('./expression');
 const NoteSet = require('../note_set');
+const TrueExp = require("./true");
 
 class OrExp extends Expression {
     static of(subExpressions) {
@@ -13,6 +14,9 @@ class OrExp extends Expression {
         else if (subExpressions.length > 0) {
             return new OrExp(subExpressions);
         }
+        else {
+            return new TrueExp();
+        }
     }
 
     constructor(subExpressions) {
diff --git a/src/services/search/expressions/order_by_and_limit.js b/src/services/search/expressions/order_by_and_limit.js
index 80f08ab2e..ee6179215 100644
--- a/src/services/search/expressions/order_by_and_limit.js
+++ b/src/services/search/expressions/order_by_and_limit.js
@@ -28,17 +28,33 @@ class OrderByAndLimitExp extends Expression {
                 let valA = valueExtractor.extract(a);
                 let valB = valueExtractor.extract(b);
 
-                if (!isNaN(valA) && !isNaN(valB)) {
+                if (valA === null && valB === null) {
+                    // neither has attribute at all
+                    continue;
+                }
+                else if (valB === null) {
+                    return smaller;
+                }
+                else if (valA === null) {
+                    return larger;
+                }
+
+                // if both are numbers then parse them for numerical comparison
+                // beware that isNaN will return false for empty string and null
+                if (valA.trim() !== "" && valB.trim() !== "" && !isNaN(valA) && !isNaN(valB)) {
                     valA = parseFloat(valA);
                     valB = parseFloat(valB);
                 }
 
-                if (valA < valB) {
+                if (!valA && !valB) {
+                    // the attribute is not defined in either note so continue to next order definition
+                    continue;
+                } else if (!valB || valA < valB) {
                     return smaller;
-                } else if (valA > valB) {
+                } else if (!valA || valA > valB) {
                     return larger;
                 }
-                // else go to next order definition
+                // else the values are equal and continue to next order definition
             }
 
             return 0;
diff --git a/src/services/search/expressions/true.js b/src/services/search/expressions/true.js
new file mode 100644
index 000000000..1ffab8bf1
--- /dev/null
+++ b/src/services/search/expressions/true.js
@@ -0,0 +1,11 @@
+"use strict";
+
+const Expression = require('./expression');
+
+class TrueExp extends Expression {
+    execute(inputNoteSet, executionContext) {
+        return inputNoteSet;
+    }
+}
+
+module.exports = TrueExp;
diff --git a/src/services/search/services/parse.js b/src/services/search/services/parse.js
index 2371c318c..afaf5389c 100644
--- a/src/services/search/services/parse.js
+++ b/src/services/search/services/parse.js
@@ -329,6 +329,9 @@ function getExpression(tokens, searchContext, level = 0) {
         else if (op === 'or') {
             return OrExp.of(expressions);
         }
+        else {
+            throw new Error(`Unrecognized op=${op}`);
+        }
     }
 
     for (i = 0; i < tokens.length; i++) {
@@ -358,8 +361,7 @@ function getExpression(tokens, searchContext, level = 0) {
                 continue;
             }
 
-            exp.subExpression = getAggregateExpression();
-
+            exp.subExpression = getAggregateExpression();console.log(exp);
             return exp;
         }
         else if (token === 'not') {