EBA-19381 | EBA | Member Search - Add confirmed saved-search deletion

CHANGELOG.md

@@ -11,6 +11,13 @@ change public APIs; each entry says how.
 
 ### Added
 
+- Confirmed per-entry saved-search deletion through the CSRF-protected POST
+  form. Stores implement `delete(context:, id:)` with a current-user-scoped
+  lookup and a boolean result. Deletion preserves the active query, sort and
+  page, updates the Saved star, and refuses denied scopes and GET mutations.
+  The saved-search popover stays inside narrow mobile viewports. Each entry
+  shares one hover/focus highlight with its inline delete icon, while Apply
+  and Delete keep separate, non-overlapping click targets.
 - `config.logger`: querylane logs why a search was unavailable (Meilisearch
   unreachable, timed out, or an API error such as a missing index). API
   errors are logged by code, type and HTTP status only, never their message,
@@ -72,6 +79,9 @@ change public APIs; each entry says how.
 
 ### Fixed
 
+- The saved-search POST-only guard and request parser share normalization for
+  symbol- and string-keyed params, including false-valued host params, even
+  when GET search queries are enabled.
 - Space on a record right after a new answer drew its rows opens the preview.
   The preview state was reset on `turbo:frame-render`, which Turbo fires a few
   repaints after the rows are in, so the preview's answer was dropped as if

app/assets/stylesheets/querylane.css

@@ -278,18 +278,29 @@
 .ql-saved-refusal { margin: 0; padding: 10px 16px; border-bottom: 1px solid var(--ql-rule); font-size: 13px; color: var(--ql-text); }
 .ql-saved-heading { padding: 10px 16px 4px; font-size: 13px; font-weight: 600; letter-spacing: 0.06em; font-variant-caps: all-small-caps; color: var(--ql-muted); }
 .ql-saved-none { margin: 0; padding: 4px 16px 14px; color: var(--ql-muted); font-size: 13px; }
-.ql-saved-item { display: grid; grid-template-columns: 1fr auto; gap: 2px 12px; width: 100%; padding: 8px 16px; border: 0;
-  background: none; text-align: left; }
-.ql-saved-item:hover, .ql-saved-item:focus-visible { background: var(--ql-accent-soft); }
-.ql-saved-name { font-weight: 600; color: var(--ql-heading); }
-.ql-saved-query { grid-column: 1; font: 400 12px/18px var(--ql-font-mono); color: var(--ql-muted); }
+.ql-saved-row { display: grid; grid-template-columns: minmax(0, 1fr) auto; align-items: center; margin: 2px 8px; padding-right: 4px; border-radius: 4px; }
+.ql-saved-row:hover, .ql-saved-row:focus-within { background: var(--ql-accent-soft); }
+.ql-saved-delete { min-width: 32px; height: 32px; padding: 4px; border: 0; border-radius: 4px; background: none; color: var(--ql-muted); }
+.ql-saved-delete:hover, .ql-saved-delete:focus-visible { color: var(--ql-error); }
+.ql-saved-item { display: grid; grid-template-columns: 1fr auto; gap: 2px 12px; width: 100%; padding: 8px; border: 0;
+  min-width: 0; background: none; text-align: left; }
+.ql-saved-name { overflow-wrap: anywhere; font-weight: 600; color: var(--ql-heading); }
+.ql-saved-query { grid-column: 1; overflow-wrap: anywhere; font: 400 12px/18px var(--ql-font-mono); color: var(--ql-muted); }
 .ql-saved-used { grid-column: 2; grid-row: 1 / span 2; align-self: center; font-size: 12px; color: var(--ql-faint); }
 .ql-notice { margin-top: 8px; font-size: 13px; color: var(--ql-text); }
 .ql-notice-refused, .ql-notice-store_failed { color: var(--ql-error); }
 @supports (anchor-name: --ql-saved) {
   .ql-box-saved { anchor-name: --ql-saved; }
   .ql-saved { position-anchor: --ql-saved; inset: auto; top: anchor(bottom); right: anchor(right); margin: 8px 0 0; }
 }
+@media (max-width: 600px) {
+  .ql-saved { left: 16px; right: 16px; width: auto; }
+  .ql-saved-head { grid-template-columns: minmax(0, 1fr) auto; gap: 8px; }
+  .ql-saved-head label { grid-column: 1 / -1; }
+  .ql-saved-head input { min-width: 0; }
+  .ql-saved-item { grid-template-columns: minmax(0, 1fr); }
+  .ql-saved-used { grid-column: 1; grid-row: auto; }
+}
 
 /* Searching, too short, reasons, unavailable, denied ------------------------ */
 .ql-box-busy-label { display: none; flex-shrink: 0; font-size: 13px; color: var(--ql-muted); }

app/javascript/querylane/controllers/box_controller.js

@@ -90,7 +90,7 @@ export default class extends Controller {
   // new revision; the rewrite comes back at that same revision.
   submitted(event) {
     clearTimeout(this.timer)
-    if (["toggle", "remove", "clear", "compose_submit", "saved", "fix", "instead", "widen", "relax", "as_words"].includes(event.submitter?.name)) this.bump()
+    if (["toggle", "remove", "clear", "compose_submit", "saved", "delete_saved", "fix", "instead", "widen", "relax", "as_words"].includes(event.submitter?.name)) this.bump()
   }
 
   setQuery(event) {

app/javascript/querylane/controllers/results_controller.js

@@ -81,6 +81,10 @@ export default class extends Controller {
     document.getElementById(event.target.dataset.querylaneEnter)?.click()
   }
 
+  deleteSaved(event) {
+    if (!window.confirm(event.currentTarget.dataset.confirm)) event.preventDefault()
+  }
+
   closePreview() {
     if (!this.hasPreviewTarget) return
     this.previewTarget.innerHTML = ""

app/views/querylane/_saved.html.erb

@@ -3,6 +3,7 @@
     Name and Save belong to the search form; choosing one sends its id and the
     server rewrites the box. %>
 <div id="<%= id %>-saved" popover class="ql-saved">
+  <input type="hidden" name="delete_saved_page" value="<%= result.page %>" form="<%= id %>-form">
   <div class="ql-saved-head">
     <label for="<%= id %>-saved-name"><%= t("querylane.saved.title") %></label>
     <input type="text" id="<%= id %>-saved-name" name="saved_name" form="<%= id %>-form" maxlength="80" autocomplete="off"
@@ -23,10 +24,17 @@
     <p class="ql-saved-none"><%= t("querylane.saved.none") %></p>
   <% end %>
   <% result.saved_searches.each do |saved| %>
-    <button type="submit" form="<%= id %>-form" name="saved" value="<%= saved.id %>" class="ql-saved-item">
-      <span class="ql-saved-name"><%= saved.name %></span>
-      <code class="ql-saved-query"><%= saved.query %></code>
-      <% if saved.used_at %><span class="ql-saved-used"><%= t("querylane.saved.used", date: l(saved.used_at.to_date, format: :short)) %></span><% end %>
-    </button>
+    <div class="ql-saved-row" data-test="saved-search">
+      <button type="submit" form="<%= id %>-form" name="saved" value="<%= saved.id %>" class="ql-saved-item">
+        <span class="ql-saved-name"><%= saved.name %></span>
+        <code class="ql-saved-query"><%= saved.query %></code>
+        <% if saved.used_at %><span class="ql-saved-used"><%= t("querylane.saved.used", date: l(saved.used_at.to_date, format: :short)) %></span><% end %>
+      </button>
+      <button type="submit" form="<%= id %>-form" name="delete_saved" value="<%= saved.id %>" class="ql-saved-delete"
+              aria-label="<%= t("querylane.saved.delete_label", name: saved.name) %>" title="<%= t("querylane.saved.delete_label", name: saved.name) %>"
+              data-action="click->querylane--results#deleteSaved" data-confirm="<%= t("querylane.saved.delete_confirm") %>">
+        <%= respond_to?(:icon) ? icon("trash-2") : t("querylane.saved.delete") %>
+      </button>
+    </div>
   <% end %>
 </div>

config/locales/querylane.en.yml

@@ -44,6 +44,9 @@ en:
       title: "Save this search"
       name: "Name"
       save: "Save"
+      delete: "Delete"
+      delete_label: "Delete saved search %{name}"
+      delete_confirm: "Delete this saved search?"
       yours: "Your saved searches"
       none: "Nothing saved yet."
       used: "used %{date}"
@@ -53,6 +56,8 @@ en:
       refusal_alone: "Remove it first."
       notice:
         saved: "Search saved."
+        deleted: "Search deleted."
+        not_available: "Saved search is not available."
         refused: "This search was not saved: it holds a member ID, SSN or last four."
         store_failed: "Saved searches are unavailable right now."
     reasons:

docs/guides/saved-searches.md

@@ -10,19 +10,24 @@ d.saved store: "SavedSearchStore"
 ```
 
 The box gets a **Saved** button that opens a popover with a name field, a Save
-button and the user's saved searches.
+button and the user's saved searches. Each saved entry also has a delete
+control, with a confirmation before it is removed.
+The entry shares one hover/focus highlight. Apply and Delete remain separate
+sibling buttons with non-overlapping click targets; the trash icon sits
+inside the row, not inside the Apply button.
 
 ## The store contract
 
 `store` is an object, or a class or module name resolved on every call. It
-responds to three methods. Each gets the request's `Querylane::Context` as
+responds to four methods. Each gets the request's `Querylane::Context` as
 `context:` so you can scope by user.
 
 | Method | Returns |
 | --- | --- |
 | `all(context:)` | The user's saved searches, in the order to show them. Each responds to `id`, `name`, `query` and `used_at` (a time or `nil`) |
 | `create(context:, name:, query:)` | Anything (ignored) |
 | `use(context:, id:)` | The saved search with that id, or `nil`. Record that it was used here |
+| `delete(context:, id:)` | `true` after deleting the current user's selected entry; `false` for a missing or unowned entry, or no current user. Scope the lookup before deleting, never trust the submitted id alone |
 
 The dummy app keeps them in memory:
 
@@ -37,16 +42,23 @@ module MemorySavedSearches
   def all(context:) = items.sort_by { |saved| saved.used_at || Time.at(0) }.reverse
 
   def create(context:, name:, query:)
-    items << Saved.new((items.size + 1).to_s, name, query, nil)
+    @next_id = (@next_id || 0) + 1
+    items << Saved.new(@next_id.to_s, name, query, nil)
   end
 
   def use(context:, id:)
     items.find { |saved| saved.id == id.to_s }&.tap { |saved| saved.used_at = Time.current }
   end
+
+  def delete(context:, id:)
+    saved = items.find { |item| item.id == id.to_s }
+    saved ? !!items.delete(saved) : false
+  end
 end
 ```
 
-A database-backed store is the same shape:
+The reference store is a shared in-memory list, not an authorization example.
+A database-backed store must scope deletion to the current user:
 
 ```ruby
 class SavedSearchStore
@@ -57,6 +69,16 @@ class SavedSearchStore
   def self.use(context:, id:)
     context.user.saved_searches.find_by(id: id)&.tap { |saved| saved.touch(:used_at) }
   end
+
+  def self.delete(context:, id:)
+    return false unless context.user && id.to_s.match?(/\A[1-9]\d*\z/)
+
+    saved = context.user.saved_searches.find_by(id: id)
+    return false unless saved
+
+    saved.destroy!
+    true
+  end
 end
 ```
 
@@ -77,6 +99,19 @@ querylane_search(PEOPLE, scope: scope, context: Querylane::Context.new(user: cur
 - The Saved star is filled while the box holds exactly the text of one of the
   user's saved searches.
 - After saving, a notice says "Search saved."
+- **Delete** asks "Delete this saved search?". Cancel changes nothing;
+  confirmation sends `delete_saved=<id>` and `delete_saved_page=<current page>`
+  through the existing POST form and its CSRF token. The host must authorize
+  access before calling `querylane_search`; a denied scope never calls deletion.
+- Successful deletion says "Search deleted." and refreshes the saved list.
+  The current query, sort and page remain unchanged. The star stays filled if
+  another saved entry has the same query. Deleting the final entry shows
+  "Nothing saved yet.".
+- Missing and unowned entries both say "Saved search is not available.".
+  GET deletion is rejected even when `allow_get_query` is enabled. Stores
+  without `delete` return the unavailable-entry notice rather than mutating data.
+- A host with an `icon` helper supplies `icon("trash-2")`; otherwise the
+  control reads "Delete". Its accessible label and tooltip identify the entry.
 
 ## What is refused
 

lib/querylane/controller.rb

@@ -63,7 +63,11 @@ def querylane_preview(definition, params: self.params, config: Querylane.configu
     # in), and no response is cached. Only the URL's own query string is
     # checked: a host may pass a query it restored on the server (e.g. a
     # remembered search) to a GET.
-    def querylane_guard!(_params, config)
+    def querylane_guard!(params, config)
+      if Request.delete_saved_param(params) && !request.post?
+        raise ActionController::BadRequest, "Saved searches are deleted by POST"
+      end
+
       url = request.query_parameters
       carries_text = [config.param_name, :compose, :group].any? { |name| url[name.to_s].present? }
       if request.get? && carries_text && !config.allow_get_query

lib/querylane/request.rb

@@ -7,29 +7,44 @@ module Querylane
   # remove (chip index), as_words (chip index), clear, instead, widen, relax, compose (with
   # compose_submit), options, sort, page, per, revision.
   class Request
-    attr_reader :fix, :saved, :save_search, :saved_name, :group, :query, :toggle, :remove, :as_words, :clear, :instead, :widen, :relax, :compose, :options, :sort, :page, :per, :revision
+    attr_reader :fix, :saved, :delete_saved, :save_search, :saved_name, :group, :query, :toggle, :remove, :as_words, :clear, :instead, :widen, :relax, :compose, :options, :sort, :page, :per, :revision
 
     # @param params [ActionController::Parameters, Hash]
     # @param definition [Querylane::Definition]
     def self.from_params(params, definition:, config: Querylane.configuration)
-      values = params.respond_to?(:to_unsafe_h) ? params.to_unsafe_h : params.to_h
-      values = values.transform_keys(&:to_s)
+      values = normalized_params(params)
+      values["page"] = values["delete_saved_page"] if values["delete_saved"].present? && values["delete_saved_page"].present?
       new(definition: definition, query: values[config.param_name.to_s], toggle: values["toggle"], remove: values["remove"],
         as_words: values["as_words"], clear: values["clear"],
         instead: values["instead"], widen: values["widen"], relax: values["relax"],
         compose: (values["compose"] if values["compose_submit"].present?), options: values["options"],
-        fix: values["fix"], group: values["group"], saved: values["saved"], save_search: values["save_search"], saved_name: values["saved_name"], sort: values["sort"], page: values["page"], per: values["per"], revision: values["revision"])
+        fix: values["fix"], group: values["group"], saved: values["saved"], delete_saved: values["delete_saved"], save_search: values["save_search"], saved_name: values["saved_name"], sort: values["sort"], page: values["page"], per: values["per"], revision: values["revision"])
+    end
+
+    # The normalized, bounded deletion value used by both the request parser and POST guard.
+    #
+    # @param params [ActionController::Parameters, Hash]
+    # @return [String, nil]
+    def self.delete_saved_param(params)
+      normalized_params(params)["delete_saved"].to_s.presence&.first(64)
+    end
+
+    def self.normalized_params(params)
+      values = params.respond_to?(:to_unsafe_h) ? params.to_unsafe_h : params.to_h
+      values.transform_keys(&:to_s)
     end
+    private_class_method :normalized_params
 
     # instead: "chip.alternative" (use another reading of an interpreted term);
     # widen: a facet key (drop that facet's terms); relax: match fewer words;
     # as_words: a chip index (search that filter's typed value as plain words).
     # compose: options-panel fields {"name" => "…", "policy" => "1 2", "is" => [...], "options" => [...]}
     # (applied only with compose_submit); options: open the options panel;
     # group: a GroupToken (all records of one group, see Search#related);
-    # saved: a saved search id to run; save_search with saved_name: save the query.
+    # saved: a saved search id to run; save_search with saved_name: save the query;
+    # delete_saved: an owned saved search id to delete without rewriting the query.
     def initialize(definition:, query: nil, toggle: nil, remove: nil, as_words: nil, clear: nil, instead: nil, widen: nil, relax: nil,
-      compose: nil, options: nil, fix: nil, group: nil, saved: nil, save_search: nil, saved_name: nil, sort: nil, page: nil, per: nil, revision: nil)
+      compose: nil, options: nil, fix: nil, group: nil, saved: nil, delete_saved: nil, save_search: nil, saved_name: nil, sort: nil, page: nil, per: nil, revision: nil)
       @query = query.to_s
       @clear = clear.present?
       @instead = instead.to_s.match(/\A(\d+)\.(\d+)\z/)&.captures&.map(&:to_i)
@@ -38,8 +53,9 @@ def initialize(definition:, query: nil, toggle: nil, remove: nil, as_words: nil,
       @options = options.present?
       @group = group.presence
       @fix = fix.present?
-      @saved = saved.to_s.presence&.first(64)
-      @save_search = save_search.present?
+      @delete_saved = delete_saved.to_s.presence&.first(64)
+      @saved = saved.to_s.presence&.first(64) unless @delete_saved
+      @save_search = save_search.present? && !@delete_saved
       @saved_name = saved_name.to_s.squish.first(80)
       @compose = compose.is_a?(Hash) ? compose.to_h.transform_keys(&:to_s) : nil
       @toggle = parse_toggle(definition, toggle)

lib/querylane/search.rb

@@ -33,6 +33,7 @@ def call
       query = Query.analyze(@definition, @request.query, config: @config)
       query, rewritten = rewrite(query)
       @notice = save(query) if @request.save_search && !@scope.denied?
+      @notice = delete_saved(@request.delete_saved) if @request.delete_saved && !@scope.denied?
       return result(:first_load, query, rewritten) if query.valid? && query.blank?
       return result(:invalid, query, rewritten, error: query.error) unless query.valid?
       return result(:short, query, rewritten, chips: Chips.for(query)) if query.too_short?(@definition.min_text_length)
@@ -311,6 +312,15 @@ def saved_text(id)
       nil
     end
 
+    def delete_saved(id)
+      store = @definition.saved_searches
+      return :not_available unless store&.respond_to?(:delete)
+
+      store.delete(context: @context, id: id) ? :deleted : :not_available
+    rescue => error
+      saved_store_failed(error)
+    end
+
     def saved_list
       store = @definition.saved_searches
       store && Array(store.all(context: @context))

spec/dummy/app/models/memory_saved_searches.rb

@@ -8,15 +8,25 @@ module MemorySavedSearches
 
   def items = (@items ||= [])
 
-  def reset! = (@items = [])
+  def reset!
+    @items = []
+    @next_id = 0
+  end
 
   def all(context:) = items.sort_by { |saved| saved.used_at || Time.at(0) }.reverse
 
   def create(context:, name:, query:)
-    items << Saved.new((items.size + 1).to_s, name, query, nil)
+    @next_id = (@next_id || 0) + 1
+    items << Saved.new(@next_id.to_s, name, query, nil)
   end
 
   def use(context:, id:)
     items.find { |saved| saved.id == id.to_s }&.tap { |saved| saved.used_at = Time.current }
   end
+
+  # @return [Boolean] whether the reference store held the selected entry
+  def delete(context:, id:)
+    saved = items.find { |item| item.id == id.to_s }
+    saved ? !!items.delete(saved) : false
+  end
 end