Merge pull request #669 from basecamp/time-filters

Add support for more time-related queries
2025-06-27 13:42:11 +02:00
parent fe8e73015c f5832f1a36
commit d06a38f883
36 changed files with 246252 additions and 31 deletions
@@ -24,6 +24,8 @@ class Card < ApplicationRecord
    when "oldest"  then chronologically
    when "latest"  then latest
    when "stalled" then stalled.chronologically
+    when "closing_soon" then closing_soon.chronologically
+    when "falling_back_soon" then falling_back_soon.chronologically
    when "closed"  then closed
    end
  end
@@ -8,6 +8,7 @@ module Card::Closeable
    scope :open, -> { where.missing(:closure) }

    scope :recently_closed_first, -> { closed.order("closures.created_at": :desc) }
+    scope :closed_at_window, ->(window) { closed.where("closures.created_at": window) }
  end

  def closed?
@@ -9,7 +9,23 @@ module Card::Entropic
    end

    scope :stagnated, -> { doing.entropic_by(:auto_reconsider_period) }
+
    scope :due_to_be_closed, -> { considering.entropic_by(:auto_close_period) }
+
+    scope :closing_soon, -> do
+      considering
+        .left_outer_joins(collection: :entropy_configuration)
+        .where("last_active_at >  DATETIME('now', '-' || COALESCE(entropy_configurations.auto_close_period, ?) || ' seconds')", Entropy::Configuration.default.auto_close_period)
+        .where("last_active_at <= DATETIME('now', '-' || CAST(COALESCE(entropy_configurations.auto_close_period, ?) * 0.75 AS INTEGER) || ' seconds')", Entropy::Configuration.default.auto_close_period)
+    end
+
+    scope :falling_back_soon, -> do
+      doing
+        .left_outer_joins(collection: :entropy_configuration)
+        .where("last_active_at >  DATETIME('now', '-' || COALESCE(entropy_configurations.auto_reconsider_period, ?) || ' seconds')", Entropy::Configuration.default.auto_reconsider_period)
+        .where("last_active_at <= DATETIME('now', '-' || CAST(COALESCE(entropy_configurations.auto_reconsider_period, ?) * 0.75 AS INTEGER) || ' seconds')", Entropy::Configuration.default.auto_reconsider_period)
+    end
+
    delegate :auto_close_period, :auto_reconsider_period, to: :collection
  end

@@ -43,7 +43,7 @@ class Command::Ai::Parser
      normalized_query.tap do |query_json|
        if query_context = query_json[:context].presence
          query_context[:assignee_ids] = query_context[:assignee_ids]&.filter_map { |name| assignee_from(name)&.id }
-          query_context[:creator_id] = assignee_from(query_context[:creator_id])&.id if query_context[:creator_id]
+          query_context[:creator_ids] = query_context[:creator_ids]&.filter_map { |name| assignee_from(name)&.id }
          query_context[:collection_ids] = query_context[:collection_ids]&.filter_map { |name| Collection.where("lower(name) like ?", "%#{name.downcase}%").first&.id }
          query_context[:tag_ids] = query_context[:tag_ids]&.filter_map { |name| ::Tag.find_by_title(name)&.id }
          query_context.compact!
@@ -40,13 +40,17 @@ class Command::Ai::Translator
        {
          "context": {                        /* REQUIRED unless empty */
            "terms": string[],
-            "indexed_by": "newest" | "oldest" | "latest" | "stalled" | "closed",
+            "indexed_by": "newest" | "oldest" | "latest" | "stalled" | "closed" | "closing_soon" | "falling_back_soon"
            "assignee_ids": string[],
            "assignment_status": "unassigned",
            "card_ids": number[],
-            "creator_id": string,
+            "creator_ids": string[],
            "collection_ids": string[],
-            "tag_ids": string[]
+            "tag_ids": string[],
+            "creation": "today" | "yesterday" | "thisweek" | "thismonth" | "thisyear"
+                      | "lastweek" | "lastmonth" | "lastyear",
+            "closure": "today" | "yesterday" | "thisweek" | "thismonth" | "thisyear"
+                        | "lastweek" | "lastmonth" | "lastyear"
          },
          "commands": string[]                /* OPTIONAL, each starts with "/" */
        }
