Refactor notes selection to use selector struct

Replaces the legacy select_notes function with a new version that takes a selector struct, simplifying and clarifying the API for selecting notes. Updates all usages throughout the codebase, including tests, to use the new selector-based approach. Removes the bible_selector field from the selector struct and adjusts logic to handle Bible filtering via the bibles vector. Cleans up related comments and improves code readability.
#1046

Author: teusbenschop

changes/change.cpp

@@ -122,19 +122,23 @@ std::string changes_change (Webserver_Request& webserver_request)
 
 
   // Get notes for the passage.
-  std::vector<int> notes = database_notes.select_notes (bibles, // Bibles.
-                                                        passage.m_book, passage.m_chapter, filter::strings::convert_to_int (passage.m_verse),
-                                                        0,  // Passage selector.
-                                                        0,  // Edit selector.
-                                                        0,  // Non-edit selector.
-                                                        {}, // Status selectors.
-                                                        "", // Bible selector.
-                                                        "", // Assignment selector.
-                                                        0,  // Subscription selector.
-                                                        -1, // Severity selector.
-                                                        0,  // Text selector.
-                                                        "", // Search text.
-                                                        -1); // Limit.
+  Database_Notes::selector selector {
+    .bibles = bibles,
+    .book = passage.m_book, \
+    .chapter = passage.m_chapter,
+    .verse = filter::strings::convert_to_int (passage.m_verse),
+    .passage_selector = 0,
+    .edit_selector = 0,
+    .non_edit_selector = 0,
+    .status_selectors = {},
+    .assignment_selector = "",
+    .subscription_selector = 0,
+    .severity_selector = -1,
+    .text_selector = 0,
+    .search_text = "",
+    .limit = -1
+  };
+  std::vector<int> notes = database_notes.select_notes(selector);
 
   // Remove the ones marked for deletion.
   {
@@ -144,7 +148,7 @@ std::string changes_change (Webserver_Request& webserver_request)
         notes2.push_back (note);
       }
     }
-    notes = notes2;
+    notes = std::move(notes2);
   }
 
   // Sort them, most recent notes first.
@@ -154,7 +158,7 @@ std::string changes_change (Webserver_Request& webserver_request)
     timestamps.push_back (timestap);
   }
   filter::strings::quick_sort (timestamps, notes, 0, static_cast <unsigned int> (notes.size ()));
-  reverse (notes.begin(), notes.end());
+  std::reverse (notes.begin(), notes.end());
 
 
   // Whether there"s a live notes editor available.

database/notes.cpp

@@ -596,190 +596,7 @@ int Database_Notes::store_new_note (const std::string& bible, int book, int chap
 }
 
 
-// Returns an array of note identifiers selected.
-std::vector <int> Database_Notes::select_notes (std::vector <std::string> bibles, int book, int chapter, int verse, int passage_selector, int edit_selector, int non_edit_selector, const std::vector<std::string>& status_selectors, std::string bible_selector, std::string assignment_selector, bool subscription_selector, int severity_selector, int text_selector, const std::string& search_text, int limit)// Todo phase out.
-{
-  const std::string& username = m_webserver_request.session_logic ()->get_username ();
-  std::vector <int> identifiers;
-  // SQL SELECT statement.
-  std::string query = notes_select_identifier ();
-  // SQL optional fulltext search statement sorted on relevance.
-  if (text_selector == 1) {
-    query.append (notes_optional_fulltext_search_relevance_statement (search_text));
-  }
-  // SQL FROM ... WHERE statement.
-  query.append (notes_from_where_statement ());
-  // Consider passage selector.
-  std::string passage;
-  switch (passage_selector) {
-    case 0:
-      // Select notes that refer to the current verse.
-      // It means that the book, the chapter, and the verse, should match.
-      passage = encode_passage (book, chapter, verse);
-      query.append (" AND passage LIKE '%" + passage + "%' ");
-      break;
-    case 1:
-      // Select notes that refer to the current chapter.
-      // It means that the book and the chapter should match.
-      passage = encode_passage (book, chapter, -1);
-      query.append (" AND passage LIKE '%" + passage + "%' ");
-      break;
-    case 2:
-      // Select notes that refer to the current book.
-      // It means that the book should match.
-      passage = encode_passage (book, -1, -1);
-      query.append (" AND passage LIKE '%" + passage + "%' ");
-      break;
-    case 3:
-      // Select notes that refer to any passage: No constraint to apply here.
-      break;
-    default: break;
-  }
-  // Consider edit selector.
-  int time { 0 };
-  switch (edit_selector) {
-    case 0:
-      // Select notes that have been edited at any time. Apply no constraint.
-      time = 0;
-      break;
-    case 1:
-      // Select notes that have been edited during the last 30 days.
-      time = filter::date::seconds_since_epoch () - 30 * 24 * 3600;
-      break;
-    case 2:
-      // Select notes that have been edited during the last 7 days.
-      time = filter::date::seconds_since_epoch () - 7 * 24 * 3600;
-      break;
-    case 3:
-      // Select notes that have been edited since yesterday.
-      time = filter::date::seconds_since_epoch () - 1 * 24 * 3600 - filter::date::numerical_hour (filter::date::seconds_since_epoch ()) * 3600;
-      break;
-    case 4:
-      // Select notes that have been edited today.
-      time = filter::date::seconds_since_epoch () - filter::date::numerical_hour (filter::date::seconds_since_epoch ()) * 3600;
-      break;
-    default: break;
-  }
-  if (time != 0) {
-    query.append (" AND modified >= ");
-    query.append (std::to_string (time));
-    query.append (" ");
-  }
-  // Consider non-edit selector.
-  int nonedit { 0 };
-  switch (non_edit_selector) {
-    case 0:
-      // Select notes that have not been edited at any time. Apply no constraint.
-      nonedit = 0;
-      break;
-    case 1:
-      // Select notes that have not been edited for a day.
-      nonedit = filter::date::seconds_since_epoch () - 1 * 24 * 3600;
-      break;
-    case 2:
-      // Select notes that have not been edited for two days.
-      nonedit = filter::date::seconds_since_epoch () - 2 * 24 * 3600;
-      break;
-    case 3:
-      // Select notes that have not been edited for a week.
-      nonedit = filter::date::seconds_since_epoch () - 7 * 24 * 3600;
-      break;
-    case 4:
-      // Select notes that have not been edited for a month.
-      nonedit = filter::date::seconds_since_epoch () - 30 * 24 * 3600;
-      break;
-    case 5:
-      // Select notes that have not been edited for a year.
-      nonedit = filter::date::seconds_since_epoch () - 365 * 24 * 3600;
-      break;
-    default: break;
-  }
-  if (nonedit != 0) {
-    query.append (" AND modified <= ");
-    query.append (std::to_string (nonedit));
-    query.append (" ");
-  }
-  // Consider the status selectors.
-  if (!status_selectors.empty()) {
-    query.append (" AND (status = '' ");
-    for (const auto& status : status_selectors) {
-      query.append (" OR status = '");
-      query.append (status);
-      query.append ("' ");
-    }
-    query.append (" ) ");
-  }
-  
-  // Consider two different Bible constraints:
-  // 1. The vector of bibles: "bibles".
-  //    This contains all the Bibles a user has access to, so only notes that refer to any Bible in this lot are going to be selected.
-  // 2. The string "bible_selector".
-  //    If this is left empty, then it selects notes that refer to Bibles in the vector above.
-  //    If this contains a Bible, then it selects notes that refer to this Bible.
-  // In addition to the above two selectors, it always selects note that refer to any Bible.
-  if (!bible_selector.empty()) {
-    bibles.clear ();
-    bibles.push_back (bible_selector);
-  }
-  if (!bibles.empty ()) {
-    query.append (" AND (bible = '' ");
-    for (auto bible : bibles) {
-      bible = database::sqlite::no_sql_injection (bible);
-      query.append (" OR bible = '");
-      query.append (bible);
-      query.append ("' ");
-    }
-    query.append (" ) ");
-  }
-  // Consider note assignment constraints.
-  if (assignment_selector != "") {
-    assignment_selector = database::sqlite::no_sql_injection (assignment_selector);
-    query.append (" AND assigned LIKE '% ");
-    query.append (assignment_selector);
-    query.append (" %' ");
-  }
-  // Consider note subscription constraints.
-  if (subscription_selector) {
-    query.append (" AND subscriptions LIKE '% ");
-    query.append (username);
-    query.append (" %' ");
-  }
-  // Consider the note severity.
-  if (severity_selector != -1) {
-    query.append (" AND severity = ");
-    query.append (std::to_string (severity_selector));
-    query.append (" ");
-  }
-  // Consider text contained in notes.
-  if (text_selector == 1) {
-    query.append (notes_optional_fulltext_search_statement (search_text));
-  }
-  if (text_selector == 1) {
-    // If searching in fulltext mode, notes get ordered on relevance of search hits.
-    query.append (notes_order_by_relevance_statement ());
-  } else {
-    // Notes get ordered by the passage they refer to. It is a rough method and better ordering is needed.
-    query.append (" ORDER BY ABS (passage) ");
-  }
-  // Limit the selection if a limit is given.
-  if (limit >= 0) {
-    query.append (" LIMIT ");
-    query.append (std::to_string (limit));
-    query.append (", 50 ");
-  }
-  query.append (";");
-
-  SqliteDatabase sql (database_notes);
-  sql.set_sql (query);
-  const std::vector <std::string> result = sql.query () ["identifier"];
-  for (const auto& id : result) {
-    identifiers.push_back (filter::strings::convert_to_int (id));
-  }
-  return identifiers;
-}
-
-
-std::vector <int> Database_Notes::select_notes (const selector& selector)
+std::vector<int> Database_Notes::select_notes(const selector& selector)
 {
   const std::string& username = m_webserver_request.session_logic ()->get_username ();
   std::vector <int> identifiers;
@@ -892,21 +709,15 @@ std::vector <int> Database_Notes::select_notes (const selector& selector)
     query.append (" ) ");
   }
 
