Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/controllers/units_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,11 @@ def unit_params
]

# Add :id to permitted fields if UNIT_BADGES is enabled
permitted_fields << :id if unit_badges_enabled?
# but only for create actions (not update)
create_actions = %w[create create_from_inspection]
if unit_badges_enabled? && create_actions.include?(action_name)
permitted_fields << :id
end

permitted_params = params.require(:unit).permit(*permitted_fields)

Expand Down
12 changes: 9 additions & 3 deletions app/views/units/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,15 @@
i18n_base: 'forms.units',
url: form_url do |form| %>
<%= render 'chobble_forms/fieldset', legend_key: 'unit_details' do %>
<%# Show ID field when UNIT_BADGES is enabled and creating new unit %>
<% if ENV["UNIT_BADGES"] == "true" && !unit.persisted? %>
<%= render 'chobble_forms/text_field', field: :id, required: true %>
<%# Show ID field when UNIT_BADGES is enabled %>
<% if ENV["UNIT_BADGES"] == "true" %>
<% if unit.persisted? %>
<%# Display ID as read-only for existing units %>
<%= render 'chobble_forms/display_field', field: :id %>
<% else %>
<%# Editable ID field for new units %>
<%= render 'chobble_forms/text_field', field: :id, required: true %>
<% end %>
<% end %>

<%= render 'chobble_forms/text_field', field: :name, required: true %>
Expand Down
37 changes: 37 additions & 0 deletions spec/features/units/unit_badges_spec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: false
# frozen_string_literal: true

require "rails_helper"
Expand Down Expand Up @@ -121,4 +122,40 @@
expect(created_unit.id).to match(/\A[A-Z0-9]{8}\z/)
end
end

context "ID field immutability on edit" do
let(:unit_with_badge) { create(:unit, id: badge.id, user: user) }

before do
ENV["UNIT_BADGES"] = "true"
end

after do
ENV.delete("UNIT_BADGES")
end

scenario "does not show ID field on edit form" do
visit edit_unit_path(unit_with_badge)
expect_field_not_present(:units, :id)
end

scenario "ID remains unchanged after editing other fields" do
original_id = unit_with_badge.id

visit edit_unit_path(unit_with_badge)
fill_in_form(:units, :name, "Updated Name")
submit_form(:units)

expect_i18n_content("units.messages.updated")

unit_with_badge.reload
expect(unit_with_badge.id).to eq(original_id)
expect(unit_with_badge.name).to eq("Updated Name")
end

scenario "shows badge ID in display format on edit page" do
visit edit_unit_path(unit_with_badge)
expect(page).to have_content(badge.id)
end
end
end
46 changes: 46 additions & 0 deletions spec/requests/units/units_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,52 @@
end
end

describe "Unit badge ID immutability" do
let(:badge_batch) { create(:badge_batch) }
let(:badge1) { create(:badge, badge_batch: badge_batch) }
let(:badge2) { create(:badge, badge_batch: badge_batch) }

before do
ENV["UNIT_BADGES"] = "true"
login_as(user)
end

after do
ENV.delete("UNIT_BADGES")
end

it "prevents changing ID on update via raw request" do
unit_with_badge = create(:unit, id: badge1.id, user: user)
original_id = unit_with_badge.id

patch unit_path(unit_with_badge), params: {
unit: {
id: badge2.id,
name: "Updated Name"
}
}

unit_with_badge.reload
expect(unit_with_badge.id).to eq(original_id)
expect(unit_with_badge.name).to eq("Updated Name")
end

it "ignores ID parameter in update requests" do
unit_with_badge = create(:unit, id: badge1.id, user: user)

patch unit_path(unit_with_badge), params: {
unit: {
id: "NEWID123",
description: "Updated Description"
}
}

unit_with_badge.reload
expect(unit_with_badge.id).to eq(badge1.id)
expect(unit_with_badge.description).to eq("Updated Description")
end
end

describe "Access control and error handling" do
describe "inactive user restrictions" do
let(:inactive_user) { create(:user, active_until: Date.current - 1.day) }
Expand Down