add sorting to active users

[mdroidian]

Summary by CodeRabbit

• New Features
  • Enhanced ACTIVEUSERS command with a new optional Sort parameter. Users can now sort active user results by their text in ascending order (ASC), descending order (DESC), or preserve the default original order (NONE). This enhancement provides greater control and flexibility when viewing active user lists.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The ACTIVEUSERS command handler is updated with a new sort parameter to enable sorting active users by text in ascending, descending, or no particular order. Internal logic refactored from direct formatting to a collect-sort-format workflow, with help text updated to document the optional sort option.

Changes

Cohort / File(s)	Summary
ACTIVEUSERS Command Enhancement src/utils/core.ts	Updated handler signature to include sort parameter (ASC, DESC, NONE). Refactored internal logic from immediate formatting to collect users as objects, sort by specified order, then format results. Help text updated to document optional Sort parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Attention areas:
  • Verify sort parameter logic correctly handles all three cases (ASC, DESC, NONE)
  • Confirm formatting is properly applied to sorted results
  • Validate help text accuracy and parameter documentation

Poem

🐰 A sorting spell we weave with care,
Users sorted here and there,
ASC, DESC, or stand in place,
Order now adorns this space!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'add sorting to active users' directly and clearly describes the main change - adding a sort parameter to the ACTIVEUSERS command handler with sorting capabilities.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch add-sorting-active-users

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/utils/core.ts (1)

2309-2313: Guard against parseNlpDate returning null/undefined before .valueOf()

If the NLP expression cannot be parsed, parseNlpDate(...) may return a falsy value; calling .valueOf() on that will throw and break the workflow instead of failing gracefully.

You can mirror the DATE handler pattern by first resolving the date and only then calling .valueOf():

-      const timestamp =
-        parseNlpDate(nlp, getDateBasisDate()).valueOf() ||
-        // chrono fails basic parsing requiring forward date if ambiguous
-        // https://github.com/wanasit/chrono/commit/4f264a9f21fbd04eb740bf48f5616f6e6e0e78b7
-        parseNlpDate(`in ${nlp}`, getDateBasisDate()).valueOf();
+      const parsedDate =
+        parseNlpDate(nlp, getDateBasisDate()) ||
+        // chrono fails basic parsing requiring forward date if ambiguous
+        // https://github.com/wanasit/chrono/commit/4f264a9f21fbd04eb740bf48f5616f6e6e0e78b7
+        parseNlpDate(`in ${nlp}`, getDateBasisDate());
+      if (!parsedDate) {
+        return `Could not return a valid date with text "${nlp}"`;
+      }
+      const timestamp = parsedDate.valueOf();

This keeps behavior predictable even for bad NLP input.

🧹 Nitpick comments (1)

src/utils/core.ts (1)

2317-2341: ACTIVEUSERS result shaping and sorting: consider uniqueness, normalization, and minor ergonomics

Functionally this works, but a few improvements are worth considering:

1. Potential duplicate users
   The Datalog query returns one row per qualifying block, so a user who edited many blocks after timestamp will appear multiple times in activeUsers. If the intended semantics are “unique active users”, consider de‑duplicating by uid:

-      const activeUsers = (
-        window.roamAlphaAPI.data.fast.q(`...`) as [PullBlock, PullBlock][]
-      ).map(([user]) => ({
-        text: user[":node/title"] || user[":block/uid"] || "",
-        uid: user[":block/uid"],
-      }));
+      const activeUsersMap = new Map<string, { text: string; uid: string }>();
+      (
+        window.roamAlphaAPI.data.fast.q(`...`) as PullBlock[][]
+      ).forEach(([user]) => {
+        const uid = user[":block/uid"] as string;
+        const text = (user[":node/title"] as string) || uid || "";
+        if (uid && !activeUsersMap.has(uid)) {
+          activeUsersMap.set(uid, { text, uid });
+        }
+      });
+      const activeUsers = Array.from(activeUsersMap.values());

2. Normalize sort and avoid in‑place mutation / repeated formatter creation
   To be more forgiving (asc vs ASC) and a bit cleaner:

-      const sortedUsers =
-        sort === "NONE"
-          ? activeUsers
-          : activeUsers.sort(
-              sort === "DESC"
-                ? (a, b) => b.text.localeCompare(a.text)
-                : (a, b) => a.text.localeCompare(b.text)
-            );
-      return sortedUsers.map((user) => getFormatter(format)(user));
+      const normalizedSort = (sort || "NONE").toUpperCase();
+      const formatter = getFormatter(format);
+      const sortedUsers =
+        normalizedSort === "NONE"
+          ? activeUsers
+          : [...activeUsers].sort(
+              normalizedSort === "DESC"
+                ? (a, b) => b.text.localeCompare(a.text)
+                : (a, b) => a.text.localeCompare(b.text)
+            );
+      return sortedUsers.map((user) => formatter(user));

These changes are optional but would make the command more robust and predictable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2d2b23 and f7310a1.

📒 Files selected for processing (1)

• src/utils/core.ts (2 hunks)

🔇 Additional comments (1)

src/utils/core.ts (1)

2303-2308: ACTIVEUSERS: new sort parameter and help text look consistent

The added sort argument (with default "NONE") matches the updated help text and is backward compatible for existing calls that only pass 0–2 arguments. No issues here.