diff --git a/apps/api/spec/requests/admin_pipelines_spec.rb b/apps/api/spec/requests/admin_pipelines_spec.rb new file mode 100644 index 0000000..81baec2 --- /dev/null +++ b/apps/api/spec/requests/admin_pipelines_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" +require "warden/test/helpers" + +# The pipeline admin form could not save at all: the resource never declared +# `permit_params`, so ActiveAdmin handed unpermitted attributes to the model and Rails +# raised ForbiddenAttributesError on every edit. No model spec could have caught that — +# the fault was one layer up — so the check belongs here, where the host's ActiveAdmin +# instance actually runs. +RSpec.describe "Admin pipelines", type: :request do + include Warden::Test::Helpers + + let(:admin) { AdminUser.create!(email: "pipelines-spec@example.org", password: "password123") } + let(:software) { create(:software) } + let!(:holder) do + WarpEngine::Pipeline.create!(woodpecker_repo_id: 990_001, repo_owner: "spec", repo_name: "holder", + platform: "tic80", software: software) + end + let!(:taker) do + WarpEngine::Pipeline.create!(woodpecker_repo_id: 990_002, repo_owner: "spec", repo_name: "taker", + platform: "tic80") + end + + before do + Warden.test_mode! + login_as(admin, scope: :admin_user) + end + + after { Warden.test_reset! } + + # This app keeps forgery protection on in the test environment, and a request spec has + # no rendered form to take a token from. The token is not what is under test here, so it + # is switched off for the duration and put back afterwards. + around do |example| + protection = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = false + example.run + ActionController::Base.allow_forgery_protection = protection + end + + it "saves the form" do + put "/admin/pipelines/#{taker.id}", params: { pipeline: { platform: "godot" } } + + expect(response).to have_http_status(:found) + expect(taker.reload.platform).to eq("godot") + end + + it "moves the software off the pipeline that had it" do + put "/admin/pipelines/#{taker.id}", + params: { pipeline: { platform: "tic80", software_id: software.id } } + + expect(response).to have_http_status(:found) + expect(taker.reload.software).to eq(software) + expect(holder.reload.software).to be_nil + expect(software.reload.pipeline).to eq(taker) + end +end diff --git a/libs/ruby/warp_engine/app/admin/pipelines.rb b/libs/ruby/warp_engine/app/admin/pipelines.rb index df101f7..5b5791a 100644 --- a/libs/ruby/warp_engine/app/admin/pipelines.rb +++ b/libs/ruby/warp_engine/app/admin/pipelines.rb @@ -1,6 +1,11 @@ ActiveAdmin.register WarpEngine::Pipeline, as: "Pipeline" do actions :index, :show, :edit, :update + # Without this the edit form cannot save at all: ActiveAdmin hands unpermitted params to + # the model and Rails raises ForbiddenAttributesError. The two fields here are the two + # the form offers; everything else about a pipeline comes from the Woodpecker sync. + permit_params :platform, :software_id + menu parent: "🌀 WarpEngine", priority: 10, label: "🚀 Pipelines" config.sort_order = "repo_name_asc" @@ -48,24 +53,12 @@ ActiveAdmin.register WarpEngine::Pipeline, as: "Pipeline" do collection: WarpEngine::Software.order(:title).map { |s| [ s.title, s.id ] }, include_blank: "- none -", hint: "One pipeline per software. Picking one that another pipeline already " \ - "has moves the link here rather than refusing it." + "has moves the link here — that pipeline is left without a software, " \ + "and the move is written to the log." end f.actions end - controller do - # The reassignment itself is the model's job; this only makes it visible. Without a - # word about it, the other pipeline loses its software with nothing on screen to say - # that it happened. - def update - super - taken = resource.software_taken_from - return if taken.blank? - - flash[:notice] = [ flash[:notice], "Software taken from #{taken.join(', ')}." ].compact.join(" ") - end - end - sidebar "Details", only: :show do attributes_table_for resource do row :id diff --git a/libs/ruby/warp_engine/app/models/warp_engine/pipeline.rb b/libs/ruby/warp_engine/app/models/warp_engine/pipeline.rb index 8f5013a..5394455 100644 --- a/libs/ruby/warp_engine/app/models/warp_engine/pipeline.rb +++ b/libs/ruby/warp_engine/app/models/warp_engine/pipeline.rb @@ -55,6 +55,16 @@ module WarpEngine others = Pipeline.where(software_id: software_id).where.not(id: id) @software_taken_from = others.map(&:full_name) + return if @software_taken_from.empty? + + # Logged rather than flashed. The first attempt at this put a message on screen by + # overriding the admin's `update` action, which bypassed the permitted-params path + # and made every pipeline edit fail with ForbiddenAttributesError. A silent + # reassignment is a small problem; an admin page that cannot save is a large one. + Rails.logger.info( + "[WarpEngine::Pipeline] #{full_name} took software #{software_id} from " \ + "#{@software_taken_from.join(', ')}" + ) others.update_all(software_id: nil, updated_at: Time.current) end