feat(api): expose valuation history index (#1596)

* feat(api): expose valuation history index

* fix(api): hide valuation exception details

* fix(api): reuse eager-loaded valuation entries

* fix(api): tighten valuation index contracts

* fix(api): scope valuation filter errors

* docs(api): nest valuation account filter format

* Fix merge conflict mistakes

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <jjmata@jjmata.com>

app/controllers/api/v1/valuations_controller.rb

@@ -1,10 +1,47 @@
 # frozen_string_literal: true
 
 class Api::V1::ValuationsController < Api::V1::BaseController
-  before_action :ensure_read_scope, only: [ :show ]
+  include Pagy::Backend
+
+  InvalidFilterError = Class.new(StandardError)
+
+  before_action :ensure_read_scope, only: [ :index, :show ]
   before_action :ensure_write_scope, only: [ :create, :update ]
   before_action :set_valuation, only: [ :show, :update ]
 
+  def index
+    family = current_resource_owner.family
+    accessible_account_ids = family.accounts.accessible_by(current_resource_owner).select(:id)
+    valuations_query = family.entries
+      .where(entryable_type: "Valuation", account_id: accessible_account_ids)
+      .includes(:account, :entryable)
+
+    valuations_query = apply_filters(valuations_query).reverse_chronological
+    @per_page = safe_per_page_param
+
+    @pagy, @entries = pagy(
+      valuations_query,
+      page: safe_page_param,
+      limit: @per_page
+    )
+
+    render :index
+  rescue InvalidFilterError => e
+    render json: {
+      error: "validation_failed",
+      message: e.message,
+      errors: [ e.message ]
+    }, status: :unprocessable_entity
+  rescue => e
+    Rails.logger.error "ValuationsController#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
     render :show
   rescue => e
@@ -208,6 +245,43 @@ class Api::V1::ValuationsController < Api::V1::BaseController
       authorize_scope!(:write)
     end
 
+    def apply_filters(query)
+      if params[:account_id].present?
+        raise InvalidFilterError, "account_id must be a valid UUID" unless valid_uuid?(params[:account_id])
+
+        query = query.where(account_id: params[:account_id])
+      end
+      query = query.where("entries.date >= ?", parse_date_param(:start_date)) if params[:start_date].present?
+      query = query.where("entries.date <= ?", parse_date_param(:end_date)) if params[:end_date].present?
+      query
+    end
+
+    def parse_date_param(key)
+      Date.iso8601(params[key].to_s)
+    rescue ArgumentError
+      raise InvalidFilterError, "#{key} must be an ISO 8601 date"
+    end
+
+    def valid_uuid?(value)
+      value.to_s.match?(/\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i)
+    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
+      end
+    end
+
     def valuation_account_id
       params.dig(:valuation, :account_id)
     end

app/views/api/v1/valuations/_valuation.json.jbuilder

@@ -1,17 +1,19 @@
 # frozen_string_literal: true
 
-json.id valuation.entry.id
-json.date valuation.entry.date
-json.amount valuation.entry.amount_money.format
-json.currency valuation.entry.currency
-json.notes valuation.entry.notes
+entry = local_assigns[:entry] || valuation.entry
+
+json.id entry.id
+json.date entry.date
+json.amount entry.amount_money.format
+json.currency entry.currency
+json.notes entry.notes
 json.kind valuation.kind
 
 # Account information
 json.account do
-  json.id valuation.entry.account.id
-  json.name valuation.entry.account.name
-  json.account_type valuation.entry.account.accountable_type.underscore
+  json.id entry.account.id
+  json.name entry.account.name
+  json.account_type entry.account.accountable_type.underscore
 end
 
 # Additional metadata

app/views/api/v1/valuations/index.json.jbuilder

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+json.valuations @entries do |entry|
+  json.partial! "valuation", valuation: entry.entryable, entry: entry
+end
+
+json.pagination do
+  json.page @pagy.page
+  json.per_page @per_page
+  json.total_count @pagy.count
+  json.total_pages @pagy.pages
+end

config/routes.rb

@@ -427,7 +427,7 @@ Rails.application.routes.draw do
       resources :transactions, only: [ :index, :show, :create, :update, :destroy ]
       resources :trades, only: [ :index, :show, :create, :update, :destroy ]
       resources :holdings, only: [ :index, :show ]
-      resources :valuations, only: [ :create, :update, :show ]
+      resources :valuations, only: [ :index, :create, :update, :show ]
       resources :imports, only: [ :index, :show, :create ]
       resource :usage, only: [ :show ], controller: :usage
       resource :balance_sheet, only: [ :show ], controller: :balance_sheet

docs/api/openapi.yaml

@@ -579,6 +579,18 @@ components:
         updated_at:
           type: string
           format: date-time
+    ValuationCollection:
+      type: object
+      required:
+      - valuations
+      - pagination
+      properties:
+        valuations:
+          type: array
+          items:
+            "$ref": "#/components/schemas/Valuation"
+        pagination:
+          "$ref": "#/components/schemas/Pagination"
     DeleteResponse:
       type: object
       required:
@@ -2952,19 +2964,72 @@ paths:
               schema:
                 "$ref": "#/components/schemas/ErrorResponse"
   "/api/v1/valuations":
+    get:
+      summary: List valuations
+      tags:
+      - Valuations
+      security:
+      - apiKeyAuth: []
+      parameters:
+      - name: page
+        in: query
+        required: false
+        description: 'Page number (default: 1)'
+        schema:
+          type: integer
+      - name: per_page
+        in: query
+        required: false
+        description: 'Items per page (default: 25, max: 100)'
+        schema:
+          type: integer
+      - name: account_id
+        in: query
+        required: false
+        description: Filter by account ID
+        schema:
+          type: string
+          format: uuid
+      - name: start_date
+        in: query
+        required: false
+        description: Filter valuations from this date
+        schema:
+          type: string
+          format: date
+      - name: end_date
+        in: query
+        required: false
+        description: Filter valuations until this date
+        schema:
+          type: string
+          format: date
+      responses:
+        '200':
+          description: valuations listed
+          content:
+            application/json:
+              schema:
+                "$ref": "#/components/schemas/ValuationCollection"
+        '401':
+          description: unauthorized
+          content:
+            application/json:
+              schema:
+                "$ref": "#/components/schemas/ErrorResponse"
+        '422':
+          description: invalid account filter
+          content:
+            application/json:
+              schema:
+                "$ref": "#/components/schemas/ErrorResponse"
     post:
       summary: Create valuation
       tags:
       - Valuations
       security:
       - apiKeyAuth: []
-      parameters:
-      - name: Authorization
-        in: header
-        required: true
-        schema:
-          type: string
-        description: Bearer token with write scope
+      parameters: []
       responses:
         '201':
           description: valuation created
@@ -3016,12 +3081,6 @@ paths:
         required: true
   "/api/v1/valuations/{id}":
     parameters:
-    - name: Authorization
-      in: header
-      required: true
-      schema:
-        type: string
-      description: Bearer token
     - name: id
       in: path
       required: true

spec/requests/api/v1/valuations_spec.rb

@@ -20,25 +20,18 @@ RSpec.describe 'API V1 Valuations', type: :request do
     )
   end
 