@@ -77,34 +81,53 @@ class Command::Ai::Translator

        Context properties you may use
          * terms — array of keywords
-          * indexed_by — "newest", "oldest", "latest", "stalled", "closed"
+          * indexed_by — "newest", "oldest", "latest", "stalled", "closed", "closing_soon", "falling_back_soon"
          * assignee_ids — array of assignee names
          * assignment_status — "unassigned". Important: ONLY when the user asks for unassigned cards.
          * card_ids — array of card IDs
-          * creator_id — creator’s name
+          * creator_id — array of creator’s names
          * collection_ids — array of collections
          * tag_ids — array of tag names
+          * creation — relative range when the card was **created** (values listed above). Use it only
+            when the user asks for cards created in a specific timeframe.
+          * closure — relative range when the card was **completed/closed** (values listed above). Use it
+            only when the user asks for cards completed/closed in a specific timeframe.
+          * "Falling back soon" cards are cards in "Doing" that are going to be moved back to "Reconsidering" automatically soon.
+            - Falling back soon means to be reconsidered soon too.
+          * "Closing soon" cards are cards in "Considering" that are going to be closed  automatically soon.

        ---------------------- EXPLICIT FILTERING RULES ----------------------

        * Use terms only if the query explicitly refers to cards; plain-text searches go to /search.
        * Numbers without the word "card(s)" default to terms **unless the number is the direct object of an
          action verb that operates on cards (move, assign, tag, close, stage, consider, do, etc.).**
-            – "123"                         → terms: ["123"]
-            – "card 123"                          → card_id: ["123"]
-            – "card 1,2"                    → card_ids: [1, 2]
-            – "move 1 and 2 to doing"       → context.card_ids = [1, 2];  command /do
-            – "123" (with no action verb)   → terms: ["123"]
-          * Quick mnemonic
-              WORD “card(s)” present? → card_ids
-              ACTION verb present?   → card_ids + command
-              Otherwise              → terms
+            – "123" (with no action verb)   → context: { terms: ["123"] }
+            – "card 123"                    → context: { card_ids: [123] }
+            – "card 1,2"                    → context: { card_ids: [1, 2] }
+            – "move 1 and 2 to doing"       → context: { card_ids: [1, 2] }, commands: ["/do"]
+
+          Quick mnemonic
+            WORD “card(s)” present? → card_ids
+            ACTION verb present?    → card_ids + command
+            Otherwise               → terms
+
+        * "Completed/closed cards" ( **and NO words like
+          today, yesterday, thisweek, thismonth, thisyear,
+          lastweek, lastmonth, lastyear** ) → indexed_by: "closed"
+
+          – Never add "closure" unless one of the eight
+            timeframe tokens is present in the user text.
+
+        * Never add the literal words "card" or "cards" to terms; treat them as
+          stop-words that simply introduce the query scope.
        * "X collection"                  → collection_ids: ["X"]
        * **Past-tense** “assigned to X”  → assignee_ids: ["X"]  (filter)
        * **Imperative** “assign to X”, “assign to me” → command /assign X
          – Never use assignee_ids when the user gives an imperative assignment
-        * "Created by X"                  → creator_id: "X"
+        * "Created by X"                  → creator_id: ["X"]
        * "Stagnated or stalled cards"    → indexed_by: "stalled"
+        * "Closing soon" cards            → indexed_by: "closing_soon"
+        * "Falling back soon" cards       → indexed_by: "falling_back_soon"
        * **Past-tense** “tagged with #X”, “#X cards” → tag_ids: ["X"]           (filter)
        * **Imperative** “tag …”, “tag with #X”, “add the #X tag”, “apply #X”
          → command /tag #X   (never a filter)
@@ -112,20 +135,24 @@ class Command::Ai::Translator
          → assignment_status: "unassigned".
          – IMPORTANT: Only set assignment_status when the user **explicitly** asks for an unassigned state
          – Do NOT infer unassigned just because an assignment follows
-          – “Assign to David” → /assign david (do NOT include assignment_status)
        * "My cards"                      → assignee_ids of requester (if identifiable)
        * “Recent cards” (i.e., newly created) → indexed_by: "newest"
        * “Cards with recent activity”, “recently updated cards” → indexed_by: "latest"
          – Only use "latest" if the user mentions activity, updates, or changes
          – Otherwise, prefer "newest" for generic mentions of “recent”