-  // Consider two different Bible constraints:
+  // Consider the Bible constraints:
   // 1. The vector of bibles: "bibles".
-  //    This contains all the Bibles a user has access to, so only notes that refer to any Bible in this lot are going to be selected.
-  // 2. The string "bible_selector".
-  //    If this is left empty, then it selects notes that refer to Bibles in the vector above.
-  //    If this contains a Bible, then it selects notes that refer to this Bible.
-  // In addition to the above two selectors, it always selects note that refer to any Bible.
-  std::vector<std::string> bibles {selector.bibles};
-  if (!selector.bible_selector.empty()) {
-    bibles.clear ();
-    bibles.push_back (selector.bible_selector);
-  }
-  if (!bibles.empty ()) {
+  //    This contains all the Bibles a user has access to,
+  //    so only notes that refer to any Bible in this lot are going to be selected.
+  //    Or it contains the Bible to be searched for notes.
+  // In addition to the above s electors, it always selects notes that refer to any Bible.
+  if (!selector.bibles.empty ()) {
     query.append (" AND (bible = '' ");
-    for (auto bible : bibles) {
+    for (auto bible : selector.bibles) {
       bible = database::sqlite::no_sql_injection (bible);
       query.append (" OR bible = '");
       query.append (bible);

database/notes.h

@@ -71,7 +71,9 @@ class Database_Notes
 
 public:
   struct selector {
-    // Contaiiner of Bible names the user has read access to.
+    // Container of Bible names that is going to be searched.
+    // Can contains all Bibles the user has read access to.
+    // Or can contain one Bible to be searched.
     std::vector<std::string> bibles{};
     // Four related selectors that can limit the selection.
     int book{};
@@ -81,10 +83,8 @@ class Database_Notes
     // Optionally constrains selection based on modification time.
     int edit_selector{}; // Todo default and enum?
     int non_edit_selector{}; // Todo default and enum?
-    // Optionally constrains selection based on note status.
+    // Optionally constrains selection based on list of note statuses.
     std::vector<std::string> status_selectors; // Todo enums?
-    // Optionally constrains the selection, based on the note's Bible.
-    std::string bible_selector{};
     // Optionally constrains the selection based on a note being assigned to somebody.
     std::string assignment_selector{};
     // Optionally limits the selection based on a note's subscription.
@@ -98,7 +98,6 @@ class Database_Notes
     // If >= 0, it indicates the starting limit for the selection.
     int limit{0};
   };
-  std::vector <int> select_notes (std::vector <std::string> bibles, int book, int chapter, int verse, int passage_selector, int edit_selector, int non_edit_selector, const std::vector<std::string>& status_selectors, std::string bible_selector, std::string assignment_selector, bool subscription_selector, int severity_selector, int text_selector, const std::string& search_text, int limit);
   std::vector<int> select_notes (const selector& selector);
 private:
   std::string notes_select_identifier ();

notes/bulk.cpp

@@ -69,15 +69,15 @@ std::string notes_bulk (Webserver_Request& webserver_request)
   std::string success, error;
 
 
-  std::vector <std::string> bibles = access_bible::bibles (webserver_request);
+  std::vector <std::string> bibles = {webserver_request.database_config_user()->get_consultation_notes_bible_selector()};
+  if (bibles.empty()) bibles = access_bible::bibles (webserver_request);
   int book = Ipc_Focus::getBook (webserver_request);
   int chapter = Ipc_Focus::getChapter (webserver_request);
   int verse = Ipc_Focus::getVerse (webserver_request);
   int passage_selector = webserver_request.database_config_user()->get_consultation_notes_passage_selector();
   int edit_selector = webserver_request.database_config_user()->get_consultation_notes_edit_selector();
   int non_edit_selector = webserver_request.database_config_user()->get_consultation_notes_non_edit_selector();
   const std::vector<std::string> status_selectors = webserver_request.database_config_user()->get_consultation_notes_status_selectors();
-  std::string bible_selector = webserver_request.database_config_user()->get_consultation_notes_bible_selector();
   std::string assignment_selector = webserver_request.database_config_user()->get_consultation_notes_assignment_selector();
   bool subscription_selector = webserver_request.database_config_user()->get_consultation_notes_subscription_selector();
   int severity_selector = webserver_request.database_config_user()->get_consultation_notes_severity_selector();
@@ -111,21 +111,23 @@ std::string notes_bulk (Webserver_Request& webserver_request)
   // This is done to remember them as long as this page is active.
   // Thus erroneous bulk operations on notes can be rectified somewhat easier.
   if (!subscribe && !unsubscribe && !assign && !unassign && !status && !severity && !bible && !erase) {
-    std::vector <int> identifiers = database_notes.select_notes (bibles,
-                                                                 book,
-                                                                 chapter,
-                                                                 verse,
-                                                                 passage_selector,
-                                                                 edit_selector,
-                                                                 non_edit_selector,
-                                                                 status_selectors,
-                                                                 bible_selector,
-                                                                 assignment_selector,
-                                                                 subscription_selector,
-                                                                 severity_selector,
-                                                                 text_selector,
-                                                                 search_text,
-                                                                 -1);
+    Database_Notes::selector selector {
+      .bibles = bibles,
+      .book = book,
+      .chapter = chapter,
+      .verse = verse,
+      .passage_selector = passage_selector,
+      .edit_selector = edit_selector,
+      .non_edit_selector = non_edit_selector,
+      .status_selectors = status_selectors,
+      .assignment_selector = assignment_selector,
+      .subscription_selector = subscription_selector,
+      .severity_selector = severity_selector,
+      .text_selector = text_selector,
+      .search_text = search_text,
+      .limit = -1
+    };
+    std::vector <int> identifiers = database_notes.select_notes (selector);
     std::vector <std::string> sids;
     for (auto id : identifiers) sids.push_back (std::to_string (id));
     database::temporal::set_value (userid, "identifiers", filter::strings::implode (sids, " "));