use preference service inside user update service - for backlogs as well

app/controllers/my_controller.rb

@@ -158,7 +158,10 @@ class MyController < ApplicationController
   end
 
   def user_params
-    permitted_params.my_account_settings.to_h
+    # The Users::UpdateService updates the user's pref using the UserPreferences::UpdateService
+    # which has a contract/schema applied to the values which is why it is ok
+    # to blindly allow all scalar values in pref.
+    permitted_params.user.to_h.merge(params.permit(pref: {}))
   end
 
   def notice_account_updated

app/models/permitted_params.rb

@@ -199,10 +199,6 @@ class PermittedParams
     params.require(:placeholder_user).permit(*self.class.permitted_attributes[:placeholder_user])
   end
 
-  def my_account_settings
-    user.merge(pref:)
-  end
-
   def user_register_via_omniauth
     permitted_params = params
       .require(:user)

app/services/users/update_service.rb

@@ -45,8 +45,9 @@ module Users
     def persist(_service_result)
       service_result = super
 
-      if service_result.success?
-        service_result.success = model.pref.save
+      if service_result.success? && params[:pref].present?
+        preference_service = UserPreferences::UpdateService.new(user:, model: model.pref)
+        service_result.add_dependent!(preference_service.call(params[:pref]))
       end
 
       service_result

modules/backlogs/app/forms/my/backlogs_form.rb

@@ -30,16 +30,14 @@
 
 class My::BacklogsForm < ApplicationForm
   form do |f|
-    f.text_field name: :task_color,
-                 label: I18n.t("backlogs.task_color"),
-                 value: @color,
+    f.text_field name: :backlogs_task_color,
+                 label: I18n.t("activerecord.attributes.user_preference.backlogs_task_color"),
                  input_width: :xsmall
 
-    f.check_box name: :versions_default_fold_state,
+    f.check_box name: :backlogs_versions_default_fold_state,
                 value: DEFAULT_FOLD_STATE,
                 unchecked_value: DEFAULT_EXPAND_STATE,
-                checked: default_fold_state_checked?,
-                label: I18n.t("backlogs.label_sprints_default_fold_state"),
+                label: I18n.t("activerecord.attributes.user_preference.backlogs_versions_default_fold_state"),
                 caption: I18n.t("backlogs.caption_sprints_default_fold_state")
 
     f.submit(name: :submit, label: I18n.t("backlogs.user_preference.button_update_backlogs"), scheme: :default)
@@ -47,16 +45,4 @@ class My::BacklogsForm < ApplicationForm
 
   DEFAULT_FOLD_STATE = "closed"
   DEFAULT_EXPAND_STATE = "open"
-
-  def initialize(color:, versions_default_fold_state:)
-    super()
-    @color = color
-    @versions_default_fold_state = versions_default_fold_state
-  end
-
-  private
-
-  def default_fold_state_checked?
-    @versions_default_fold_state == DEFAULT_FOLD_STATE
-  end
 end

modules/backlogs/app/views/shared/_view_my_settings.html.erb

@@ -33,7 +33,7 @@ See COPYRIGHT and LICENSE files for more details.
   end
 %>
 <%=
-  settings_primer_form_with(model: user.pref, scope: :backlogs, url: { action: "update_settings" }, data: { turbo: false }) do |form|
-    render(My::BacklogsForm.new(form, color:, versions_default_fold_state:))
+  settings_primer_form_with(model: user.pref, scope: :pref, url: { action: "update_settings" }, data: { turbo: false }) do |form|
+    render(My::BacklogsForm.new(form))
   end
 %>

modules/backlogs/config/locales/en.yml

@@ -48,6 +48,9 @@ en:
         sprint_sharing: "Sprint sharing"
       sprint:
         duration: "Sprint duration"
+      user_preference:
+        backlogs_task_color: "Task color"
+        backlogs_versions_default_fold_state: "Show sprints folded"
       work_package:
         backlogs_work_package_type: "Backlog type"
         position: "Position"
