From 4189261cd03f95a256ec4c161d9395b38f104d2b Mon Sep 17 00:00:00 2001 From: Italo Matos Date: Sun, 4 Jan 2026 10:02:58 -0300 Subject: [PATCH 1/7] Refactor: Replace pluck(:id) with ids method Use the more idiomatic ActiveRecord ids method instead of pluck(:id) across controllers and models. The ids method is more readable and explicitly conveys the intent to retrieve primary key values. Changes: - BoardsController#edit: Use @board.users.ids - Board::Storage: Use cards.ids, Comment.where().ids, and ActionText::RichText.where().ids - User::Accessor: Use account.boards.all_access.ids This change improves code clarity while maintaining the same functionality. --- app/controllers/boards_controller.rb | 2 +- app/models/board/storage.rb | 7 +++---- app/models/user/accessor.rb | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb index 721e69b0d..5b8763b71 100644 --- a/app/controllers/boards_controller.rb +++ b/app/controllers/boards_controller.rb @@ -30,7 +30,7 @@ class BoardsController < ApplicationController end def edit - selected_user_ids = @board.users.pluck :id + selected_user_ids = @board.users.ids @selected_users, @unselected_users = \ @board.account.users.active.alphabetically.includes(:identity).partition { |user| selected_user_ids.include? user.id } end diff --git a/app/models/board/storage.rb b/app/models/board/storage.rb index 24028e714..126ba96ab 100644 --- a/app/models/board/storage.rb +++ b/app/models/board/storage.rb @@ -21,7 +21,7 @@ module Board::Storage end def card_ids - @card_ids ||= cards.pluck(:id) + @card_ids ||= cards.ids end def card_image_bytes @@ -36,7 +36,7 @@ module Board::Storage def comment_embed_bytes card_ids.each_slice(BATCH_SIZE).sum do |batch| - sum_embed_bytes_for "Comment", Comment.where(card_id: batch).pluck(:id) + sum_embed_bytes_for "Comment", Comment.where(card_id: batch).ids end end @@ -46,8 +46,7 @@ module Board::Storage def sum_embed_bytes_for(record_type, record_ids) rich_text_ids = ActionText::RichText \ - .where(record_type: record_type, record_id: record_ids) - .pluck(:id) + .where(record_type: record_type, record_id: record_ids).ids sum_blob_bytes_in_batches \ ActiveStorage::Attachment.where(record_type: "ActionText::RichText", name: "embeds"), diff --git a/app/models/user/accessor.rb b/app/models/user/accessor.rb index a9cc5e459..e8a24830f 100644 --- a/app/models/user/accessor.rb +++ b/app/models/user/accessor.rb @@ -13,6 +13,6 @@ module User::Accessor private def grant_access_to_boards - Access.insert_all account.boards.all_access.pluck(:id).collect { |board_id| { id: ActiveRecord::Type::Uuid.generate, board_id: board_id, user_id: id, account_id: account.id } } + Access.insert_all account.boards.all_access.ids.collect { |board_id| { id: ActiveRecord::Type::Uuid.generate, board_id: board_id, user_id: id, account_id: account.id } } end end From 27093e4e785247e129ca630d47b4631a7f6e2bee Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 4 Jan 2026 11:05:19 -0500 Subject: [PATCH 2/7] Remove unnecessary recurring execution cleanup task This task is unnecessary because the `solid_queue_recurring_executions` table has a foreign key to `solid_queue_jobs` with `on_delete: :cascade`. This config was cargo-culted from HEY (in 44429644), which *does* need this task to run because we've omitted the FK constraints there. --- config/recurring.yml | 6 ------ db/queue_schema.rb | 1 + 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/config/recurring.yml b/config/recurring.yml index 6485278b3..bc1d069cc 100644 --- a/config/recurring.yml +++ b/config/recurring.yml @@ -18,9 +18,6 @@ production: &production clear_solid_queue_finished_jobs: command: "SolidQueue::Job.clear_finished_in_batches(sleep_between_batches: 0.3)" schedule: every hour at minute 12 - clear_solid_queue_recurring_executions: - command: "SolidQueue::RecurringExecution.clear_in_batches" - schedule: every hour at minute 52 cleanup_webhook_deliveries: command: "Webhook::Delivery.cleanup" schedule: every 4 hours at minute 51 @@ -43,9 +40,6 @@ beta: clear_solid_queue_finished_jobs: command: "SolidQueue::Job.clear_finished_in_batches(sleep_between_batches: 0.3)" schedule: every hour at minute 12 - clear_solid_queue_recurring_executions: - command: "SolidQueue::RecurringExecution.clear_in_batches" - schedule: every hour at minute 52 staging: *production development: *production diff --git a/db/queue_schema.rb b/db/queue_schema.rb index c4713f8d1..84bc6b8a6 100644 --- a/db/queue_schema.rb +++ b/db/queue_schema.rb @@ -132,6 +132,7 @@ ActiveRecord::Schema[8.2].define(version: 1) do t.index ["key"], name: "index_solid_queue_semaphores_on_key", unique: true end + # If these FKs are removed, make sure to periodically run `RecurringExecution.clear_in_batches` add_foreign_key "solid_queue_blocked_executions", "solid_queue_jobs", column: "job_id", on_delete: :cascade add_foreign_key "solid_queue_claimed_executions", "solid_queue_jobs", column: "job_id", on_delete: :cascade add_foreign_key "solid_queue_failed_executions", "solid_queue_jobs", column: "job_id", on_delete: :cascade From 96dfcb9d4c6b3a0d5acd09b426301942d2e54e4c Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 4 Jan 2026 14:09:42 -0500 Subject: [PATCH 3/7] Add a "*" to the queues handled by the solid queue worker pool Specifically this is to make sure all queues are managed, like the one we use for incineration in the fizzy-saas engine. --- config/queue.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/queue.yml b/config/queue.yml index 3f5615fa8..722da40b1 100644 --- a/config/queue.yml +++ b/config/queue.yml @@ -3,7 +3,7 @@ default: &default - polling_interval: 1 batch_size: 500 workers: - - queues: [ "default", "solid_queue_recurring", "backend", "webhooks" ] + - queues: [ "default", "solid_queue_recurring", "backend", "webhooks", "*" ] threads: 3 processes: <%= Integer(ENV.fetch("JOB_CONCURRENCY") { Concurrent.physical_processor_count }) %> polling_interval: 0.1 From 300801117538e57e08c1073633e1f38bf2154e17 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Mon, 5 Jan 2026 15:57:48 +0100 Subject: [PATCH 4/7] Make sure new card drafts are refreshed when reused To deal with old timestamps messing with card ordering and so. https://app.fizzy.do/5986089/cards/3495 --- app/controllers/cards_controller.rb | 2 +- app/models/user/accessor.rb | 6 ++++++ test/models/user/accessor_test.rb | 32 +++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/app/controllers/cards_controller.rb b/app/controllers/cards_controller.rb index daf22a5bc..4b7b4748f 100644 --- a/app/controllers/cards_controller.rb +++ b/app/controllers/cards_controller.rb @@ -12,7 +12,7 @@ class CardsController < ApplicationController def create respond_to do |format| format.html do - card = @board.cards.find_or_create_by!(creator: Current.user, status: "drafted") + card = Current.user.draft_new_card_at(@board) redirect_to card end diff --git a/app/models/user/accessor.rb b/app/models/user/accessor.rb index a9cc5e459..b9744a667 100644 --- a/app/models/user/accessor.rb +++ b/app/models/user/accessor.rb @@ -11,6 +11,12 @@ module User::Accessor after_create_commit :grant_access_to_boards, unless: :system? end + def draft_new_card_at(board) + board.cards.find_or_initialize_by(creator: self, status: "drafted").tap do |card| + card.update!(created_at: Time.current, updated_at: Time.current, last_active_at: Time.current) + end + end + private def grant_access_to_boards Access.insert_all account.boards.all_access.pluck(:id).collect { |board_id| { id: ActiveRecord::Type::Uuid.generate, board_id: board_id, user_id: id, account_id: account.id } } diff --git a/test/models/user/accessor_test.rb b/test/models/user/accessor_test.rb index 95cb84edb..300c2c741 100644 --- a/test/models/user/accessor_test.rb +++ b/test/models/user/accessor_test.rb @@ -12,4 +12,36 @@ class User::AccessorTest < ActiveSupport::TestCase system_user = User.create!(account: accounts("37s"), role: "system", name: "Test System User") assert_empty system_user.boards end + + test "creating a new card draft sets current timestamps" do + user = users(:david) + board = boards(:writebook) + + freeze_time do + card = user.draft_new_card_at(board) + + assert card.persisted? + assert card.drafted? + assert_equal user, card.creator + assert_equal board, card.board + assert_equal Time.current, card.created_at + assert_equal Time.current, card.updated_at + assert_equal Time.current, card.last_active_at + end + end + + test "reusing an existing card draft refreshes timestamps" do + existing_draft = cards(:unfinished_thoughts) + user = existing_draft.creator + board = existing_draft.board + + freeze_time do + card = user.draft_new_card_at(board) + + assert_equal existing_draft, card + assert_equal Time.current, card.created_at + assert_equal Time.current, card.updated_at + assert_equal Time.current, card.last_active_at + end + end end From 045ddf78f3517f9b983b2c68e4baf0088750b448 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Mon, 5 Jan 2026 15:58:16 +0100 Subject: [PATCH 5/7] Revert "Make sure new card drafts are refreshed when reused" This reverts commit 300801117538e57e08c1073633e1f38bf2154e17. --- app/controllers/cards_controller.rb | 2 +- app/models/user/accessor.rb | 6 ------ test/models/user/accessor_test.rb | 32 ----------------------------- 3 files changed, 1 insertion(+), 39 deletions(-) diff --git a/app/controllers/cards_controller.rb b/app/controllers/cards_controller.rb index 4b7b4748f..daf22a5bc 100644 --- a/app/controllers/cards_controller.rb +++ b/app/controllers/cards_controller.rb @@ -12,7 +12,7 @@ class CardsController < ApplicationController def create respond_to do |format| format.html do - card = Current.user.draft_new_card_at(@board) + card = @board.cards.find_or_create_by!(creator: Current.user, status: "drafted") redirect_to card end diff --git a/app/models/user/accessor.rb b/app/models/user/accessor.rb index b9744a667..a9cc5e459 100644 --- a/app/models/user/accessor.rb +++ b/app/models/user/accessor.rb @@ -11,12 +11,6 @@ module User::Accessor after_create_commit :grant_access_to_boards, unless: :system? end - def draft_new_card_at(board) - board.cards.find_or_initialize_by(creator: self, status: "drafted").tap do |card| - card.update!(created_at: Time.current, updated_at: Time.current, last_active_at: Time.current) - end - end - private def grant_access_to_boards Access.insert_all account.boards.all_access.pluck(:id).collect { |board_id| { id: ActiveRecord::Type::Uuid.generate, board_id: board_id, user_id: id, account_id: account.id } } diff --git a/test/models/user/accessor_test.rb b/test/models/user/accessor_test.rb index 300c2c741..95cb84edb 100644 --- a/test/models/user/accessor_test.rb +++ b/test/models/user/accessor_test.rb @@ -12,36 +12,4 @@ class User::AccessorTest < ActiveSupport::TestCase system_user = User.create!(account: accounts("37s"), role: "system", name: "Test System User") assert_empty system_user.boards end - - test "creating a new card draft sets current timestamps" do - user = users(:david) - board = boards(:writebook) - - freeze_time do - card = user.draft_new_card_at(board) - - assert card.persisted? - assert card.drafted? - assert_equal user, card.creator - assert_equal board, card.board - assert_equal Time.current, card.created_at - assert_equal Time.current, card.updated_at - assert_equal Time.current, card.last_active_at - end - end - - test "reusing an existing card draft refreshes timestamps" do - existing_draft = cards(:unfinished_thoughts) - user = existing_draft.creator - board = existing_draft.board - - freeze_time do - card = user.draft_new_card_at(board) - - assert_equal existing_draft, card - assert_equal Time.current, card.created_at - assert_equal Time.current, card.updated_at - assert_equal Time.current, card.last_active_at - end - end end From 5f390f72dfaa7b12a174b82f5f22cb4022d0f34c Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Mon, 5 Jan 2026 13:23:32 +0100 Subject: [PATCH 6/7] Support local installations where the app is loaded over HTTP In this case requests won't be performed from a secure context [1] and the browser won't send the Sec-Fetch-Site header. This means non-GET requests will be rejected because CSRF protection will fail. With this change, we allow these requests with missing Sec-Fetch-Site headers if: - They happen over HTTP - The app is not configured to force SSL The Origin check happens in any case. [1] https://developer.mozilla.org/en-US/docs/Web/Security/Defenses/Secure_Contexts#potentially_trustworthy_origins --- .../concerns/request_forgery_protection.rb | 26 ++--- .../request_forgery_protection_test.rb | 110 +++++------------- 2 files changed, 36 insertions(+), 100 deletions(-) diff --git a/app/controllers/concerns/request_forgery_protection.rb b/app/controllers/concerns/request_forgery_protection.rb index c8481beb3..fdff7ba8f 100644 --- a/app/controllers/concerns/request_forgery_protection.rb +++ b/app/controllers/concerns/request_forgery_protection.rb @@ -2,31 +2,19 @@ module RequestForgeryProtection extend ActiveSupport::Concern included do - after_action :append_sec_fetch_site_to_vary_header + protect_from_forgery using: :header_only, with: :exception end private - def append_sec_fetch_site_to_vary_header - vary_header = response.headers["Vary"].to_s.split(",").map(&:strip).reject(&:blank?) - response.headers["Vary"] = (vary_header + [ "Sec-Fetch-Site" ]).join(",") + def verified_via_header_only? + super || allowed_api_request? || allowed_insecure_context_request? end - def verified_request? - request.get? || request.head? || !protect_against_forgery? || - (valid_request_origin? && safe_fetch_site?) + def allowed_api_request? + sec_fetch_site_value.nil? && request.format.json? end - SAFE_FETCH_SITES = %w[ same-origin same-site ] - - def safe_fetch_site? - SAFE_FETCH_SITES.include?(sec_fetch_site_value) || (sec_fetch_site_value.nil? && api_request?) - end - - def api_request? - request.format.json? - end - - def sec_fetch_site_value - request.headers["Sec-Fetch-Site"].to_s.downcase.presence + def allowed_insecure_context_request? + sec_fetch_site_value.nil? && !request.ssl? && !Rails.configuration.force_ssl end end diff --git a/test/controllers/concerns/request_forgery_protection_test.rb b/test/controllers/concerns/request_forgery_protection_test.rb index 233c1e361..db1a84e63 100644 --- a/test/controllers/concerns/request_forgery_protection_test.rb +++ b/test/controllers/concerns/request_forgery_protection_test.rb @@ -6,83 +6,16 @@ class RequestForgeryProtectionTest < ActionDispatch::IntegrationTest @original_allow_forgery_protection = ActionController::Base.allow_forgery_protection ActionController::Base.allow_forgery_protection = true + + @original_force_ssl = Rails.configuration.force_ssl end teardown do ActionController::Base.allow_forgery_protection = @original_allow_forgery_protection + Rails.configuration.force_ssl = @original_force_ssl end - test "fails if Sec-Fetch-Site is cross-site" do - assert_no_difference -> { Board.count } do - post boards_path, - params: { board: { name: "Test Board" } }, - headers: { "Sec-Fetch-Site" => "cross-site" } - end - - assert_response :unprocessable_entity - end - - test "succeeds with same-origin Sec-Fetch-Site" do - assert_difference -> { Board.count }, +1 do - post boards_path, - params: { board: { name: "Test Board" } }, - headers: { "Sec-Fetch-Site" => "same-origin" } - end - - assert_response :redirect - end - - test "succeeds with same-site Sec-Fetch-Site" do - assert_difference -> { Board.count }, +1 do - post boards_path, - params: { board: { name: "Test Board" } }, - headers: { "Sec-Fetch-Site" => "same-site" } - end - - assert_response :redirect - end - - test "fails with none Sec-Fetch-Site" do - assert_no_difference -> { Board.count } do - post boards_path, - params: { board: { name: "Test Board" } }, - headers: { "Sec-Fetch-Site" => "none" } - end - - assert_response :unprocessable_entity - end - - test "fails when Sec-Fetch-Site header is missing" do - assert_no_difference -> { Board.count } do - post boards_path, params: { board: { name: "Test Board" } } - end - - assert_response :unprocessable_entity - end - - test "GET requests succeed regardless of Sec-Fetch-Site header" do - get board_path(boards(:writebook)), headers: { "Sec-Fetch-Site" => "cross-site" } - - assert_response :success - end - - test "appends Sec-Fetch-Site to Vary header on GET requests" do - get board_path(boards(:writebook)) - - assert_response :success - assert_includes response.headers["Vary"], "Sec-Fetch-Site" - end - - test "appends Sec-Fetch-Site to Vary header on POST requests" do - post boards_path, - params: { board: { name: "Test Board" } }, - headers: { "Sec-Fetch-Site" => "same-origin" } - - assert_response :redirect - assert_includes response.headers["Vary"], "Sec-Fetch-Site" - end - - test "JSON request succeeds with missing Sec-Fetch-Site" do + test "JSON request succeeds with missing Sec-Fetch-Site header" do assert_difference -> { Board.count }, +1 do post boards_path, params: { board: { name: "Test Board" } }, @@ -92,22 +25,37 @@ class RequestForgeryProtectionTest < ActionDispatch::IntegrationTest assert_response :created end - test "JSON request fails with cross-site Sec-Fetch-Site" do + test "HTTP request succeeds with missing Sec-Fetch-Site header when force_ssl is disabled" do + Rails.configuration.force_ssl = false + + assert_difference -> { Board.count }, +1 do + post boards_path, + params: { board: { name: "Test Board" } } + end + + assert_response :redirect + end + + test "HTTP request fails with missing Sec-Fetch-Site header when force_ssl is enabled" do + Rails.configuration.force_ssl = true + assert_no_difference -> { Board.count } do post boards_path, - params: { board: { name: "Test Board" } }, - headers: { "Sec-Fetch-Site" => "cross-site" }, - as: :json + params: { board: { name: "Test Board" } } end assert_response :unprocessable_entity end - private - def csrf_token - @csrf_token ||= begin - get new_board_path - response.body[/name="authenticity_token" value="([^"]+)"/, 1] - end + test "HTTPS request fails with missing Sec-Fetch-Site header" do + Rails.configuration.force_ssl = false + + assert_no_difference -> { Board.count } do + post boards_path, + params: { board: { name: "Test Board" } }, + headers: { "X-Forwarded-Proto" => "https" } end + + assert_response :unprocessable_entity + end end From 2ee7af6fdebaf306688543d7db3efc622eb339d7 Mon Sep 17 00:00:00 2001 From: Andy Smith Date: Mon, 5 Jan 2026 15:00:36 -0600 Subject: [PATCH 7/7] Only hide the blank slate when there are no cards present --- app/assets/stylesheets/blank-slates.css | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/blank-slates.css b/app/assets/stylesheets/blank-slates.css index 0725dd336..3e0e7615c 100644 --- a/app/assets/stylesheets/blank-slates.css +++ b/app/assets/stylesheets/blank-slates.css @@ -24,12 +24,12 @@ border: 2px dashed var(--color-ink-light); border-radius: 1ch; color: var(--color-ink-dark); - margin-block-start: 5dvh; + margin-block-start: 2dvh; padding: 1ch 2ch; rotate: -3deg; font-weight: 500; - .cards:not(.cards--grid) & { + .cards:has(.card) & { display: none; } }