-        * "Completed/closed cards"       → indexed_by: "closed"
+        * "Completed/closed cards" (no date range) → indexed_by: "closed"
+          – VERY IMPORTANT: Do **not** set "closure" filter unless the user explicitly supplies a timeframe
+            (e.g., “completed this month”, “closed last week”).
+          (If the timeframe is supplied with “closed” instead of “completed”, treat it the same way.)
+
        * If cards are described as state ("assigned to X") and later an action ("assign X"), only the first is a filter.
+
        * ❗ Once you produce a valid context **or** command list, do not add a fallback /search.

        -------------------- COMMAND INTERPRETATION RULES --------------------

-        * /do                       → engage with card and move it to "doing"
-        * /consider                → move card back to "considering" (reconsider)
+        * /do                        → engage with card and move it to "doing"
+        * /consider                 → move card back to "considering" (reconsider)
        * Unless a clear command applies, fallback to /search with the verbatim text.
        * When searching for nouns (non-person), prefer /search over terms.
        * Respect the spoken order of commands.
@@ -137,15 +164,17 @@ class Command::Ai::Translator
        * /visit [url or path] lets you visit arbitrary URLs and paths. E.g: /visit /cards/123
        * /stage [workflow stage]    → assign the card to the given stage
          – /stage never takes card IDs as arguments.
-        * “Move <ID(s)> to doing”        → context.card_ids = [IDs]; command /do
-        * “Move <ID(s)> to considering”  → context.card_ids = [IDs]; command /consider
        * “Move <ID(s)> to <Stage>”      → context.card_ids = [IDs]; command /stage <Stage>
+        * “Move <ID(s)> to doing”        → context.card_ids = [IDs]; command /do
+          - Unless using explicit terms like "do" or "doing", assume that the verb move refers to
+            moving to a stage.#{'        '}
+        * “Move <ID(s)> to considering”  → context.card_ids = [IDs]; command /consider
        * /add_card → Create a new card with a blank title
        * /add_card [title] → Create a new card with the provided title

        ---------------------------- VISIT SCREENS ---------------------------

-        You can open these screens by using /visits with their urls:
+        You can open these screens by using /visit with their urls:

        * My profile → /visit #{user_path(user)}
        * Edit my profile (including your name and avatar) → /visit #{edit_user_path(user)}
@@ -157,12 +186,15 @@ class Command::Ai::Translator
        * Never use names, tags, or stage names mentioned **inside commands** (like /assign, /tag, /stage) as filters.
          – e.g., “assign to jason” → only /assign jason (NOT assignee_ids)
          – e.g., “set the stage to Investigating” → only /stage Investigating (NOT terms)
-        * Never duplicate the assignee in both `commands` and `context`.
-          – If the request says “assign to X”, produce only `/assign X`, never assignee_ids
+        * Never duplicate the assignee in both commands and context.
+          – If the request says “assign to X”, produce only /assign X, never assignee_ids
        * Never add properties tied to UI view ("card", "list", etc.).
-        * When you see a word with a # prefix, assume it refers to a tag (either a filter or a command argument, but don't search for it').
+        * To filter completed or closed cards, use "indexed_by: closed", don't set a "closure" filter unless the user is
+          asking for cards completed in a certain window of time.
+        * When you see a word with a # prefix, assume it refers to a tag (either a filter or a command argument, but don't search for it).
        * All filters, including terms, must live **inside** context.
        * Do not duplicate terms across properties.
+        * Don't use "creation" and "closure" filters at the same time.
        * Avoid redundant terms.

        ---------------------------- OUTPUT CLEANLINESS ----------------------------
@@ -171,7 +203,7 @@ class Command::Ai::Translator
          – Do NOT include empty arrays (e.g., [], []).
          – Do NOT include empty strings ("") or default values that don't apply.
          – Do NOT emit unused or null context keys — omit them entirely.
-          – Example of bad output: {context: {terms: ["123"], card_ids: [], creator_id: ""}}
+          – Example of bad output: {context: {terms: ["123"], card_ids: [], creator_id: []}}
            ✅ Instead: {context: {terms: ["123"]}}

        * Similarly, only include commands if there are valid actions.
@@ -203,6 +235,18 @@ class Command::Ai::Translator
          "commands": ["/tag #design"]
        }

+        User: completed cards
+        Output:
+        {
+          "context": { "indexed_by": "closed" }
+        }
+
+        User: completed cards yesterday
+        Output:
+        {
+          "context": { "indexed_by": "closed", "closure": "yesterday" }
+        }
+
        User: "cards tagged with #design" or "#design cards"
        Output:
        {
@@ -222,6 +266,18 @@ class Command::Ai::Translator
          "commands": ["/close", "/assign kevin"]
        }

+        User: cards created yesterday
+        Output:
+        {
+          "context": { "creation": "yesterday" }
+        }
+
+        User: cards completed last week
+        Output:
+        {
+          "context": { "closure": "lastweek", "indexed_by": "closed" }
+        }
+
        Fallback search example (when nothing matches):
        { "commands": ["/search what's blocking deploy"] }

@@ -20,7 +20,7 @@ class Filter < ApplicationRecord
    @cards ||= begin
      result = creator.accessible_cards.indexed_by(indexed_by)
      result = result.where(id: card_ids) if card_ids.present?
-      result = result.open unless indexed_by.closed? || card_ids.present?
+      result = result.open unless indexed_by.closed? || closure_window || card_ids.present?
      result = result.by_engagement_status(engagement_status) if engagement_status.present?
      result = result.unassigned if assignment_status.unassigned?
      result = result.assigned_to(assignees.ids) if assignees.present?
@@ -28,6 +28,8 @@ class Filter < ApplicationRecord
      result = result.where(collection: collections.ids) if collections.present?
      result = result.in_stage(stages.ids) if stages.present?
      result = result.tagged_with(tags.ids) if tags.present?
+      result = result.where("cards.created_at": creation_window) if creation_window
+      result = result.closed_at_window(closure_window) if closure_window
      result = terms.reduce(result) do |result, term|
        result.mentioning(term)
      end
@@ -1,7 +1,7 @@
 module Filter::Fields
  extend ActiveSupport::Concern

-  INDEXES = %w[ newest oldest latest stalled ]
+  INDEXES = %w[ newest oldest latest stalled closing_soon falling_back_soon ]

  delegate :default_value?, to: :class

@@ -16,7 +16,8 @@ module Filter::Fields
  end

  included do
-    store_accessor :fields, :assignment_status, :indexed_by, :terms, :engagement_status, :card_ids
+    store_accessor :fields, :assignment_status, :indexed_by, :terms,
+      :engagement_status, :card_ids, :creation, :closure

    def assignment_status
      super.to_s.inquiry
@@ -30,6 +31,14 @@ module Filter::Fields
      super&.inquiry
    end

+    def creation_window
+      TimeWindowParser.parse(creation)
+    end
+
+    def closure_window
+      TimeWindowParser.parse(closure)
+    end
+
    def terms
      Array(super)
    end
@@ -5,6 +5,8 @@ module Filter::Params
    :assignment_status,
    :indexed_by,
    :engagement_status,
+    :creation,
+    :closure,
    card_ids: [],
    assignee_ids: [],
    creator_ids: [],
@@ -38,6 +40,8 @@ module Filter::Params
    {}.tap do |params|
      params[:indexed_by]        = indexed_by
      params[:engagement_status] = engagement_status
+      params[:creation]          = creation
+      params[:closure]           = closure
      params[:assignment_status] = assignment_status
      params[:terms]             = terms
      params[:tag_ids]           = tags.ids
@@ -0,0 +1,58 @@
+class TimeWindowParser
+  attr_reader :now
+
+  HUMAN_NAMES_BY_VALUE = {
+    "today" => "Today",
+    "yesterday" => "Yesterday",
+    "thisweek" => "This week",
+    "thismonth" => "This month",
+    "thisyear" => "This year",
+    "lastweek" => "Last week",
+    "lastmonth" => "Last month",
+    "lastyear" => "Last year"
+  }
+
+  VALUES = HUMAN_NAMES_BY_VALUE.keys
+
+  class << self
+    def parse(string)
+      new.parse(string)
+    end
+
+    def human_name_for(value)
+      HUMAN_NAMES_BY_VALUE[value]
+    end
+  end
+
+  def initialize(now: Time.current)
+    @now = now
+  end
+
+  def parse(string)
+    case normalize(string)
+    when "today"
+      now.beginning_of_day..now.end_of_day
+    when "yesterday"
+      (now - 1.day).beginning_of_day..(now - 1.day).end_of_day
+    when "thisweek"
+      now.beginning_of_week..now.end_of_week
+    when "thismonth"
+      now.beginning_of_month..now.end_of_month
+    when "thisyear"
+      now.beginning_of_year..now.end_of_year
+    when "lastweek"
+      (now - 1.week).beginning_of_week..(now - 1.week).end_of_week
+    when "lastmonth"
+      (now - 1.month).beginning_of_month..(now - 1.month).end_of_month
+    when "lastyear"
+      (now - 1.year).beginning_of_year..(now - 1.year).end_of_year
+    end
+  end
+
+  private
+    def normalize(string)
+      if string
+        string.downcase.gsub(/[\s_\-]/, "")
+      end
+    end
+end
@@ -18,10 +18,12 @@
    <%= render "filters/tags", filter: filter %>
    <%= render "filters/assignees", filter: filter %>
    <%= render "filters/creators", filter: filter %>
+    <%= render "filters/time_window", filter: filter, name: :creation, label: "Created" %>
+    <%= render "filters/time_window", filter: filter, name: :closure, label: "Closed" %>
    <%# FIXME LATER: this needs to be able to allow selection of unique stages across all active workflows %>
    <%#= render "filters/stages", filter: filter %>
    <%= render "filters/cards", filter: filter %>
-    
+

    <% filter.terms.each do |term| %>
      <%= filter_chip_tag %Q("#{term}"), filter.as_params_without(:terms, term) %>
@@ -0,0 +1,32 @@
+<% if filter.public_send("#{name}_window").present? %>
+  <div class="quick-filter position-relative" data-controller="dialog" data-action="keydown.esc->dialog#close click@document->dialog#closeOnClickOutside">
+    <button class="btn input input--select flex-inline txt-x-small" data-action="click->dialog#open:stop">
+      <span class="overflow-ellipsis">
+        <%= "#{label} #{TimeWindowParser.human_name_for(filter.public_send(name))&.downcase}" %>
+      </span>
+    </button>
+
+    <dialog class="events__popup popup panel flex-column align-start gap-half fill-white shadow"
+      aria-label="Created…" aria-description="Created…"
+      data-dialog-target="dialog" data-action="turbo:before-cache@document->dialog#close">
+      <strong class="popup__title margin-block-start-half pad-inline-half"><%= label %>…</strong>
+      <%= form_with url: cards_path, method: :get, class: "popup__list",
+            data: { controller: "form" } do |form| %>
+        <% filter.as_params.except(name).each do |key, value| %>
+          <%= filter_hidden_field_tag key, value %>
+        <% end %>
+
+        <% TimeWindowParser::VALUES.each do |value| %>
+          <div class="btn popup__item">
+            <%= form.radio_button name, value,
+                  checked: filter.public_send(name) == value,
+                  data: { action: "change->form#submit" } %>
+
+            <%= form.label name, TimeWindowParser.human_name_for(value), value: value, class: "overflow-ellipsis" %>
+            <%= icon_tag "check", class: "checked flex-item-justify-end" %>
+          </div>
+        <% end %>
+      <% end %>
+    </dialog>
+  </div>
+<% end %>
@@ -70,6 +70,16 @@ class Card::EntropicTest < ActiveSupport::TestCase
    assert_not cards(:shipping).reload.closed?
  end

+  test "closing soon scope" do
+    cards(:logo, :shipping).each(&:published!).each(&:reconsider)
+
+    cards(:logo).update!(last_active_at: entropy_configurations(:writebook_collection).auto_close_period.seconds.ago + 2.days)
+    cards(:shipping).update!(last_active_at: entropy_configurations(:writebook_collection).auto_close_period.seconds.ago - 2.days)
+
+    assert_includes Card.closing_soon, cards(:logo)
+    assert_not_includes Card.closing_soon, cards(:shipping)
+  end
+
  test "auto consider all stagnated using the default account entropy configuration" do
    travel_to Time.current

@@ -104,4 +114,16 @@ class Card::EntropicTest < ActiveSupport::TestCase
    assert cards(:logo).reload.considering?
    assert_equal Time.current, cards(:logo).last_active_at
  end
