From 6634ef3a7366ba08c42d44f305ee239b0583e88e Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Fri, 13 Jun 2025 15:37:11 +0200 Subject: [PATCH 01/44] [#64347] add new format "calculated_value" to project attribute list --- config/locales/en.yml | 1 + lib/open_project/custom_field_format.rb | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index 74c2e03e86f..ad8b83e97bf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2721,6 +2721,7 @@ en: label_select_main_menu_item: Select new main menu item label_required_disk_storage: "Required disk storage" label_send_invitation: Send invitation + label_calculated_value: "Calculated value" label_change_plural: "Changes" label_change_properties: "Change properties" label_change_status: "Change status" diff --git a/lib/open_project/custom_field_format.rb b/lib/open_project/custom_field_format.rb index 3130617ba15..354c5e3456b 100644 --- a/lib/open_project/custom_field_format.rb +++ b/lib/open_project/custom_field_format.rb @@ -80,6 +80,17 @@ module OpenProject available.keys end + # TODO: Consider changing the initializer order and moving this to the CustomFieldFormat initializer instead + def ensure_calculated_value_if_active! + if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? && !@@available.key?("calculated_value") + order_of_last_entry = @@available.values.last.order + register OpenProject::CustomFieldFormat.new("calculated_value", + label: :label_calculated_value, + only: %w(Project), + order: order_of_last_entry + 1) + end + end + def find_by(name:) registered[name.to_s] end From 89d75262239659303d88831f031c6cc4ace66e03 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Fri, 13 Jun 2025 16:20:08 +0200 Subject: [PATCH 02/44] [#64347] add formula column (and index) to custom_fields --- ...0613141234_add_formula_to_custom_fields.rb | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 db/migrate/20250613141234_add_formula_to_custom_fields.rb diff --git a/db/migrate/20250613141234_add_formula_to_custom_fields.rb b/db/migrate/20250613141234_add_formula_to_custom_fields.rb new file mode 100644 index 00000000000..42865578f9f --- /dev/null +++ b/db/migrate/20250613141234_add_formula_to_custom_fields.rb @@ -0,0 +1,37 @@ +# 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. +#++ + +class AddFormulaToCustomFields < ActiveRecord::Migration[8.0] + def change + add_column :custom_fields, :formula, :jsonb, null: true + + add_index :custom_fields, :formula, using: :gin + end +end From fb60aabbe4f65f0c2c63ba8cb2311f568a1f168a Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 17 Jun 2025 14:56:21 +0200 Subject: [PATCH 03/44] [#64347] introduce formula field to custom field admin views --- app/contracts/custom_fields/base_contract.rb | 1 + .../admin/settings/project_custom_fields_controller.rb | 2 +- app/models/permitted_params.rb | 1 + app/views/custom_fields/_form.html.erb | 8 ++++++++ config/locales/en.yml | 3 +++ .../controllers/dynamic/admin/custom-fields.controller.ts | 2 ++ lib/open_project/custom_field_format_dependent.rb | 5 +++-- 7 files changed, 19 insertions(+), 3 deletions(-) diff --git a/app/contracts/custom_fields/base_contract.rb b/app/contracts/custom_fields/base_contract.rb index 6648bfc213b..ef77d39f10c 100644 --- a/app/contracts/custom_fields/base_contract.rb +++ b/app/contracts/custom_fields/base_contract.rb @@ -41,6 +41,7 @@ module CustomFields attribute :name attribute :possible_values attribute :regexp + attribute :formula attribute :searchable attribute :admin_only attribute :default_value diff --git a/app/controllers/admin/settings/project_custom_fields_controller.rb b/app/controllers/admin/settings/project_custom_fields_controller.rb index a3e2330eb93..e537cf365c0 100644 --- a/app/controllers/admin/settings/project_custom_fields_controller.rb +++ b/app/controllers/admin/settings/project_custom_fields_controller.rb @@ -53,7 +53,7 @@ module Admin::Settings def show # quick fixing redirect issue from perform_update - # perform_update is always redirecting to the show action altough configured otherwise + # perform_update is always redirecting to the show action although configured otherwise render :edit end diff --git a/app/models/permitted_params.rb b/app/models/permitted_params.rb index e20d2d1735f..fb542d9033f 100644 --- a/app/models/permitted_params.rb +++ b/app/models/permitted_params.rb @@ -481,6 +481,7 @@ class PermittedParams custom_field: [ :editable, :field_format, + :formula, :is_filter, :is_for_all, :is_required, diff --git a/app/views/custom_fields/_form.html.erb b/app/views/custom_fields/_form.html.erb index 192fc485768..9ae12e18fdb 100644 --- a/app/views/custom_fields/_form.html.erb +++ b/app/views/custom_fields/_form.html.erb @@ -76,6 +76,14 @@ See COPYRIGHT and LICENSE files for more details. +
> + <%= f.text_field :formula, + container_class: "-wide" %> + + <%= t("custom_fields.instructions.formula") %> + +
+ <% if @custom_field.new_record? || @custom_field.multi_value_possible? %>
> <%= f.check_box :multi_value, diff --git a/config/locales/en.yml b/config/locales/en.yml index ad8b83e97bf..b8fdfe6c783 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -303,6 +303,7 @@ en: is_filter: > Allow the custom field to be used in a filter in work package views. Note that only with 'For all projects' selected, the custom field will show up in global views. + formula: "Add numeric values or type / to search for an attribute or a mathematical operator." tab: no_results_title_text: There are currently no custom fields. @@ -981,6 +982,7 @@ en: default_value: "Default value" editable: "Editable" field_format: "Format" + formula: "Formula" is_filter: "Used as a filter" is_for_all: "For all projects" is_required: "Required" @@ -2857,6 +2859,7 @@ en: label_follows: "follows" label_force_user_language_to_default: "Set language of users having a non allowed language to default" label_form_configuration: "Form configuration" + label_formula: "Formula" label_gantt_chart: "Gantt chart" label_gantt_chart_plural: "Gantt charts" label_general: "General" diff --git a/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts b/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts index 4ae929c6518..c7e5710789e 100644 --- a/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts @@ -47,6 +47,7 @@ export default class CustomFieldsController extends Controller { 'multiSelect', 'possibleValues', 'regexp', + 'formula', 'searchable', 'textOrientation', @@ -80,6 +81,7 @@ export default class CustomFieldsController extends Controller { declare readonly multiSelectTargets:HTMLElement[]; declare readonly possibleValuesTargets:HTMLElement[]; declare readonly regexpTargets:HTMLElement[]; + declare readonly formulaTargets:HTMLElement[]; declare readonly searchableTargets:HTMLInputElement[]; declare readonly textOrientationTargets:HTMLElement[]; diff --git a/lib/open_project/custom_field_format_dependent.rb b/lib/open_project/custom_field_format_dependent.rb index 0843c97f3f5..6a95d8a61f1 100644 --- a/lib/open_project/custom_field_format_dependent.rb +++ b/lib/open_project/custom_field_format_dependent.rb @@ -36,8 +36,9 @@ module OpenProject length: [:except, %w[list bool date user version link hierarchy]], multiSelect: [:only, %w[list user version hierarchy]], possibleValues: [:only, %w[list]], - regexp: [:except, %w[list bool date user version hierarchy]], - searchable: [:except, %w[bool date float int user version hierarchy]], + regexp: [:except, %w[list bool date user version hierarchy calculated_value]], + formula: [:only, %w[calculated_value]], + searchable: [:except, %w[bool date float int user version hierarchy calculated_value]], textOrientation: [:only, %w[text]], enterpriseBanner: [:only, %w[hierarchy]] }.freeze From a5f30703e83a524fe52d6fd6e6d7bc8a79faae07 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 16 Jun 2025 11:26:13 +0200 Subject: [PATCH 04/44] [#64347] primerize view for calculated value --- .../details_component.html.erb | 15 ++++ .../calculated_values/details_component.rb | 53 +++++++++++++ .../calculated_values/details_form.rb | 75 +++++++++++++++++++ .../custom_field_row_component.html.erb | 2 +- .../custom_field_row_component.rb | 10 ++- ...stom_field_calculated_values_controller.rb | 56 ++++++++++++++ .../project_custom_fields_controller.rb | 2 + .../concerns/custom_fields/shared_actions.rb | 6 +- app/models/custom_field.rb | 4 + .../edit.html.erb | 43 +++++++++++ .../new.html.erb | 36 +++++++++ config/routes.rb | 4 + .../custom_field_format_dependent.rb | 4 +- 13 files changed, 305 insertions(+), 5 deletions(-) create mode 100644 app/components/admin/custom_fields/calculated_values/details_component.html.erb create mode 100644 app/components/admin/custom_fields/calculated_values/details_component.rb create mode 100644 app/components/admin/custom_fields/calculated_values/details_form.rb create mode 100644 app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb create mode 100644 app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb create mode 100644 app/views/admin/settings/project_custom_field_calculated_values/new.html.erb diff --git a/app/components/admin/custom_fields/calculated_values/details_component.html.erb b/app/components/admin/custom_fields/calculated_values/details_component.html.erb new file mode 100644 index 00000000000..848b0f996e4 --- /dev/null +++ b/app/components/admin/custom_fields/calculated_values/details_component.html.erb @@ -0,0 +1,15 @@ +<%= + component_wrapper do + flex_layout do |content| + content.with_row do + settings_primer_form_with( + model:, + scope: :custom_field, + id: "custom_field_form", + url: form_url, + method: form_method + ) { |form| render Admin::CustomFields::CalculatedValues::DetailsForm.new(form) } + end + end + end +%> diff --git a/app/components/admin/custom_fields/calculated_values/details_component.rb b/app/components/admin/custom_fields/calculated_values/details_component.rb new file mode 100644 index 00000000000..74d88666d0f --- /dev/null +++ b/app/components/admin/custom_fields/calculated_values/details_component.rb @@ -0,0 +1,53 @@ +# 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. +#++ +# +module Admin::CustomFields::CalculatedValues + class DetailsComponent < ApplicationComponent + include ApplicationHelper + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + + alias_method :custom_field, :model + + private + + def form_url + if custom_field.new_record? + admin_settings_project_custom_fields_path + else + admin_settings_project_custom_field_path(custom_field) + end + end + + def form_method + custom_field.new_record? ? :post : :put + end + end +end diff --git a/app/components/admin/custom_fields/calculated_values/details_form.rb b/app/components/admin/custom_fields/calculated_values/details_form.rb new file mode 100644 index 00000000000..d051e60cf89 --- /dev/null +++ b/app/components/admin/custom_fields/calculated_values/details_form.rb @@ -0,0 +1,75 @@ +# 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. +#++ + +module Admin::CustomFields::CalculatedValues + class DetailsForm < ApplicationForm + form do |details_form| + details_form.text_field( + name: :name, + label: I18n.t(:label_name), + required: true + ) + + details_form.select_list( + name: :custom_field_section_id, + label: I18n.t("activerecord.attributes.project_custom_field.custom_field_section"), + required: true + ) do |li| + ProjectCustomFieldSection.all.collect.each do |cs| + li.option(value: cs.id, label: cs.name) + end + end + + details_form.text_field( + name: :formula, + label: I18n.t(:label_formula), + required: true, + caption: I18n.t("custom_fields.instructions.formula") + ) + + details_form.check_box( + name: :is_for_all, + label: I18n.t("activerecord.attributes.custom_field.is_for_all"), + caption: I18n.t("custom_fields.instructions.is_for_all") + ) + + details_form.check_box( + name: :admin_only, + label: I18n.t("activerecord.attributes.custom_field.admin_only"), + caption: I18n.t("custom_fields.instructions.admin_only") + ) + + details_form.hidden(name: :field_format) + details_form.hidden(name: :type, scope_name_to_model: false) + + details_form.submit(name: :submit, label: I18n.t(:button_save), scheme: :default) + end + end +end diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb index 37916605dc1..4a8b84596a1 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -8,7 +8,7 @@ content_container.with_column(mr: 2) do render( Primer::Beta::Link.new( - href: edit_admin_settings_project_custom_field_path(@project_custom_field), + href: edit_path, underline: false, font_weight: :bold, data: { turbo: "false" } diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.rb b/app/components/settings/project_custom_field_sections/custom_field_row_component.rb index 74cade38187..a181c078dbc 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.rb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.rb @@ -42,9 +42,17 @@ module Settings private + def edit_path + if @project_custom_field.field_format_calculated_value? + edit_admin_settings_project_custom_field_calculated_value_path(@project_custom_field) + else + edit_admin_settings_project_custom_field_path(@project_custom_field) + end + end + def edit_action_item(menu) menu.with_item(label: t("label_edit"), - href: edit_admin_settings_project_custom_field_path(@project_custom_field), + href: edit_path, data: { turbo: "false", test_selector: "project-custom-field-edit" }) do |item| item.with_leading_visual_icon(icon: :pencil) end diff --git a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb new file mode 100644 index 00000000000..a7d331eb14b --- /dev/null +++ b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb @@ -0,0 +1,56 @@ +# 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. +#++ + +module Admin::Settings + class ProjectCustomFieldCalculatedValuesController < ::Admin::SettingsController + include CustomFields::SharedActions + include OpTurbo::ComponentStream + include OpTurbo::DialogStreamHelper + include FlashMessagesOutputSafetyHelper + include Admin::Settings::ProjectCustomFields::ComponentStreams + + def show + # quick fixing redirect issue from perform_update + # perform_update is always redirecting to the show action although configured otherwise + render :edit + end + + def new + @custom_field = ProjectCustomField.new(custom_field_section_id: params[:custom_field_section_id], + field_format: "calculated_value") + + respond_to :html + end + + def edit + @custom_field = ProjectCustomField.find(params[:id]) + end + end +end diff --git a/app/controllers/admin/settings/project_custom_fields_controller.rb b/app/controllers/admin/settings/project_custom_fields_controller.rb index e537cf365c0..cf801f9ad2c 100644 --- a/app/controllers/admin/settings/project_custom_fields_controller.rb +++ b/app/controllers/admin/settings/project_custom_fields_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH diff --git a/app/controllers/concerns/custom_fields/shared_actions.rb b/app/controllers/concerns/custom_fields/shared_actions.rb index d02ad148891..0b35ca37a63 100644 --- a/app/controllers/concerns/custom_fields/shared_actions.rb +++ b/app/controllers/concerns/custom_fields/shared_actions.rb @@ -41,7 +41,11 @@ module CustomFields def edit_path(custom_field, params = {}) if custom_field.type == "ProjectCustomField" - admin_settings_project_custom_field_path(**params) + if custom_field.field_format_calculated_value? + admin_settings_project_custom_field_calculated_values_path(**params) + else + admin_settings_project_custom_fields_path(**params) + end else edit_custom_field_path(**params) end diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 3fcab7a3a61..8b2f42fd876 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -299,6 +299,10 @@ class CustomField < ApplicationRecord field_format == "hierarchy" end + def field_format_calculated_value? + field_format == "calculated_value" + end + def multi_value_possible? OpenProject::CustomFieldFormat.find_by(name: field_format)&.multi_value_possible? end diff --git a/app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb b/app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb new file mode 100644 index 00000000000..909df98646f --- /dev/null +++ b/app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb @@ -0,0 +1,43 @@ +<%#-- 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. + +++#%> + +<% html_title t(:label_administration), t("settings.project_attributes.heading"), @custom_field.name %> + +<%= + render( + Settings::ProjectCustomFields::EditFormHeaderComponent.new( + custom_field: @custom_field, + selected: :project_custom_field_edit + ) + ) +%> + +<%= error_messages_for "custom_field" %> + +<%= render Admin::CustomFields::CalculatedValues::DetailsComponent.new(@custom_field) %> diff --git a/app/views/admin/settings/project_custom_field_calculated_values/new.html.erb b/app/views/admin/settings/project_custom_field_calculated_values/new.html.erb new file mode 100644 index 00000000000..67d84e9a517 --- /dev/null +++ b/app/views/admin/settings/project_custom_field_calculated_values/new.html.erb @@ -0,0 +1,36 @@ +<%#-- 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. + +++#%> + +<% html_title t(:label_administration), t("settings.project_attributes.heading"), t("settings.project_attributes.new.heading") %> + +<%= render(Settings::ProjectCustomFields::NewFormHeaderComponent.new) %> + +<%= error_messages_for "custom_field" %> + +<%= render Admin::CustomFields::CalculatedValues::DetailsComponent.new(@custom_field) %> diff --git a/config/routes.rb b/config/routes.rb index 363e9a1fe5a..fe2743311e1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -601,6 +601,10 @@ Rails.application.routes.draw do delete :unlink end end + + resources :project_custom_field_calculated_values, + controller: "/admin/settings/project_custom_field_calculated_values" + resources :project_custom_field_sections, controller: "/admin/settings/project_custom_field_sections", only: %i[create update destroy] do member do diff --git a/lib/open_project/custom_field_format_dependent.rb b/lib/open_project/custom_field_format_dependent.rb index 6a95d8a61f1..a07c6d0828d 100644 --- a/lib/open_project/custom_field_format_dependent.rb +++ b/lib/open_project/custom_field_format_dependent.rb @@ -32,8 +32,8 @@ module OpenProject allowNonOpenVersions: [:only, %w[version]], defaultBool: [:only, %w[bool]], defaultLongText: [:only, %w[text]], - defaultText: [:except, %w[list bool date text user version hierarchy]], - length: [:except, %w[list bool date user version link hierarchy]], + defaultText: [:except, %w[list bool date text user version hierarchy calculated_value]], + length: [:except, %w[list bool date user version link hierarchy calculated_value]], multiSelect: [:only, %w[list user version hierarchy]], possibleValues: [:only, %w[list]], regexp: [:except, %w[list bool date user version hierarchy calculated_value]], From 6774c6e9f1ac08c18cdcf52376a6de7572fbb4e5 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Wed, 18 Jun 2025 11:11:18 +0200 Subject: [PATCH 05/44] [#64347] translate custom field formats on index --- .../custom_field_row_component.html.erb | 2 +- .../custom_field_row_component.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb index 7e00e30035c..8bcc2cf52d0 100644 --- a/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -13,7 +13,7 @@ end title_container.with_column(pt: 1, mr: 2, data: { test_selector: "custom-field-type" }) do render(Primer::Beta::Text.new(font_size: :small, color: :subtle)) do - @project_custom_field.field_format.capitalize + t(@project_custom_field.field_format) end end if @project_custom_field.required? diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb index 4a8b84596a1..a56d115f78f 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -19,7 +19,7 @@ end content_container.with_column(mr: 2) do render(Primer::Beta::Text.new(font_size: :small, color: :subtle)) do - @project_custom_field.field_format.capitalize + t(@project_custom_field.field_format) end end content_container.with_column(mr: 2) do From 68f14261b28d538fc45471e4099992f960bcb108 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Wed, 18 Jun 2025 15:52:57 +0200 Subject: [PATCH 06/44] [#64347] show correct active menu item --- .../project_custom_field_calculated_values_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb index a7d331eb14b..41e71cf75fd 100644 --- a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb +++ b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb @@ -36,6 +36,8 @@ module Admin::Settings include FlashMessagesOutputSafetyHelper include Admin::Settings::ProjectCustomFields::ComponentStreams + menu_item :project_custom_fields_settings + def show # quick fixing redirect issue from perform_update # perform_update is always redirecting to the show action although configured otherwise From 35954c7eb1aabc1e36342669c9456b8223bbb1f8 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Wed, 18 Jun 2025 17:21:36 +0200 Subject: [PATCH 07/44] [#64347] controller checks feature flag --- ...roject_custom_field_calculated_values_controller.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb index 41e71cf75fd..4c591e4296d 100644 --- a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb +++ b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb @@ -36,6 +36,8 @@ module Admin::Settings include FlashMessagesOutputSafetyHelper include Admin::Settings::ProjectCustomFields::ComponentStreams + before_action :check_feature_flag + menu_item :project_custom_fields_settings def show @@ -52,7 +54,13 @@ module Admin::Settings end def edit - @custom_field = ProjectCustomField.find(params[:id]) + @custom_field = ProjectCustomField.find_by(id: params[:id], field_format: "calculated_value") + end + + private + + def check_feature_flag + render_404 unless OpenProject::FeatureDecisions.calculated_value_project_attribute_active? end end end From fb8148f0287ef21e2a9246d10c2328e8fb0af18a Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Fri, 20 Jun 2025 13:59:17 +0200 Subject: [PATCH 08/44] [#64347] check feature flag in default scope for custom fields --- app/models/custom_field.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 8b2f42fd876..f95a61aac62 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -30,7 +30,6 @@ class CustomField < ApplicationRecord include CustomField::OrderStatements - scope :required, -> { where(is_required: true) } has_many :custom_values, dependent: :delete_all # WARNING: the inverse_of option is also required in order # for the 'touch: true' option on the custom_field association in CustomOption @@ -49,6 +48,15 @@ class CustomField < ApplicationRecord inverse_of: "custom_field" scope :hierarchy_root_and_children, -> { includes(hierarchy_root: { children: :children }) } + scope :required, -> { where(is_required: true) } + + default_scope -> { + if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? + all + else + where.not(field_format: "calculated_value") + end + } acts_as_list scope: [:type] From 373591a5fd9d75f31b7f368b6f65ff6c7e36cc5b Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Fri, 20 Jun 2025 14:30:39 +0200 Subject: [PATCH 09/44] [#64347] remove non-existent module import --- .../project_custom_field_calculated_values_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb index 4c591e4296d..f862485a2d1 100644 --- a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb +++ b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb @@ -32,7 +32,6 @@ module Admin::Settings class ProjectCustomFieldCalculatedValuesController < ::Admin::SettingsController include CustomFields::SharedActions include OpTurbo::ComponentStream - include OpTurbo::DialogStreamHelper include FlashMessagesOutputSafetyHelper include Admin::Settings::ProjectCustomFields::ComponentStreams From ed7c27c2082333c45690e956dec0110fb4af9701 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 23 Jun 2025 10:33:56 +0200 Subject: [PATCH 10/44] [#64347] write/read formula, prepare validation --- .../calculated_values/details_form.rb | 1 + app/models/custom_field.rb | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/app/components/admin/custom_fields/calculated_values/details_form.rb b/app/components/admin/custom_fields/calculated_values/details_form.rb index d051e60cf89..79abd41c760 100644 --- a/app/components/admin/custom_fields/calculated_values/details_form.rb +++ b/app/components/admin/custom_fields/calculated_values/details_form.rb @@ -49,6 +49,7 @@ module Admin::CustomFields::CalculatedValues details_form.text_field( name: :formula, + value: model.formula_string, label: I18n.t(:label_formula), required: true, caption: I18n.t("custom_fields.instructions.formula") diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index f95a61aac62..5f03bc3fddf 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -80,6 +80,7 @@ class CustomField < ApplicationRecord validate :validate_default_value validate :validate_regex + validate :validate_formula validates :min_length, numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :max_length, numericality: { only_integer: true, greater_than_or_equal_to: 0 } @@ -137,6 +138,15 @@ class CustomField < ApplicationRecord errors.add(:regexp, :invalid) end + def validate_formula + return unless field_format_calculated_value? + + # TODO: add meaningful validation + if formula_string == "foo" + errors.add(:formula, :invalid) + end + end + def has_regexp? regexp.present? end @@ -311,6 +321,19 @@ class CustomField < ApplicationRecord field_format == "calculated_value" end + def formula=(value) + if value.is_a?(String) + # TODO perform string parsing here + super({ formula: value }) + else + super + end + end + + def formula_string + formula.is_a?(Hash) ? formula["formula"] : formula + end + def multi_value_possible? OpenProject::CustomFieldFormat.find_by(name: field_format)&.multi_value_possible? end From d38c6244a515888a459dabea0e1ed9ad16fe0868 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 23 Jun 2025 11:03:59 +0200 Subject: [PATCH 11/44] [#64347] perform basic formula validation --- app/models/custom_field.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 5f03bc3fddf..e891af58154 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -141,10 +141,18 @@ class CustomField < ApplicationRecord def validate_formula return unless field_format_calculated_value? - # TODO: add meaningful validation - if formula_string == "foo" - errors.add(:formula, :invalid) - end + # List of allowed characters in a formula. This only performs a very basic validation. + # Allowed characters are: + # + - / * ( ) whitespace digits and decimal points + # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of + # the custom field. + # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST + # and ensures that the formula is really valid. A welcome side-effect of the basic validation done here is that + # it prevents built-in functions from being used in the formula, which we do not want to allow. + common_chars = '[\+\-\/\*\(\)\s\d\.]' + pattern = /\A#{common_chars}+(?:cf_\d+#{common_chars}*)*\z/ + + errors.add(:formula, :invalid) unless formula_string.match?(pattern) end def has_regexp? From af23dd7bdc9b6638827ebdcfc26e2db1f09ca286 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 23 Jun 2025 14:22:32 +0200 Subject: [PATCH 12/44] [#64347] extract custom field IDs from formula --- Gemfile | 3 +++ Gemfile.lock | 5 +++++ app/models/custom_field.rb | 18 +++++++++++++++--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index cac0a32a5f8..c91cc7a019a 100644 --- a/Gemfile +++ b/Gemfile @@ -390,6 +390,9 @@ gem "googleauth", require: false # Required for contracts gem "disposable", "~> 0.6.2" +# Used for formula evaluation of calculated values +gem "dentaku", "~> 3.5" + platforms :mri, :mingw, :x64_mingw do group :postgres do gem "pg", "~> 1.5.0" diff --git a/Gemfile.lock b/Gemfile.lock index aaca71eccfd..34b8bd2a6b2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -464,6 +464,9 @@ GEM deckar01-task_list (2.3.4) html-pipeline (~> 2.0) declarative (0.0.20) + dentaku (3.5.4) + bigdecimal + concurrent-ruby descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.6.2) @@ -1371,6 +1374,7 @@ DEPENDENCIES dashboards! date_validator (~> 0.12.0) deckar01-task_list (~> 2.3.1) + dentaku (~> 3.5) disposable (~> 0.6.2) doorkeeper (~> 5.8.0) dotenv-rails @@ -1639,6 +1643,7 @@ CHECKSUMS date_validator (0.12.0) sha256=68c9834da240347b9c17441c553a183572508617ebfbe8c020020f3192ce3058 deckar01-task_list (2.3.4) sha256=66abdc7e009ea759732bb53867e1ea42de550e2aa03ac30a015cbf42a04c1667 declarative (0.0.20) sha256=8021dd6cb17ab2b61233c56903d3f5a259c5cf43c80ff332d447d395b17d9ff9 + dentaku (3.5.4) sha256=0f897acf360776c43c3b3629134224abbf6a90a59084707af6e194c5d69ad9c7 descendants_tracker (0.0.4) sha256=e9c41dd4cfbb85829a9301ea7e7c48c2a03b26f09319db230e6479ccdc780897 diff-lcs (1.6.2) sha256=9ae0d2cba7d4df3075fe8cd8602a8604993efc0dfa934cff568969efb1909962 disposable (0.6.3) sha256=7f2a3fb251bff6cd83f25b164043d4ec3531209b51b066ed476a9df9c2d384cc diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index e891af58154..06f76c891e9 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -147,12 +147,19 @@ class CustomField < ApplicationRecord # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of # the custom field. # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST - # and ensures that the formula is really valid. A welcome side-effect of the basic validation done here is that + # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that # it prevents built-in functions from being used in the formula, which we do not want to allow. common_chars = '[\+\-\/\*\(\)\s\d\.]' pattern = /\A#{common_chars}+(?:cf_\d+#{common_chars}*)*\z/ errors.add(:formula, :invalid) unless formula_string.match?(pattern) + + # TODO: check for valid (i.e. visible & enabled) custom field references (see #cf_ids_used_in_formula) + + # Dentaku will return nil if the formula is invalid. + # TODO: add support for referenced custom fields by injecting them as variables, + # e.g. Dentaku(formula_string, cf_123: CustomField.find(123).value) + errors.add(:formula, :invalid) unless Dentaku(formula_string) end def has_regexp? @@ -331,8 +338,7 @@ class CustomField < ApplicationRecord def formula=(value) if value.is_a?(String) - # TODO perform string parsing here - super({ formula: value }) + super({ formula: value, referenced_custom_fields: cf_ids_used_in_formula }) else super end @@ -361,6 +367,12 @@ class CustomField < ApplicationRecord private + # Returns a list of custom field IDs used in the formula. + # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. + def cf_ids_used_in_formula + (formula_string || "").scan(/cf_(\d+)/).flatten.map(&:to_i) + end + def possible_versions(obj, options: {}) project = deduce_project(obj) deduce_versions(project, options:) From 399a233fa29f4a24b154212ebbb41801950e123b Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 23 Jun 2025 14:42:07 +0200 Subject: [PATCH 13/44] [#64347] remove unnecessary calculated values controller With the menu rework, this is no longer necessary --- ...stom_field_calculated_values_controller.rb | 65 ------------------- .../edit.html.erb | 43 ------------ .../new.html.erb | 36 ---------- config/routes.rb | 4 -- 4 files changed, 148 deletions(-) delete mode 100644 app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb delete mode 100644 app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb delete mode 100644 app/views/admin/settings/project_custom_field_calculated_values/new.html.erb diff --git a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb b/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb deleted file mode 100644 index f862485a2d1..00000000000 --- a/app/controllers/admin/settings/project_custom_field_calculated_values_controller.rb +++ /dev/null @@ -1,65 +0,0 @@ -# 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. -#++ - -module Admin::Settings - class ProjectCustomFieldCalculatedValuesController < ::Admin::SettingsController - include CustomFields::SharedActions - include OpTurbo::ComponentStream - include FlashMessagesOutputSafetyHelper - include Admin::Settings::ProjectCustomFields::ComponentStreams - - before_action :check_feature_flag - - menu_item :project_custom_fields_settings - - def show - # quick fixing redirect issue from perform_update - # perform_update is always redirecting to the show action although configured otherwise - render :edit - end - - def new - @custom_field = ProjectCustomField.new(custom_field_section_id: params[:custom_field_section_id], - field_format: "calculated_value") - - respond_to :html - end - - def edit - @custom_field = ProjectCustomField.find_by(id: params[:id], field_format: "calculated_value") - end - - private - - def check_feature_flag - render_404 unless OpenProject::FeatureDecisions.calculated_value_project_attribute_active? - end - end -end diff --git a/app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb b/app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb deleted file mode 100644 index 909df98646f..00000000000 --- a/app/views/admin/settings/project_custom_field_calculated_values/edit.html.erb +++ /dev/null @@ -1,43 +0,0 @@ -<%#-- 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. - -++#%> - -<% html_title t(:label_administration), t("settings.project_attributes.heading"), @custom_field.name %> - -<%= - render( - Settings::ProjectCustomFields::EditFormHeaderComponent.new( - custom_field: @custom_field, - selected: :project_custom_field_edit - ) - ) -%> - -<%= error_messages_for "custom_field" %> - -<%= render Admin::CustomFields::CalculatedValues::DetailsComponent.new(@custom_field) %> diff --git a/app/views/admin/settings/project_custom_field_calculated_values/new.html.erb b/app/views/admin/settings/project_custom_field_calculated_values/new.html.erb deleted file mode 100644 index 67d84e9a517..00000000000 --- a/app/views/admin/settings/project_custom_field_calculated_values/new.html.erb +++ /dev/null @@ -1,36 +0,0 @@ -<%#-- 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. - -++#%> - -<% html_title t(:label_administration), t("settings.project_attributes.heading"), t("settings.project_attributes.new.heading") %> - -<%= render(Settings::ProjectCustomFields::NewFormHeaderComponent.new) %> - -<%= error_messages_for "custom_field" %> - -<%= render Admin::CustomFields::CalculatedValues::DetailsComponent.new(@custom_field) %> diff --git a/config/routes.rb b/config/routes.rb index fe2743311e1..363e9a1fe5a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -601,10 +601,6 @@ Rails.application.routes.draw do delete :unlink end end - - resources :project_custom_field_calculated_values, - controller: "/admin/settings/project_custom_field_calculated_values" - resources :project_custom_field_sections, controller: "/admin/settings/project_custom_field_sections", only: %i[create update destroy] do member do From 87f5dc8940c5c7f563af71ed9ab16d7412157e7d Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 23 Jun 2025 15:11:45 +0200 Subject: [PATCH 14/44] [#64347] inject Primer view for calculated values into existing admin view much like the hierarchy before --- .../custom_field_row_component.rb | 6 +---- .../concerns/custom_fields/shared_actions.rb | 6 +---- .../project_custom_fields/edit.html.erb | 26 +++++++++++-------- .../project_custom_fields/new.html.erb | 26 +++++++++++-------- config/initializers/custom_field_format.rb | 7 +++++ lib/open_project/custom_field_format.rb | 26 +++++++++---------- 6 files changed, 52 insertions(+), 45 deletions(-) diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.rb b/app/components/settings/project_custom_field_sections/custom_field_row_component.rb index a181c078dbc..4195f72e4ba 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.rb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.rb @@ -43,11 +43,7 @@ module Settings private def edit_path - if @project_custom_field.field_format_calculated_value? - edit_admin_settings_project_custom_field_calculated_value_path(@project_custom_field) - else - edit_admin_settings_project_custom_field_path(@project_custom_field) - end + edit_admin_settings_project_custom_field_path(@project_custom_field) end def edit_action_item(menu) diff --git a/app/controllers/concerns/custom_fields/shared_actions.rb b/app/controllers/concerns/custom_fields/shared_actions.rb index 0b35ca37a63..cedc24046d9 100644 --- a/app/controllers/concerns/custom_fields/shared_actions.rb +++ b/app/controllers/concerns/custom_fields/shared_actions.rb @@ -41,11 +41,7 @@ module CustomFields def edit_path(custom_field, params = {}) if custom_field.type == "ProjectCustomField" - if custom_field.field_format_calculated_value? - admin_settings_project_custom_field_calculated_values_path(**params) - else - admin_settings_project_custom_fields_path(**params) - end + admin_settings_project_custom_fields_path(**params) else edit_custom_field_path(**params) end diff --git a/app/views/admin/settings/project_custom_fields/edit.html.erb b/app/views/admin/settings/project_custom_fields/edit.html.erb index 02678f1ccdb..a2b1c26facf 100644 --- a/app/views/admin/settings/project_custom_fields/edit.html.erb +++ b/app/views/admin/settings/project_custom_fields/edit.html.erb @@ -38,18 +38,22 @@ See COPYRIGHT and LICENSE files for more details. ) %> -<%= error_messages_for "custom_field" %> +<% if @custom_field.field_format_calculated_value? %> + <%= render Admin::CustomFields::CalculatedValues::DetailsComponent.new(@custom_field) %> +<% else %> + <%= error_messages_for "custom_field" %> -<% content_controller "admin--custom-fields", - "admin--custom-fields-format-value": @custom_field.field_format, - "admin--custom-fields-format-config-value": OpenProject::CustomFieldFormatDependent.stimulus_config %> + <% content_controller "admin--custom-fields", + "admin--custom-fields-format-value": @custom_field.field_format, + "admin--custom-fields-format-config-value": OpenProject::CustomFieldFormatDependent.stimulus_config %> -<%= labelled_tabular_form_for @custom_field, as: :custom_field, - url: admin_settings_project_custom_field_path(@custom_field), - html: { method: :put, id: "custom_field_form" } do |f| %> - <%= render partial: "custom_fields/form", locals: { f: f, custom_field: @custom_field } %> - <% if @custom_field.new_record? %> - <%= hidden_field_tag "type", @custom_field.type %> + <%= labelled_tabular_form_for @custom_field, as: :custom_field, + url: admin_settings_project_custom_field_path(@custom_field), + html: { method: :put, id: "custom_field_form" } do |f| %> + <%= render partial: "custom_fields/form", locals: { f: f, custom_field: @custom_field } %> + <% if @custom_field.new_record? %> + <%= hidden_field_tag "type", @custom_field.type %> + <% end %> + <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %> <% end %> - <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %> <% end %> diff --git a/app/views/admin/settings/project_custom_fields/new.html.erb b/app/views/admin/settings/project_custom_fields/new.html.erb index 565857a63a9..70b15251716 100644 --- a/app/views/admin/settings/project_custom_fields/new.html.erb +++ b/app/views/admin/settings/project_custom_fields/new.html.erb @@ -31,18 +31,22 @@ See COPYRIGHT and LICENSE files for more details. <%= render(Settings::ProjectCustomFields::NewFormHeaderComponent.new) %> -<%= error_messages_for "custom_field" %> +<% if @custom_field.field_format_calculated_value? %> + <%= render Admin::CustomFields::CalculatedValues::DetailsComponent.new(@custom_field) %> +<% else %> + <%= error_messages_for "custom_field" %> -<% content_controller "admin--custom-fields", - "admin--custom-fields-format-value": @custom_field.field_format, - "admin--custom-fields-format-config-value": OpenProject::CustomFieldFormatDependent.stimulus_config %> + <% content_controller "admin--custom-fields", + "admin--custom-fields-format-value": @custom_field.field_format, + "admin--custom-fields-format-config-value": OpenProject::CustomFieldFormatDependent.stimulus_config %> -<%= labelled_tabular_form_for @custom_field, as: :custom_field, - url: admin_settings_project_custom_fields_path, - html: { id: "custom_field_form" } do |f| %> - <%= render partial: "custom_fields/form", locals: { f: f, custom_field: @custom_field } %> - <% if @custom_field.new_record? %> - <%= hidden_field_tag "type", @custom_field.type %> + <%= labelled_tabular_form_for @custom_field, as: :custom_field, + url: admin_settings_project_custom_fields_path, + html: { id: "custom_field_form" } do |f| %> + <%= render partial: "custom_fields/form", locals: { f: f, custom_field: @custom_field } %> + <% if @custom_field.new_record? %> + <%= hidden_field_tag "type", @custom_field.type %> + <% end %> + <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %> <% end %> - <%= styled_button_tag t(:button_save), class: "-highlight -with-icon icon-checkmark" %> <% end %> diff --git a/config/initializers/custom_field_format.rb b/config/initializers/custom_field_format.rb index 2d773b3950e..71971d18c13 100644 --- a/config/initializers/custom_field_format.rb +++ b/config/initializers/custom_field_format.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -88,4 +90,9 @@ OpenProject::CustomFieldFormat.map do |fields| multi_value_possible: true, enterprise_feature: :custom_field_hierarchies, formatter: "CustomValue::HierarchyStrategy") + + fields.register OpenProject::CustomFieldFormat.new("calculated_value", + label: :label_calculated_value, + only: %w(Project), + order: 13) end diff --git a/lib/open_project/custom_field_format.rb b/lib/open_project/custom_field_format.rb index 354c5e3456b..295c2d89697 100644 --- a/lib/open_project/custom_field_format.rb +++ b/lib/open_project/custom_field_format.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -72,26 +74,24 @@ module OpenProject end def available - registered - .select { |_, format| !format.enterprise_feature || EnterpriseToken.allows_to?(format.enterprise_feature) } + formats = registered.select do |_, format| + !format.enterprise_feature || EnterpriseToken.allows_to?(format.enterprise_feature) + end + + if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? + formats + else + formats.except("calculated_value") + end end def available_formats available.keys end - # TODO: Consider changing the initializer order and moving this to the CustomFieldFormat initializer instead - def ensure_calculated_value_if_active! - if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? && !@@available.key?("calculated_value") - order_of_last_entry = @@available.values.last.order - register OpenProject::CustomFieldFormat.new("calculated_value", - label: :label_calculated_value, - only: %w(Project), - order: order_of_last_entry + 1) - end - end - def find_by(name:) + return nil if name.to_s == "calculated_value" && !OpenProject::FeatureDecisions.calculated_value_project_attribute_active? + registered[name.to_s] end From 127cee26aefb95714b69a95e644057c6b44bb42c Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 23 Jun 2025 16:43:41 +0200 Subject: [PATCH 15/44] [#64347] get rid of inefficient regular expression --- app/models/custom_field.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 06f76c891e9..0924d3c70f3 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -149,10 +149,14 @@ class CustomField < ApplicationRecord # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that # it prevents built-in functions from being used in the formula, which we do not want to allow. - common_chars = '[\+\-\/\*\(\)\s\d\.]' - pattern = /\A#{common_chars}+(?:cf_\d+#{common_chars}*)*\z/ + allowed_chars = %w[+ - / * ( )] + [" "] + split_pattern = /\s|(\+)|(-)|(\*)|(\()|(\))/ + pattern = /^(cf_\d+|\d+\.?\d*|\.\d+.|[#{allowed_chars.join}]+)$/ + valid_formula_string = formula_string.split(split_pattern).reject(&:empty?).all? do |token| + token.match?(pattern) + end - errors.add(:formula, :invalid) unless formula_string.match?(pattern) + errors.add(:formula, :invalid) unless valid_formula_string # TODO: check for valid (i.e. visible & enabled) custom field references (see #cf_ids_used_in_formula) From af910f5e2c7db4bc239ab73dd841e339dcab49b1 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 09:12:16 +0200 Subject: [PATCH 16/44] [#64347] move calculated value logic into its own module --- app/models/custom_field.rb | 48 +----------- app/models/custom_field/calculated_value.rb | 81 +++++++++++++++++++++ 2 files changed, 83 insertions(+), 46 deletions(-) create mode 100644 app/models/custom_field/calculated_value.rb diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 0924d3c70f3..8e3ab0d0f26 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -30,6 +30,8 @@ class CustomField < ApplicationRecord include CustomField::OrderStatements + include CustomField::CalculatedValue + has_many :custom_values, dependent: :delete_all # WARNING: the inverse_of option is also required in order # for the 'touch: true' option on the custom_field association in CustomOption @@ -138,34 +140,6 @@ class CustomField < ApplicationRecord errors.add(:regexp, :invalid) end - def validate_formula - return unless field_format_calculated_value? - - # List of allowed characters in a formula. This only performs a very basic validation. - # Allowed characters are: - # + - / * ( ) whitespace digits and decimal points - # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of - # the custom field. - # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST - # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that - # it prevents built-in functions from being used in the formula, which we do not want to allow. - allowed_chars = %w[+ - / * ( )] + [" "] - split_pattern = /\s|(\+)|(-)|(\*)|(\()|(\))/ - pattern = /^(cf_\d+|\d+\.?\d*|\.\d+.|[#{allowed_chars.join}]+)$/ - valid_formula_string = formula_string.split(split_pattern).reject(&:empty?).all? do |token| - token.match?(pattern) - end - - errors.add(:formula, :invalid) unless valid_formula_string - - # TODO: check for valid (i.e. visible & enabled) custom field references (see #cf_ids_used_in_formula) - - # Dentaku will return nil if the formula is invalid. - # TODO: add support for referenced custom fields by injecting them as variables, - # e.g. Dentaku(formula_string, cf_123: CustomField.find(123).value) - errors.add(:formula, :invalid) unless Dentaku(formula_string) - end - def has_regexp? regexp.present? end @@ -340,18 +314,6 @@ class CustomField < ApplicationRecord field_format == "calculated_value" end - def formula=(value) - if value.is_a?(String) - super({ formula: value, referenced_custom_fields: cf_ids_used_in_formula }) - else - super - end - end - - def formula_string - formula.is_a?(Hash) ? formula["formula"] : formula - end - def multi_value_possible? OpenProject::CustomFieldFormat.find_by(name: field_format)&.multi_value_possible? end @@ -371,12 +333,6 @@ class CustomField < ApplicationRecord private - # Returns a list of custom field IDs used in the formula. - # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. - def cf_ids_used_in_formula - (formula_string || "").scan(/cf_(\d+)/).flatten.map(&:to_i) - end - def possible_versions(obj, options: {}) project = deduce_project(obj) deduce_versions(project, options:) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb new file mode 100644 index 00000000000..afb6d1a004c --- /dev/null +++ b/app/models/custom_field/calculated_value.rb @@ -0,0 +1,81 @@ +# 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. +#++ + +# Methods for custom fields with a format of "calculated value". +# Should be included in the CustomField model. +module CustomField::CalculatedValue + def validate_formula + return unless field_format_calculated_value? + + # List of allowed characters in a formula. This only performs a very basic validation. + # Allowed characters are: + # + - / * ( ) whitespace digits and decimal points + # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of + # the custom field. + # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST + # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that + # it prevents built-in functions from being used in the formula, which we do not want to allow. + allowed_chars = %w[+ - / * ( )] + [" "] + split_pattern = /\s|(\+)|(-)|(\*)|(\()|(\))/ + pattern = /^(cf_\d+|\d+\.?\d*|\.\d+.|[#{allowed_chars.join}]+)$/ + valid_formula_string = formula_string.split(split_pattern).reject(&:empty?).all? do |token| + token.match?(pattern) + end + + errors.add(:formula, :invalid) unless valid_formula_string + + # WP-64348: check for valid (i.e., visible & enabled) custom field references (see #cf_ids_used_in_formula) + + # Dentaku will return nil if the formula is invalid. + # TODO WP-64348: add support for referenced custom fields by injecting them as variables, + # e.g. Dentaku(formula_string, cf_123: CustomField.find(123).value) + errors.add(:formula, :invalid) unless Dentaku(formula_string) + end + + def formula=(value) + if value.is_a?(String) + super({ formula: value, referenced_custom_fields: cf_ids_used_in_formula }) + else + super + end + end + + def formula_string + formula.is_a?(Hash) ? formula["formula"] : formula + end + + private + + # Returns a list of custom field IDs used in the formula. + # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. + def cf_ids_used_in_formula + (formula_string || "").scan(/cf_(\d+)/).flatten.map(&:to_i) + end +end From 9eb195442233df1c6ed23b45d823c15634407530 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 10:51:38 +0200 Subject: [PATCH 17/44] [#64347] calculated value model specs --- app/models/custom_field/calculated_value.rb | 46 ++++--- spec/factories/custom_field_factory.rb | 7 +- .../custom_field/calculated_value_spec.rb | 116 ++++++++++++++++++ spec/models/custom_field_spec.rb | 9 ++ 4 files changed, 157 insertions(+), 21 deletions(-) create mode 100644 spec/models/custom_field/calculated_value_spec.rb diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index afb6d1a004c..dd70e5ebf65 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -33,23 +33,10 @@ module CustomField::CalculatedValue def validate_formula return unless field_format_calculated_value? + # An empty formula is okay. + return if formula_string.blank? - # List of allowed characters in a formula. This only performs a very basic validation. - # Allowed characters are: - # + - / * ( ) whitespace digits and decimal points - # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of - # the custom field. - # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST - # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that - # it prevents built-in functions from being used in the formula, which we do not want to allow. - allowed_chars = %w[+ - / * ( )] + [" "] - split_pattern = /\s|(\+)|(-)|(\*)|(\()|(\))/ - pattern = /^(cf_\d+|\d+\.?\d*|\.\d+.|[#{allowed_chars.join}]+)$/ - valid_formula_string = formula_string.split(split_pattern).reject(&:empty?).all? do |token| - token.match?(pattern) - end - - errors.add(:formula, :invalid) unless valid_formula_string + errors.add(:formula, :invalid) unless formula_contains_only_allowed_characters? # WP-64348: check for valid (i.e., visible & enabled) custom field references (see #cf_ids_used_in_formula) @@ -61,21 +48,40 @@ module CustomField::CalculatedValue def formula=(value) if value.is_a?(String) - super({ formula: value, referenced_custom_fields: cf_ids_used_in_formula }) + super({ formula: value, referenced_custom_fields: cf_ids_used_in_formula(value) }) else super end end + # Returns the formula as a string. Will return an empty string if the formula is not set. def formula_string - formula.is_a?(Hash) ? formula["formula"] : formula + formula.is_a?(Hash) ? formula.fetch("formula", "") : formula.to_s end private + def formula_contains_only_allowed_characters? + # List of allowed characters in a formula. This only performs a very basic validation. + # Allowed characters are: + # + - / * ( ) whitespace digits and decimal points + # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of + # the custom field. + # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST + # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that + # it prevents built-in functions from being used in the formula, which we do not want to allow. + allowed_chars = %w[+ - / * ( )] + [" "] + split_pattern = /\s|(\+)|(-)|(\/)|(\*)|(\()|(\))/ + allowed_tokens = /^(cf_\d+|\d+\.?\d*|\.\d+.|[#{allowed_chars.join}]+)$/ + + formula_string.split(split_pattern).reject(&:empty?).all? do |token| + token.match?(allowed_tokens) + end + end + # Returns a list of custom field IDs used in the formula. # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. - def cf_ids_used_in_formula - (formula_string || "").scan(/cf_(\d+)/).flatten.map(&:to_i) + def cf_ids_used_in_formula(formula_str = formula_string) + formula_str.scan(/cf_(\d+)/).flatten.map(&:to_i) end end diff --git a/spec/factories/custom_field_factory.rb b/spec/factories/custom_field_factory.rb index 0dcb1686805..b5ef114a7b7 100644 --- a/spec/factories/custom_field_factory.rb +++ b/spec/factories/custom_field_factory.rb @@ -81,6 +81,10 @@ FactoryBot.define do field_format { "int" } end + trait :calculated_value do + field_format { "calculated_value" } + end + trait :float do field_format { "float" } end @@ -96,7 +100,7 @@ FactoryBot.define do end field_format { "list" } multi_value { false } - possible_values { ["A", "B", "C", "D", "E", "F", "G"] } + possible_values { %w[A B C D E F G] } # update custom options default value from the default_option transient # field for non-multiselect field @@ -203,6 +207,7 @@ FactoryBot.define do factory :string_project_custom_field, traits: [:string] factory :text_project_custom_field, traits: [:text] factory :integer_project_custom_field, traits: [:integer] + factory :calculated_value_project_custom_field, traits: [:calculated_value] factory :float_project_custom_field, traits: [:float] factory :date_project_custom_field, traits: [:date] factory :list_project_custom_field, traits: [:list] diff --git a/spec/models/custom_field/calculated_value_spec.rb b/spec/models/custom_field/calculated_value_spec.rb new file mode 100644 index 00000000000..1c645a5a745 --- /dev/null +++ b/spec/models/custom_field/calculated_value_spec.rb @@ -0,0 +1,116 @@ +# 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 CustomField::CalculatedValue, with_flag: { calculated_value_project_attribute: true } do + context "when calculated value" do + subject(:custom_field) { create(:calculated_value_project_custom_field) } + + describe "#formula=" do + it "splits formula and referenced custom fields on persist if given a string" do + formula = "1 * cf_7 + cf_42" + subject.formula = formula + + expect(subject.formula).to eq({ "formula" => formula, "referenced_custom_fields" => [7, 42] }) + end + + it "omits referenced custom fields if none are given" do + formula = "2 + 3 * (8 / 7)" + subject.formula = formula + + expect(subject.formula).to eq({ "formula" => formula, "referenced_custom_fields" => [] }) + end + end + + describe "#formula_string" do + it "returns an empty string if no formula is set" do + expect(subject.formula_string).to eq("") + end + + it "returns the formula as a string" do + formula = "1 * cf_7 + cf_42" + subject.formula = formula + + expect(subject.formula_string).to eq(formula) + end + end + + describe "#validate_formula" do + let(:formula) { "" } + + before do + subject.formula = formula + subject.validate_formula + end + + context "with an empty formula" do + it "is valid" do + expect(subject).to be_valid + end + end + + context "with a formula containing only allowed characters" do + let(:formula) { "1 / 2 + (3 * 4.5) - 0.0" } + + it "is valid" do + expect(subject).to be_valid + end + end + + context "with a formula containing forbidden characters" do + let(:formula) { "abc + 2" } + + it "is invalid" do + expect(subject).not_to be_valid + expect(subject.errors[:formula]).to include("is invalid.") + end + end + + context "with a formula that is not a valid equation" do + let(:formula) { "1 / + - 3" } + + it "is invalid" do + expect(subject).not_to be_valid + expect(subject.errors[:formula]).to include("is invalid.") + end + end + + context "with a formula that causes a division by zero error" do + let(:formula) { "1 / 0" } + + it "is invalid" do + expect(subject).not_to be_valid + expect(subject.errors[:formula]).to include("is invalid.") + end + end + end + end +end diff --git a/spec/models/custom_field_spec.rb b/spec/models/custom_field_spec.rb index 7607c938203..2b28935f2d6 100644 --- a/spec/models/custom_field_spec.rb +++ b/spec/models/custom_field_spec.rb @@ -485,6 +485,15 @@ RSpec.describe CustomField do end end + context "with a project calculated value cf" do + let(:field) { build_stubbed(:calculated_value_project_custom_field) } + + it "is false" do + expect(field) + .not_to be_multi_value_possible + end + end + context "with a time_entry user cf" do let(:field) { build_stubbed(:time_entry_custom_field, :user) } From bdf05da508b5bdbc5ab7cbff24b595915d67bb83 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 11:19:04 +0200 Subject: [PATCH 18/44] [#64347] remove falsy dot from token pattern --- app/models/custom_field/calculated_value.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index dd70e5ebf65..58b088ae4bb 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -72,7 +72,7 @@ module CustomField::CalculatedValue # it prevents built-in functions from being used in the formula, which we do not want to allow. allowed_chars = %w[+ - / * ( )] + [" "] split_pattern = /\s|(\+)|(-)|(\/)|(\*)|(\()|(\))/ - allowed_tokens = /^(cf_\d+|\d+\.?\d*|\.\d+.|[#{allowed_chars.join}]+)$/ + allowed_tokens = /^(cf_\d+|\d+\.?\d*|\.\d+|[#{allowed_chars.join}]+)$/ formula_string.split(split_pattern).reject(&:empty?).all? do |token| token.match?(allowed_tokens) From 5519b9e4efaec11e083c6e25f8a524b5c66411b8 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 11:21:47 +0200 Subject: [PATCH 19/44] [#64347] error messages --- app/models/custom_field/calculated_value.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index 58b088ae4bb..2174ab82543 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -44,6 +44,9 @@ module CustomField::CalculatedValue # TODO WP-64348: add support for referenced custom fields by injecting them as variables, # e.g. Dentaku(formula_string, cf_123: CustomField.find(123).value) errors.add(:formula, :invalid) unless Dentaku(formula_string) + + # TODO: consider differentiating between a formula that contains invalid characters, missing variables, invalid + # syntax, or mathematical errors. end def formula=(value) From 40aba89ed0a232679d2e54430bf75a5e699e2bc6 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 15:02:49 +0200 Subject: [PATCH 20/44] [#64347] add missing translations for field formats --- .../custom_field_row_component.html.erb | 2 +- config/locales/en.yml | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb index a56d115f78f..58673930538 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -19,7 +19,7 @@ end content_container.with_column(mr: 2) do render(Primer::Beta::Text.new(font_size: :small, color: :subtle)) do - t(@project_custom_field.field_format) + t("label_#{@project_custom_field.field_format}") end end content_container.with_column(mr: 2) do diff --git a/config/locales/en.yml b/config/locales/en.yml index b8fdfe6c783..69958298153 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2707,6 +2707,7 @@ en: label_forum_new: "New forum" label_forum_plural: "Forums" label_forum_sticky: "Sticky" + label_bool: "Boolean" label_boolean: "Boolean" label_board_plural: "Boards" label_branch: "Branch" @@ -2895,6 +2896,7 @@ en: label_information: "Information" label_information_plural: "Information" label_installation_guides: "Installation guides" + label_int: "Integer" label_integer: "Integer" label_interface: "Interface" label_internal: "Internal" From ac6e6155b2dbfa408cd9adcbd3d598f113366035 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 16:23:20 +0200 Subject: [PATCH 21/44] [#64347] project custom fields index spec --- lib/open_project/custom_field_format.rb | 2 - .../custom_fields/projects/index_spec.rb | 51 ++++++++++++++++++- .../open_project/custom_field_format_spec.rb | 44 ++++++++++++---- .../custom_fields_projects/index.rb | 20 ++++++++ 4 files changed, 105 insertions(+), 12 deletions(-) diff --git a/lib/open_project/custom_field_format.rb b/lib/open_project/custom_field_format.rb index 295c2d89697..0ce379e26ff 100644 --- a/lib/open_project/custom_field_format.rb +++ b/lib/open_project/custom_field_format.rb @@ -90,8 +90,6 @@ module OpenProject end def find_by(name:) - return nil if name.to_s == "calculated_value" && !OpenProject::FeatureDecisions.calculated_value_project_attribute_active? - registered[name.to_s] end diff --git a/spec/features/admin/custom_fields/projects/index_spec.rb b/spec/features/admin/custom_fields/projects/index_spec.rb index 556cba513f3..f6c4eec10b7 100644 --- a/spec/features/admin/custom_fields/projects/index_spec.rb +++ b/spec/features/admin/custom_fields/projects/index_spec.rb @@ -129,6 +129,55 @@ RSpec.describe "List project custom fields", :js do end describe "managing project custom fields" do + context "with calculated value feature flag active", with_flag: { calculated_value_project_attribute: true } do + it "offers the type for creation" do + cf_index_page.expect_having_create_item("Calculated value") + end + + context "with fields of type calculated value" do + let!(:calculated_value_project_custom_field) do + create(:calculated_value_project_custom_field, + name: "Calculated value field", + project_custom_field_section: section_for_input_fields) + end + + before do + login_as(admin) + cf_index_page.visit! + end + + it "lists the calculated value custom field" do + within_project_custom_field_section_container(section_for_input_fields) do + containers = page.all(".op-project-custom-field-container") + + expect(containers.last.text).to include(calculated_value_project_custom_field.name) + end + end + + it "does not list calculated values if the feature flag is deactivated later" do + # This spec tests that calculated values are no longer shown after the feature flag is deactivated. + # First, a custom field of type calculated value is created. This must be done while the feature flag is active, + # or else the model validation will fail. + # Next, we simulate that the feature flag is off: + allow(OpenProject::FeatureDecisions).to receive(:calculated_value_project_attribute_active?).and_return(false) + + # Revisit the page and check that the field is no longer listed: + cf_index_page.visit! + within_project_custom_field_section_container(section_for_input_fields) do + containers = page.all(".op-project-custom-field-container") + + expect(containers.last.text).not_to include(calculated_value_project_custom_field.name) + end + end + end + end + + context "without calculated value feature flag active" do + it "does not offer the type for creation" do + cf_index_page.expect_not_having_create_item("Calculated value") + end + end + it "shows all custom fields in the correct order within their section and allows reordering via menu or drag and drop" do within_project_custom_field_section_container(section_for_input_fields) do containers = page.all(".op-project-custom-field-container") @@ -253,7 +302,7 @@ RSpec.describe "List project custom fields", :js do within_project_custom_field_section_menu(section) do click_on(action) end - sleep 0.5 # quick fix: allow the brower to process the action + sleep 0.5 # quick fix: allow the browser to process the action end def within_project_custom_field_container(custom_field, &) diff --git a/spec/lib/open_project/custom_field_format_spec.rb b/spec/lib/open_project/custom_field_format_spec.rb index b1e45393c1b..928d5c12ff3 100644 --- a/spec/lib/open_project/custom_field_format_spec.rb +++ b/spec/lib/open_project/custom_field_format_spec.rb @@ -41,47 +41,55 @@ RSpec.describe OpenProject::CustomFieldFormat do end context "for a 'Project' class" do - it_behaves_like "custom field formats", - "Project", - ["bool", "date", "float", "int", "link", "list", "string", "text", "user", "version"] + context "with calculated values feature flag enabled", with_flag: { calculated_value_project_attribute: true } do + it_behaves_like "custom field formats", + "Project", + %w[bool calculated_value date float int link list string text user version] + end + + context "with calculated values feature flag disabled" do + it_behaves_like "custom field formats", + "Project", + %w[bool date float int link list string text user version] + end end context "for a 'WorkPackage' class" do context "with a custom_field_hierarchies ee", with_ee: [:custom_field_hierarchies] do it_behaves_like "custom field formats", "WorkPackage", - ["bool", "date", "float", "int", "link", "list", "string", "text", "user", "version", "hierarchy"] + %w[bool date float int link list string text user version hierarchy] end context "without a custom_field_hierarchies ee" do it_behaves_like "custom field formats", "WorkPackage", - ["bool", "date", "float", "int", "link", "list", "string", "text", "user", "version"] + %w[bool date float int link list string text user version] end end context "for a 'Version' class" do it_behaves_like "custom field formats", "Version", - ["bool", "date", "float", "int", "list", "string", "text", "user", "version"] + %w[bool date float int list string text user version] end context "for a 'TimeEntry' class" do it_behaves_like "custom field formats", "TimeEntry", - ["bool", "date", "float", "int", "list", "string", "text", "user", "version"] + %w[bool date float int list string text user version] end context "for a 'User' class" do it_behaves_like "custom field formats", "User", - ["bool", "date", "float", "int", "list", "string", "text"] + %w[bool date float int list string text] end context "for a 'Group' class" do it_behaves_like "custom field formats", "Group", - ["bool", "date", "float", "int", "list", "string", "text"] + %w[bool date float int list string text] end end @@ -93,6 +101,15 @@ RSpec.describe OpenProject::CustomFieldFormat do .to contain_exactly("bool", "date", "float", "int", "link", "list", "string", "text", "user", "version", "hierarchy", "empty") end + + context "with calculated values feature flag enabled", with_flag: { calculated_value_project_attribute: true } do + it "returns all custom field formats including calculated values" do + formats = described_class.available_formats + expect(formats) + .to contain_exactly("bool", "calculated_value", "date", "float", "int", "link", "list", "string", "text", + "user", "version", "hierarchy", "empty") + end + end end context "without a custom_field_hierarchies ee" do @@ -102,6 +119,15 @@ RSpec.describe OpenProject::CustomFieldFormat do .to contain_exactly("bool", "date", "float", "int", "link", "list", "string", "text", "user", "version", "empty") end + + context "with calculated values feature flag enabled", with_flag: { calculated_value_project_attribute: true } do + it "returns all custom field formats including calculated values" do + formats = described_class.available_formats + expect(formats) + .to contain_exactly("bool", "calculated_value", "date", "float", "int", "link", "list", "string", "text", + "user", "version", "empty") + end + end end end end diff --git a/spec/support/pages/admin/custom_fields/custom_fields_projects/index.rb b/spec/support/pages/admin/custom_fields/custom_fields_projects/index.rb index 92e9e18b48e..08a26e93352 100644 --- a/spec/support/pages/admin/custom_fields/custom_fields_projects/index.rb +++ b/spec/support/pages/admin/custom_fields/custom_fields_projects/index.rb @@ -48,6 +48,26 @@ module Pages click_on type end + + def expect_having_create_item(type) + wait_for_network_idle + + click_button "Add" + + click_button "Project attribute" + + expect(page).to have_link(type) + end + + def expect_not_having_create_item(type) + wait_for_network_idle + + click_button "Add" + + click_button "Project attribute" + + expect(page).to have_no_link(type) + end end end end From f49ea9998262a5e16c1c2a07b10bd19608795760 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 17:20:06 +0200 Subject: [PATCH 22/44] [#64347] refactor hidden calculated values w/o feature flag --- .../show_component.rb | 5 +++++ .../settings/project_custom_fields_controller.rb | 6 ++++++ app/models/custom_field.rb | 8 -------- app/models/project_custom_field.rb | 16 +++++++++++----- .../base_contract_spec.rb | 6 ++++-- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/app/components/settings/project_custom_field_sections/show_component.rb b/app/components/settings/project_custom_field_sections/show_component.rb index d2e1ee86a50..1230e1e449f 100644 --- a/app/components/settings/project_custom_field_sections/show_component.rb +++ b/app/components/settings/project_custom_field_sections/show_component.rb @@ -38,6 +38,11 @@ module Settings @project_custom_field_section = project_custom_field_section @project_custom_fields = project_custom_field_section.custom_fields + + unless OpenProject::FeatureDecisions.calculated_value_project_attribute_active? + @project_custom_fields = @project_custom_fields.where.not(field_format: "calculated_value") + end + @first_and_last = first_and_last end diff --git a/app/controllers/admin/settings/project_custom_fields_controller.rb b/app/controllers/admin/settings/project_custom_fields_controller.rb index cf801f9ad2c..9699a1fed63 100644 --- a/app/controllers/admin/settings/project_custom_fields_controller.rb +++ b/app/controllers/admin/settings/project_custom_fields_controller.rb @@ -210,6 +210,8 @@ module Admin::Settings def find_custom_field @custom_field = ProjectCustomField.find(params[:id]) + + render_404 if @custom_field.field_format_calculated_value? && !calculated_value_feature_flag? end def drop_success_streams(call) @@ -222,5 +224,9 @@ module Admin::Settings def include_sub_projects? ActiveRecord::Type::Boolean.new.cast(params.to_unsafe_h[:project_custom_field_project_mapping][:include_sub_projects]) end + + def calculated_value_feature_flag? + OpenProject::FeatureDecisions.calculated_value_project_attribute_active? + end end end diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 8e3ab0d0f26..ef66d647cf1 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -52,14 +52,6 @@ class CustomField < ApplicationRecord scope :hierarchy_root_and_children, -> { includes(hierarchy_root: { children: :children }) } scope :required, -> { where(is_required: true) } - default_scope -> { - if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? - all - else - where.not(field_format: "calculated_value") - end - } - acts_as_list scope: [:type] validates :field_format, presence: true diff --git a/app/models/project_custom_field.rb b/app/models/project_custom_field.rb index df4c67428ff..5a21220fb53 100644 --- a/app/models/project_custom_field.rb +++ b/app/models/project_custom_field.rb @@ -40,12 +40,18 @@ class ProjectCustomField < CustomField class << self def visible(user = User.current, project: nil) - if user.admin? - all - elsif user.allowed_in_any_project?(:select_project_custom_fields) || user.allowed_globally?(:add_project) - where(admin_only: false) + cfs = if user.admin? + all + elsif user.allowed_in_any_project?(:select_project_custom_fields) || user.allowed_globally?(:add_project) + where(admin_only: false) + else + where(admin_only: false).where(mappings_with_view_project_attributes_permission(user, project).exists) + end + + if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? + cfs else - where(admin_only: false).where(mappings_with_view_project_attributes_permission(user, project).exists) + cfs.where.not(field_format: "calculated_value") end end diff --git a/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb b/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb index d50157db756..951288f98ea 100644 --- a/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb +++ b/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb @@ -49,10 +49,12 @@ RSpec.describe ProjectCustomFieldProjectMappings::BaseContract do end context "with non-visible custom field and admin user" do - let(:project_custom_field) { build_stubbed(:project_custom_field, admin_only: true) } + let(:project_custom_field) { create(:project_custom_field, admin_only: true) } before do - allow(ProjectCustomField).to receive(:all).and_return([project_custom_field]) + # Stub `all` to return an ActiveRecord::Relation, not an array, so that further query chaining + # (e.g., .where) works as expected. + allow(ProjectCustomField).to receive(:all).and_return(ProjectCustomField.where(id: project_custom_field.id)) end it_behaves_like "contract is valid" From 385cd588c41255d2a62ad91d74e63b6223950c6a Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 17:23:21 +0200 Subject: [PATCH 23/44] [#64347] use proper translation for field format --- .../custom_field_row_component.html.erb | 2 +- .../projects/project_custom_fields/settings/mapping_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb index 8bcc2cf52d0..608e951411d 100644 --- a/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -13,7 +13,7 @@ end title_container.with_column(pt: 1, mr: 2, data: { test_selector: "custom-field-type" }) do render(Primer::Beta::Text.new(font_size: :small, color: :subtle)) do - t(@project_custom_field.field_format) + t("label_#{@project_custom_field.field_format}") end end if @project_custom_field.required? diff --git a/spec/features/projects/project_custom_fields/settings/mapping_spec.rb b/spec/features/projects/project_custom_fields/settings/mapping_spec.rb index e30f1ccb76f..50bbd09f72a 100644 --- a/spec/features/projects/project_custom_fields/settings/mapping_spec.rb +++ b/spec/features/projects/project_custom_fields/settings/mapping_spec.rb @@ -142,7 +142,7 @@ RSpec.describe "Projects custom fields mapping via project settings", :js do end within_custom_field_container(string_project_custom_field) do expect(page).to have_content("String field") - expect_type("String") + expect_type("Text") expect_unchecked_state end end From 06ebbd09cc130649137c975338c920076121e5da Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 18:16:34 +0200 Subject: [PATCH 24/44] [#64347] fix unused code and wrong edit path --- .../custom_field_row_component.html.erb | 2 +- .../custom_field_row_component.rb | 6 +----- app/controllers/concerns/custom_fields/shared_actions.rb | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb index 58673930538..028629bf860 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -8,7 +8,7 @@ content_container.with_column(mr: 2) do render( Primer::Beta::Link.new( - href: edit_path, + href: edit_admin_settings_project_custom_field_path(@project_custom_field), underline: false, font_weight: :bold, data: { turbo: "false" } diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.rb b/app/components/settings/project_custom_field_sections/custom_field_row_component.rb index 4195f72e4ba..74cade38187 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.rb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.rb @@ -42,13 +42,9 @@ module Settings private - def edit_path - edit_admin_settings_project_custom_field_path(@project_custom_field) - end - def edit_action_item(menu) menu.with_item(label: t("label_edit"), - href: edit_path, + href: edit_admin_settings_project_custom_field_path(@project_custom_field), data: { turbo: "false", test_selector: "project-custom-field-edit" }) do |item| item.with_leading_visual_icon(icon: :pencil) end diff --git a/app/controllers/concerns/custom_fields/shared_actions.rb b/app/controllers/concerns/custom_fields/shared_actions.rb index cedc24046d9..d02ad148891 100644 --- a/app/controllers/concerns/custom_fields/shared_actions.rb +++ b/app/controllers/concerns/custom_fields/shared_actions.rb @@ -41,7 +41,7 @@ module CustomFields def edit_path(custom_field, params = {}) if custom_field.type == "ProjectCustomField" - admin_settings_project_custom_fields_path(**params) + admin_settings_project_custom_field_path(**params) else edit_custom_field_path(**params) end From 82c63443d7cea274bfe6c7012b2cc221c690e4fa Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 18:28:09 +0200 Subject: [PATCH 25/44] [#64347] only use hidden fields for new records --- .../admin/custom_fields/calculated_values/details_form.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/components/admin/custom_fields/calculated_values/details_form.rb b/app/components/admin/custom_fields/calculated_values/details_form.rb index 79abd41c760..b1a483daf9e 100644 --- a/app/components/admin/custom_fields/calculated_values/details_form.rb +++ b/app/components/admin/custom_fields/calculated_values/details_form.rb @@ -31,6 +31,11 @@ module Admin::CustomFields::CalculatedValues class DetailsForm < ApplicationForm form do |details_form| + if model.new_record? + details_form.hidden(name: :field_format) + details_form.hidden(name: :type, scope_name_to_model: false) + end + details_form.text_field( name: :name, label: I18n.t(:label_name), @@ -67,9 +72,6 @@ module Admin::CustomFields::CalculatedValues caption: I18n.t("custom_fields.instructions.admin_only") ) - details_form.hidden(name: :field_format) - details_form.hidden(name: :type, scope_name_to_model: false) - details_form.submit(name: :submit, label: I18n.t(:button_save), scheme: :default) end end From c4826a264614809c165c81596cd6a096a8cd1d59 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 24 Jun 2025 18:45:03 +0200 Subject: [PATCH 26/44] [#64347] fix custom field Primer bugs In the admin view, when you edit a custom fields name to be empty and try to save that, the display would incorrectly assume the empty title from the model, even if the model would be invalid with these changes. Updated to fetch the name attribute from the database instead. The bug only happens when you use a Primer component. So for now, this is only true for Hierarchy and Calculated Value CFs. Angular prevents this bug from happening since there is no page reload. --- .../admin/custom_fields/edit_form_header_component.html.erb | 2 +- .../admin/custom_fields/edit_form_header_component.rb | 2 +- .../project_custom_fields/edit_form_header_component.html.erb | 2 +- .../project_custom_fields/edit_form_header_component.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/components/admin/custom_fields/edit_form_header_component.html.erb b/app/components/admin/custom_fields/edit_form_header_component.html.erb index 3a87af24a09..c6398961ce8 100644 --- a/app/components/admin/custom_fields/edit_form_header_component.html.erb +++ b/app/components/admin/custom_fields/edit_form_header_component.html.erb @@ -29,7 +29,7 @@ See COPYRIGHT and LICENSE files for more details. <%= render(Primer::OpenProject::PageHeader.new(test_selector: "custom-fields--page-header")) do |header| - header.with_title { @custom_field.name } + header.with_title { @custom_field.attribute_in_database("name") || @custom_field.name } header.with_breadcrumbs(breadcrumbs_items) diff --git a/app/components/admin/custom_fields/edit_form_header_component.rb b/app/components/admin/custom_fields/edit_form_header_component.rb index cb60d1ff695..04eac57114e 100644 --- a/app/components/admin/custom_fields/edit_form_header_component.rb +++ b/app/components/admin/custom_fields/edit_form_header_component.rb @@ -71,7 +71,7 @@ module Admin [{ href: admin_index_path, text: t(:label_administration) }, { href: custom_fields_path, text: t(:label_custom_field_plural) }, { href: custom_fields_path(tab: @custom_field.type), text: I18n.t(@custom_field.type_name) }, - @custom_field.name] + @custom_field.attribute_in_database("name")] end end end diff --git a/app/components/settings/project_custom_fields/edit_form_header_component.html.erb b/app/components/settings/project_custom_fields/edit_form_header_component.html.erb index 22ff5ee0bf1..3c2cc5f0908 100644 --- a/app/components/settings/project_custom_fields/edit_form_header_component.html.erb +++ b/app/components/settings/project_custom_fields/edit_form_header_component.html.erb @@ -28,7 +28,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= render(Primer::OpenProject::PageHeader.new) do |header| - header.with_title { @custom_field.name } + header.with_title { @custom_field.attribute_in_database("name") || @custom_field.name } header.with_description { t("settings.project_attributes.edit.description") } header.with_breadcrumbs(breadcrumbs_items) diff --git a/app/components/settings/project_custom_fields/edit_form_header_component.rb b/app/components/settings/project_custom_fields/edit_form_header_component.rb index 652211fbef1..2bd01df5ef8 100644 --- a/app/components/settings/project_custom_fields/edit_form_header_component.rb +++ b/app/components/settings/project_custom_fields/edit_form_header_component.rb @@ -53,7 +53,7 @@ module Settings [{ href: admin_index_path, text: t("label_administration") }, { href: admin_settings_project_custom_fields_path, text: t("label_project_plural") }, { href: admin_settings_project_custom_fields_path, text: t("settings.project_attributes.heading") }, - @custom_field.name] + @custom_field.attribute_in_database("name")] end end end From 017e95b68d16303d8b08a74b1fc8033cb510187b Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Wed, 25 Jun 2025 09:44:55 +0200 Subject: [PATCH 27/44] [#64347] details form spec, better model validation --- app/models/custom_field/calculated_value.rb | 14 +- config/locales/en.yml | 1 + .../projects/calculated_value_spec.rb | 137 ++++++++++++++++++ .../custom_fields/projects/index_spec.rb | 1 + .../custom_field/calculated_value_spec.rb | 10 +- 5 files changed, 155 insertions(+), 8 deletions(-) create mode 100644 spec/features/admin/custom_fields/projects/calculated_value_spec.rb diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index 2174ab82543..39c32ff0d0d 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -33,10 +33,16 @@ module CustomField::CalculatedValue def validate_formula return unless field_format_calculated_value? - # An empty formula is okay. - return if formula_string.blank? - errors.add(:formula, :invalid) unless formula_contains_only_allowed_characters? + if formula_string.blank? + errors.add(:formula, :blank) + return + end + + unless formula_contains_only_allowed_characters? + errors.add(:formula, :invalid_characters) + return + end # WP-64348: check for valid (i.e., visible & enabled) custom field references (see #cf_ids_used_in_formula) @@ -45,7 +51,7 @@ module CustomField::CalculatedValue # e.g. Dentaku(formula_string, cf_123: CustomField.find(123).value) errors.add(:formula, :invalid) unless Dentaku(formula_string) - # TODO: consider differentiating between a formula that contains invalid characters, missing variables, invalid + # TODO: consider differentiating between a formula that contains missing variables, invalid # syntax, or mathematical errors. end diff --git a/config/locales/en.yml b/config/locales/en.yml index 69958298153..6e358c15a57 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1277,6 +1277,7 @@ en: inclusion: "is not set to one of the allowed values." inclusion_nested: "is not set to one of the allowed values at path '%{path}'." invalid: "is invalid." + invalid_characters: "contains invalid characters." invalid_url: "is not a valid URL." invalid_url_scheme: "is not a supported protocol (allowed: %{allowed_schemes})." less_than_or_equal_to: "must be less than or equal to %{count}." diff --git a/spec/features/admin/custom_fields/projects/calculated_value_spec.rb b/spec/features/admin/custom_fields/projects/calculated_value_spec.rb new file mode 100644 index 00000000000..bb56be33f19 --- /dev/null +++ b/spec/features/admin/custom_fields/projects/calculated_value_spec.rb @@ -0,0 +1,137 @@ +# 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" +require_relative "shared_context" + +RSpec.describe "Edit project custom field calculated value", :js, with_flag: { calculated_value_project_attribute: true } do + include_context "with seeded project custom fields" + + let!(:calculated_value) do + create(:calculated_value_project_custom_field, + name: "Calculated value field", + formula: "42 + 1", + project_custom_field_section: section_for_input_fields) + end + + context "with insufficient permissions" do + it "is not accessible" do + login_as(non_admin) + visit edit_admin_settings_project_custom_field_path(calculated_value) + + expect(page).to have_text("You are not authorized to access this page.") + end + end + + context "with sufficient permissions" do + before do + login_as(admin) + visit edit_admin_settings_project_custom_field_path(calculated_value) + end + + it "shows a correct breadcrumb menu" do + within ".PageHeader-breadcrumbs" do + expect(page).to have_link("Administration") + expect(page).to have_link("Projects") + expect(page).to have_link("Project attributes") + expect(page).to have_text(calculated_value.name) + end + end + + it "shows tab navigation" do + within_test_selector("project_attribute_detail_header") do + expect(page).to have_link("Details") + expect(page).to have_link("Projects") + end + end + + it "allows to change basic attributes and the section of the calculated value" do + expect(page).to have_css(".PageHeader-title", text: calculated_value.name) + + fill_in("custom_field_name", with: "Updated name", fill_options: { clear: :backspace }) + select(section_for_select_fields.name, from: "custom_field_custom_field_section_id") + fill_in("custom_field_formula", with: "1 + 1", fill_options: { clear: :backspace }) + + click_on "Save" + + expect(page).to have_text("Successful update") + + expect(page).to have_css(".PageHeader-title", text: "Updated name") + + expect(calculated_value.reload.name).to eq("Updated name") + expect(calculated_value.reload.project_custom_field_section).to eq(section_for_select_fields) + expect(calculated_value.reload.formula_string).to eq("1 + 1") + + within ".PageHeader-breadcrumbs" do + expect(page).to have_link("Administration") + expect(page).to have_link("Projects") + expect(page).to have_link("Project attributes") + expect(page).to have_text("Updated name") + end + end + + it "prevents saving a calculated value with an empty name" do + original_name = calculated_value.name + + fill_in("custom_field_name", with: "") + click_on "Save" + + expect(page).to have_text("Name can't be blank") + + expect(page).to have_no_text("Successful update") + + expect(page).to have_css(".PageHeader-title", text: original_name) + expect(calculated_value.reload.name).to eq(original_name) + end + + it "prevents saving a calculated value with an empty formula" do + original_formula = calculated_value.formula_string + + fill_in("custom_field_formula", with: "") + click_on "Save" + + expect(page).to have_text("Formula can't be blank") + expect(page).to have_no_text("Successful update") + + expect(calculated_value.reload.formula_string).to eq(original_formula) + end + end + + context "without the feature flag" do + it "renders a 404" do + allow(OpenProject::FeatureDecisions).to receive(:calculated_value_project_attribute_active?).and_return(false) + + login_as(admin) + visit edit_admin_settings_project_custom_field_path(calculated_value) + + expect(page).to have_http_status(:not_found) + end + end +end diff --git a/spec/features/admin/custom_fields/projects/index_spec.rb b/spec/features/admin/custom_fields/projects/index_spec.rb index f6c4eec10b7..6891df6bf9d 100644 --- a/spec/features/admin/custom_fields/projects/index_spec.rb +++ b/spec/features/admin/custom_fields/projects/index_spec.rb @@ -138,6 +138,7 @@ RSpec.describe "List project custom fields", :js do let!(:calculated_value_project_custom_field) do create(:calculated_value_project_custom_field, name: "Calculated value field", + formula: "42 + 1", project_custom_field_section: section_for_input_fields) end diff --git a/spec/models/custom_field/calculated_value_spec.rb b/spec/models/custom_field/calculated_value_spec.rb index 1c645a5a745..afdab20d6bb 100644 --- a/spec/models/custom_field/calculated_value_spec.rb +++ b/spec/models/custom_field/calculated_value_spec.rb @@ -32,7 +32,7 @@ require "spec_helper" RSpec.describe CustomField::CalculatedValue, with_flag: { calculated_value_project_attribute: true } do context "when calculated value" do - subject(:custom_field) { create(:calculated_value_project_custom_field) } + subject(:custom_field) { create(:calculated_value_project_custom_field, formula: "1 + 1") } describe "#formula=" do it "splits formula and referenced custom fields on persist if given a string" do @@ -52,6 +52,7 @@ RSpec.describe CustomField::CalculatedValue, with_flag: { calculated_value_proje describe "#formula_string" do it "returns an empty string if no formula is set" do + subject.formula = nil expect(subject.formula_string).to eq("") end @@ -72,8 +73,9 @@ RSpec.describe CustomField::CalculatedValue, with_flag: { calculated_value_proje end context "with an empty formula" do - it "is valid" do - expect(subject).to be_valid + it "is invalid" do + expect(subject).not_to be_valid + expect(subject.errors[:formula]).to include("can't be blank.") end end @@ -90,7 +92,7 @@ RSpec.describe CustomField::CalculatedValue, with_flag: { calculated_value_proje it "is invalid" do expect(subject).not_to be_valid - expect(subject.errors[:formula]).to include("is invalid.") + expect(subject.errors[:formula]).to include("contains invalid characters.") end end From 49baedfce7ddfb4e81fd4708607bffe62e1afb72 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Wed, 25 Jun 2025 09:52:07 +0200 Subject: [PATCH 28/44] [#64347] copyright --- .../details_component.html.erb | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/app/components/admin/custom_fields/calculated_values/details_component.html.erb b/app/components/admin/custom_fields/calculated_values/details_component.html.erb index 848b0f996e4..3f0b557de44 100644 --- a/app/components/admin/custom_fields/calculated_values/details_component.html.erb +++ b/app/components/admin/custom_fields/calculated_values/details_component.html.erb @@ -1,3 +1,32 @@ +<%#-- 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. + +++#%> + <%= component_wrapper do flex_layout do |content| From 8cb4f16034b479e7a01b39f9ad25bb3bbeefa008 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Fri, 27 Jun 2025 10:07:11 +0200 Subject: [PATCH 29/44] [#64347] use existing helper for cf label --- .../custom_field_row_component.html.erb | 2 +- .../custom_field_row_component.html.erb | 2 +- config/locales/en.yml | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb index 608e951411d..df223298392 100644 --- a/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/projects/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -13,7 +13,7 @@ end title_container.with_column(pt: 1, mr: 2, data: { test_selector: "custom-field-type" }) do render(Primer::Beta::Text.new(font_size: :small, color: :subtle)) do - t("label_#{@project_custom_field.field_format}") + helpers.label_for_custom_field_format(@project_custom_field.field_format) end end if @project_custom_field.required? diff --git a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb index 028629bf860..6a9fb982a3b 100644 --- a/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb +++ b/app/components/settings/project_custom_field_sections/custom_field_row_component.html.erb @@ -19,7 +19,7 @@ end content_container.with_column(mr: 2) do render(Primer::Beta::Text.new(font_size: :small, color: :subtle)) do - t("label_#{@project_custom_field.field_format}") + helpers.label_for_custom_field_format(@project_custom_field.field_format) end end content_container.with_column(mr: 2) do diff --git a/config/locales/en.yml b/config/locales/en.yml index 6e358c15a57..a3708ab155e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2708,7 +2708,6 @@ en: label_forum_new: "New forum" label_forum_plural: "Forums" label_forum_sticky: "Sticky" - label_bool: "Boolean" label_boolean: "Boolean" label_board_plural: "Boards" label_branch: "Branch" @@ -2897,7 +2896,6 @@ en: label_information: "Information" label_information_plural: "Information" label_installation_guides: "Installation guides" - label_int: "Integer" label_integer: "Integer" label_interface: "Interface" label_internal: "Internal" From fb2e9f0c1d8fc2036fd3d1477d180e294d3cda2e Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 10:32:04 +0200 Subject: [PATCH 30/44] [#64347] remove unnecessary default argument --- app/models/custom_field/calculated_value.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index 39c32ff0d0d..af17cca718a 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -90,7 +90,7 @@ module CustomField::CalculatedValue # Returns a list of custom field IDs used in the formula. # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. - def cf_ids_used_in_formula(formula_str = formula_string) + def cf_ids_used_in_formula(formula_str) formula_str.scan(/cf_(\d+)/).flatten.map(&:to_i) end end From bf1af534745feabe6ae4ba56bf0811c65f278f0f Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 10:47:05 +0200 Subject: [PATCH 31/44] [#64347] is_for_all -> is_required --- .../admin/custom_fields/calculated_values/details_form.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/admin/custom_fields/calculated_values/details_form.rb b/app/components/admin/custom_fields/calculated_values/details_form.rb index b1a483daf9e..7d48d0e05e4 100644 --- a/app/components/admin/custom_fields/calculated_values/details_form.rb +++ b/app/components/admin/custom_fields/calculated_values/details_form.rb @@ -61,9 +61,9 @@ module Admin::CustomFields::CalculatedValues ) details_form.check_box( - name: :is_for_all, - label: I18n.t("activerecord.attributes.custom_field.is_for_all"), - caption: I18n.t("custom_fields.instructions.is_for_all") + name: :is_required, + label: I18n.t("activerecord.attributes.project_custom_field.is_required"), + caption: I18n.t("custom_fields.instructions.is_required_for_project") ) details_form.check_box( From 0954f5f0466b12b57a179ebaec47d4ea99183e1f Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 10:57:11 +0200 Subject: [PATCH 32/44] [#64347] use shared examples for #available_formats --- .../open_project/custom_field_format_spec.rb | 41 +++++++------------ 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/spec/lib/open_project/custom_field_format_spec.rb b/spec/lib/open_project/custom_field_format_spec.rb index 928d5c12ff3..de9eb9ed06f 100644 --- a/spec/lib/open_project/custom_field_format_spec.rb +++ b/spec/lib/open_project/custom_field_format_spec.rb @@ -94,39 +94,28 @@ RSpec.describe OpenProject::CustomFieldFormat do end describe ".available_formats" do - context "with a custom_field_hierarchies ee", with_ee: [:custom_field_hierarchies] do - it "returns all custom field formats including hierarchy" do + shared_examples_for "available custom field formats" do |suffix, expected_formats| + it "returns all custom field formats #{suffix}", :aggregate_failures do formats = described_class.available_formats - expect(formats) - .to contain_exactly("bool", "date", "float", "int", "link", "list", "string", "text", "user", - "version", "hierarchy", "empty") - end - - context "with calculated values feature flag enabled", with_flag: { calculated_value_project_attribute: true } do - it "returns all custom field formats including calculated values" do - formats = described_class.available_formats - expect(formats) - .to contain_exactly("bool", "calculated_value", "date", "float", "int", "link", "list", "string", "text", - "user", "version", "hierarchy", "empty") - end + expect(formats).to match_array(expected_formats) end end + context "with a custom_field_hierarchies ee", with_ee: [:custom_field_hierarchies] do + it_behaves_like "available custom field formats", + "including hierarchy", + %w[bool date float int link list string text user version hierarchy empty] + end + context "without a custom_field_hierarchies ee" do - it "returns all custom field formats excluding hierarchy" do - formats = described_class.available_formats - expect(formats) - .to contain_exactly("bool", "date", "float", "int", "link", "list", "string", "text", "user", - "version", "empty") - end + it_behaves_like "available custom field formats", + "excluding hierarchy", + %w[bool date float int link list string text user version empty] context "with calculated values feature flag enabled", with_flag: { calculated_value_project_attribute: true } do - it "returns all custom field formats including calculated values" do - formats = described_class.available_formats - expect(formats) - .to contain_exactly("bool", "calculated_value", "date", "float", "int", "link", "list", "string", "text", - "user", "version", "empty") - end + it_behaves_like "available custom field formats", + "including calculated values", + %w[bool calculated_value date float int link list string text user version empty] end end end From a2daa82dc791f1e227e90c9937a0d2019ae3c2c5 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 11:05:20 +0200 Subject: [PATCH 33/44] [#64347] remove unnecessary stub --- .../base_contract_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb b/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb index 951288f98ea..0c5e75e950a 100644 --- a/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb +++ b/spec/contracts/project_custom_field_project_mappings/base_contract_spec.rb @@ -51,12 +51,6 @@ RSpec.describe ProjectCustomFieldProjectMappings::BaseContract do context "with non-visible custom field and admin user" do let(:project_custom_field) { create(:project_custom_field, admin_only: true) } - before do - # Stub `all` to return an ActiveRecord::Relation, not an array, so that further query chaining - # (e.g., .where) works as expected. - allow(ProjectCustomField).to receive(:all).and_return(ProjectCustomField.where(id: project_custom_field.id)) - end - it_behaves_like "contract is valid" end From 7d37ac98cfcdfea00fac05a5597816f8f2b9d7b0 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 11:16:12 +0200 Subject: [PATCH 34/44] [#64347] remove formula field from regular custom fields --- app/views/custom_fields/_form.html.erb | 8 -------- .../controllers/dynamic/admin/custom-fields.controller.ts | 2 -- 2 files changed, 10 deletions(-) diff --git a/app/views/custom_fields/_form.html.erb b/app/views/custom_fields/_form.html.erb index 9ae12e18fdb..192fc485768 100644 --- a/app/views/custom_fields/_form.html.erb +++ b/app/views/custom_fields/_form.html.erb @@ -76,14 +76,6 @@ See COPYRIGHT and LICENSE files for more details.
-
> - <%= f.text_field :formula, - container_class: "-wide" %> - - <%= t("custom_fields.instructions.formula") %> - -
- <% if @custom_field.new_record? || @custom_field.multi_value_possible? %>
> <%= f.check_box :multi_value, diff --git a/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts b/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts index c7e5710789e..4ae929c6518 100644 --- a/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/admin/custom-fields.controller.ts @@ -47,7 +47,6 @@ export default class CustomFieldsController extends Controller { 'multiSelect', 'possibleValues', 'regexp', - 'formula', 'searchable', 'textOrientation', @@ -81,7 +80,6 @@ export default class CustomFieldsController extends Controller { declare readonly multiSelectTargets:HTMLElement[]; declare readonly possibleValuesTargets:HTMLElement[]; declare readonly regexpTargets:HTMLElement[]; - declare readonly formulaTargets:HTMLElement[]; declare readonly searchableTargets:HTMLInputElement[]; declare readonly textOrientationTargets:HTMLElement[]; From 4f7f370204eac139ff9afbb75c18f9dab6a778e0 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 11:21:09 +0200 Subject: [PATCH 35/44] [#64347] all.collect.each -> find_each --- .../admin/custom_fields/calculated_values/details_form.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/admin/custom_fields/calculated_values/details_form.rb b/app/components/admin/custom_fields/calculated_values/details_form.rb index 7d48d0e05e4..5b3da9c9c5f 100644 --- a/app/components/admin/custom_fields/calculated_values/details_form.rb +++ b/app/components/admin/custom_fields/calculated_values/details_form.rb @@ -47,7 +47,7 @@ module Admin::CustomFields::CalculatedValues label: I18n.t("activerecord.attributes.project_custom_field.custom_field_section"), required: true ) do |li| - ProjectCustomFieldSection.all.collect.each do |cs| + ProjectCustomFieldSection.find_each do |cs| li.option(value: cs.id, label: cs.name) end end From 30b946d9d613ea6280b62bb1f4e89051b0a5b0cb Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 11:26:43 +0200 Subject: [PATCH 36/44] [#64347] name in database cannot be falsy --- .../admin/custom_fields/edit_form_header_component.html.erb | 2 +- .../project_custom_fields/edit_form_header_component.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/admin/custom_fields/edit_form_header_component.html.erb b/app/components/admin/custom_fields/edit_form_header_component.html.erb index c6398961ce8..66808da18b3 100644 --- a/app/components/admin/custom_fields/edit_form_header_component.html.erb +++ b/app/components/admin/custom_fields/edit_form_header_component.html.erb @@ -29,7 +29,7 @@ See COPYRIGHT and LICENSE files for more details. <%= render(Primer::OpenProject::PageHeader.new(test_selector: "custom-fields--page-header")) do |header| - header.with_title { @custom_field.attribute_in_database("name") || @custom_field.name } + header.with_title { @custom_field.attribute_in_database("name") } header.with_breadcrumbs(breadcrumbs_items) diff --git a/app/components/settings/project_custom_fields/edit_form_header_component.html.erb b/app/components/settings/project_custom_fields/edit_form_header_component.html.erb index 3c2cc5f0908..fc0ff62837f 100644 --- a/app/components/settings/project_custom_fields/edit_form_header_component.html.erb +++ b/app/components/settings/project_custom_fields/edit_form_header_component.html.erb @@ -28,7 +28,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> <%= render(Primer::OpenProject::PageHeader.new) do |header| - header.with_title { @custom_field.attribute_in_database("name") || @custom_field.name } + header.with_title { @custom_field.attribute_in_database("name") } header.with_description { t("settings.project_attributes.edit.description") } header.with_breadcrumbs(breadcrumbs_items) From ebddccc2f75a51d07104ad610ad2216e93c231e1 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 13:27:54 +0200 Subject: [PATCH 37/44] [#64347] use Regexp.union --- app/models/custom_field/calculated_value.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index af17cca718a..bf6826cf3ce 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -80,10 +80,9 @@ module CustomField::CalculatedValue # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that # it prevents built-in functions from being used in the formula, which we do not want to allow. allowed_chars = %w[+ - / * ( )] + [" "] - split_pattern = /\s|(\+)|(-)|(\/)|(\*)|(\()|(\))/ allowed_tokens = /^(cf_\d+|\d+\.?\d*|\.\d+|[#{allowed_chars.join}]+)$/ - formula_string.split(split_pattern).reject(&:empty?).all? do |token| + formula_string.split(Regexp.union(allowed_chars)).reject(&:empty?).all? do |token| token.match?(allowed_tokens) end end From 2e203f0e3b042f950a677ce940be68d7c2d3965b Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 14:25:27 +0200 Subject: [PATCH 38/44] [#64347] simplify formula validation --- app/models/custom_field/calculated_value.rb | 4 +- .../custom_field/calculated_value_spec.rb | 130 ++++++++++-------- 2 files changed, 72 insertions(+), 62 deletions(-) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index bf6826cf3ce..220dbb438d6 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -65,7 +65,7 @@ module CustomField::CalculatedValue # Returns the formula as a string. Will return an empty string if the formula is not set. def formula_string - formula.is_a?(Hash) ? formula.fetch("formula", "") : formula.to_s + formula ? formula.fetch("formula", "") : "" end private @@ -80,7 +80,7 @@ module CustomField::CalculatedValue # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that # it prevents built-in functions from being used in the formula, which we do not want to allow. allowed_chars = %w[+ - / * ( )] + [" "] - allowed_tokens = /^(cf_\d+|\d+\.?\d*|\.\d+|[#{allowed_chars.join}]+)$/ + allowed_tokens = /\A(cf_\d+|\d+\.?\d*|\.\d+)\z/ formula_string.split(Regexp.union(allowed_chars)).reject(&:empty?).all? do |token| token.match?(allowed_tokens) diff --git a/spec/models/custom_field/calculated_value_spec.rb b/spec/models/custom_field/calculated_value_spec.rb index afdab20d6bb..859dd9aed38 100644 --- a/spec/models/custom_field/calculated_value_spec.rb +++ b/spec/models/custom_field/calculated_value_spec.rb @@ -31,88 +31,98 @@ require "spec_helper" RSpec.describe CustomField::CalculatedValue, with_flag: { calculated_value_project_attribute: true } do - context "when calculated value" do - subject(:custom_field) { create(:calculated_value_project_custom_field, formula: "1 + 1") } + subject(:custom_field) { create(:calculated_value_project_custom_field, formula: "1 + 1") } - describe "#formula=" do - it "splits formula and referenced custom fields on persist if given a string" do - formula = "1 * cf_7 + cf_42" - subject.formula = formula + describe "#formula=" do + it "splits formula and referenced custom fields on persist if given a string" do + formula = "1 * cf_7 + cf_42" + subject.formula = formula - expect(subject.formula).to eq({ "formula" => formula, "referenced_custom_fields" => [7, 42] }) - end - - it "omits referenced custom fields if none are given" do - formula = "2 + 3 * (8 / 7)" - subject.formula = formula - - expect(subject.formula).to eq({ "formula" => formula, "referenced_custom_fields" => [] }) - end + expect(subject.formula).to eq({ "formula" => formula, "referenced_custom_fields" => [7, 42] }) end - describe "#formula_string" do - it "returns an empty string if no formula is set" do - subject.formula = nil - expect(subject.formula_string).to eq("") - end + it "omits referenced custom fields if none are given" do + formula = "2 + 3 * (8 / 7)" + subject.formula = formula - it "returns the formula as a string" do - formula = "1 * cf_7 + cf_42" - subject.formula = formula + expect(subject.formula).to eq({ "formula" => formula, "referenced_custom_fields" => [] }) + end + end - expect(subject.formula_string).to eq(formula) - end + describe "#formula_string" do + it "returns an empty string if no formula is set" do + subject.formula = nil + expect(subject.formula_string).to eq("") end - describe "#validate_formula" do - let(:formula) { "" } + it "returns the formula as a string" do + formula = "1 * cf_7 + cf_42" + subject.formula = formula - before do + expect(subject.formula_string).to eq(formula) + end + end + + describe "#validate_formula" do + shared_examples_for "valid formula" do + it "is valid", :aggregate_failures do subject.formula = formula subject.validate_formula + + expect(subject).to be_valid end + end - context "with an empty formula" do - it "is invalid" do - expect(subject).not_to be_valid - expect(subject.errors[:formula]).to include("can't be blank.") - end + shared_examples_for "invalid formula" do |error_message| + it "is invalid", :aggregate_failures do + subject.formula = formula + subject.validate_formula + + expect(subject).not_to be_valid + expect(subject.errors[:formula]).to include(error_message) end + end - context "with a formula containing only allowed characters" do - let(:formula) { "1 / 2 + (3 * 4.5) - 0.0" } + let(:formula) { "" } - it "is valid" do - expect(subject).to be_valid - end - end + context "with an empty formula" do + it_behaves_like "invalid formula", "can't be blank." + end - context "with a formula containing forbidden characters" do - let(:formula) { "abc + 2" } + context "with a formula containing only allowed characters" do + let(:formula) { "1 / 2 + (3 * 4.5) - 0.0" } - it "is invalid" do - expect(subject).not_to be_valid - expect(subject.errors[:formula]).to include("contains invalid characters.") - end - end + it_behaves_like "valid formula" + end - context "with a formula that is not a valid equation" do - let(:formula) { "1 / + - 3" } + context "when omitting leading decimals before a decimal point" do + let(:formula) { "1.5 + .0 - 3.25" } - it "is invalid" do - expect(subject).not_to be_valid - expect(subject.errors[:formula]).to include("is invalid.") - end - end + it_behaves_like "valid formula" + end - context "with a formula that causes a division by zero error" do - let(:formula) { "1 / 0" } + context "when omitting trailing decimals after a decimal point" do + let(:formula) { "1.5 + 1. - 3.25" } - it "is invalid" do - expect(subject).not_to be_valid - expect(subject.errors[:formula]).to include("is invalid.") - end - end + it_behaves_like "invalid formula", "is invalid." + end + + context "with a formula containing forbidden characters" do + let(:formula) { "abc + 2" } + + it_behaves_like "invalid formula", "contains invalid characters." + end + + context "with a formula that is not a valid equation" do + let(:formula) { "1 / + - 3" } + + it_behaves_like "invalid formula", "is invalid." + end + + context "with a formula that causes a division by zero error" do + let(:formula) { "1 / 0" } + + it_behaves_like "invalid formula", "is invalid." end end end From 2245e1c2e10ccaa51b3d93464671cb0e240a8f1e Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 14:27:53 +0200 Subject: [PATCH 39/44] [#64347] use word boundaries in custom field regex --- app/models/custom_field/calculated_value.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index 220dbb438d6..f9995e52e25 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -90,6 +90,6 @@ module CustomField::CalculatedValue # Returns a list of custom field IDs used in the formula. # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. def cf_ids_used_in_formula(formula_str) - formula_str.scan(/cf_(\d+)/).flatten.map(&:to_i) + formula_str.scan(/\bcf_(\d+)\b/).flatten.map(&:to_i) end end From e30f7a958824dcc599f6fba787128396d289130c Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 14:30:24 +0200 Subject: [PATCH 40/44] [#64347] use Dentaku instance instead of shortcut --- app/models/custom_field/calculated_value.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index f9995e52e25..64f2d4753d1 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -48,8 +48,8 @@ module CustomField::CalculatedValue # Dentaku will return nil if the formula is invalid. # TODO WP-64348: add support for referenced custom fields by injecting them as variables, - # e.g. Dentaku(formula_string, cf_123: CustomField.find(123).value) - errors.add(:formula, :invalid) unless Dentaku(formula_string) + # e.g. calculator.evaluate(formula_string, cf_123: CustomField.find(123).value) + errors.add(:formula, :invalid) unless Dentaku::Calculator.new.evaluate(formula_string) # TODO: consider differentiating between a formula that contains missing variables, invalid # syntax, or mathematical errors. From f0c119ca85872e093fc9351c32083220ff6ea1b1 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 15:12:36 +0200 Subject: [PATCH 41/44] [#64347] Allow viewing/editing calculated values without FF We still show these CFs, but we will not allow creating new ones. This explictly only applies to enabled/disabled custom fields. We are stricter with the enterprise flag. --- .../details_component.html.erb | 4 ++++ .../show_component.rb | 4 ---- .../project_custom_fields_controller.rb | 6 ----- app/models/custom_field.rb | 14 +++++++++-- app/models/project_custom_field.rb | 16 ++++--------- config/initializers/custom_field_format.rb | 5 +++- lib/open_project/custom_field_format.rb | 24 ++++++++++++------- .../projects/calculated_value_spec.rb | 18 +++++++++----- .../custom_fields/projects/index_spec.rb | 8 +++---- .../open_project/custom_field_format_spec.rb | 14 +++++++++++ 10 files changed, 71 insertions(+), 42 deletions(-) diff --git a/app/components/admin/custom_fields/calculated_values/details_component.html.erb b/app/components/admin/custom_fields/calculated_values/details_component.html.erb index 3f0b557de44..6bdceda3369 100644 --- a/app/components/admin/custom_fields/calculated_values/details_component.html.erb +++ b/app/components/admin/custom_fields/calculated_values/details_component.html.erb @@ -30,6 +30,10 @@ See COPYRIGHT and LICENSE files for more details. <%= component_wrapper do flex_layout do |content| + content.with_row do + render BaseErrorsComponent.new(model, keys: %w[field_format], mb: 3) + end + content.with_row do settings_primer_form_with( model:, diff --git a/app/components/settings/project_custom_field_sections/show_component.rb b/app/components/settings/project_custom_field_sections/show_component.rb index 1230e1e449f..7f47572b426 100644 --- a/app/components/settings/project_custom_field_sections/show_component.rb +++ b/app/components/settings/project_custom_field_sections/show_component.rb @@ -39,10 +39,6 @@ module Settings @project_custom_field_section = project_custom_field_section @project_custom_fields = project_custom_field_section.custom_fields - unless OpenProject::FeatureDecisions.calculated_value_project_attribute_active? - @project_custom_fields = @project_custom_fields.where.not(field_format: "calculated_value") - end - @first_and_last = first_and_last end diff --git a/app/controllers/admin/settings/project_custom_fields_controller.rb b/app/controllers/admin/settings/project_custom_fields_controller.rb index 9699a1fed63..cf801f9ad2c 100644 --- a/app/controllers/admin/settings/project_custom_fields_controller.rb +++ b/app/controllers/admin/settings/project_custom_fields_controller.rb @@ -210,8 +210,6 @@ module Admin::Settings def find_custom_field @custom_field = ProjectCustomField.find(params[:id]) - - render_404 if @custom_field.field_format_calculated_value? && !calculated_value_feature_flag? end def drop_success_streams(call) @@ -224,9 +222,5 @@ module Admin::Settings def include_sub_projects? ActiveRecord::Type::Boolean.new.cast(params.to_unsafe_h[:project_custom_field_project_mapping][:include_sub_projects]) end - - def calculated_value_feature_flag? - OpenProject::FeatureDecisions.calculated_value_project_attribute_active? - end end end diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index ef66d647cf1..ce71762809a 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -70,8 +70,7 @@ class CustomField < ApplicationRecord errors.add(:name, :taken) if name.in?(taken_names) end - validates :field_format, inclusion: { in: -> { OpenProject::CustomFieldFormat.available_formats } } - + validate :validate_field_format_inclusion validate :validate_default_value validate :validate_regex validate :validate_formula @@ -108,6 +107,17 @@ class CustomField < ApplicationRecord end end + def validate_field_format_inclusion + available = OpenProject::CustomFieldFormat.available_formats + # When creating a new custom field, only the available formats are allowed. + # But you can edit and update existing custom fields, even if they have a field format that is disabled. + allowed = new_record? ? available : (available + OpenProject::CustomFieldFormat.disabled_formats).uniq + + unless allowed.include?(field_format) + errors.add(:field_format, :inclusion) + end + end + def validate_default_value # It is not possible to determine the validity of a value, when there is no valid format. # another validation will take care of adding an error, but here we need to abort. diff --git a/app/models/project_custom_field.rb b/app/models/project_custom_field.rb index 5a21220fb53..df4c67428ff 100644 --- a/app/models/project_custom_field.rb +++ b/app/models/project_custom_field.rb @@ -40,18 +40,12 @@ class ProjectCustomField < CustomField class << self def visible(user = User.current, project: nil) - cfs = if user.admin? - all - elsif user.allowed_in_any_project?(:select_project_custom_fields) || user.allowed_globally?(:add_project) - where(admin_only: false) - else - where(admin_only: false).where(mappings_with_view_project_attributes_permission(user, project).exists) - end - - if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? - cfs + if user.admin? + all + elsif user.allowed_in_any_project?(:select_project_custom_fields) || user.allowed_globally?(:add_project) + where(admin_only: false) else - cfs.where.not(field_format: "calculated_value") + where(admin_only: false).where(mappings_with_view_project_attributes_permission(user, project).exists) end end diff --git a/config/initializers/custom_field_format.rb b/config/initializers/custom_field_format.rb index 71971d18c13..329b8c06b43 100644 --- a/config/initializers/custom_field_format.rb +++ b/config/initializers/custom_field_format.rb @@ -94,5 +94,8 @@ OpenProject::CustomFieldFormat.map do |fields| fields.register OpenProject::CustomFieldFormat.new("calculated_value", label: :label_calculated_value, only: %w(Project), - order: 13) + order: 13, + enabled: lambda do + OpenProject::FeatureDecisions.calculated_value_project_attribute_active? + end) end diff --git a/lib/open_project/custom_field_format.rb b/lib/open_project/custom_field_format.rb index 0ce379e26ff..a416dc33103 100644 --- a/lib/open_project/custom_field_format.rb +++ b/lib/open_project/custom_field_format.rb @@ -43,6 +43,7 @@ module OpenProject only: nil, multi_value_possible: false, enterprise_feature: nil, + enabled: lambda { true }, formatter: "CustomValue::StringStrategy") @name = name @label = label @@ -51,6 +52,7 @@ module OpenProject @class_names = only @multi_value_possible = multi_value_possible @enterprise_feature = enterprise_feature + @enabled = enabled @formatter = formatter end @@ -63,6 +65,14 @@ module OpenProject Kernel.const_get(@formatter) end + def enabled? + @enabled.call + end + + def disabled? + !enabled? + end + class << self def map(&) yield self @@ -74,14 +84,8 @@ module OpenProject end def available - formats = registered.select do |_, format| - !format.enterprise_feature || EnterpriseToken.allows_to?(format.enterprise_feature) - end - - if OpenProject::FeatureDecisions.calculated_value_project_attribute_active? - formats - else - formats.except("calculated_value") + registered.select do |_, format| + format.enabled? && (!format.enterprise_feature || EnterpriseToken.allows_to?(format.enterprise_feature)) end end @@ -100,6 +104,10 @@ module OpenProject .sort_by(&:order) .reject { |format| format.label.nil? } end + + def disabled_formats + registered.select { |_, format| format.disabled? }.keys + end end end end diff --git a/spec/features/admin/custom_fields/projects/calculated_value_spec.rb b/spec/features/admin/custom_fields/projects/calculated_value_spec.rb index bb56be33f19..bbf28fd1a91 100644 --- a/spec/features/admin/custom_fields/projects/calculated_value_spec.rb +++ b/spec/features/admin/custom_fields/projects/calculated_value_spec.rb @@ -124,14 +124,20 @@ RSpec.describe "Edit project custom field calculated value", :js, with_flag: { c end end - context "without the feature flag" do - it "renders a 404" do - allow(OpenProject::FeatureDecisions).to receive(:calculated_value_project_attribute_active?).and_return(false) + context "without the feature flag", with_flag: { calculated_value_project_attribute: false } do + let!(:calculated_value) { nil } - login_as(admin) - visit edit_admin_settings_project_custom_field_path(calculated_value) + it "prevents saving a calculated value" do + expect do + login_as(admin) + visit new_admin_settings_project_custom_field_path(field_format: "calculated_value", + custom_field_section_id: section_for_input_fields.id) + fill_in("custom_field_name", with: "New calculated value") + fill_in("custom_field_formula", with: "1 + 1") + click_on "Save" - expect(page).to have_http_status(:not_found) + expect(page).to have_text("Format is not set to one of the allowed values.") + end.not_to change(CustomField, :count) end end end diff --git a/spec/features/admin/custom_fields/projects/index_spec.rb b/spec/features/admin/custom_fields/projects/index_spec.rb index 6891df6bf9d..4d71c7c66e2 100644 --- a/spec/features/admin/custom_fields/projects/index_spec.rb +++ b/spec/features/admin/custom_fields/projects/index_spec.rb @@ -155,19 +155,19 @@ RSpec.describe "List project custom fields", :js do end end - it "does not list calculated values if the feature flag is deactivated later" do - # This spec tests that calculated values are no longer shown after the feature flag is deactivated. + it "lists calculated values even if the feature flag is deactivated later" do + # This spec tests that calculated values are still shown after the feature flag is deactivated. # First, a custom field of type calculated value is created. This must be done while the feature flag is active, # or else the model validation will fail. # Next, we simulate that the feature flag is off: allow(OpenProject::FeatureDecisions).to receive(:calculated_value_project_attribute_active?).and_return(false) - # Revisit the page and check that the field is no longer listed: + # Revisit the page and check that the field is still listed: cf_index_page.visit! within_project_custom_field_section_container(section_for_input_fields) do containers = page.all(".op-project-custom-field-container") - expect(containers.last.text).not_to include(calculated_value_project_custom_field.name) + expect(containers.last.text).to include(calculated_value_project_custom_field.name) end end end diff --git a/spec/lib/open_project/custom_field_format_spec.rb b/spec/lib/open_project/custom_field_format_spec.rb index de9eb9ed06f..7bc6da0066e 100644 --- a/spec/lib/open_project/custom_field_format_spec.rb +++ b/spec/lib/open_project/custom_field_format_spec.rb @@ -119,4 +119,18 @@ RSpec.describe OpenProject::CustomFieldFormat do end end end + + describe ".disabled_formats" do + it "returns disabled formats" do + formats = described_class.disabled_formats + expect(formats).to match_array(%w[calculated_value]) + end + + context "with calculated values feature flag enabled", with_flag: { calculated_value_project_attribute: true } do + it "returns no disabled formats" do + formats = described_class.disabled_formats + expect(formats).to be_empty + end + end + end end From fe3dba09de8ec19decaabab01ff76148012baca4 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 16:08:36 +0200 Subject: [PATCH 42/44] [#64347] button: primary --- .../admin/custom_fields/calculated_values/details_form.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/admin/custom_fields/calculated_values/details_form.rb b/app/components/admin/custom_fields/calculated_values/details_form.rb index 5b3da9c9c5f..4ea618de393 100644 --- a/app/components/admin/custom_fields/calculated_values/details_form.rb +++ b/app/components/admin/custom_fields/calculated_values/details_form.rb @@ -72,7 +72,7 @@ module Admin::CustomFields::CalculatedValues caption: I18n.t("custom_fields.instructions.admin_only") ) - details_form.submit(name: :submit, label: I18n.t(:button_save), scheme: :default) + details_form.submit(name: :submit, label: I18n.t(:button_save), scheme: :primary) end end end From 3e4b8cb4c1f586d6ea7e6f4a20bdc95847516711 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 16:19:51 +0200 Subject: [PATCH 43/44] [#64347] use concern helpers --- app/models/custom_field.rb | 1 - app/models/custom_field/calculated_value.rb | 104 +++++++++++--------- 2 files changed, 55 insertions(+), 50 deletions(-) diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index ce71762809a..2e4c6ba2230 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -73,7 +73,6 @@ class CustomField < ApplicationRecord validate :validate_field_format_inclusion validate :validate_default_value validate :validate_regex - validate :validate_formula validates :min_length, numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :max_length, numericality: { only_integer: true, greater_than_or_equal_to: 0 } diff --git a/app/models/custom_field/calculated_value.rb b/app/models/custom_field/calculated_value.rb index 64f2d4753d1..ffe933e6e45 100644 --- a/app/models/custom_field/calculated_value.rb +++ b/app/models/custom_field/calculated_value.rb @@ -31,65 +31,71 @@ # Methods for custom fields with a format of "calculated value". # Should be included in the CustomField model. module CustomField::CalculatedValue - def validate_formula - return unless field_format_calculated_value? + extend ActiveSupport::Concern - if formula_string.blank? - errors.add(:formula, :blank) - return + included do + validate :validate_formula + + def validate_formula + return unless field_format_calculated_value? + + if formula_string.blank? + errors.add(:formula, :blank) + return + end + + unless formula_contains_only_allowed_characters? + errors.add(:formula, :invalid_characters) + return + end + + # WP-64348: check for valid (i.e., visible & enabled) custom field references (see #cf_ids_used_in_formula) + + # Dentaku will return nil if the formula is invalid. + # TODO WP-64348: add support for referenced custom fields by injecting them as variables, + # e.g. calculator.evaluate(formula_string, cf_123: CustomField.find(123).value) + errors.add(:formula, :invalid) unless Dentaku::Calculator.new.evaluate(formula_string) + + # TODO: consider differentiating between a formula that contains missing variables, invalid + # syntax, or mathematical errors. end - unless formula_contains_only_allowed_characters? - errors.add(:formula, :invalid_characters) - return + def formula=(value) + if value.is_a?(String) + super({ formula: value, referenced_custom_fields: cf_ids_used_in_formula(value) }) + else + super + end end - # WP-64348: check for valid (i.e., visible & enabled) custom field references (see #cf_ids_used_in_formula) - - # Dentaku will return nil if the formula is invalid. - # TODO WP-64348: add support for referenced custom fields by injecting them as variables, - # e.g. calculator.evaluate(formula_string, cf_123: CustomField.find(123).value) - errors.add(:formula, :invalid) unless Dentaku::Calculator.new.evaluate(formula_string) - - # TODO: consider differentiating between a formula that contains missing variables, invalid - # syntax, or mathematical errors. - end - - def formula=(value) - if value.is_a?(String) - super({ formula: value, referenced_custom_fields: cf_ids_used_in_formula(value) }) - else - super + # Returns the formula as a string. Will return an empty string if the formula is not set. + def formula_string + formula ? formula.fetch("formula", "") : "" end - end - # Returns the formula as a string. Will return an empty string if the formula is not set. - def formula_string - formula ? formula.fetch("formula", "") : "" - end + private - private + def formula_contains_only_allowed_characters? + # List of allowed characters in a formula. This only performs a very basic validation. + # Allowed characters are: + # + - / * ( ) whitespace digits and decimal points + # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of + # the custom field. + # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST + # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that + # it prevents built-in functions from being used in the formula, which we do not want to allow. + allowed_chars = %w[+ - / * ( )] + [" "] + allowed_tokens = /\A(cf_\d+|\d+\.?\d*|\.\d+)\z/ - def formula_contains_only_allowed_characters? - # List of allowed characters in a formula. This only performs a very basic validation. - # Allowed characters are: - # + - / * ( ) whitespace digits and decimal points - # Additionally, the formula may contain references to custom fields in the form of `cf_123` where 123 is the ID of - # the custom field. - # Once this basic validation passes, the formula will be parsed and validated by Dentaku, which builds an AST - # and ensures that the formula is really valid. A welcome side effect of the basic validation done here is that - # it prevents built-in functions from being used in the formula, which we do not want to allow. - allowed_chars = %w[+ - / * ( )] + [" "] - allowed_tokens = /\A(cf_\d+|\d+\.?\d*|\.\d+)\z/ - - formula_string.split(Regexp.union(allowed_chars)).reject(&:empty?).all? do |token| - token.match?(allowed_tokens) + formula_string.split(Regexp.union(allowed_chars)).reject(&:empty?).all? do |token| + token.match?(allowed_tokens) + end end - end - # Returns a list of custom field IDs used in the formula. - # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. - def cf_ids_used_in_formula(formula_str) - formula_str.scan(/\bcf_(\d+)\b/).flatten.map(&:to_i) + # Returns a list of custom field IDs used in the formula. + # For a formula like `2 + cf_12 + cf_4` it returns `[12, 4]`. + def cf_ids_used_in_formula(formula_str) + formula_str.scan(/\bcf_(\d+)\b/).flatten.map(&:to_i) + end end end From ee09c0e0c9c9e739741c103b3f4b7812aa66d83d Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Mon, 30 Jun 2025 16:29:52 +0200 Subject: [PATCH 44/44] [#64347] move details form to `app/forms` --- .../details_component.html.erb | 2 +- .../calculated_values/details_form.rb | 78 ------------------ .../calculated_values/details_form.rb | 80 +++++++++++++++++++ 3 files changed, 81 insertions(+), 79 deletions(-) delete mode 100644 app/components/admin/custom_fields/calculated_values/details_form.rb create mode 100644 app/forms/custom_fields/calculated_values/details_form.rb diff --git a/app/components/admin/custom_fields/calculated_values/details_component.html.erb b/app/components/admin/custom_fields/calculated_values/details_component.html.erb index 6bdceda3369..00b49b65975 100644 --- a/app/components/admin/custom_fields/calculated_values/details_component.html.erb +++ b/app/components/admin/custom_fields/calculated_values/details_component.html.erb @@ -41,7 +41,7 @@ See COPYRIGHT and LICENSE files for more details. id: "custom_field_form", url: form_url, method: form_method - ) { |form| render Admin::CustomFields::CalculatedValues::DetailsForm.new(form) } + ) { |form| render CustomFields::CalculatedValues::DetailsForm.new(form) } end end end diff --git a/app/components/admin/custom_fields/calculated_values/details_form.rb b/app/components/admin/custom_fields/calculated_values/details_form.rb deleted file mode 100644 index 4ea618de393..00000000000 --- a/app/components/admin/custom_fields/calculated_values/details_form.rb +++ /dev/null @@ -1,78 +0,0 @@ -# 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. -#++ - -module Admin::CustomFields::CalculatedValues - class DetailsForm < ApplicationForm - form do |details_form| - if model.new_record? - details_form.hidden(name: :field_format) - details_form.hidden(name: :type, scope_name_to_model: false) - end - - details_form.text_field( - name: :name, - label: I18n.t(:label_name), - required: true - ) - - details_form.select_list( - name: :custom_field_section_id, - label: I18n.t("activerecord.attributes.project_custom_field.custom_field_section"), - required: true - ) do |li| - ProjectCustomFieldSection.find_each do |cs| - li.option(value: cs.id, label: cs.name) - end - end - - details_form.text_field( - name: :formula, - value: model.formula_string, - label: I18n.t(:label_formula), - required: true, - caption: I18n.t("custom_fields.instructions.formula") - ) - - details_form.check_box( - name: :is_required, - label: I18n.t("activerecord.attributes.project_custom_field.is_required"), - caption: I18n.t("custom_fields.instructions.is_required_for_project") - ) - - details_form.check_box( - name: :admin_only, - label: I18n.t("activerecord.attributes.custom_field.admin_only"), - caption: I18n.t("custom_fields.instructions.admin_only") - ) - - details_form.submit(name: :submit, label: I18n.t(:button_save), scheme: :primary) - end - end -end diff --git a/app/forms/custom_fields/calculated_values/details_form.rb b/app/forms/custom_fields/calculated_values/details_form.rb new file mode 100644 index 00000000000..3bb8fd62f79 --- /dev/null +++ b/app/forms/custom_fields/calculated_values/details_form.rb @@ -0,0 +1,80 @@ +# 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. +#++ + +module CustomFields + module CalculatedValues + class DetailsForm < ApplicationForm + form do |details_form| + if model.new_record? + details_form.hidden(name: :field_format) + details_form.hidden(name: :type, scope_name_to_model: false) + end + + details_form.text_field( + name: :name, + label: I18n.t(:label_name), + required: true + ) + + details_form.select_list( + name: :custom_field_section_id, + label: I18n.t("activerecord.attributes.project_custom_field.custom_field_section"), + required: true + ) do |li| + ProjectCustomFieldSection.find_each do |cs| + li.option(value: cs.id, label: cs.name) + end + end + + details_form.text_field( + name: :formula, + value: model.formula_string, + label: I18n.t(:label_formula), + required: true, + caption: I18n.t("custom_fields.instructions.formula") + ) + + details_form.check_box( + name: :is_required, + label: I18n.t("activerecord.attributes.project_custom_field.is_required"), + caption: I18n.t("custom_fields.instructions.is_required_for_project") + ) + + details_form.check_box( + name: :admin_only, + label: I18n.t("activerecord.attributes.custom_field.admin_only"), + caption: I18n.t("custom_fields.instructions.admin_only") + ) + + details_form.submit(name: :submit, label: I18n.t(:button_save), scheme: :primary) + end + end + end +end