From 52053c6971461fb4c0a34f392b600b565aca566f Mon Sep 17 00:00:00 2001 From: ulferts Date: Wed, 10 Sep 2025 17:16:24 +0200 Subject: [PATCH] encorporate dombesz proposals --- .../admin/enumerations/item_form.rb | 4 +-- app/models/enumeration.rb | 8 +++--- config/locales/en.yml | 4 --- .../dynamic/admin/enumerations.controller.ts | 9 +++---- .../spec/models/time_entry_activity_spec.rb | 4 +-- .../spec/models/document_category_spec.rb | 4 +-- .../shared_enumeration_examples.rb | 27 ++++++++++++++----- spec/models/issue_priority_spec.rb | 4 +-- 8 files changed, 35 insertions(+), 29 deletions(-) diff --git a/app/components/admin/enumerations/item_form.rb b/app/components/admin/enumerations/item_form.rb index d85507eb470..47ed6c87448 100644 --- a/app/components/admin/enumerations/item_form.rb +++ b/app/components/admin/enumerations/item_form.rb @@ -53,8 +53,8 @@ module Admin form.check_box( name: :active, label: object.class.human_attribute_name(:active), - data: { action: "admin--enumerations#lockstepDefault", - "admin--enumerations-target": "active" } + disabled: object.is_default, + data: { "admin--enumerations-target": "active" } ) if object.class.can_have_default_value? diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb index dda4edc2a39..1f25fa8f186 100644 --- a/app/models/enumeration.rb +++ b/app/models/enumeration.rb @@ -37,6 +37,7 @@ class Enumeration < ApplicationRecord acts_as_tree order: "position ASC" before_save :unmark_old_default_value, if: :became_default_value? + before_save :ensure_activated, if: -> { self.class.can_have_default_value? && is_default? } before_destroy :check_integrity validates :name, presence: true @@ -44,7 +45,6 @@ class Enumeration < ApplicationRecord uniqueness: { scope: %i(type project_id), case_sensitive: false } validates :name, length: { maximum: 256 } - validate :default_is_active, if: -> { self.class.can_have_default_value? } scope :shared, -> { where(project_id: nil) } scope :active, -> { where(active: true) } @@ -175,9 +175,7 @@ class Enumeration < ApplicationRecord raise "Can't delete enumeration" if in_use? end - def default_is_active - if is_default? && !active? - errors.add(:is_default, :active_required) - end + def ensure_activated + self.active = true end end diff --git a/config/locales/en.yml b/config/locales/en.yml index b3b5c6b2577..85b39faac3d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1463,10 +1463,6 @@ en: only_one_trial: "Only one trial token can be active. Please delete the previous trial token before adding another." unreadable: "can't be read. Are you sure it is a support token?" already_added: "This token has already been added." - enumeration: - attributes: - active: - active_required: "needs the 'Active' property to be true." grids/grid: overlaps: "overlap." outside: "is outside of the grid." diff --git a/frontend/src/stimulus/controllers/dynamic/admin/enumerations.controller.ts b/frontend/src/stimulus/controllers/dynamic/admin/enumerations.controller.ts index e996ebf878e..b30dbc1cd3e 100644 --- a/frontend/src/stimulus/controllers/dynamic/admin/enumerations.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/admin/enumerations.controller.ts @@ -44,12 +44,9 @@ export default class EnumerationsController extends Controller { public lockstepActive() { if (this.defaultTarget.checked) { this.activeTarget.checked = true; - } - } - - public lockstepDefault() { - if (!this.activeTarget.checked && this.hasDefaultTarget) { - this.defaultTarget.checked = false; + this.activeTarget.disabled = true; + } else { + this.activeTarget.disabled = false; } } } diff --git a/modules/costs/spec/models/time_entry_activity_spec.rb b/modules/costs/spec/models/time_entry_activity_spec.rb index 57e8010df1a..4ff2642feb8 100644 --- a/modules/costs/spec/models/time_entry_activity_spec.rb +++ b/modules/costs/spec/models/time_entry_activity_spec.rb @@ -55,7 +55,7 @@ RSpec.describe TimeEntryActivity do end end - it_behaves_like "enumeration validation", false do - let(:enumeration) { build_stubbed(:time_entry_activity) } + it_behaves_like "enumeration#active handling", false do + let(:enumeration) { described_class.new(attributes_for(:time_entry_activity)) } end end diff --git a/modules/documents/spec/models/document_category_spec.rb b/modules/documents/spec/models/document_category_spec.rb index bedf1cb7e2b..16bbbd09cf8 100644 --- a/modules/documents/spec/models/document_category_spec.rb +++ b/modules/documents/spec/models/document_category_spec.rb @@ -61,7 +61,7 @@ RSpec.describe DocumentCategory do end.to change { old_default.is_default? }.from(true).to(false) end - it_behaves_like "enumeration validation", true do - let(:enumeration) { build_stubbed(:document_category) } + it_behaves_like "enumeration#active handling", true do + let(:enumeration) { described_class.new(attributes_for(:document_category)) } end end diff --git a/spec/models/enumerations/shared_enumeration_examples.rb b/spec/models/enumerations/shared_enumeration_examples.rb index 28c9bb0b670..08eb15509f4 100644 --- a/spec/models/enumerations/shared_enumeration_examples.rb +++ b/spec/models/enumerations/shared_enumeration_examples.rb @@ -30,22 +30,37 @@ require "spec_helper" -RSpec.shared_context "enumeration validation" do |default_supported| # rubocop:disable RSpec/ContextWording +RSpec.shared_context "enumeration#active handling" do |default_supported| # rubocop:disable RSpec/ContextWording let(:enumeration) do super() rescue NoMethodError raise "'enumeration' let needs to be set" end - describe "#valid?" do + describe "#active" do if default_supported - context "with the enumeration being inactive and default" do - it "is invalid", :aggregate_failures do + context "with the enumeration being inactive and default before saving" do + it "sets the enumeration to be active", :aggregate_failures do enumeration.active = false enumeration.is_default = true - expect(enumeration.valid?).to be false - expect(enumeration.errors.symbols_for(:is_default)).to contain_exactly(:active_required) + enumeration.save + + expect(enumeration).to be_persisted + expect(enumeration.active).to be true + end + end + + context "with the enumeration being inactive and not default before saving" do + it "keeps the value of active", :aggregate_failures do + enumeration.active = false + enumeration.is_default = false + + enumeration.save + + expect(enumeration).to be_persisted + expect(enumeration.active).to be false + expect(enumeration.is_default).to be false end end end diff --git a/spec/models/issue_priority_spec.rb b/spec/models/issue_priority_spec.rb index 9c00131ae5f..b82cb17a410 100644 --- a/spec/models/issue_priority_spec.rb +++ b/spec/models/issue_priority_spec.rb @@ -37,8 +37,8 @@ RSpec.describe IssuePriority do let(:stubbed_priority) { build_stubbed(:priority) } - it_behaves_like "enumeration validation", true do - let(:enumeration) { build_stubbed(:priority) } + it_behaves_like "enumeration#active handling", true do + let(:enumeration) { described_class.new(attributes_for(:priority)) } end describe ".ancestors" do