@@ -103,7 +106,6 @@ en:
     sharing_description: "This project can either share its own sprints, receive shared sprints or handle sprints independently (no sharing)."
     sharing: "Sharing"
     impediment: "Impediment"
-    label_sprints_default_fold_state: "Show sprints folded"
     caption_sprints_default_fold_state: "Sprints will not be expanded by default when viewing the 'Backlog and sprints' page. Each one has to be manually expanded."
     work_package_is_closed: "Work package is done, when"
     label_is_done_status: "Status %{status_name} means done"

modules/backlogs/lib/open_project/backlogs/engine.rb

@@ -179,7 +179,8 @@ module OpenProject::Backlogs
         "definitions/UserPreferences/properties",
         {
           "backlogs_task_color" => {
-            "type" => "string"
+            "type" => "string",
+            "pattern" => "^#[0-9a-fA-F]{6}$"
           },
           "backlogs_versions_default_fold_state" => {
             "type" => "string",
@@ -228,7 +229,6 @@ module OpenProject::Backlogs
 
     config.to_prepare do
       OpenProject::Backlogs::Hooks::LayoutHook
-      OpenProject::Backlogs::Hooks::UserSettingsHook
     end
 
     initializer "openproject_backlogs.event_subscriptions" do

modules/backlogs/lib/open_project/backlogs/hooks/layout_hook.rb

@@ -35,10 +35,7 @@ module OpenProject::Backlogs::Hooks
         :render_to_string,
         partial: "shared/view_my_settings",
         locals: {
-          user: context[:user],
-          color: context[:user].backlogs_preference(:task_color),
-          versions_default_fold_state:
-            context[:user].backlogs_preference(:versions_default_fold_state)
+          user: context[:user]
         }
       )
     end

modules/backlogs/lib/open_project/backlogs/hooks/user_settings_hook.rb

@@ -1,51 +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.
-#++
-
-class OpenProject::Backlogs::Hooks::UserSettingsHook < OpenProject::Hook::ViewListener
-  # Updates the backlogs settings before saving the user
-  #
-  # Context:
-  # * params => Request parameters
-  # * permitted_params => whitelisted params
-  # * user => user being altered
-  def service_update_user_before_save(context = {})
-    params = context[:params]
-    user = context[:user]
-
-    backlogs_params = params.delete(:backlogs)
-    return unless backlogs_params
-
-    versions_default_fold_state = backlogs_params[:versions_default_fold_state] || "open"
-    user.backlogs_preference(:versions_default_fold_state, versions_default_fold_state)
-
-    color = backlogs_params[:task_color] || ""
-    if color == "" || color.match(/^#[A-Fa-f0-9]{6}$/)
-      user.backlogs_preference(:task_color, color)
-    end
-  end
-end

modules/backlogs/lib/open_project/backlogs/patches/permitted_params_patch.rb

@@ -41,11 +41,6 @@ module OpenProject::Backlogs::Patches::PermittedParamsPatch
       permitted_params
     end
 
-    def my_account_settings
-      backlogs_params = params.fetch(:backlogs, {}).permit(:task_color, :versions_default_fold_state)
-      super.merge(backlogs: backlogs_params)
-    end
-
     def backlogs_admin_settings
       params
         .require(:settings)

spec/services/users/update_service_spec.rb

@@ -43,7 +43,7 @@ RSpec.describe Users::UpdateService do
     let(:current_user) { create(:admin) }
     let(:update_user) { create(:user, mail: "correct@example.org") }
 
-    subject { instance.call(attributes:) }
+    subject { instance.call(**attributes) }
 
     context "when invalid" do
       let(:attributes) { { mail: "invalid" } }
@@ -97,7 +97,7 @@ RSpec.describe Users::UpdateService do
     end
 
     describe "updating prefs" do
-      let(:attributes) { {} }
+      let(:attributes) { { pref: { increase_theme_contrast: 0 } } }
 
       before do
         allow(update_user).to receive(:save).and_return(user_save_result)