Merge pull request #23457 from opf/filter-form-with-wp-query

Updates to `Filters::FilterForm` to make it work with `Query`

app/forms/filters/inputs/base_filter_form.rb

@@ -106,11 +106,12 @@ class Filters::Inputs::BaseFilterForm < ApplicationForm
         "filter-name": @filter.name
       }
     ) do |select|
-      @filter.available_operators.each do |op|
+      @filter.available_operators.each do |operator|
         select.option(
-          label: op.human_name,
-          value: op.symbol,
-          selected: op.symbol == selected_operator
+          label: operator.human_name,
+          value: operator.symbol,
+          selected: operator.symbol == selected_operator,
+          **(operator.requires_value? ? {} : { "data-no-value": true })
         )
       end
     end

app/models/queries/work_packages/filter/filter_for_wp_mixin.rb

@@ -37,6 +37,27 @@ module Queries::WorkPackages::Filter::FilterForWpMixin
     raise NotImplementedError, "There would be too many candidates"
   end
 
+  # Tell `Filters::FilterForm`'s dispatch to render these filters with a
+  # server-side autocompleter (the candidate set is too large for an inline
+  # `<select>`). Mirrors what the legacy Angular WP filter UI does — see
+  # `filter-searchable-multiselect-value.component.html`, which renders an
+  # `op-autocompleter` against the `work_packages` resource using the
+  # `typeahead` filter for search.
+  #
+  # Subclasses that override `type` to something other than `:list` (e.g.
+  # `SearchFilter` / `SubjectOrIdFilter` use `:search`, `RelatableFilter`
+  # uses `:relation`) get an empty hash so the dispatch falls back to the
+  # appropriate non-autocomplete input.
+  def autocomplete_options
+    return {} unless type == :list
+
+    {
+      component: "opce-autocompleter",
+      resource: "work_packages",
+      searchKey: "typeahead"
+    }
+  end
+
   def value_objects
     objects = visible_scope.find(no_templated_values)
 

app/models/query.rb

@@ -63,6 +63,7 @@ class Query < ApplicationRecord
   validate :validate_timestamps
 
   include Scopes::Scoped
+
   scopes :visible,
          :having_views
 
@@ -217,6 +218,16 @@ class Query < ApplicationRecord
     filters.detect { |f| f.name == name }
   end
 
+  # The manual-sort filter is added programmatically when the user drags
+  # work packages to reorder them — it has no operator/value UI of its own
+  # (type `:empty_value`), so it doesn't belong in the picker that
+  # `Filters::FilterForm` builds. Mirrors how
+  # `Queries::Filters::AvailableFilters#available_advanced_filters` already
+  # excludes the inline `name_and_identifier` quick-filter on projects.
+  def available_advanced_filters
+    super.grep_v(::Queries::WorkPackages::Filter::ManualSortFilter)
+  end
+
   def normalized_name
     name.parameterize.underscore
   end

config/locales/en.yml

@@ -69,10 +69,10 @@ en:
       not_available_in_semantic_mode: "Reserved identifiers are only available in numeric identifier mode."
       filter_label: "Filter identifiers"
       btn_release: "Release"
-      released_notice: "Identifier \"%{identifier}\" has been released."
+      released_notice: 'Identifier "%{identifier}" has been released.'
       dialog:
         title: "Release identifier"
-        heading: "Release \"%{identifier}\"?"
+        heading: 'Release "%{identifier}"?'
         description: "Releasing this identifier cannot be undone. External links and integrations using it will stop resolving, and the name becomes available for any new project to claim."
         checkbox_label: "I understand that this cannot be undone."
         confirm_button: "Release identifier"
@@ -1548,7 +1548,7 @@ en:
             one: "Successfully copied workflow to '%{type_name}' type."
             other: "Successfully copied workflow to %{count} types."
       new:
