Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions app/forms/filters/inputs/base_filter_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,12 @@ def add_operator(group)
"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
Expand Down
2 changes: 1 addition & 1 deletion app/forms/filters/inputs/date_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def on_date_div(builder, filter_name, value)
label: :singleDay,
hidden: value.nil?,
leading_visual: { icon: :calendar },
value:,
value: value || "",
datepicker_options: { input_attributes: { "data-filter--filters-form-target" => "singleDay" } },
data: { "filter-name": filter_name }
)
Expand Down
12 changes: 12 additions & 0 deletions app/models/persisted_view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class PersistedView < ApplicationRecord

scope :public_views, -> { where(public: true) }
scope :private_views, ->(principal = User.current) { where(public: false, principal_id: principal.id) }
scope :with_children, -> { includes(:children) }

scope :visible, (lambda do |principal = User.current|
public_views.or(private_views(principal))
Expand All @@ -68,6 +69,17 @@ class << self
attr_writer :allowed_children
end

# Resolves a (potentially user-supplied) class name to a view class, but only
# if it is an allowed child of this view. Returns nil for any unknown or
# forbidden name.
#
# `constantize` is applied only to the developer-controlled `allowed_children`
# list, never to the passed-in name, so untrusted input can never resolve an
# arbitrary constant.
def self.allowed_child_class(name)
allowed_children.index_with(&:constantize)[name.to_s]
end

# Returns the query of this view or, if not set, the query of the parent view.
def effective_query
query || parent&.effective_query
Expand Down
21 changes: 21 additions & 0 deletions app/models/queries/work_packages/filter/filter_for_wp_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,27 @@ def allowed_values
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)

Expand Down
11 changes: 11 additions & 0 deletions app/models/query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class Query < ApplicationRecord
validate :validate_timestamps

include Scopes::Scoped

scopes :visible,
:having_views

Expand Down Expand Up @@ -217,6 +218,16 @@ def find_active_filter(name)
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
Expand Down
17 changes: 9 additions & 8 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@
not_available_in_semantic_mode: "Reserved identifiers are only available in classic identifier mode."
filter_label: "Filter identifiers"
btn_release: "Release"
released_notice: "Identifier \"%{identifier}\" has been released."
released_notice: 'Identifier "%{identifier}" has been released.'

Check failure on line 72 in config/locales/en.yml

View workflow job for this annotation

GitHub Actions / yamllint

[yamllint] reported by reviewdog 🐶 [error] wrong ordering of key "released_notice" in mapping (key-ordering) Raw Output: config/locales/en.yml:72:7: [error] wrong ordering of key "released_notice" in mapping (key-ordering)
dialog:
title: "Release identifier"
heading: "Release \"%{identifier}\"?"
heading: 'Release "%{identifier}"?'

Check failure on line 75 in config/locales/en.yml

View workflow job for this annotation

GitHub Actions / yamllint

[yamllint] reported by reviewdog 🐶 [error] wrong ordering of key "heading" in mapping (key-ordering) Raw Output: config/locales/en.yml:75:9: [error] wrong ordering of key "heading" in mapping (key-ordering)
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"
Expand Down Expand Up @@ -1548,7 +1548,7 @@
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"
Expand Down Expand Up @@ -1829,7 +1829,7 @@
changeset:
repository: "Repository"
comment:
commented: "Commented" # an object that this comment belongs to
commented: "Commented" # an object that this comment belongs to

Check failure on line 1832 in config/locales/en.yml

View workflow job for this annotation

GitHub Actions / yamllint

[yamllint] reported by reviewdog 🐶 [warning] too few spaces before comment: expected 2 (comments) Raw Output: config/locales/en.yml:1832:32: [warning] too few spaces before comment: expected 2 (comments)
custom_action:
actions: "Actions"
custom_field:
Expand Down Expand Up @@ -2124,7 +2124,7 @@
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

Check failure on line 2127 in config/locales/en.yml

View workflow job for this annotation

GitHub Actions / yamllint

[yamllint] reported by reviewdog 🐶 [warning] too few spaces before comment: expected 2 (comments) Raw Output: config/locales/en.yml:2127:36: [warning] too few spaces before comment: expected 2 (comments)
begin_insertion: "Begin of the insertion"
begin_deletion: "Begin of the deletion"
children: "Subelements"
Expand All @@ -2142,13 +2142,14 @@
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

Check failure on line 2145 in config/locales/en.yml

View workflow job for this annotation

GitHub Actions / yamllint

[yamllint] reported by reviewdog 🐶 [warning] too few spaces before comment: expected 2 (comments) Raw Output: config/locales/en.yml:2145:26: [warning] too few spaces before comment: expected 2 (comments)
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"
Expand Down Expand Up @@ -4861,8 +4862,8 @@
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

Check failure on line 4865 in config/locales/en.yml

View workflow job for this annotation

GitHub Actions / yamllint

[yamllint] reported by reviewdog 🐶 [warning] too few spaces before comment: expected 2 (comments) Raw Output: config/locales/en.yml:4865:60: [warning] too few spaces before comment: expected 2 (comments)
b: "Lock or delete an existing user ([here](users_url))" # here turned into a link

Check failure on line 4866 in config/locales/en.yml

View workflow job for this annotation

GitHub Actions / yamllint

[yamllint] reported by reviewdog 🐶 [warning] too few spaces before comment: expected 2 (comments) Raw Output: config/locales/en.yml:4866:64: [warning] too few spaces before comment: expected 2 (comments)
label: "To allow the user to sign in you can either: "

more_actions: "More functions"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', '');
}
}
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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 });
Expand All @@ -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 });
Expand Down Expand Up @@ -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) {
Expand All @@ -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 [];
}

Expand Down
7 changes: 3 additions & 4 deletions lookbook/docs/patterns/11-filter-forms.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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=[...]`
Expand Down
14 changes: 14 additions & 0 deletions lookbook/previews/open_project/filter/filter_form_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ module Filter
# @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"])
Expand All @@ -48,16 +50,28 @@ def with_active_filter
# @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 })
end

# @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
Original file line number Diff line number Diff line change
@@ -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 %>
Loading
Loading