diff --git a/modules/resource_management/app/components/resource_allocations/list_item_component.rb b/modules/resource_management/app/components/resource_allocations/list_item_component.rb index 21c5110e2d3..d92288cf1fd 100644 --- a/modules/resource_management/app/components/resource_allocations/list_item_component.rb +++ b/modules/resource_management/app/components/resource_allocations/list_item_component.rb @@ -115,12 +115,13 @@ module ResourceAllocations end # An allocation without a user yet (filter placeholder or lost principal) - # shows a person-add icon instead of an avatar. + # shows a person-add icon instead of an avatar. Sized to match the avatar so + # the leading column keeps the same width and the row stays aligned. def leading_visual if allocation.principal Primer::OpenProject::AvatarWithFallback.new(size: AVATAR_SIZE, **avatar_options) else - Primer::Beta::Octicon.new(icon: :"person-add", color: :muted, "aria-hidden": true) + Primer::Beta::Octicon.new(icon: :"person-add", size: :medium, color: :muted, "aria-hidden": true) end end diff --git a/modules/resource_management/app/models/resource_allocation.rb b/modules/resource_management/app/models/resource_allocation.rb index da15a7813cd..d7f07b163f6 100644 --- a/modules/resource_management/app/models/resource_allocation.rb +++ b/modules/resource_management/app/models/resource_allocation.rb @@ -31,6 +31,11 @@ class ResourceAllocation < ApplicationRecord ALLOWED_ENTITY_TYPES = %w[WorkPackage].freeze + # `allocated_time` is stored in minutes in an integer column. Cap it at 5000 + # hours so an absurdly large input is rejected with a validation error rather + # than overflowing the column and raising ActiveModel::RangeError on save. + MAX_ALLOCATED_TIME = (5000.hours / 1.minute).to_i + belongs_to :entity, polymorphic: true, optional: false belongs_to :principal, class_name: "User", optional: true, inverse_of: :resource_allocations belongs_to :requested_by, class_name: "User", optional: true @@ -119,6 +124,8 @@ class ResourceAllocation < ApplicationRecord presence: true, numericality: { only_integer: true, greater_than: 0 } + validate :allocated_time_within_limit + validates :entity_type, inclusion: { in: ALLOWED_ENTITY_TYPES }, allow_blank: true @@ -207,6 +214,17 @@ class ResourceAllocation < ApplicationRecord private + # Capped above to keep `allocated_time` within the integer column. The field is + # entered in hours, so the message reports the limit in the same duration + # format rather than the raw minutes the column stores. + def allocated_time_within_limit + return if allocated_time.blank? + return if allocated_time <= MAX_ALLOCATED_TIME + + errors.add(:allocated_time, :less_than_or_equal_to, + count: DurationConverter.output(MAX_ALLOCATED_TIME / 60.0)) + end + def starts_before_entity? entity_start_date.present? && start_date.present? && start_date < entity_start_date end diff --git a/modules/resource_management/spec/models/resource_allocation_spec.rb b/modules/resource_management/spec/models/resource_allocation_spec.rb index fd54dea341f..e5d47a6e826 100644 --- a/modules/resource_management/spec/models/resource_allocation_spec.rb +++ b/modules/resource_management/spec/models/resource_allocation_spec.rb @@ -401,6 +401,17 @@ RSpec.describe ResourceAllocation do allocation.allocated_time = 1 expect(allocation).to be_valid end + + it "is valid at the upper cap" do + allocation.allocated_time = described_class::MAX_ALLOCATED_TIME + expect(allocation).to be_valid + end + + it "is invalid (rather than overflowing the column) above the cap" do + allocation.allocated_time = described_class::MAX_ALLOCATED_TIME + 1 + expect(allocation).not_to be_valid + expect(allocation.errors.symbols_for(:allocated_time)).to include(:less_than_or_equal_to) + end end describe "date range" do diff --git a/modules/resource_management/spec/requests/resource_allocations_spec.rb b/modules/resource_management/spec/requests/resource_allocations_spec.rb index 1a8b1ac9717..41f4b15cfba 100644 --- a/modules/resource_management/spec/requests/resource_allocations_spec.rb +++ b/modules/resource_management/spec/requests/resource_allocations_spec.rb @@ -525,6 +525,18 @@ RSpec.describe "ResourceAllocations requests", expect(response).to have_http_status(:unprocessable_entity) expect(allocation.reload.allocated_time).to eq(600) end + + # Regression: an absurdly large value used to overflow the integer column + # and raise ActiveModel::RangeError (500) instead of failing validation. + it "rejects a value above the maximum with an hours-formatted message" do + perform(allocated_hours: "999999999999h") + + expect(response).to have_http_status(:unprocessable_entity) + expect(allocation.reload.allocated_time).to eq(600) + expect(response.body).to include( + DurationConverter.output(ResourceAllocation::MAX_ALLOCATED_TIME / 60.0) + ) + end end context "when the update would overbook the assigned user" do