Release OpenProject 16.3.2

Author: oliverguenther

app/contracts/projects/base_contract.rb

@@ -148,5 +148,13 @@ def validate_changing_active
 
       validate_and_merge_errors(contract)
     end
+
+    def all_available_custom_fields
+      if user.admin?
+        model.all_available_custom_fields
+      else
+        model.all_available_custom_fields.where(admin_only: false)
+      end
+    end
   end
 end

app/contracts/projects/update_contract.rb

@@ -32,11 +32,13 @@ module Projects
   class UpdateContract < BaseContract
     def writable_attributes
       if allow_project_attributes_only
-        with_custom_fields_only(super)
+        with_available_custom_fields_only(super)
       elsif allow_edit_attributes_only
         without_custom_fields(super)
       elsif allow_all_attributes
-        super
+        # When all attributes are updated (API-only case), allow writing to all available custom
+        # fields (including disabled ones) to maintain backward compatibility with the API.
+        with_all_available_custom_fields_only(super)
       else
         []
       end
@@ -63,7 +65,13 @@ def allow_all_attributes
 
     def without_custom_fields(changes) = changes.grep_v(/^custom_field_/)
 
-    def with_custom_fields_only(changes) = changes.grep(/^custom_field_/)
+    def with_available_custom_fields_only(changes) = changes & available_custom_fields.map(&:attribute_name)
+
+    def with_all_available_custom_fields_only(changes)
+      allowed_attributes = changes.grep_v(/^custom_field_/)
+      allowed_attributes += changes & all_available_custom_fields.map(&:attribute_name)
+      allowed_attributes
+    end
 
     def manage_permission
       if changed_by_user == ["active"]

app/contracts/work_packages/base_contract.rb

@@ -32,6 +32,7 @@ module WorkPackages
   class BaseContract < ::ModelContract
     include ::Attachments::ValidateReplacements
     include AssignableValuesContract
+    include WorkPackages::SetAttributesService::ProgressValuesCalculations
 
     attribute :subject
     attribute :description
@@ -397,9 +398,7 @@ def validate_percent_complete_is_set_when_work_and_remaining_work_are_set
     end
 
     def validate_percent_complete_matches_work_and_remaining_work
-      return if percent_complete_derivation_unapplicable?
-
-      if !percent_complete_range_derived_from_work_and_remaining_work.cover?(percent_complete)
+      if correctable_percent_complete_value?(work:, remaining_work:, percent_complete:)
         errors.add(:done_ratio, :does_not_match_work_and_remaining_work)
       end
     end
@@ -475,21 +474,6 @@ def percent_complete_empty?
       percent_complete.nil?
     end
 
-    def percent_complete_derivation_unapplicable?
-      WorkPackage.status_based_mode? || # only applicable in work-based mode
-        work_empty? || remaining_work_empty? || percent_complete_empty? || # only applicable if all 3 values are set
-        work == 0 || percent_complete == 100 # only applicable if not in special cases leading to divisions by zero
-    end
-
-    def percent_complete_range_derived_from_work_and_remaining_work
-      work_done = work - remaining_work
-      percentage = (100 * work_done.to_f / work)
-
-      lower_bound = percentage.truncate
-      upper_bound = lower_bound + 1
-      lower_bound..upper_bound
-    end
-
     def validate_no_reopen_on_closed_version
       if model.version_id && model.reopened? && model.version.closed?
         errors.add :base, I18n.t(:error_can_not_reopen_work_package_on_closed_version)

app/forms/work_packages/activities_tab/journals/notes_form.rb

@@ -40,6 +40,7 @@ class NotesForm < ApplicationForm
         rich_text_options: {
           showAttachments: false,
           resource:,
+          storageKey: "work_package-#{object.journable.id}-notes-#{object.id || 'new'}",
           editor_type: "constrained"
         }
       )

app/helpers/colors_helper.rb

@@ -122,8 +122,7 @@ def hl_background_class(name, model)
   end
 
   def icon_for_color(color, options = {})
