diff --git a/modules/purchase_trade/pricing.py b/modules/purchase_trade/pricing.py index 0d18e37..80be547 100755 --- a/modules/purchase_trade/pricing.py +++ b/modules/purchase_trade/pricing.py @@ -439,15 +439,18 @@ class ImportPrices(Wizard): price_value = row_values.get(index) if price_value in (None, ''): continue - result.append({ - '_row_number': excel_row_number, - 'price_index': ( - '%s %s' % (price_index, month_term) - if price_index else ''), - 'price_date': price_date, - 'price_value': price_value, - }) - return result + result.append({ + '_row_number': excel_row_number, + 'price_index': ( + '%s %s' % (price_index, month_term) + if price_index else ''), + 'price_date': price_date, + 'high_price': price_value, + 'low_price': price_value, + 'open_price': price_value, + 'price_value': price_value, + }) + return result @staticmethod def _read_shared_strings(workbook): diff --git a/modules/purchase_trade/tests/test_module.py b/modules/purchase_trade/tests/test_module.py index 9f0aac8..b8332d6 100644 --- a/modules/purchase_trade/tests/test_module.py +++ b/modules/purchase_trade/tests/test_module.py @@ -4,6 +4,7 @@ import datetime from decimal import Decimal from unittest.mock import Mock, patch +from xml.etree import ElementTree from trytond.pool import Pool from trytond.pyson import Eval @@ -2058,6 +2059,37 @@ class PurchaseTradeTestCase(ModuleTestCase): self.assertTrue(Trigger.default_average()) self.assertEqual(Trigger.last.string, 'Latest') + @with_transaction() + def test_import_forward_prices_copies_value_to_ohl_fields(self): + 'forward import uses price values for open, low and high prices' + ImportPrices = Pool().get('purchase_trade.import_prices') + ns = {'s': 'http://schemas.openxmlformats.org/spreadsheetml/2006/main'} + sheet = ElementTree.fromstring(''' + + + + Price Index + Price Date + 2026-03 + + + ICE Cotton + 2026-03-01 + 75.25 + + + + ''') + rows = sheet.findall('.//s:sheetData/s:row', ns) + + result = ImportPrices._read_forward_price_rows(rows, [], ns) + + self.assertEqual(len(result), 1) + self.assertEqual(result[0]['price_value'], '75.25') + self.assertEqual(result[0]['open_price'], '75.25') + self.assertEqual(result[0]['low_price'], '75.25') + self.assertEqual(result[0]['high_price'], '75.25') + def test_pricing_component_matrix_returns_generic_line_price(self): 'matrix pricing can use an unconditional matrix line as a component price' Component = Pool().get('pricing.component') diff --git a/notes/purchase_trade_import_prices_backlog.md b/notes/purchase_trade_import_prices_backlog.md new file mode 100644 index 0000000..0eac6b3 --- /dev/null +++ b/notes/purchase_trade_import_prices_backlog.md @@ -0,0 +1,83 @@ +# Purchase Trade Import Prices Backlog + +Backlog of potential issues found during a targeted review of the +`purchase_trade.import_prices` wizard. + +## Findings + +### Incomplete auto-created price indexes + +- Area: `modules/purchase_trade/pricing.py`, `_price_index_values` +- Risk: medium +- Current behavior: when `create_missing_price_index` is enabled, the wizard + creates a `price.price` even if default references cannot be found. +- Details: default references are searched by exact names: `Market price`, + `USD`, `Argus EU`, and `Mt`. +- Possible impact: imported prices may be attached to indexes without unit, + currency, type, or calendar, which can later break or distort price + conversions. +- Suggested action: fail clearly when required defaults are missing, or expose + /import a controlled mapping instead of relying on silent best-effort lookup. + +### Duplicate price values for the same index and date + +- Area: `modules/purchase_trade/pricing.py`, `_import_rows`; `modules/price/price_value.py` +- Risk: medium +- Current behavior: the wizard searches an existing `price.price_value` with + `limit=1` for a given `price` and `price_date`. +- Details: no uniqueness constraint was found on `price.price_value` for + `(price, price_date)`. +- Possible impact: if duplicates already exist, only one record is updated, and + later price reads may use a non-deterministic record. +- Suggested action: add a duplicate detection step in the import wizard. Longer + term, consider a unique constraint or a cleanup/migration if the database + already contains duplicates. + +### Strict forward month term formats + +- Area: `modules/purchase_trade/pricing.py`, `_as_month_term` +- Risk: low to medium +- Current behavior: forward month columns accept Excel date values or text in + `YYYY-MM` format. +- Details: common business labels such as `MAR26`, `Mar-26`, or `March 2026` + are rejected. +- Possible impact: valid user files may fail with `Invalid month term`. +- Suggested action: confirm the real Excel template used by operators. If + needed, support month abbreviations while keeping unambiguous validation. + +### Fragile text number parsing + +- Area: `modules/purchase_trade/pricing.py`, `_as_float` +- Risk: low to medium +- Current behavior: blank values become `None`; comma decimals such as `75,25` + are accepted. +- Details: formatted text numbers such as `1 234,56`, `1,234.56`, or values + with non-breaking spaces are not handled safely. +- Possible impact: imports may fail on files where prices are pasted as + formatted text instead of stored as numeric Excel cells. +- Suggested action: normalize spaces and common thousand separators, or produce + a clearer validation error with row/index context. + +### Excel 1904 date system is not handled + +- Area: `modules/purchase_trade/pricing.py`, `_as_date` +- Risk: low +- Current behavior: numeric Excel dates are interpreted with the 1899-12-30 + base. +- Details: Excel workbooks using the 1904 date system would be shifted by about + four years. +- Possible impact: imported `price_date` or forward month terms could be wrong + for uncommon workbook settings. +- Suggested action: inspect workbook properties for the date system before + converting numeric dates. + +## Recently Fixed + +### Forward prices only populated `price_value` + +- Area: `modules/purchase_trade/pricing.py`, `_read_forward_price_rows` +- Status: fixed locally +- Change: forward imports now copy each forward cell value into `price_value`, + `open_price`, `low_price`, and `high_price`. +- Regression coverage: `test_import_forward_prices_copies_value_to_ohl_fields` + in `modules/purchase_trade/tests/test_module.py`.