From b861fe81b85371c7a4b1b779f56ac3c51e3d81b4 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Thu, 20 Nov 2025 19:01:10 +0100 Subject: [PATCH 1/5] fix: show remove_connections value in Moves index and forms --- app/controllers/moves_controller.rb | 18 ++++------ app/decorators/move_decorator.rb | 11 ++++-- app/models/move.rb | 23 ++++++++++--- app/views/moves/_form.html.erb | 7 ++-- app/views/moves/index.html.erb | 8 +++-- config/locales/en.yml | 1 + config/locales/fr.yml | 1 + spec/decorators/move_decorator_spec.rb | 46 ++++++++++++++++++-------- spec/models/move_spec.rb | 10 +++++- test/models/move_test.rb | 6 ++-- 10 files changed, 92 insertions(+), 39 deletions(-) diff --git a/app/controllers/moves_controller.rb b/app/controllers/moves_controller.rb index 0d0307dad..a7202d7c9 100644 --- a/app/controllers/moves_controller.rb +++ b/app/controllers/moves_controller.rb @@ -51,12 +51,8 @@ def create authorize! @move = @moves_project_step.moves.build(move_params) - if params[:move][:remove_connections] == "Oui" - @move.clear_connections - end - respond_to do |format| - if @move.save + if @move.clear_connections_and_save format.html { redirect_to moves_project_path(@moves_project_step.moves_project), notice: t(".flashes.created") } format.json { render :show, status: :created, location: @move } else @@ -68,12 +64,10 @@ def create end def update - respond_to do |format| - if @move.update(move_params) - if params[:move][:remove_connections] == "Oui" - @move.clear_connections - end + @move.assign_attributes(move_params) + respond_to do |format| + if @move.clear_connections_and_save format.html { redirect_to moves_project_path(@moves_project_step.moves_project), notice: t(".flashes.updated") } format.json { render :show, status: :ok, location: @move } else @@ -219,11 +213,11 @@ def set_move # Never trust parameters from the scary internet, only allow the white list through. def move_params - params.expect(move: %i[moveable_type moveable_id frame_id moves_project_step_id position]) + params.expect(move: %i[moveable_type moveable_id frame_id moves_project_step_id position remove_connections]) end def unscoped_move_params - params.expect(move: %i[moveable_type moveable_id moves_project_step_id]) + params.expect(move: %i[moveable_type moveable_id moves_project_step_id remove_connections]) end def moved_connection_params diff --git a/app/decorators/move_decorator.rb b/app/decorators/move_decorator.rb index 254cf53f9..eb6621529 100644 --- a/app/decorators/move_decorator.rb +++ b/app/decorators/move_decorator.rb @@ -3,6 +3,10 @@ class MoveDecorator < ApplicationDecorator include ActionView::Helpers + def steps_options_for_select + options_for_select(moves_project.steps.pluck(:name, :id), { selected: moves_project_step_id }) + end + def status_to_badge_component text = I18n.t(".activerecord.attributes.move.statuses.#{status}") color = executed? ? :success : :primary @@ -10,8 +14,11 @@ def status_to_badge_component BadgeComponent.new(text, color:, variant: :pill) end - def steps_options_for_select - options_for_select(moves_project.steps.pluck(:name, :id), { selected: moves_project_step_id }) + def moved_connections_to_badge_component + has_connections = moved_connections.any? + color = has_connections ? :success : :danger + + BadgeComponent.new(I18n.t("boolean.#{has_connections}"), color:, variant: :pill) end def display_name diff --git a/app/models/move.rb b/app/models/move.rb index 646701027..a77acebf3 100644 --- a/app/models/move.rb +++ b/app/models/move.rb @@ -3,7 +3,7 @@ class Move < ApplicationRecord has_changelog - attr_accessor :remove_connections + attribute :remove_connections, :boolean, default: false belongs_to :step, class_name: "MovesProjectStep", foreign_key: :moves_project_step_id, inverse_of: :moves belongs_to :moveable, polymorphic: true @@ -21,10 +21,25 @@ class Move < ApplicationRecord scope :not_executed, -> { where(executed_at: nil) } + def clear_connections_and_save + clear_connections + save + end + + def moved_connections + return [] unless moveable + + MovedConnection.per_servers([moveable]) + end + def clear_connections server = moveable + # Delete current moved connections - MovedConnection.per_servers([server]).delete_all + moved_connections.delete_all + + return unless remove_connections + # Add moved connection for each port server.ports.each do |p| MovedConnection.create( @@ -49,7 +64,7 @@ def execute!(apply_connections: true) equipment.position = position if equipment.save! - MovedConnection.per_servers([equipment]).map(&:execute!) if apply_connections + moved_connections.map(&:execute!) if apply_connections # Update prev_frame and prev_position for incoming moves Move.not_executed @@ -59,7 +74,7 @@ def execute!(apply_connections: true) move.update(prev_frame_id: frame.id, prev_position: position) end - update!(executed_at: Time.zone.now) + update_columns(executed_at: Time.zone.now) # rubocop:disable Rails/SkipsModelValidations end end end diff --git a/app/views/moves/_form.html.erb b/app/views/moves/_form.html.erb index 80fe96441..d15ca674e 100644 --- a/app/views/moves/_form.html.erb +++ b/app/views/moves/_form.html.erb @@ -76,9 +76,10 @@
<%= f.label :remove_connections, class: "form-label" %> <%= f.select :remove_connections, - options_for_select(["Oui", "Non"], { selected: "Non" }), - {}, - class: "form-select" + options_for_select( + [["Oui", "true"], ["Non", "false"]], { selected: move.moved_connections.any? ? "true" : "false" }, + ), + {}, class: "form-select" %>
diff --git a/app/views/moves/index.html.erb b/app/views/moves/index.html.erb index da5c48eb2..84fe76f82 100644 --- a/app/views/moves/index.html.erb +++ b/app/views/moves/index.html.erb @@ -27,7 +27,7 @@
<%= turbo_frame_tag(dom_id(Move, :table), data: { turbo_action: :advance }) do %>
- <%= render List::DataTableComponent.new(@moves) do |table| %> + <%= render List::DataTableComponent.new(decorate(@moves)) do |table| %> <% table.with_column(Server.model_name.human) do |move| %> <%= link_to move.moveable, server_path(move.moveable), data: { turbo_frame: :_top } %>
@@ -79,12 +79,16 @@ <% table.with_column(Move.human_attribute_name(:status)) do |move| %> <%= render move.decorated.status_to_badge_component %> <% end %> + <% table.with_column(t(".remove_connections")) do |move| %> + <%= render move.moved_connections_to_badge_component %> + <% end %> + <% table.with_column(style: "min-width: 120px; width: 120px") do |move| %>
<% unless @moves_project.archived? || move.executed? %> diff --git a/config/locales/en.yml b/config/locales/en.yml index dab2b4736..9c8308804 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -255,6 +255,7 @@ en: execute_confirmation: Do you really want to apply this move? frame_table_title: Frames after moving edit_connections: Update connections + remove_connections: Delete connections? action: execute_move: Move the server and its connections new: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 213243a33..f204105ab 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -550,6 +550,7 @@ fr: frame_table_title: Châssis après déplacements edit_connections: Modifier les connexions execute_confirmation: Voulez-vous vraiment appliquer ce déplacement ? + remove_connections: Supprimer les connexion ? action: execute_move: Déplacer le serveur et ses connexions new: diff --git a/spec/decorators/move_decorator_spec.rb b/spec/decorators/move_decorator_spec.rb index 8e259bc56..97d609520 100644 --- a/spec/decorators/move_decorator_spec.rb +++ b/spec/decorators/move_decorator_spec.rb @@ -6,32 +6,52 @@ let(:object) { moves(:planned) } let(:decorated_move) { described_class.decorate(object) } + describe "#steps_options_for_select" do + let(:object) { moves(:move_step_one) } + let(:options) { decorated_move.steps_options_for_select } + + it { expect(options).to have_tag("option", count: 3) } + it { expect(options).to have_tag("option", text: "Step 1", with: { value: 6, selected: "selected" }) } + it { expect(options).to have_tag("option", text: "Step 2", with: { value: 7 }) } + it { expect(options).to have_tag("option", text: "Step 3", with: { value: 8 }) } + end + describe "#status_to_badge_component" do subject(:badge) { decorated_move.status_to_badge_component } context "with move planned" do - it { is_expected.to be_a BadgeComponent } - it { expect(badge.instance_variable_get(:@color)).to eq :primary } - it { expect(badge.content).to eq "Planifié" } + it { is_expected.to be_a(BadgeComponent) } + it { expect(badge.instance_variable_get(:@color)).to eq(:primary) } + it { expect(badge.content).to eq("Planifié") } end context "with move executed" do let(:object) { moves(:executed) } - it { is_expected.to be_a BadgeComponent } - it { expect(badge.instance_variable_get(:@color)).to eq :success } - it { expect(badge.content).to eq "Exécuté" } + it { is_expected.to be_a(BadgeComponent) } + it { expect(badge.instance_variable_get(:@color)).to eq(:success) } + it { expect(badge.content).to eq("Exécuté") } end end - describe "#steps_options_for_select" do - let(:object) { moves(:move_step_one) } - let(:options) { decorated_move.steps_options_for_select } + describe "#moved_connections_to_badge_component" do + subject(:badge) { decorated_move.moved_connections_to_badge_component } - it { expect(options).to have_tag("option", count: 3) } - it { expect(options).to have_tag("option", text: "Step 1", with: { value: 6, selected: "selected" }) } - it { expect(options).to have_tag("option", text: "Step 2", with: { value: 7 }) } - it { expect(options).to have_tag("option", text: "Step 3", with: { value: 8 }) } + context "with moved_connections" do + let(:object) { moves(:one) } + + it { is_expected.to be_a(BadgeComponent) } + it { expect(badge.instance_variable_get(:@color)).to eq(:success) } + it { expect(badge.content).to eq("Oui") } + end + + context "without moved_connections" do + let(:object) { Move.new } + + it { is_expected.to be_a(BadgeComponent) } + it { expect(badge.instance_variable_get(:@color)).to eq(:danger) } + it { expect(badge.content).to eq("Non") } + end end describe "#display_name" do diff --git a/spec/models/move_spec.rb b/spec/models/move_spec.rb index 8d5b66e82..67cceef46 100644 --- a/spec/models/move_spec.rb +++ b/spec/models/move_spec.rb @@ -37,7 +37,15 @@ end describe "#clear_connections" do - pending + let(:move) { moves(:planned) } + + it { expect { move.clear_connections }.to change { move.moved_connections.count }.from(2).to(0) } + + context "when remove_connections set to true" do + let(:move) { moves(:planned).tap { |m| m.remove_connections = true } } + + it { expect { move.clear_connections }.to change { move.moved_connections.count }.from(2).to(4) } + end end describe "#status" do diff --git a/test/models/move_test.rb b/test/models/move_test.rb index 2ccc8113e..ff83bceb8 100644 --- a/test/models/move_test.rb +++ b/test/models/move_test.rb @@ -25,6 +25,7 @@ def setup assert_empty(@moved_connections.select { |c| c.port_from_id == 2 }) # Re-init moved connections + @move.remove_connections = true @move.clear_connections # After @@ -48,12 +49,13 @@ def setup end test "execution of a movement with connections" do - @moved_connection = MovedConnection.per_servers([@move.moveable]).first + @moved_connection = @move.moved_connections.first @port_from = @moved_connection.port_from assert @port_from.cable_name != @moved_connection.cablename assert_nil @move.executed_at @move.execute! + @move.reload @moved_connection.reload assert @move.moveable.reload.frame == @move.frame @@ -62,6 +64,6 @@ def setup assert @move.executed_at assert Move.where(id: @move.id) assert @moved_connection.executed_at - assert MovedConnection.where(id: @moved_connection.id) + assert MovedConnection.find_by(id: @moved_connection.id) end end From 0cb2faf9748905a94cf47b8a5b759981fa426626 Mon Sep 17 00:00:00 2001 From: B_Rass Date: Wed, 3 Dec 2025 16:19:18 +0100 Subject: [PATCH 2/5] Small cleans --- app/views/moves/index.html.erb | 2 +- config/locales/fr.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/moves/index.html.erb b/app/views/moves/index.html.erb index 84fe76f82..3f700de88 100644 --- a/app/views/moves/index.html.erb +++ b/app/views/moves/index.html.erb @@ -85,7 +85,7 @@ <% end %> - <% table.with_column(t(".remove_connections")) do |move| %> + <% table.with_column(t(".remove_connections"), text_align: :center) do |move| %> <%= render move.moved_connections_to_badge_component %> <% end %> diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f204105ab..fd27bfcbc 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -550,7 +550,7 @@ fr: frame_table_title: Châssis après déplacements edit_connections: Modifier les connexions execute_confirmation: Voulez-vous vraiment appliquer ce déplacement ? - remove_connections: Supprimer les connexion ? + remove_connections: Supprimer les connexions ? action: execute_move: Déplacer le serveur et ses connexions new: From c235feba2d787e0b35c07a5fc2bbf68fed6411e7 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Wed, 17 Dec 2025 16:27:37 +0100 Subject: [PATCH 3/5] wip --- app/models/move.rb | 10 ++++++---- app/models/moved_connection.rb | 4 +++- ...10161815_add_step_reference_to_moved_connections.rb | 7 +++++++ db/schema.rb | 3 +++ 4 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20251210161815_add_step_reference_to_moved_connections.rb diff --git a/app/models/move.rb b/app/models/move.rb index a77acebf3..cf5aa7429 100644 --- a/app/models/move.rb +++ b/app/models/move.rb @@ -12,6 +12,8 @@ class Move < ApplicationRecord has_one :moves_project, through: :step + has_many :moved_connections, through: :step + validates :moveable_id, uniqueness: { scope: %i[step moveable_type] } validates :position, presence: true @@ -26,17 +28,17 @@ def clear_connections_and_save save end - def moved_connections + def moveable_moved_connections return [] unless moveable - MovedConnection.per_servers([moveable]) + moved_connections.per_servers([moveable]) end def clear_connections server = moveable # Delete current moved connections - moved_connections.delete_all + moveable_moved_connections.delete_all return unless remove_connections @@ -64,7 +66,7 @@ def execute!(apply_connections: true) equipment.position = position if equipment.save! - moved_connections.map(&:execute!) if apply_connections + moveable_moved_connections.map(&:execute!) if apply_connections # Update prev_frame and prev_position for incoming moves Move.not_executed diff --git a/app/models/moved_connection.rb b/app/models/moved_connection.rb index c94df2fea..114e8dc4a 100644 --- a/app/models/moved_connection.rb +++ b/app/models/moved_connection.rb @@ -3,6 +3,7 @@ class MovedConnection < ApplicationRecord has_changelog + belongs_to :step, class_name: "MoveProjectStep" belongs_to :port_from, class_name: "Port" belongs_to :port_to, class_name: "Port", optional: true @@ -10,7 +11,8 @@ class MovedConnection < ApplicationRecord def self.per_servers(servers) servers_ports_ids = servers.map(&:ports).flatten.map(&:id) - MovedConnection.where("port_from_id IN (?) OR port_to_id IN (?)", servers_ports_ids, servers_ports_ids) + + where("port_from_id IN (?) OR port_to_id IN (?)", servers_ports_ids, servers_ports_ids) end def ports diff --git a/db/migrate/20251210161815_add_step_reference_to_moved_connections.rb b/db/migrate/20251210161815_add_step_reference_to_moved_connections.rb new file mode 100644 index 000000000..8e5e6f274 --- /dev/null +++ b/db/migrate/20251210161815_add_step_reference_to_moved_connections.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddStepReferenceToMovedConnections < ActiveRecord::Migration[8.0] + def change + add_reference :moved_connections, :step, null: true, foreign_key: { to_table: :moves_project_steps } + end +end diff --git a/db/schema.rb b/db/schema.rb index b7275810c..f7a86d922 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -361,8 +361,10 @@ t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.datetime "executed_at", precision: nil + t.bigint "step_id" t.index ["port_from_id"], name: "index_moved_connections_on_port_from_id" t.index ["port_to_id"], name: "index_moved_connections_on_port_to_id" + t.index ["step_id"], name: "index_moved_connections_on_step_id" end create_table "moves", id: :serial, force: :cascade do |t| @@ -578,6 +580,7 @@ add_foreign_key "modeles", "architectures" add_foreign_key "modeles", "categories" add_foreign_key "modeles", "manufacturers" + add_foreign_key "moved_connections", "moves_project_steps", column: "step_id" add_foreign_key "moves", "frames" add_foreign_key "moves", "frames", column: "prev_frame_id" add_foreign_key "moves", "moves_project_steps" From 9b00ac782d2be15b0531813d281cd66b24dd61fa Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Wed, 17 Dec 2025 18:11:03 +0100 Subject: [PATCH 4/5] fixes --- app/models/move.rb | 4 +--- app/models/moved_connection.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/move.rb b/app/models/move.rb index cf5aa7429..b9c58c3cf 100644 --- a/app/models/move.rb +++ b/app/models/move.rb @@ -35,15 +35,13 @@ def moveable_moved_connections end def clear_connections - server = moveable - # Delete current moved connections moveable_moved_connections.delete_all return unless remove_connections # Add moved connection for each port - server.ports.each do |p| + moveable.ports.each do |p| MovedConnection.create( port_from_id: p.id, vlans: "", diff --git a/app/models/moved_connection.rb b/app/models/moved_connection.rb index 114e8dc4a..421e65a08 100644 --- a/app/models/moved_connection.rb +++ b/app/models/moved_connection.rb @@ -3,7 +3,7 @@ class MovedConnection < ApplicationRecord has_changelog - belongs_to :step, class_name: "MoveProjectStep" + belongs_to :step, class_name: "MovesProjectStep" belongs_to :port_from, class_name: "Port" belongs_to :port_to, class_name: "Port", optional: true From 2f2f4c932529521f92f8e95f959a66566ad2e086 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Wed, 22 Apr 2026 15:31:16 +0200 Subject: [PATCH 5/5] wip --- app/models/moves_project_step.rb | 2 ++ spec/models/move_spec.rb | 2 ++ spec/models/moved_connection_spec.rb | 5 ++++- test/fixtures/moved_connections.yml | 2 ++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/models/moves_project_step.rb b/app/models/moves_project_step.rb index 9e7ab56ef..c430bb920 100644 --- a/app/models/moves_project_step.rb +++ b/app/models/moves_project_step.rb @@ -4,7 +4,9 @@ class MovesProjectStep < ApplicationRecord has_changelog belongs_to :moves_project + has_many :moves, dependent: :restrict_with_error + has_many :moved_connections, dependent: :restrict_with_error acts_as_list scope: :moves_project diff --git a/spec/models/move_spec.rb b/spec/models/move_spec.rb index 29a6a3080..d36d97c94 100644 --- a/spec/models/move_spec.rb +++ b/spec/models/move_spec.rb @@ -18,6 +18,8 @@ it { is_expected.to belong_to(:prev_frame).class_name("Frame") } it { is_expected.to have_one(:moves_project).through(:step) } + + it { is_expected.to have_many(:moved_connections).through(:step) } end describe "validations" do diff --git a/spec/models/moved_connection_spec.rb b/spec/models/moved_connection_spec.rb index f7dd1bbc5..d3d3d42c2 100644 --- a/spec/models/moved_connection_spec.rb +++ b/spec/models/moved_connection_spec.rb @@ -5,9 +5,12 @@ RSpec.describe MovedConnection do # it_behaves_like "changelogable", new_attributes: { } - subject(:moved_connection) { described_class.new(color: "bleu", cablename: "cable") } + subject(:moved_connection) { described_class.new(color: "bleu", cablename: "cable", step:) } + + let(:step) { move_project_steps(:one) } describe "associations" do + it { is_expected.to belong_to(:step) } it { is_expected.to belong_to(:port_from) } it { is_expected.to belong_to(:port_to).optional(true) } end diff --git a/test/fixtures/moved_connections.yml b/test/fixtures/moved_connections.yml index ba5c84289..f68c09112 100644 --- a/test/fixtures/moved_connections.yml +++ b/test/fixtures/moved_connections.yml @@ -1,6 +1,7 @@ # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html one: + step_id: 1 port_from_id: 1 port_to_id: 2 vlans: vlan01 @@ -8,6 +9,7 @@ one: color: Blue two: + step_id: 1 port_from_id: 1 port_to_id: 2 vlans: VlanXZZ