-    return unless color
-    return if color.hexcode.blank?
+    return unless color&.valid_attribute?(:hexcode)
 
     style = join_style_arguments(
       "background-color: #{color.hexcode}",

app/models/application_record.rb

@@ -43,7 +43,12 @@ def just_created?
   ##
   # Returns whether the given attribute is free of errors
   def valid_attribute?(attribute)
-    valid? # Ensure validations have run
+    errors.clear
+
+    # run validations for specified attribute only.
+    self.class.validators_on(attribute).each do |validator|
+      validator.validate_each(self, attribute, public_send(attribute))
+    end
 
     errors[attribute].empty?
   end

app/models/color.rb

@@ -37,11 +37,9 @@ class Color < ApplicationRecord
            class_name: "Type",
            dependent: :nullify
 
-  after_initialize :normalize_hexcode
-  before_validation :normalize_hexcode
-
   validates :name, :hexcode, presence: true
+  validates :name, length: { maximum: 255 }
+  validates :hexcode, format: { with: RGB_HEX_FORMAT, message: :hexcode_invalid, allow_blank: true }
 
-  validates :name, length: { maximum: 255, unless: lambda { |e| e.name.blank? } }
-  validates :hexcode, format: { with: /\A#[0-9A-F]{6}\z/, unless: lambda { |e| e.hexcode.blank? } }
+  normalizes :hexcode, with: ::Colors::HexColor::Normalizer
 end

app/models/colors/hex_color.rb

@@ -30,6 +30,9 @@
 
 module Colors
   module HexColor
+    RGB_HEX_FORMAT = /\A#[0-9A-F]{6}\z/
+    private_constant :RGB_HEX_FORMAT
+
     ##
     # Get the fill style for this color.
     # If the color is light, use a dark font.
@@ -76,11 +79,13 @@ def brightness_yiq
     end
 
     ##
-    # Splits the hexcode into rbg color array
+    # Splits the hexcode into rgb color array
     def rgb_colors
       hexcode
-        .delete("#") # Remove trailing #
+        .delete_prefix("#") # Remove leading #
+        .ljust(6, "0") # Pad to at least 6 chars
         .scan(/../) # Pair hex chars
+        .first(3)
         .map(&:hex) # to int
     end
 
@@ -111,20 +116,22 @@ def blend(mix_value, opacity)
       "#%<r>02x%<g>02x%<b>02x" % { r:, g:, b: }
     end
 
-    # rubocop:disable Metrics/AbcSize
-    def normalize_hexcode
-      return unless hexcode.present? && hexcode_changed?
-
-      self.hexcode = hexcode.strip.upcase
-
-      unless hexcode.starts_with? "#"
-        self.hexcode = "##{hexcode}"
+    class Normalizer
+      def self.call(...)
+        new.call(...)
       end
 
-      if hexcode.size == 4 # =~ /#.../
-        self.hexcode = hexcode.gsub(/([^#])/, '\1\1')
+      def call(hex)
+        hex = hex.strip.delete_prefix("#")
+        case hex
+        when /\A[0-9a-fA-F]{3}\z/ # short form: #abc
+          "##{hex.chars.map { |c| c * 2 }.join.upcase}"
+        when /\A[0-9a-fA-F]{6}\z/ # long form: #aabbcc
+          "##{hex.upcase}"
+        else # do nothing
+          hex
+        end
       end
     end
-    # rubocop:enable Metrics/AbcSize
   end
 end

app/models/design_color.rb

@@ -31,7 +31,6 @@
 class DesignColor < ApplicationRecord
   include ::Colors::HexColor
 
-  before_validation :normalize_hexcode
   after_commit -> do
     # CustomStyle.current.updated_at determines the cache key for inline_css
     # in which the CSS color variables will be overwritten. That is why we need
@@ -44,9 +43,11 @@ class DesignColor < ApplicationRecord
     end
   end
 
+  validates :variable, :hexcode, presence: true
   validates :variable, uniqueness: true
-  validates :hexcode, :variable, presence: true
-  validates :hexcode, format: { with: /\A#[0-9A-F]{6}\z/, unless: lambda { |e| e.hexcode.blank? } }
+  validates :hexcode, format: { with: RGB_HEX_FORMAT, message: :hexcode_invalid, allow_blank: true }
+
+  normalizes :hexcode, with: ::Colors::HexColor::Normalizer
 
   class << self
     def setables

app/services/work_packages/activities_tab/comment_attachments_claims/set_attributes_service.rb

@@ -37,7 +37,10 @@ class SetAttributesService < ::BaseServices::SetAttributes
         ATTACHMENT_CSS_SELECTOR = "img.op-uc-image"
 
         def perform
-          self.params = params.reverse_merge(attachment_ids: collect_attachment_ids_from_notes)
+          ids_from_notes = collect_attachment_ids_from_notes
+          claimable_ids = filter_claimable_attachment_ids(ids_from_notes)
+
+          self.params = params.reverse_merge(attachment_ids: claimable_ids)
           super
         end
 
@@ -57,6 +60,20 @@ def collect_attachment_ids_from_notes
           end
         end
 
+        def filter_claimable_attachment_ids(ids)
+          return [] if ids.blank?
+
+          # Only claim attachments that are actually claimable. We must not try to
+          # reassign attachments that are already attached to another container
+          # (e.g., the work package, or another comment), and we must only claim unattached files of
+          # the current user to satisfy validation rules.
+          Attachment
+            .where(container: nil)
+            .or(Attachment.where(container: model))
+            .where(id: ids, author: User.current)
+            .pluck(:id)
+        end
+
         def parser
           @parser ||= Nokogiri::HTML.fragment(model.notes)
         end