+
+  test "falling back scope" do
+    travel_to Time.current
+
+    cards(:logo, :shipping).each(&:engage)
+
+    cards(:logo).update!(last_active_at: 1.day.ago - entropy_configurations("writebook_collection").auto_close_period)
+    cards(:shipping).update!(last_active_at: 1.day.from_now - entropy_configurations("writebook_collection").auto_close_period)
+
+    assert_includes Card.falling_back_soon, cards(:shipping)
+    assert_not_includes Card.falling_back_soon, cards(:logo)
+  end
 end
@@ -121,6 +121,17 @@ class Command::Ai::TranslatorTest < ActionDispatch::IntegrationTest
    assert_command({ commands: [ "/add_card urgent issue" ] }, "create card urgent issue")
  end

+  test "filter by time ranges" do
+    assert_command({ context: { closure: "thisweek", indexed_by: "closed" } }, "cards completed this week")
+    assert_command({ context: { creation: "thisweek", tag_ids: [ "design" ], creator_ids: [ "jz" ] } }, "cards created this week by jz tagged as #design")
+  end
+
+  test "closing soon and falling back soon" do
+    assert_command({ context: { indexed_by: "falling_back_soon" } }, "cards to be reconsidered soon")
+    assert_command({ context: { indexed_by: "falling_back_soon" } }, "cards to be reconsidered soon")
+    assert_command({ context: { assignee_ids: [ "david" ], indexed_by: "closing_soon" } }, "my cards that are going to be auto closed")
+  end
+
  private
    def assert_command(expected, query, context: :list)
      assert_equal expected, translate(query, context:)
@@ -133,4 +133,24 @@ class FilterTest < ActiveSupport::TestCase
    assert filter.indexed_by.closed?
    assert_equal [ "haggis" ], filter.terms
  end
+
+  test "creation window" do
+    filter = users(:david).filters.new creation: "this week"
+
+    cards(:logo).update_columns created_at: 2.weeks.ago
+    assert_not_includes filter.cards, cards(:logo)
+
+    cards(:logo).update_columns created_at: Time.current
+    assert_includes filter.cards, cards(:logo)
+  end
+
+  test "closure window" do
+    filter = users(:david).filters.new closure: "this week"
+
+    cards(:shipping).closure.update_columns created_at: 2.weeks.ago
+    assert_not_includes filter.cards, cards(:shipping)
+
+    cards(:shipping).closure.update_columns created_at: Time.current
+    assert_includes filter.cards, cards(:shipping)
+  end
 end
@@ -0,0 +1,64 @@
+require "test_helper"
+
+class TimeWindowParserTest < ActiveSupport::TestCase
+  setup do
+    @now = Time.zone.parse("2023-06-15 9am")
+    @parser = TimeWindowParser.new(now: @now)
+  end
+
+  test "parse today" do
+    assert_equal @now.beginning_of_day..@now.end_of_day,
+      @parser.parse("today")
+  end
+
+  test "parse yesterday" do
+    yesterday = @now - 1.day
+
+    assert_equal yesterday.beginning_of_day..yesterday.end_of_day,
+      @parser.parse("yesterday")
+  end
+
+  test "parse this week" do
+    assert_equal @now.beginning_of_week..@now.end_of_week,
+      @parser.parse("this week")
+  end
+
+  test "parse this month" do
+    assert_equal @now.beginning_of_month..@now.end_of_month,
+      @parser.parse("this month")
+  end
+
+  test "parse this year" do
+    assert_equal @now.beginning_of_year..@now.end_of_year,
+      @parser.parse("this year")
+  end
+
+  test "parse last week" do
+    last_week = @now - 1.week
+
+    assert_equal last_week.beginning_of_week..last_week.end_of_week,
+      @parser.parse("last week")
+  end
+
+  test "parse last month" do
+    last_month = @now - 1.month
+
+    assert_equal last_month.beginning_of_month..last_month.end_of_month,
+      @parser.parse("last month")
+  end
+
+  test "parse last year" do
+    last_year = @now - 1.year
+
+    assert_equal last_year.beginning_of_year..last_year.end_of_year,
+      @parser.parse("last year")
+  end
+
+  test "parse with unknown string returns nil" do
+    assert_nil @parser.parse("unknown time window")
+  end
+
+  test "returns nil for nil" do
+    assert_nil @parser.parse(nil)
+  end
+end