The file manager's rename prompt was empty: Kernel#j, not escape_javascript
The admin file manager builds its rename and delete buttons with inline
handlers, and interpolated the file name through `j`:
onclick: "var n=prompt('New name:','#{j entry[:name]}');..."
In a view `j` is `escape_javascript`. Inside an Arbre block it is not: Arbre
resolves unknown methods through `method_missing`, and `j` is not unknown — it
is `Kernel#j`, which prints its argument as JSON to stdout and returns nil. So
every page load wrote the file names to the server log, and the browser got
prompt('New name:','')
An admin pressing rename saw an empty prompt, and the delete confirmation asked
"Delete ''?". `escape_javascript(...)` spelled out is what those three
interpolations use now.
The page has no test, which is why nothing caught it. It has one now
(spec/requests/admin_files_spec.rb), and it asserts the file name is in both
handlers — with the icons, the folder creation, the failed folder creation and
the delete-returns-to-parent path, because those are the behaviours the
refactoring below could break silently.
Also in the page: the twenty-branch extension-to-emoji `case` moved out of the
view into `WarpEngine::FileIcon`, and the five page actions share one
`redirect_to_files` instead of repeating
`admin_files_path(dir:, picker:, field:)` six times.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,76 @@
|
||||
require "rails_helper"
|
||||
require "warden/test/helpers"
|
||||
|
||||
RSpec.describe "Admin file manager", type: :request do
|
||||
include Warden::Test::Helpers
|
||||
|
||||
let(:admin) { AdminUser.create!(email: "files-spec@example.org", password: "password123") }
|
||||
let(:container) { Rails.root.join("tmp/files-spec").to_s }
|
||||
|
||||
before do
|
||||
FileUtils.mkdir_p(File.join(container, "mygame-1.0"))
|
||||
File.write(File.join(container, "mygame-1.0.zip"), "zipdata")
|
||||
allow(WarpEngine.config).to receive(:file_container_path).and_return(container)
|
||||
Warden.test_mode!
|
||||
login_as(admin, scope: :admin_user)
|
||||
end
|
||||
|
||||
after do
|
||||
Warden.test_reset!
|
||||
FileUtils.rm_rf(container)
|
||||
end
|
||||
|
||||
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 "lists the artifact directory with an icon per entry" do
|
||||
get "/admin/files"
|
||||
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(response.body).to include("mygame-1.0.zip")
|
||||
expect(response.body).to include(WarpEngine::FileIcon.for("mygame-1.0.zip"))
|
||||
expect(response.body).to include(WarpEngine::FileIcon::DIRECTORY)
|
||||
end
|
||||
|
||||
it "puts the file name into the rename and delete prompts" do
|
||||
get "/admin/files"
|
||||
|
||||
expect(response.body).to include("prompt('New name:','mygame-1.0.zip')")
|
||||
expect(response.body).to include("Delete \\'mygame-1.0.zip\\'")
|
||||
end
|
||||
|
||||
it "shows the download count next to a file that was downloaded" do
|
||||
create(:download, file_path: "mygame-1.0.zip")
|
||||
|
||||
get "/admin/files"
|
||||
|
||||
expect(response.body).to include("mygame-1.0.zip")
|
||||
end
|
||||
|
||||
it "creates a folder and returns to the directory it was created in" do
|
||||
post "/admin/files/mkdir", params: { dir: "mygame-1.0", name: "docs" }
|
||||
|
||||
expect(response).to redirect_to("/admin/files?dir=mygame-1.0")
|
||||
expect(File.directory?(File.join(container, "mygame-1.0", "docs"))).to be(true)
|
||||
end
|
||||
|
||||
it "reports a failed folder creation instead of raising" do
|
||||
post "/admin/files/mkdir", params: { dir: "mygame-1.0", name: "" }
|
||||
|
||||
expect(response).to redirect_to("/admin/files?dir=mygame-1.0")
|
||||
expect(flash[:alert]).to include("Failed")
|
||||
end
|
||||
|
||||
it "deletes a file and returns to its parent directory" do
|
||||
File.write(File.join(container, "mygame-1.0", "readme.txt"), "hi")
|
||||
|
||||
delete "/admin/files/delete", params: { path: "mygame-1.0/readme.txt" }
|
||||
|
||||
expect(response).to redirect_to("/admin/files?dir=mygame-1.0")
|
||||
expect(File.exist?(File.join(container, "mygame-1.0", "readme.txt"))).to be(false)
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user