diff --git a/CHANGES b/CHANGES index fbde0100c..7054c1c01 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,8 @@ Changelog Unreleased ---------- +Holdings reports now value positions at the time-filter end date, matching +account reports, instead of using later prices from outside the filter. A Finnish translation was added. The date filter parser was rewritten from a regex-based implementation to a proper lexer/parser, fixing several edge-case bugs. As part of this, the undocumented `to` range separator (e.g. `2010 to diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index ea4a8c2d8..1b66c5dcd 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -61,6 +61,7 @@ type GetEndpoint = | "events" | "extract" | "help" + | "holdings" | "imports" | "income_statement" | "journal_page" @@ -89,6 +90,7 @@ type ApiEndpoint = DeleteEndpoint | GetEndpoint | PutEndpoint; type ApiParams = Partial<{ a: string; account: string; + aggregation_key: string; conversion: string; entry_hash: string; filename: string; @@ -272,6 +274,14 @@ export const get_help = define_endpoint( object({ html: string, pages: array(tuple(string, string)) }), ["page_slug"], ); +export const get_holdings = define_endpoint( + "holdings", + object({ + query_string: string, + query_result_table: query_validator, + }), + [...filters, "aggregation_key"], +); export const get_imports = define_paramless_endpoint( "imports", importable_files_validator, diff --git a/frontend/src/reports/holdings/index.ts b/frontend/src/reports/holdings/index.ts index 383b6ad97..cf9e2afc3 100644 --- a/frontend/src/reports/holdings/index.ts +++ b/frontend/src/reports/holdings/index.ts @@ -1,4 +1,4 @@ -import { get_query } from "../../api/index.ts"; +import { get_holdings } from "../../api/index.ts"; import { get_url_path } from "../../helpers.ts"; import { _ } from "../../i18n.ts"; import { get_url_filters } from "../../stores/filters.ts"; @@ -17,56 +17,6 @@ const to_report_type = (s: string | null): HoldingsReportType => ? s : "all"; -const QUERIES = { - all: ` -SELECT - account, - units(sum(position)) as units, - cost_number as cost, - first(getprice(currency, cost_currency)) as price, - cost(sum(position)) as book_value, - value(sum(position)) as market_value, - safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct, - cost_date as acquisition_date -WHERE account_sortkey(account) ~ "^[01]" -GROUP BY account, cost_date, currency, cost_currency, cost_number, account_sortkey(account) -ORDER BY account_sortkey(account), currency, cost_date -`.trim(), - by_account: ` -SELECT - account, - units(sum(position)) as units, - cost(sum(position)) as book_value, - value(sum(position)) as market_value, - safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct -WHERE account_sortkey(account) ~ "^[01]" -GROUP BY account, cost_currency, account_sortkey(account), currency -ORDER BY account_sortkey(account), currency -`.trim(), - by_currency: ` -SELECT - units(sum(position)) as units, - safediv(number(only(first(cost_currency), cost(sum(position)))), number(only(first(currency), units(sum(position))))) as average_cost, - first(getprice(currency, cost_currency)) as price, - cost(sum(position)) as book_value, - value(sum(position)) as market_value, - safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct -WHERE account_sortkey(account) ~ "^[01]" -GROUP BY currency, cost_currency -ORDER BY currency, cost_currency -`.trim(), - by_cost_currency: ` -SELECT - units(sum(position)) as units, - cost(sum(position)) as book_value, - value(sum(position)) as market_value, - safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct -WHERE account_sortkey(account) ~ "^[01]" -GROUP BY cost_currency -ORDER BY cost_currency -`.trim(), -}; - export interface HoldingsReportProps { aggregation_key: HoldingsReportType; query_string: string; @@ -79,9 +29,8 @@ export const holdings = new Route( async (url) => { const [, key = ""] = get_url_path(url).unwrap().split("/"); const aggregation_key = to_report_type(key); - const query_string = QUERIES[aggregation_key]; - const query_result_table = await get_query({ - query_string, + const { query_string, query_result_table } = await get_holdings({ + aggregation_key, ...get_url_filters(url), }); if (query_result_table.t !== "table") { diff --git a/src/fava/core/holdings.py b/src/fava/core/holdings.py new file mode 100644 index 000000000..93b9f375c --- /dev/null +++ b/src/fava/core/holdings.py @@ -0,0 +1,92 @@ +"""Holdings report queries. + +Beanquery's ``value()`` and ``getprice()`` use the latest price when no date +is given. Holdings queries run against ``entries_with_all_prices``, which +adds prices from outside the time filter back in, so an undated call would +value lots at future prices. Pin both functions to the filter end date +(the same date account reports use). +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: # pragma: no cover + import datetime + +_UNREALIZED_PROFIT_PCT = ( + "safediv((abs(sum(number(value(position{date}))))" + " - abs(sum(number(cost(position)))))," + " sum(number(cost(position)))) * 100 as unrealized_profit_pct" +) +_AVERAGE_COST = ( + "safediv(number(only(first(cost_currency), cost(sum(position))))," + " number(only(first(currency), units(sum(position))))) as average_cost" +) + +HOLDINGS_QUERIES = { + "all": f""" +SELECT + account, + units(sum(position)) as units, + cost_number as cost, + first(getprice(currency, cost_currency{{date}})) as price, + cost(sum(position)) as book_value, + value(sum(position){{date}}) as market_value, + {_UNREALIZED_PROFIT_PCT}, + cost_date as acquisition_date +WHERE account_sortkey(account) ~ "^[01]" +GROUP BY account, cost_date, currency, cost_currency, cost_number, + account_sortkey(account) +ORDER BY account_sortkey(account), currency, cost_date +""".strip(), + "by_account": f""" +SELECT + account, + units(sum(position)) as units, + cost(sum(position)) as book_value, + value(sum(position){{date}}) as market_value, + {_UNREALIZED_PROFIT_PCT} +WHERE account_sortkey(account) ~ "^[01]" +GROUP BY account, cost_currency, account_sortkey(account), currency +ORDER BY account_sortkey(account), currency +""".strip(), + "by_currency": f""" +SELECT + units(sum(position)) as units, + {_AVERAGE_COST}, + first(getprice(currency, cost_currency{{date}})) as price, + cost(sum(position)) as book_value, + value(sum(position){{date}}) as market_value, + {_UNREALIZED_PROFIT_PCT} +WHERE account_sortkey(account) ~ "^[01]" +GROUP BY currency, cost_currency +ORDER BY currency, cost_currency +""".strip(), + "by_cost_currency": f""" +SELECT + units(sum(position)) as units, + cost(sum(position)) as book_value, + value(sum(position){{date}}) as market_value, + {_UNREALIZED_PROFIT_PCT} +WHERE account_sortkey(account) ~ "^[01]" +GROUP BY cost_currency +ORDER BY cost_currency +""".strip(), +} + + +def holdings_query( + aggregation_key: str, end_date: datetime.date | None +) -> str: + """Return the holdings BQL, pinning prices to ``end_date`` when set. + + Args: + aggregation_key: One of the keys in ``HOLDINGS_QUERIES``. + end_date: Inclusive date to value at, or ``None`` for the latest price. + + Returns: + The BQL string for the holdings report. + """ + date_arg = f", {end_date.isoformat()}" if end_date is not None else "" + return HOLDINGS_QUERIES[aggregation_key].format(date=date_arg) diff --git a/src/fava/json_api.py b/src/fava/json_api.py index 5c529cb00..74443b42e 100644 --- a/src/fava/json_api.py +++ b/src/fava/json_api.py @@ -41,6 +41,8 @@ from fava.core.file import get_entry_slice from fava.core.filters import FilterError from fava.core.group_entries import group_entries_by_type +from fava.core.holdings import HOLDINGS_QUERIES +from fava.core.holdings import holdings_query from fava.core.ingest import filepath_in_primary_imports_folder from fava.core.misc import align from fava.helpers import FavaAPIError @@ -169,6 +171,15 @@ def __init__(self, filename: str) -> None: super().__init__(f"Not a file: '{filename}'") +class UnknownHoldingsAggregationKeyError(ValidationError): + """Unknown holdings aggregation key.""" + + def __init__(self, aggregation_key: str) -> None: + super().__init__( + f"unknown holdings aggregation key: `{aggregation_key}`" + ) + + @json_api.errorhandler(FavaAPIError) def _(error: FavaAPIError) -> Response: log.error("Encountered FavaAPIError.", exc_info=error) @@ -339,6 +350,27 @@ def get_query(query_string: str) -> QueryResultTable | QueryResultText: ) +class HoldingsReport(Struct, frozen=True): + """Data for the holdings report.""" + + query_string: str + query_result_table: QueryResultTable | QueryResultText + + +@api_endpoint +def get_holdings(aggregation_key: str = "all") -> HoldingsReport: + """Get the holdings report, valued at the time-filter end date.""" + if aggregation_key not in HOLDINGS_QUERIES: + raise UnknownHoldingsAggregationKeyError(aggregation_key) + query_string = holdings_query(aggregation_key, g.filtered.end_date) + return HoldingsReport( + query_string, + g.ledger.query_shell.execute_query_serialised( + g.filtered.entries_with_all_prices, query_string + ), + ) + + @api_endpoint def get_extract(filename: str, importer: str) -> Sequence[object]: """Extract entries using the ingest framework.""" diff --git a/tests/test_core_holdings.py b/tests/test_core_holdings.py new file mode 100644 index 000000000..2f0c81114 --- /dev/null +++ b/tests/test_core_holdings.py @@ -0,0 +1,91 @@ +from __future__ import annotations + +from datetime import date +from decimal import Decimal +from textwrap import dedent +from typing import TYPE_CHECKING + +import pytest + +from fava.core import FavaLedger +from fava.core.holdings import HOLDINGS_QUERIES +from fava.core.holdings import holdings_query +from fava.core.query import QueryResultTable + +if TYPE_CHECKING: # pragma: no cover + from pathlib import Path + + +@pytest.mark.parametrize("aggregation_key", list(HOLDINGS_QUERIES)) +def test_holdings_query_pins_price_functions(aggregation_key: str) -> None: + query = holdings_query(aggregation_key, date(2017, 8, 31)) + assert "value(sum(position), 2017-08-31)" in query + assert "value(position, 2017-08-31)" in query + if "getprice" in HOLDINGS_QUERIES[aggregation_key]: + assert "getprice(currency, cost_currency, 2017-08-31)" in query + + +@pytest.mark.parametrize("aggregation_key", list(HOLDINGS_QUERIES)) +def test_holdings_query_omits_date_without_filter( + aggregation_key: str, +) -> None: + query = holdings_query(aggregation_key, None) + assert "value(sum(position), " not in query + assert "value(position, " not in query + assert "getprice(currency, cost_currency, " not in query + assert "value(sum(position))" in query + assert "value(position)" in query + + +def _account_market_value( + table: QueryResultTable, account: str +) -> dict[str, Decimal]: + names = [col.name for col in table.types] + acc_i = names.index("account") + val_i = names.index("market_value") + row = next(r for r in table.rows if r[acc_i] == account) + value = row[val_i] + assert isinstance(value, dict) + return value + + +def test_holdings_values_at_filter_end_date(tmp_path: Path) -> None: + """Undated value() uses later prices; holdings must pin the filter end.""" + ledger_path = tmp_path / "prices.beancount" + ledger_path.write_text( + dedent("""\ + option "title" "Holdings Prices" + option "operating_currency" "USD" + + 2020-01-01 open Assets:Broker + 2020-01-01 open Assets:Cash USD + 2020-01-01 commodity STOCK + + 2020-06-01 * "Buy" + Assets:Broker 10 STOCK {10.00 USD} + Assets:Cash -100.00 USD + + 2020-06-30 price STOCK 12.00 USD + 2020-09-30 price STOCK 20.00 USD + """) + ) + ledger = FavaLedger(str(ledger_path)) + filtered = ledger.get_filtered(time="2020-06") + assert filtered.end_date == date(2020, 6, 30) + + undated = ledger.query_shell.execute_query_serialised( + filtered.entries_with_all_prices, + holdings_query("by_account", None), + ) + dated = ledger.query_shell.execute_query_serialised( + filtered.entries_with_all_prices, + holdings_query("by_account", filtered.end_date), + ) + assert isinstance(undated, QueryResultTable) + assert isinstance(dated, QueryResultTable) + assert _account_market_value(undated, "Assets:Broker") == { + "USD": Decimal("200.00") + } + assert _account_market_value(dated, "Assets:Broker") == { + "USD": Decimal("120.00") + } diff --git a/tests/test_json_api.py b/tests/test_json_api.py index 010151ef5..b7bd9ee1e 100644 --- a/tests/test_json_api.py +++ b/tests/test_json_api.py @@ -1012,6 +1012,54 @@ def test_api_filter_error( assert_api_error(response, status=HTTPStatus.BAD_REQUEST) +@pytest.mark.parametrize( + "aggregation_key", + ["all", "by_account", "by_currency", "by_cost_currency"], +) +def test_api_holdings_query_string_includes_end_date( + test_client: FlaskClient, + aggregation_key: str, +) -> None: + data = assert_api_success( + test_client.get( + "/long-example/api/holdings", + query_string={ + "aggregation_key": aggregation_key, + "time": "2017-08", + }, + ) + ) + assert isinstance(data, dict) + assert "value(sum(position), 2017-08-31)" in data["query_string"] + table = data["query_result_table"] + assert isinstance(table, dict) + assert table["t"] == "table" + assert table["rows"] + + +def test_api_holdings_without_time_filter(test_client: FlaskClient) -> None: + data = assert_api_success(test_client.get("/long-example/api/holdings")) + assert isinstance(data, dict) + assert "value(sum(position), " not in data["query_string"] + table = data["query_result_table"] + assert isinstance(table, dict) + assert table["t"] == "table" + + +def test_api_holdings_invalid_aggregation_key( + test_client: FlaskClient, +) -> None: + assert_api_error( + test_client.get( + "/long-example/api/holdings", + query_string={"aggregation_key": "by_something"}, + ), + "Invalid API request: unknown holdings aggregation key:" + " `by_something`", + HTTPStatus.BAD_REQUEST, + ) + + @pytest.mark.parametrize( ("name", "url"), [