Skip to content

Conversation

@mdroidian
Copy link
Contributor

@mdroidian mdroidian commented Oct 3, 2025

Intentionally left targetOptions blank, seems like it would just be confusing as it is the only option and 99% of input is raw text, but I'm 50/50 with this decision.

Summary by CodeRabbit

  • New Features
    • “With text” filters now support the {current user} tag, matching content that includes your name.
  • Bug Fixes
    • Improved accuracy and consistency of “with text” filtering across plain text and regex cases, reducing false positives/negatives.
  • Performance
    • Faster evaluations when using {current user} in “with text” filters by avoiding unnecessary pattern matching.

@linear
Copy link

linear bot commented Oct 3, 2025

@supabase
Copy link

supabase bot commented Oct 3, 2025

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@mdroidian
Copy link
Contributor Author

@CodeRabbit full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

📝 Walkthrough

Walkthrough

Adds special handling for the "{current user}" token in the "with text" translation path, reorders logic to check current-user first, retains regex handling when applicable, and introduces a general string-includes matching fallback for non-regex, non-current-user targets.

Changes

Cohort / File(s) Summary of Changes
"with text" translator branching
apps/roam/src/utils/conditionToDatalog.ts
Introduced early branch for "{current user}" to perform includes-based checks against the source String and current user name; moved existing regex path after this check; added non-regex fallback that matches block/string or node/title and applies clojure.string/includes?; preserved regex handling with re-pattern and re-find.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Translator as conditionToDatalog ("with text")
    participant User as CurrentUserResolver
    participant Regex as RegexEngine
    participant Match as StringIncludes

    Caller->>Translator: translate(condition: "with text", target)
    alt target == "{current user}"
        Translator->>User: getCurrentUser()
        User-->>Translator: userName
        Translator->>Match: includes?(sourceString, userName)
        Match-->>Translator: boolean
        Translator-->>Caller: Datalog clauses (includes-based)
    else target isRegex
        Translator->>Regex: re-pattern(target)
        Regex-->>Translator: pattern
        Translator->>Regex: re-find(pattern, sourceString)
        Regex-->>Translator: boolean
        Translator-->>Caller: Datalog clauses (regex-based)
    else non-regex general string
        Translator->>Match: includes?(sourceString from block/title, target)
        Match-->>Translator: boolean
        Translator-->>Caller: Datalog clauses (block/title + includes)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks

✅ 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 accurately and concisely summarizes the primary change—adding support for the `{current user}` token in the "with text" condition—and directly corresponds to the modifications described in the PR without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ab8b30 and 86b5c31.

📒 Files selected for processing (1)
  • apps/roam/src/utils/conditionToDatalog.ts (4 hunks)

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

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

@mdroidian mdroidian merged commit 67c327f into main Oct 3, 2025
5 checks passed
@mdroidian mdroidian deleted the eng-932-add-support-for-current-user-in-with-text-condition branch October 3, 2025 06:11
@github-project-automation github-project-automation bot moved this to Done in General Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants