Delete spent time custom field values not visible by the user after assignment (#33550).

Patch by  Marius BALTEANU.


git-svn-id: http://svn.redmine.org/redmine/trunk@20802 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/models/time_entry.rb

@@ -128,7 +128,14 @@ class TimeEntry < ActiveRecord::Base
       else
         @invalid_user_id = nil
       end
+
+      # Delete assigned custom fields not visible by the user
+      editable_custom_field_ids = editable_custom_field_values(user).map {|v| v.custom_field_id.to_s}
+      self.custom_field_values.delete_if do |c|
+        !editable_custom_field_ids.include?(c.custom_field.id.to_s)
+      end
     end
+
     attrs
   end
 
@@ -199,7 +206,7 @@ class TimeEntry < ActiveRecord::Base
 
   # Returns the custom_field_values that can be edited by the given user
   def editable_custom_field_values(user=nil)
-    visible_custom_field_values
+    visible_custom_field_values(user)
   end
 
   # Returns the custom fields that can be edited by the given user

test/unit/time_entry_custom_field_test.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 2006-2020 Jean-Philippe Lang
+#
+# 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.
+
+require File.expand_path('../../test_helper', __FILE__)
+
+class TimeEntryCustomFieldTest < ActiveSupport::TestCase
+  include Redmine::I18n
+
+  fixtures :roles
+
+  def setup
+    User.current = nil
+  end
+
+  def test_custom_field_with_visible_set_to_false_should_validate_roles
+    set_language_if_valid 'en'
+    field = TimeEntryCustomField.new(:name => 'Field', :field_format => 'string', :visible => false)
+    assert !field.save
+    assert_include "Roles cannot be blank", field.errors.full_messages
+    field.role_ids = [1, 2]
+    assert field.save
+  end
+
+  def test_changing_visible_to_true_should_clear_roles
+    field = TimeEntryCustomField.create!(:name => 'Field', :field_format => 'string', :visible => false, :role_ids => [1, 2])
+    assert_equal 2, field.roles.count
+
+    field.visible = true
+    field.save!
+    assert_equal 0, field.roles.count
+  end
+
+  def test_safe_attributes_should_include_only_custom_fields_visible_to_user
+    cf1 = TimeEntryCustomField.create!(:name => 'Visible field',
+                                       :field_format => 'string',
+                                       :visible => false, :role_ids => [1])
+    cf2 = TimeEntryCustomField.create!(:name => 'Non visible field',
+                                       :field_format => 'string',
+                                       :visible => false, :role_ids => [3])
+    user = User.find(2)
+    time_entry = TimeEntry.new(:issue_id => 1)
+
+    time_entry.send :safe_attributes=, {'custom_field_values' => {
+      cf1.id.to_s => 'value1',
+      cf2.id.to_s => 'value2'
+    }}, user
+
+    assert_equal 'value1', time_entry.custom_field_value(cf1)
+    assert_nil time_entry.custom_field_value(cf2)
+
+    time_entry.send :safe_attributes=, {'custom_fields' => [
+      {'id' => cf1.id.to_s, 'value' => 'valuea'},
+      {'id' => cf2.id.to_s, 'value' => 'valueb'}
+    ]}, user
+
+    assert_equal 'valuea', time_entry.custom_field_value(cf1)
+    assert_nil time_entry.custom_field_value(cf2)
+  end
+end

test/unit/time_entry_test.rb

@@ -29,7 +29,8 @@ class TimeEntryTest < ActiveSupport::TestCase
            :journals, :journal_details,
            :issue_categories, :enumerations,
            :groups_users,
-           :enabled_modules
+           :enabled_modules,
+           :custom_fields, :custom_fields_projects, :custom_values
 
   def setup
     User.current = nil