From 0fdcaa3a5a467bf331b35248aaed8168b8b1c377 Mon Sep 17 00:00:00 2001 From: mattsoh Date: Thu, 6 Aug 2026 19:46:25 +0200 Subject: [PATCH 1/5] add endpoint to edit comments --- app/controllers/api/v4/comments_controller.rb | 10 ++++++++++ config/routes.rb | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v4/comments_controller.rb b/app/controllers/api/v4/comments_controller.rb index 0e1e160a14..10ae99d2d0 100644 --- a/app/controllers/api/v4/comments_controller.rb +++ b/app/controllers/api/v4/comments_controller.rb @@ -31,6 +31,16 @@ def create require_oauth2_scope "comments:write", :create + def update + @comment = authorize Comment.find_by_public_id!(params[:id]) + + @comment.update!(params.permit(:content, :admin_only, :file)) + + render "show" + end + + require_oauth2_scope "comments:write", :update + end end end diff --git a/config/routes.rb b/config/routes.rb index e99313af9b..94539179da 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -820,7 +820,7 @@ resources :wires, only: [:index, :show, :create] resources :ach_transfers, only: [:create] - resources :comments, only: [:index, :create] + resources :comments, only: [:index, :create, :update] get "stripe_terminal_connection_token", to: "stripe_terminal#connection_token" From 274b9ae28b79f2498f8408c76fc6094620828d95 Mon Sep 17 00:00:00 2001 From: mattsoh Date: Thu, 6 Aug 2026 20:15:44 +0200 Subject: [PATCH 2/5] remove ability to add/change file when editing comment --- app/controllers/api/v4/comments_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/v4/comments_controller.rb b/app/controllers/api/v4/comments_controller.rb index 10ae99d2d0..2c9f51bbd4 100644 --- a/app/controllers/api/v4/comments_controller.rb +++ b/app/controllers/api/v4/comments_controller.rb @@ -34,7 +34,7 @@ def create def update @comment = authorize Comment.find_by_public_id!(params[:id]) - @comment.update!(params.permit(:content, :admin_only, :file)) + @comment.update!(params.permit(:content, :admin_only)) render "show" end From 4632577b56edef621a91ae11bac398d608fc7ef2 Mon Sep 17 00:00:00 2001 From: mattsoh Date: Sat, 8 Aug 2026 03:39:27 +0200 Subject: [PATCH 3/5] gatekeep admin only behind auditor access --- app/controllers/api/v4/comments_controller.rb | 6 +++++- app/policies/comment_policy.rb | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v4/comments_controller.rb b/app/controllers/api/v4/comments_controller.rb index 2c9f51bbd4..414ddf2af3 100644 --- a/app/controllers/api/v4/comments_controller.rb +++ b/app/controllers/api/v4/comments_controller.rb @@ -34,7 +34,11 @@ def create def update @comment = authorize Comment.find_by_public_id!(params[:id]) - @comment.update!(params.permit(:content, :admin_only)) + @comment.assign_attributes(params.permit(:content, :admin_only)) + + authorize @comment, :set_admin_only? if @comment.admin_only? + + @comment.save! render "show" end diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index 7df115a0b2..a51a1c8bc6 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -27,9 +27,15 @@ def edit? end def update? + return false if record.admin_only && !user.auditor? + user.admin? || (users.include?(user) && record.user == user) || (user.auditor? && record.user == user) end + def set_admin_only? + user.auditor? + end + def react? show? end From 99af44e978ea70d99e0305b80fbee42ee4284ae7 Mon Sep 17 00:00:00 2001 From: mattsoh Date: Sat, 8 Aug 2026 03:41:46 +0200 Subject: [PATCH 4/5] add v4 comments api tests --- .../api/v4/comments_controller_spec.rb | 216 ++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 spec/controllers/api/v4/comments_controller_spec.rb diff --git a/spec/controllers/api/v4/comments_controller_spec.rb b/spec/controllers/api/v4/comments_controller_spec.rb new file mode 100644 index 0000000000..1cac4f0e75 --- /dev/null +++ b/spec/controllers/api/v4/comments_controller_spec.rb @@ -0,0 +1,216 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Api::V4::CommentsController do + render_views + + describe "#update" do + let(:event) { create(:event) } + let(:hcb_code) { create(:disbursement, source_event: event).outgoing_disbursement.local_hcb_code } + let(:comment) { create(:comment, commentable: hcb_code, user:, content: "Original content", admin_only: false) } + + def authenticate_as(user, scopes: nil) + token = create(:api_token, user:, scopes:) + request.headers["Authorization"] = "Bearer #{token.token}" + end + + def update_comment(params) + patch :update, params: { id: comment.public_id, **params }, as: :json + end + + context "as an auditor" do + let(:user) { create(:user, :make_auditor) } + + before { authenticate_as(user, scopes: "admin:read") } + + it "updates the content" do + update_comment(content: "Edited content") + + expect(response).to have_http_status(:ok) + expect(comment.reload.content).to eq("Edited content") + expect(response.parsed_body).to include("content" => "Edited content") + end + + context "when admin_only is omitted" do + it "leaves a public comment public" do + expect { update_comment(content: "Edited content") }.not_to(change { comment.reload.admin_only }.from(false)) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).not_to have_key("admin_only") + end + + it "leaves an admin-only comment admin-only" do + comment.update!(admin_only: true) + + expect { update_comment(content: "Edited content") }.not_to(change { comment.reload.admin_only }.from(true)) + + expect(response).to have_http_status(:ok) + expect(comment.reload.content).to eq("Edited content") + expect(response.parsed_body).to include("admin_only" => true) + end + end + + context "when admin_only is a boolean" do + it "marks a public comment as admin-only, leaving the content alone" do + expect { update_comment(admin_only: true) }.to change { comment.reload.admin_only }.from(false).to(true) + + expect(response).to have_http_status(:ok) + expect(comment.reload.content).to eq("Original content") + expect(response.parsed_body).to include("admin_only" => true, "content" => "Original content") + end + + it "marks an admin-only comment as public" do + comment.update!(admin_only: true) + + expect { update_comment(admin_only: false) }.to change { comment.reload.admin_only }.from(true).to(false) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).not_to have_key("admin_only") + end + end + + context "when admin_only is not a boolean" do + { + "true" => true, + "false" => false, + "t" => true, + "f" => false, + "0" => false, + "1" => true, + 0 => false, + 1 => true, + 2 => true, + "yes" => true, + "banana" => true, + }.each do |value, expected| + it "casts #{value.inspect} to #{expected}" do + update_comment(admin_only: value) + + expect(response).to have_http_status(:ok) + expect(comment.reload.admin_only).to eq(expected) + end + end + + it "rejects a value that casts to nil" do + comment.update!(admin_only: true) + + expect { update_comment(admin_only: "") }.not_to(change { comment.reload.admin_only }.from(true)) + + expect(response).to have_http_status(:internal_server_error) + expect(response.parsed_body).to eq( + { + "error" => "internal_error", + "messages" => ["Internal database error"] + } + ) + end + end + end + + context "as an organizer who is not an auditor" do + let(:user) { create(:user) } + + before do + create(:organizer_position, user:, event:) + authenticate_as(user) + end + + it "edits the content of its own public comment" do + update_comment(content: "Edited content") + + expect(response).to have_http_status(:ok) + expect(comment.reload.content).to eq("Edited content") + end + + it "leaves admin_only alone when it is omitted" do + expect { update_comment(content: "Edited content") }.not_to(change { comment.reload.admin_only }.from(false)) + + expect(response).to have_http_status(:ok) + end + + it "cannot make its own comment admin-only" do + expect { update_comment(admin_only: true) }.not_to(change { comment.reload.admin_only }.from(false)) + + expect(response).to have_http_status(:forbidden) + expect(response.parsed_body).to eq({ "error" => "not_authorized" }) + end + + ["true", "t", "1", 1, 2, "yes", "banana"].each do |value| + it "cannot make its own comment admin-only with #{value.inspect}" do + expect { update_comment(admin_only: value) }.not_to(change { comment.reload.admin_only }.from(false)) + + expect(response).to have_http_status(:forbidden) + end + end + + it "cannot edit a comment that is already admin-only" do + comment.update!(admin_only: true) + + expect { update_comment(content: "Edited content") }.not_to(change { comment.reload.attributes }) + + expect(response).to have_http_status(:forbidden) + end + + it "cannot make an admin-only comment public" do + comment.update!(admin_only: true) + + expect { update_comment(admin_only: false) }.not_to(change { comment.reload.admin_only }.from(true)) + + expect(response).to have_http_status(:forbidden) + end + + it "ignores a non-scalar admin_only value" do + expect { update_comment(admin_only: { hacked: true }) }.not_to(change { comment.reload.admin_only }.from(false)) + + expect(response).to have_http_status(:ok) + end + + it "may still set admin_only to a falsy value" do + expect { update_comment(admin_only: "false") }.not_to(change { comment.reload.admin_only }.from(false)) + + expect(response).to have_http_status(:ok) + end + end + + context "when a file is passed" do + let(:user) { create(:user) } + + before do + create(:organizer_position, user:, event:) + authenticate_as(user) + end + + it "is ignored" do + patch :update, params: { + id: comment.public_id, + content: "Edited content", + file: fixture_file_upload("attachment1.txt", "text/plain") + }, format: :json + + expect(response).to have_http_status(:ok) + expect(comment.reload.content).to eq("Edited content") + expect(comment.file).not_to be_attached + end + end + + context "when the comment belongs to someone else" do + let(:user) { create(:user) } + + before do + create(:organizer_position, user:, event:) + + other_user = create(:user) + create(:organizer_position, user: other_user, event:) + authenticate_as(other_user) + end + + it "is forbidden" do + expect { update_comment(content: "Edited content", admin_only: true) }.not_to(change { comment.reload.attributes }) + + expect(response).to have_http_status(:forbidden) + expect(response.parsed_body).to eq({ "error" => "not_authorized" }) + end + end + end +end \ No newline at end of file From d9446f6ab78fcfe327ac5a0000d5eb6cc36d6b1b Mon Sep 17 00:00:00 2001 From: mattsoh Date: Sat, 8 Aug 2026 03:46:14 +0200 Subject: [PATCH 5/5] re-add newline --- spec/controllers/api/v4/comments_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/api/v4/comments_controller_spec.rb b/spec/controllers/api/v4/comments_controller_spec.rb index 1cac4f0e75..7301adb607 100644 --- a/spec/controllers/api/v4/comments_controller_spec.rb +++ b/spec/controllers/api/v4/comments_controller_spec.rb @@ -213,4 +213,4 @@ def update_comment(params) end end end -end \ No newline at end of file +end