-  let(:oauth_application) do
-    Doorkeeper::Application.create!(
-      name: 'API Docs',
-      redirect_uri: 'https://example.com/callback',
-      scopes: 'read read_write'
+  let(:api_key) do
+    key = ApiKey.generate_secure_key
+    ApiKey.create!(
+      user: user,
+      name: 'API Docs Key',
+      key: key,
+      scopes: %w[read_write],
+      source: 'web'
     )
   end
 
-  let(:access_token) do
-    Doorkeeper::AccessToken.create!(
-      application: oauth_application,
-      resource_owner_id: user.id,
-      scopes: 'read_write',
-      expires_in: 2.hours,
-      token: SecureRandom.hex(32)
-    )
-  end
-
-  let(:Authorization) { "Bearer #{access_token.token}" }
+  let(:'X-Api-Key') { api_key.plain_key }
 
   let(:account) do
     Account.create!(
@@ -66,13 +59,59 @@ RSpec.describe 'API V1 Valuations', type: :request do
   let!(:valuation_id) { valuation_entry.id }
 
   path '/api/v1/valuations' do
+    get 'List valuations' do
+      tags 'Valuations'
+      security [ { apiKeyAuth: [] } ]
+      produces 'application/json'
+      parameter name: :page, in: :query, type: :integer, required: false,
+                description: 'Page number (default: 1)'
+      parameter name: :per_page, in: :query, type: :integer, required: false,
+                description: 'Items per page (default: 25, max: 100)'
+      parameter name: :account_id, in: :query, required: false, description: 'Filter by account ID',
+                schema: { type: :string, format: :uuid }
+      parameter name: :start_date, in: :query, required: false,
+                description: 'Filter valuations from this date',
+                schema: { type: :string, format: :date }
+      parameter name: :end_date, in: :query, required: false,
+                description: 'Filter valuations until this date',
+                schema: { type: :string, format: :date }
+
+      response '200', 'valuations listed' do
+        schema '$ref' => '#/components/schemas/ValuationCollection'
+
+        run_test!
+      end
+
+      response '401', 'unauthorized' do
+        schema '$ref' => '#/components/schemas/ErrorResponse'
+
+        let(:'X-Api-Key') { nil }
+
+        run_test!
+      end
+
+      response '422', 'invalid date filter' do
+        schema '$ref' => '#/components/schemas/ErrorResponse'
+
+        let(:start_date) { 'not-a-date' }
+
+        run_test!
+      end
+
+      response '422', 'invalid account filter' do
+        schema '$ref' => '#/components/schemas/ErrorResponse'
+
+        let(:account_id) { 'not-a-uuid' }
+
+        run_test!
+      end
+    end
+
     post 'Create valuation' do
       tags 'Valuations'
       security [ { apiKeyAuth: [] } ]
       consumes 'application/json'
       produces 'application/json'
-      parameter name: :Authorization, in: :header, required: true, schema: { type: :string },
-                description: 'Bearer token with write scope'
       parameter name: :body, in: :body, required: true, schema: {
         type: :object,
         properties: {
@@ -170,8 +209,6 @@ RSpec.describe 'API V1 Valuations', type: :request do
   end
 
   path '/api/v1/valuations/{id}' do
-    parameter name: :Authorization, in: :header, required: true, schema: { type: :string },
-              description: 'Bearer token'
     parameter name: :id, in: :path, type: :string, required: true, description: 'Valuation ID (entry ID)'
 
     get 'Retrieve a valuation' do

spec/swagger_helper.rb

@@ -364,6 +364,17 @@ RSpec.configure do |config|
               updated_at: { type: :string, format: :'date-time' }
             }
           },
+          ValuationCollection: {
+            type: :object,
+            required: %w[valuations pagination],
+            properties: {
+              valuations: {
+                type: :array,
+                items: { '$ref' => '#/components/schemas/Valuation' }
+              },
+              pagination: { '$ref' => '#/components/schemas/Pagination' }
+            }
+          },
           DeleteResponse: {
             type: :object,
             required: %w[message],

test/controllers/api/v1/valuations_controller_test.rb

@@ -17,6 +17,7 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest
       user: @user,
       name: "Test Read-Write Key",
       scopes: [ "read_write" ],
+      source: "web",
       display_key: "test_rw_#{SecureRandom.hex(8)}"
     )
 
@@ -33,6 +34,92 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest
     Redis.new.del("api_rate_limit:#{@read_only_api_key.id}")
   end
 
+  # INDEX action tests
+  test "should get index with valid API key" do
+    get api_v1_valuations_url, headers: api_headers(@api_key)
+    assert_response :success
+
+    response_data = JSON.parse(response.body)
+    assert response_data.key?("valuations")
+    assert response_data.key?("pagination")
+    assert response_data["valuations"].is_a?(Array)
+    assert response_data["pagination"].key?("page")
+    assert response_data["pagination"].key?("per_page")
+    assert response_data["pagination"].key?("total_count")
+    assert response_data["pagination"].key?("total_pages")
+  end
+
+  test "should get index with read-only API key" do
+    get api_v1_valuations_url, headers: api_headers(@read_only_api_key)
+    assert_response :success
+  end
+
+  test "should filter index by account_id" do
+    get api_v1_valuations_url,
+        params: { account_id: @account.id },
+        headers: api_headers(@api_key)
+    assert_response :success
+
+    response_data = JSON.parse(response.body)
+    response_data["valuations"].each do |valuation|
+      assert_equal @account.id, valuation["account"]["id"]
+    end
+  end
+
+  test "should filter index by date range" do
+    entry = @valuation.entry
+
+    get api_v1_valuations_url,
+        params: { start_date: entry.date, end_date: entry.date },
+        headers: api_headers(@api_key)
+    assert_response :success
+
+    response_data = JSON.parse(response.body)
+    assert_includes response_data["valuations"].map { |valuation| valuation["id"] }, entry.id
+    response_data["valuations"].each do |valuation|
+      valuation_date = Date.iso8601(valuation["date"])
+      assert_equal entry.date, valuation_date
+    end
+  end
+
+  test "should reject index with invalid date filter" do
+    get api_v1_valuations_url,
+        params: { start_date: "04/30/2026" },
+        headers: api_headers(@api_key)
+    assert_response :unprocessable_entity
+
+    response_data = JSON.parse(response.body)
+    assert_equal "validation_failed", response_data["error"]
+  end
+
+  test "should reject index with malformed account_id filter" do
+    get api_v1_valuations_url,
+        params: { account_id: "not-a-uuid" },
+        headers: api_headers(@api_key)
+    assert_response :unprocessable_entity
+
+    response_data = JSON.parse(response.body)
+    assert_equal "validation_failed", response_data["error"]
+    assert_equal "account_id must be a valid UUID", response_data["message"]
+  end
+
+  test "should not expose internal index errors" do
+    Api::V1::ValuationsController.any_instance.stubs(:safe_page_param).raises(StandardError, "database password leaked")
+
+    get api_v1_valuations_url, headers: api_headers(@api_key)
+    assert_response :internal_server_error
+
+    response_data = JSON.parse(response.body)
+    assert_equal "internal_server_error", response_data["error"]
+    assert_equal "An unexpected error occurred", response_data["message"]
+    assert_not_includes response.body, "database password leaked"
+  end
+
+  test "should reject index without API key" do
+    get api_v1_valuations_url
+    assert_response :unauthorized
+  end
+
   # CREATE action tests
   test "should create valuation with valid parameters" do
     valuation_params = {
@@ -207,6 +294,6 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest
   private
 
     def api_headers(api_key)
-      { "X-Api-Key" => api_key.display_key }
+      { "X-Api-Key" => api_key.plain_key }
     end
 end