Files
sure/app/controllers/api/v1/transactions_controller.rb
William Wei Ming 34dd5fbc62 update transactions_controller (#1953)
* update transactions_controller

* fix FEEDBACK - Remove DISTINCT that breaks tag-filtered index queries

* fix FEEBACK from jjmata

* Ignore Brakeman EOLRails warning for Rails 7.2
Restore fingerprint-scoped ignore lost during merge from main.

* update transactions_controller

* fix FEEDBACK - Remove DISTINCT that breaks tag-filtered index queries

* fix FEEBACK from jjmata

* Ignore Brakeman EOLRails warning for Rails 7.2
Restore fingerprint-scoped ignore lost during merge from main.

* resolve review - Add .distinct to the tag filtering subquery

* fix(ci): skip scheduled preview cleanup on forks
Only run the hourly Cloudflare preview cleanup on we-promise/sure,
where the required secrets exist.

* Drop obsolete Rails EOL note and Brakeman EOLRails ignore

The EOLRails ignore and the accompanying migration-rule note were only
needed while the app ran Rails 7.2.3.1, whose support window closed
2026-08-09. Main has since moved to Rails 8.1.3, so the check no longer
warns and both changes are dead weight that only widen this PR's diff.

Keeps the PR focused on the transactions controller query optimization.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-07-26 01:42:18 +02:00

449 lines
13 KiB
Ruby

# frozen_string_literal: true
class Api::V1::TransactionsController < Api::V1::BaseController
include Pagy::Backend
# Ensure proper scope authorization for read vs write access
before_action :ensure_read_scope, only: [ :index, :show ]
before_action :ensure_write_scope, only: [ :create, :update, :destroy ]
before_action :set_transaction, only: [ :show, :update, :destroy ]
def index
family = current_resource_owner.family
accessible_account_ids = family.accounts
.accessible_by(current_resource_owner)
.where.not(status: "pending_deletion")
.select(:id)
transactions_query = family.transactions
.joins(:entry).where(entries: { account_id: accessible_account_ids })
# Apply filters
transactions_query = apply_filters(transactions_query)
# Apply search
transactions_query = apply_search(transactions_query) if params[:search].present?
# Include necessary associations for efficient queries
transactions_query = transactions_query.includes(
{ entry: :account },
:category, :merchant, :tags,
transfer_as_outflow: { inflow_transaction: { entry: :account } },
transfer_as_inflow: { outflow_transaction: { entry: :account } }
).reverse_chronological
# Handle pagination with Pagy
@pagy, @transactions = pagy(
transactions_query,
page: safe_page_param,
limit: safe_per_page_param
)
# Make per_page available to the template
@per_page = safe_per_page_param
# Rails will automatically use app/views/api/v1/transactions/index.json.jbuilder
render :index
rescue => e
Rails.logger.error "TransactionsController#index error: #{e.message}"
Rails.logger.error e.backtrace.join("\n")
render json: {
error: "internal_server_error",
message: "An unexpected error occurred"
}, status: :internal_server_error
end
def show
# Rails will automatically use app/views/api/v1/transactions/show.json.jbuilder
render :show
rescue => e
Rails.logger.error "TransactionsController#show error: #{e.message}"
Rails.logger.error e.backtrace.join("\n")
render json: {
error: "internal_server_error",
message: "An unexpected error occurred"
}, status: :internal_server_error
end
def create
family = current_resource_owner.family
# Validate account_id is present
unless account_id_param.present?
render json: {
error: "validation_failed",
message: "Account ID is required",
errors: [ "Account ID is required" ]
}, status: :unprocessable_entity
return
end
if idempotency_source_param.present? && idempotency_external_id.blank?
render json: {
error: "validation_failed",
message: "Source requires external_id",
errors: [ "Source requires external_id" ]
}, status: :unprocessable_entity
return
end
account = family.accounts.writable_by(current_resource_owner).find(account_id_param)
if idempotency_key_requested? && (existing_entry = existing_idempotent_entry(account))
return render_existing_idempotent_entry(existing_entry)
end
@entry = account.entries.new(entry_params_for_create)
if @entry.save
@entry.sync_account_later
@entry.lock_saved_attributes!
@entry.transaction.lock_attr!(:tag_ids) if @entry.transaction.tags.any?
@transaction = @entry.transaction
render :show, status: :created
else
render json: {
error: "validation_failed",
message: "Transaction could not be created",
errors: @entry.errors.full_messages
}, status: :unprocessable_entity
end
rescue ActiveRecord::RecordNotUnique
if idempotency_key_requested? && account && (existing_entry = existing_idempotent_entry(account))
render_existing_idempotent_entry(existing_entry)
else
raise
end
rescue => e
Rails.logger.error "TransactionsController#create error: #{e.message}"
Rails.logger.error e.backtrace.join("\n")
render json: {
error: "internal_server_error",
message: "An unexpected error occurred"
}, status: :internal_server_error
end
def update
if @entry.split_child?
render json: { error: "validation_failed", message: "Split child transactions cannot be edited directly. Use the split editor." }, status: :unprocessable_entity
return
end
if @entry.split_parent? && split_financial_fields_changed?
render json: { error: "validation_failed", message: "Split parent amount, date, and type cannot be changed directly. Use the split editor." }, status: :unprocessable_entity
return
end
Entry.transaction do
if @entry.update(entry_params_for_update)
# Handle tags separately - only when explicitly provided in the request
# This allows clearing tags with tag_ids: [] while preserving tags when not specified
if tags_provided?
@entry.transaction.tag_ids = transaction_params[:tag_ids] || []
@entry.transaction.save!
@entry.transaction.lock_attr!(:tag_ids) if @entry.transaction.tags.any?
end
@entry.sync_account_later
@entry.lock_saved_attributes!
@transaction = @entry.transaction
render :show
else
render json: {
error: "validation_failed",
message: "Transaction could not be updated",
errors: @entry.errors.full_messages
}, status: :unprocessable_entity
raise ActiveRecord::Rollback
end
end
rescue => e
Rails.logger.error "TransactionsController#update error: #{e.message}"
Rails.logger.error e.backtrace.join("\n")
render json: {
error: "internal_server_error",
message: "An unexpected error occurred"
}, status: :internal_server_error
end
def destroy
if @entry.split_child?
render json: { error: "validation_failed", message: "Split child transactions cannot be deleted individually." }, status: :unprocessable_entity
return
end
@entry.destroy!
@entry.sync_account_later
render json: {
message: "Transaction deleted successfully"
}, status: :ok
rescue => e
Rails.logger.error "TransactionsController#destroy error: #{e.message}"
Rails.logger.error e.backtrace.join("\n")
render json: {
error: "internal_server_error",
message: "An unexpected error occurred"
}, status: :internal_server_error
end
private
def set_transaction
raise ActiveRecord::RecordNotFound unless valid_uuid?(params[:id])
family = current_resource_owner.family
@transaction = family.transactions
.joins(entry: :account)
.merge(Account.accessible_by(current_resource_owner))
.find(params[:id])
@entry = @transaction.entry
rescue ActiveRecord::RecordNotFound
render json: {
error: "not_found",
message: "Transaction not found"
}, status: :not_found
end
def ensure_read_scope
authorize_scope!(:read)
end
def ensure_write_scope
authorize_scope!(:write)
end
def apply_filters(query)
# Account filtering
if params[:account_id].present?
query = query.where(entries: { account_id: params[:account_id] })
end
if params[:account_ids].present?
account_ids = Array(params[:account_ids])
query = query.where(entries: { account_id: account_ids })
end
# Category filtering
if params[:category_id].present?
query = query.where(category_id: params[:category_id])
end
if params[:category_ids].present?
category_ids = Array(params[:category_ids])
query = query.where(category_id: category_ids)
end
# Merchant filtering
if params[:merchant_id].present?
query = query.where(merchant_id: params[:merchant_id])
end
if params[:merchant_ids].present?
merchant_ids = Array(params[:merchant_ids])
query = query.where(merchant_id: merchant_ids)
end
# Date range filtering
if params[:start_date].present?
query = query.where("entries.date >= ?", Date.parse(params[:start_date]))
end
if params[:end_date].present?
query = query.where("entries.date <= ?", Date.parse(params[:end_date]))
end
# Amount filtering
if params[:min_amount].present?
min_amount = params[:min_amount].to_f
query = query.where("entries.amount >= ?", min_amount)
end
if params[:max_amount].present?
max_amount = params[:max_amount].to_f
query = query.where("entries.amount <= ?", max_amount)
end
# Tag filtering
if params[:tag_ids].present?
tag_ids = Array(params[:tag_ids])
query = query.where(
id: query.joins(:tags).where(tags: { id: tag_ids }).distinct.select(:id)
)
end
# Transaction type filtering (income/expense)
if params[:type].present?
case params[:type].downcase
when "income"
query = query.where("entries.amount < 0")
when "expense"
query = query.where("entries.amount > 0")
end
end
query
end
def apply_search(query)
search_term = "%#{params[:search]}%"
query
.left_joins(:merchant)
.where(
"entries.name ILIKE ? OR entries.notes ILIKE ? OR merchants.name ILIKE ?",
search_term, search_term, search_term
)
end
def transaction_params
params.require(:transaction).permit(
:date, :amount, :name, :description, :notes, :currency,
:category_id, :merchant_id, :nature, tag_ids: []
)
end
def account_id_param
params.dig(:transaction, :account_id).presence
end
def entry_params_for_create
entry_params = {
name: transaction_params[:name] || transaction_params[:description],
date: transaction_params[:date],
amount: calculate_signed_amount,
currency: transaction_params[:currency] || current_resource_owner.family.currency,
notes: transaction_params[:notes],
entryable_type: "Transaction",
entryable_attributes: {
category_id: transaction_params[:category_id],
merchant_id: transaction_params[:merchant_id],
tag_ids: transaction_params[:tag_ids] || []
}
}
if idempotency_key_requested?
entry_params[:external_id] = idempotency_external_id
entry_params[:source] = idempotency_source
end
entry_params.compact
end
def entry_params_for_update
entry_params = {
name: transaction_params[:name] || transaction_params[:description],
date: transaction_params[:date],
notes: transaction_params[:notes],
entryable_attributes: {
id: @entry.entryable_id,
category_id: transaction_params[:category_id],
merchant_id: transaction_params[:merchant_id]
# Note: tag_ids handled separately in update action to distinguish
# "not provided" from "explicitly set to empty"
}.compact_blank
}
# Only update amount if provided
if transaction_params[:amount].present?
entry_params[:amount] = calculate_signed_amount
end
entry_params.compact
end
# Check if tag_ids was explicitly provided in the request.
# This distinguishes between "user wants to update tags" vs "user didn't specify tags".
def tags_provided?
params[:transaction].key?(:tag_ids)
end
def split_financial_fields_changed?
params.dig(:transaction, :amount).present? ||
params.dig(:transaction, :date).present? ||
params.dig(:transaction, :nature).present?
end
def idempotency_key_requested?
idempotency_external_id.present?
end
def idempotency_external_id
idempotency_param_value(:external_id)
end
def idempotency_source
idempotency_source_param.presence || "api"
end
def idempotency_source_param
idempotency_param_value(:source)
end
def idempotency_param_value(key)
value = params.dig(:transaction, key)
value.to_s.presence if value.is_a?(String) || value.is_a?(Numeric)
end
def existing_idempotent_entry(account)
account.entries.find_by(
external_id: idempotency_external_id,
source: idempotency_source
)
end
def render_existing_idempotent_entry(entry)
unless entry.entryable.is_a?(Transaction)
render json: {
error: "validation_failed",
message: "External ID already exists for a non-transaction entry",
errors: [ "External ID already exists for a non-transaction entry" ]
}, status: :unprocessable_entity
return
end
@entry = entry
@transaction = entry.transaction
render :show, status: :ok
end
def calculate_signed_amount
amount = transaction_params[:amount].to_f
nature = transaction_params[:nature]
case nature&.downcase
when "income", "inflow"
-amount.abs # Income is negative
when "expense", "outflow"
amount.abs # Expense is positive
else
amount # Use as provided
end
end
def safe_page_param
page = params[:page].to_i
page > 0 ? page : 1
end
def safe_per_page_param
per_page = params[:per_page].to_i
case per_page
when 1..100
per_page
else
25 # Default
end
end
end