fix(section): preserve user-defined subscription order in UCI and backend

luci-app-tachyon/htdocs/luci-static/resources/view/tachyon/section.js

@@ -1674,6 +1674,12 @@ const SettingsDynamicList = form.DynamicList.extend({
         itemIds,
         this.ownerOption,
       );
+      itemIds.forEach((itemId, index) => {
+        uci.set(UCI_PACKAGE, itemId, "order", `${index}`);
+      });
+      if (typeof uci.reorder === "function") {
+        uci.reorder(UCI_PACKAGE, itemIds);
+      }
       if (typeof this.afterMaterializeChildItems === "function") {
         this.afterMaterializeChildItems(ownerId, itemIds);
       }
@@ -4823,16 +4829,28 @@ function getChildItemIds(section_id, typeName, ownerOption) {
     .sections(UCI_PACKAGE, typeName)
     .filter((item) => item[ownerKey] === section_id);
 
-  if (typeName === "priority_level") {
+  items.sort((a, b) => {
-    items.sort((a, b) => {
+    const hasOrderA = a && a.order != null && a.order !== "";
+    const hasOrderB = b && b.order != null && b.order !== "";
+    if (hasOrderA && hasOrderB) {
       const orderDiff = childItemOrder(a) - childItemOrder(b);
       if (orderDiff !== 0) {
         return orderDiff;
       }
+    } else if (hasOrderA) {
+      return -1;
+    } else if (hasOrderB) {
+      return 1;
+    }
 
+    if (typeName === "priority_level") {
       return `${a[".name"] || ""}`.localeCompare(`${b[".name"] || ""}`);
-    });
+    }
-  }
+
+    const indexA = a && a[".index"] != null ? a[".index"] : 0;
+    const indexB = b && b[".index"] != null ? b[".index"] : 0;
+    return indexA - indexB;
+  });
 
   return items.map((item) => item[".name"]).filter(Boolean);
 }
@@ -10317,6 +10335,11 @@ function createSectionContent(section) {
   o.renderListItemLabel = function (section_id, itemId) {
     return childItemInputValue(section_id, itemId, "subscription_url", "url");
   };
+  o.afterMaterializeChildItems = function (_section_id, itemIds) {
+    itemIds.forEach((itemId, index) => {
+      uci.set(UCI_PACKAGE, itemId, "order", `${index}`);
+    });
+  };
   o.validate = validateSubscriptionUrlEntry;
   o.validate = function (section_id, value) {
     return validateSubscriptionUrlEntry(

tachyon/files/usr/lib/config/connections.uc

@@ -165,11 +165,28 @@ function owned_child_section(parent, type_name, item_id) {
     return option(child, "section", "") == section_name(parent) ? child : null;
 }
 
+function child_item_sort(left, right) {
+    let left_has_order = (raw_option(left, "order") != null && raw_option(left, "order") != "");
+    let right_has_order = (raw_option(right, "order") != null && raw_option(right, "order") != "");
+    if (left_has_order && right_has_order) {
+        let left_order = int(option(left, "order", "0"), 10);
+        let right_order = int(option(right, "order", "0"), 10);
+        if (left_order != right_order)
+            return left_order < right_order ? -1 : 1;
+    } else if (left_has_order) {
+        return -1;
+    } else if (right_has_order) {
+        return 1;
+    }
+    return 0;
+}
+
 function child_items(parent, type_name) {
     let result = [];
     for (let child in object_or_empty(object_or_empty(get_item_sections())[type_name]).list || [])
         if (option(child, "section", "") == section_name(parent))
             push(result, child);
+    sort(result, child_item_sort);
     return result;
 }
 
@@ -179,6 +196,7 @@ function child_items_by_owner(type_name, owner_key, owner_value) {
     for (let child in object_or_empty(object_or_empty(get_item_sections())[type_name]).list || [])
         if (option(child, owner_key, "") == owner_value)
             push(result, child);
+    sort(result, child_item_sort);
     return result;
 }
 

tests/subscription_section_order.sh

@@ -0,0 +1,162 @@
+#!/usr/bin/env bash
+set -eo pipefail
+
+ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+TACHYON_LIB="${TACHYON_LIB:-$ROOT_DIR/tachyon/files/usr/lib}"
+WORK_DIR="$(mktemp -d)"
+
+cleanup() {
+  rm -rf "$WORK_DIR"
+}
+trap cleanup EXIT
+
+fail() {
+  printf 'FAIL: %s\n' "$1" >&2
+  exit 1
+}
+
+# 1. Create a mock uci state with 3 subscriptions in a section, where UCI file order is sub1, sub2, sub3,
+# but order options specify: sub2 (order 0), sub3 (order 1), sub1 (order 2)
+cat >"$WORK_DIR/uci.uc" <<'UCODE'
+let state = {
+    tachyon: {
+        sec1: {
+            ".name": "sec1",
+            ".type": "section",
+            action: "connection",
+            enabled: "1"
+        },
+        sub1: {
+            ".name": "sub1",
+            ".type": "subscription_url",
+            section: "sec1",
+            url: "https://example.com/third.txt",
+            order: "2"
+        },
+        sub2: {
+            ".name": "sub2",
+            ".type": "subscription_url",
+            section: "sec1",
+            url: "https://example.com/first.txt",
+            order: "0"
+        },
+        sub3: {
+            ".name": "sub3",
+            ".type": "subscription_url",
+            section: "sec1",
+            url: "https://example.com/second.txt",
+            order: "1"
+        }
+    }
+};
+
+function cursor() {
+    return {
+        load: function(_package_name) {
+            return true;
+        },
+        foreach: function(package_name, type_name, callback) {
+            let pkg = state["" + package_name] || {};
+            for (let name in pkg) {
+                let section = pkg[name];
+                if (section && section[".type"] == type_name) {
+                    callback(section);
+                }
+            }
+        }
+    };
+}
+
+return { cursor };
+UCODE
+
+ucode -L "$WORK_DIR" -L "$TACHYON_LIB" -e '
+let connections = require("config.connections");
+let cursor = require("uci").cursor();
+
+function assert(val, msg) {
+    if (!val) {
+        warn("Assertion failed: " + msg + "\n");
+        exit(1);
+    }
+}
+
+let index = connections.item_index_from_cursor(cursor, "tachyon");
+connections.set_item_sections_from_cursor(cursor, "tachyon");
+
+let sec = { ".name": "sec1", action: "connection" };
+let urls = connections.subscription_urls(sec);
+
+assert(length(urls) == 3, "expected 3 subscription urls, got " + length(urls));
+assert(urls[0] == "https://example.com/first.txt", "expected first url at index 0, got " + urls[0]);
+assert(urls[1] == "https://example.com/second.txt", "expected second url at index 1, got " + urls[1]);
+assert(urls[2] == "https://example.com/third.txt", "expected third url at index 2, got " + urls[2]);
+'
+
+# 2. Test fallback when items do not have order option: they should maintain their original order
+cat >"$WORK_DIR/uci_no_order.uc" <<'UCODE'
+let state = {
+    tachyon: {
+        sec1: {
+            ".name": "sec1",
+            ".type": "section",
+            action: "connection",
+            enabled: "1"
+        },
+        subA: {
+            ".name": "subA",
+            ".type": "subscription_url",
+            section: "sec1",
+            url: "https://example.com/alpha.txt"
+        },
+        subB: {
+            ".name": "subB",
+            ".type": "subscription_url",
+            section: "sec1",
+            url: "https://example.com/beta.txt"
+        }
+    }
+};
+
+function cursor() {
+    return {
+        load: function(_package_name) {
+            return true;
+        },
+        foreach: function(package_name, type_name, callback) {
+            let pkg = state["" + package_name] || {};
+            for (let name in pkg) {
+                let section = pkg[name];
+                if (section && section[".type"] == type_name) {
+                    callback(section);
+                }
+            }
+        }
+    };
+}
+
+return { cursor };
+UCODE
+
+ucode -L "$WORK_DIR" -L "$TACHYON_LIB" -e '
+let connections = require("config.connections");
+let cursor = require("uci_no_order").cursor();
+
+function assert(val, msg) {
+    if (!val) {
+        warn("Assertion failed: " + msg + "\n");
+        exit(1);
+    }
+}
+
+connections.set_item_sections_from_cursor(cursor, "tachyon");
+
+let sec = { ".name": "sec1", action: "connection" };
+let urls = connections.subscription_urls(sec);
+
+assert(length(urls) == 2, "expected 2 subscription urls");
+assert(urls[0] == "https://example.com/alpha.txt", "expected alpha first");
+assert(urls[1] == "https://example.com/beta.txt", "expected beta second");
+'
+
+echo "PASS: subscription_section_order"