-        title: "Copy workflow of \"%{source_type}\""
+        title: 'Copy workflow of "%{source_type}"'
     form:
       matrix_caption: "Workflow matrix"
       matrix_caption_assignee: "Workflow matrix for assignee"
@@ -1829,7 +1829,7 @@ en:
       changeset:
         repository: "Repository"
       comment:
-        commented: "Commented"  # an object that this comment belongs to
+        commented: "Commented" # an object that this comment belongs to
       custom_action:
         actions: "Actions"
       custom_field:
@@ -2124,7 +2124,7 @@ en:
         redirect_existing_links: "Redirect existing links"
         text: "Page content"
       work_package:
-        ancestor: "Descendants of"  # used for filtering of work packages that are descendants of a given work package
+        ancestor: "Descendants of" # used for filtering of work packages that are descendants of a given work package
         begin_insertion: "Begin of the insertion"
         begin_deletion: "Begin of the deletion"
         children: "Subelements"
@@ -2142,13 +2142,14 @@ en:
           false: "working days only"
           true: "include non-working days"
         journal_internal: Internal Journal
-        notify: "Notify"  # used in custom actions
+        notify: "Notify" # used in custom actions
         parent: "Parent"
         parent_issue: "Parent"
         parent_work_package: "Parent"
         priority: "Priority"
         progress: "% Complete"
         readonly: "Read only"
+        relatable: "Relatable to"
         remaining_hours: "Remaining work"
         remaining_time: "Remaining work"
         sequence_number: "Sequence number"
@@ -4861,8 +4862,8 @@ en:
       A new user (%{email}) tried to create an account on an OpenProject environment that you manage (%{host}).
       The user cannot activate their account since the user limit has been reached.
     steps:
-      a: "Upgrade your payment plan ([here](upgrade_url))"  # here turned into a link
-      b: "Lock or delete an existing user ([here](users_url))"  # here turned into a link
+      a: "Upgrade your payment plan ([here](upgrade_url))" # here turned into a link
+      b: "Lock or delete an existing user ([here](users_url))" # here turned into a link
       label: "To allow the user to sign in you can either: "
 
   more_actions: "More functions"

frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts

@@ -153,7 +153,7 @@ export default class FiltersFormController extends Controller {
     const filterName = target.getAttribute('data-filter-name');
     if (filterName) {
       const operator = this.findTargetByName(filterName, this.operatorTargets);
-      if (operator && this.noValueOperators.includes(operator.value)) {
+      if (operator && this.operatorRequiresNoValue(operator)) {
         target.setAttribute('hidden', '');
       }
     }
@@ -354,16 +354,24 @@ export default class FiltersFormController extends Controller {
     inputElement.dispatchEvent(inputEvent);
   }
 
-  private readonly noValueOperators = ['*', '!*', 't', 'w'];
   private readonly daysOperators = ['>t-', '<t-', 't-', '<t+', '>t+', 't+'];
   private readonly onDateOperator = '=d';
   private readonly betweenDatesOperator = '<>d';
 
+  // Whether the operator currently selected in `operatorElement` declares
+  // itself value-less. Driven by the `data-no-value` attribute that
+  // `Filters::Inputs::BaseFilterForm#add_operator` emits on each `<option>`
+  // whose `Operator.requires_value?` is false — keeps the symbol list in
+  // one place (Ruby) instead of duplicating it here.
+  private operatorRequiresNoValue(operatorElement:HTMLSelectElement):boolean {
+    return operatorElement.selectedOptions[0]?.hasAttribute('data-no-value') ?? false;
+  }
+
   setValueVisibility({ target, params: { filterName } }:{ target:HTMLSelectElement, params:{ filterName:string } }) {
     const selectedOperator = target.value;
     const valueContainer = this.findTargetByName(filterName, this.filterValueContainerTargets);
     if (valueContainer) {
-      if (this.noValueOperators.includes(selectedOperator)) {
+      if (this.operatorRequiresNoValue(target)) {
         valueContainer.setAttribute('hidden', '');
       } else {
         valueContainer.removeAttribute('hidden');
@@ -456,7 +464,9 @@ export default class FiltersFormController extends Controller {
       const type = filter.getAttribute('data-filter-type');
       const operator = filter.getAttribute('data-filter-operator');
       if (name && type && operator) {
-        const value = this.parseFilterValue(filter, name, type, operator) as string[]|null;
+        // Quick filters carry an operator that always requires a value
+        // (the inline search input).
+        const value = this.parseFilterValue(filter, name, type, operator, false) as string[]|null;
 
         if (value) {
           filters.push({ name, operator, value });
@@ -473,11 +483,13 @@ export default class FiltersFormController extends Controller {
     advancedFilters.forEach((filter) => {
       const filterName = filter.getAttribute('data-filter-name')!;
       const filterType = filter.getAttribute('data-filter-type');
-      const parsedOperator = this.findTargetByName(filterName, this.operatorTargets)?.value;
+      const operatorTarget = this.findTargetByName(filterName, this.operatorTargets);
+      const parsedOperator = operatorTarget?.value;
       const valueContainer = this.findTargetByName(filterName, this.filterValueContainerTargets);
 
-      if (valueContainer && filterName && filterType && parsedOperator) {
-        const parsedValue = this.parseFilterValue(valueContainer, filterName, filterType, parsedOperator) as string[]|null;
+      if (valueContainer && filterName && filterType && parsedOperator && operatorTarget) {
+        const requiresNoValue = this.operatorRequiresNoValue(operatorTarget);
+        const parsedValue = this.parseFilterValue(valueContainer, filterName, filterType, parsedOperator, requiresNoValue) as string[]|null;
 
         if (parsedValue) {
           filters.push({ name: filterName, operator: parsedOperator, value: parsedValue });
@@ -509,10 +521,9 @@ export default class FiltersFormController extends Controller {
     return value && value.length > 0 ? value.replace(/"/g, '\\"') : '';
   }
 
-  private readonly operatorsWithoutValues = ['*', '!*', 't', 'w'];
   private readonly dateFilterTypes = ['datetime_past', 'date'];
 
-  private parseFilterValue(valueContainer:HTMLElement, filterName:string, filterType:string, operator:string) {
+  private parseFilterValue(valueContainer:HTMLElement, filterName:string, filterType:string, operator:string, requiresNoValue:boolean) {
     const checkbox = valueContainer.querySelector<HTMLInputElement>('input[type="checkbox"]');
 
     if (checkbox) {
@@ -523,7 +534,7 @@ export default class FiltersFormController extends Controller {
       return (valueContainer.querySelector<HTMLInputElement>('input[name="value"]'))?.value.split(',');
     }
 
-    if (this.operatorsWithoutValues.includes(operator)) {
+    if (requiresNoValue) {
       return [];
     }
 

lookbook/docs/patterns/11-filter-forms.md.erb

@@ -130,10 +130,7 @@ form forwards it as `appendTo` to every autocompleter it renders.
 | You want…                                                | Use                       |
 |----------------------------------------------------------|---------------------------|
 | The standard OpenProject filter panel on an index page   | `Filter::FilterComponent` |
-| Filters inside a dialog or a non-filter form             | `Filters::FilterForm`     |
-| Coordination with a quick filter input in a sub-header   | `Filter::FilterComponent` (the sub-header attaches the controller) |
-| The serialized filter string in `params` on submit       | `Filters::FilterForm` + `hidden_input_name:` |
-| Autocompleter dropdowns to escape a clipping container   | `Filters::FilterForm` + `autocomplete_append_to:` |
+| Filters inside a dialog or a non-filter form             | `Filters::FilterForm` + `hidden_input_name:`   |
 
 ## Stimulus controller placement
 
@@ -164,6 +161,8 @@ default.
 with the same signature, so passing a `Query` (or any of its subclasses)
 to `FilterForm` works exactly like passing a `BaseQuery` subclass.
 
+<%= embed OpenProject::Filter::FilterFormPreview, :for_a_work_package_query, panels: %i[preview source] %>
+
 What `FilterForm` does *not* do for you on the legacy side: parsing the
 form submission back into a `Query#filters` collection. The work-package
 filter pipeline still uses its own serialization (URL `filters=[...]`

lookbook/previews/open_project/filter/filter_form_preview.rb

@@ -33,12 +33,14 @@ module OpenProject
     # @logical_path OpenProject/Filter
     class FilterFormPreview < Lookbook::Preview
       # @display min_height 600px
+      # Using the `UserQuery` as an example.
       def default
         render_with_template(locals: { query: UserQuery.new })
       end
 
       # @display min_height 600px
       # @label With an active filter
+      # Using the `UserQuery` as an example.
       def with_active_filter
         query = UserQuery.new
         query.where(:login, "~", ["admin"])
@@ -48,6 +50,7 @@ module OpenProject
       # @display min_height 600px
       # @label Hidden input mode
       # @param output_format [Symbol] select [json, params]
+      # Using the `UserQuery` as an example.
       # This also renders a field that shows the value of the hidden input to show the different serialization formats.
       def with_hidden_input(output_format: :json)
         render_with_template(locals: { query: UserQuery.new, output_format: output_format.to_sym })
@@ -55,9 +58,20 @@ module OpenProject
 
       # @display min_height 600px
       # @label Combined with other inputs
+      # Using the `UserQuery` as an example
       def combined_with_other_inputs
         render_with_template(locals: { query: UserQuery.new })
       end
+
+      # @display min_height 600px
+      # @label Work package query (legacy `Query`)
+      # Renders against the legacy work-package `Query` (the one that # predates `Queries::BaseQuery`).
+      def for_a_work_package_query
+        query = ::Query.new
+        query.add_filter(:status_id, "o", [])
+        query.add_filter(:due_date, "=d", [Date.current.end_of_year.iso8601])
+        render_with_template(locals: { query: query })
+      end
     end
   end
 end

lookbook/previews/open_project/filter/filter_form_preview/for_a_work_package_query.html.erb

@@ -0,0 +1,17 @@
+<%# `Query` is the legacy work-package query model. `Filters::FilterForm` %>
+<%# works against it the same way it does against `Queries::BaseQuery` %>
+<%# subclasses — the only requirement is `available_advanced_filters`, %>
+<%# `filters`, and `find_active_filter(name)`, all of which `Query` %>
+<%# exposes (the last one was mirrored from `BaseQuery` to bring the %>
+<%# legacy query in line with the new form). %>
+<%= primer_form_with(url: "/foo", method: :post) do |f| %>
+  <%=
+    render(
+      Filters::FilterForm.new(
+        f,
+        query: query,
+        wrap_with_controller: true
+      )
+    )
+  %>
+<% end %>

modules/storages/config/locales/en.yml

@@ -31,10 +31,15 @@ en:
         authentication_method: Authentication Method
         creator: Creator
         drive: Drive ID
+        file_link_origin: File link origin ID
         host: Host
+        linkable_to_storage: Linkable to storage
+        linkable_to_storage_url: Linkable to storage URL
         name: Name
         password: Application password
         provider_type: Provider type
+        storage: Storage
+        storage_url: Storage URL
         tenant: Directory (tenant) ID
     errors:
       messages:

spec/forms/filters/filter_form_spec.rb

@@ -156,6 +156,34 @@ RSpec.describe Filters::FilterForm, type: :forms do
     end
   end
 
+  describe "value-less operators (operator `data-no-value` attribute)" do
+    # A date custom field surfaces operators whose `requires_value?` is
+    # false (`t`, `w`, `!*`) alongside ones that require a value (`=d`,
+    # `>t-`, …). That's enough to assert both branches of the dispatch.
+    let!(:date_field) { create(:user_custom_field, field_format: "date") }
+
+    it "marks value-less operators with data-no-value and leaves the rest untouched" do
+      render_form
+
+      operator_select = page.find(:element, :select,
+                                  name: "operator_cf_#{date_field.id}",
+                                  visible: :all)
+
+      # `t` (Today) — `Queries::Operators::Today` declares `require_value false`.
+      expect(operator_select).to have_element :option,
+                                              value: "t",
+                                              "data-no-value": "true",
+                                              visible: :all
+
+      # `=d` (OnDate) — declares `require_value true` (the default).
+      expect(operator_select).to have_element :option,
+                                              value: "=d",
+                                              visible: :all do |option|
+        expect(option["data-no-value"]).to be_nil
+      end
+    end
+  end
+
   describe "autocomplete_append_to:" do
     # `appendTo` arrives at the angular component as a JSON-encoded data
     # attribute (`angular_component_tag` json-encodes its `inputs:` hash).

spec/models/queries/work_packages/filter/filter_for_wp_mixin_spec.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+
+RSpec.describe Queries::WorkPackages::Filter::FilterForWpMixin do
+  describe "#autocomplete_options" do
+    context "on a list-type filter (e.g. ParentFilter)" do
+      subject(:filter) { Queries::WorkPackages::Filter::ParentFilter.create!(name: :parent) }
+
+      it "returns the WP autocompleter config so FilterForm routes the filter to AutocompleteForm" do
+        # The candidate set is too large to enumerate (`allowed_values`
+        # intentionally raises), so the filter must render as a server-side
+        # autocompleter against the work_packages resource. Matches the
+        # Angular `op-filter-searchable-multiselect-value` behaviour.
+        expect(filter.autocomplete_options).to eq(
+          component: "opce-autocompleter",
+          resource: "work_packages",
+          searchKey: "typeahead"
+        )
+      end
+    end
+
+    context "on a relation-type filter (RelatableFilter)" do
+      subject(:filter) { Queries::WorkPackages::Filter::RelatableFilter.create!(name: :relatable) }
+
+      it "returns an empty hash so the FilterForm dispatch falls through to a non-autocomplete input" do
+        expect(filter.type).to eq(:relation)
+        expect(filter.autocomplete_options).to eq({})
+      end
+    end
+
+    context "on a search-type filter (SearchFilter)" do
+      subject(:filter) { Queries::WorkPackages::Filter::SearchFilter.create!(name: :search) }
+
+      it "returns an empty hash so the FilterForm dispatch falls through to a text input" do
+        expect(filter.type).to eq(:search)
+        expect(filter.autocomplete_options).to eq({})
+      end
+    end
+  end
+end

spec/models/query_spec.rb

@@ -87,6 +87,27 @@ RSpec.describe Query,
     end
   end
 
+  describe "#available_advanced_filters" do
+    let(:query) { described_class.new }
+
+    it "excludes the ManualSortFilter so it doesn't appear in the FilterForm picker" do
+      classes = query.available_advanced_filters.map(&:class)
+
+      expect(classes).not_to include(Queries::WorkPackages::Filter::ManualSortFilter)
+    end
+
+    it "still exposes regular work-package filters" do
+      classes = query.available_advanced_filters.map(&:class)
+
+      # Sanity: a couple of well-known WP filters are still advertised.
+      # Picked ones whose `available?` doesn't depend on DB records (Status
+      # / Priority would require seeding statuses/priorities first).
+      expect(classes).to include(Queries::WorkPackages::Filter::SubjectFilter,
+                                 Queries::WorkPackages::Filter::CreatedAtFilter,
+                                 Queries::WorkPackages::Filter::DueDateFilter)
+    end
+  end
+
   describe "include_subprojects" do
     let(:query) { described_class.new name: "foo" }