Import Price Bug Fix
This commit is contained in:
@@ -439,15 +439,18 @@ class ImportPrices(Wizard):
|
|||||||
price_value = row_values.get(index)
|
price_value = row_values.get(index)
|
||||||
if price_value in (None, ''):
|
if price_value in (None, ''):
|
||||||
continue
|
continue
|
||||||
result.append({
|
result.append({
|
||||||
'_row_number': excel_row_number,
|
'_row_number': excel_row_number,
|
||||||
'price_index': (
|
'price_index': (
|
||||||
'%s %s' % (price_index, month_term)
|
'%s %s' % (price_index, month_term)
|
||||||
if price_index else ''),
|
if price_index else ''),
|
||||||
'price_date': price_date,
|
'price_date': price_date,
|
||||||
'price_value': price_value,
|
'high_price': price_value,
|
||||||
})
|
'low_price': price_value,
|
||||||
return result
|
'open_price': price_value,
|
||||||
|
'price_value': price_value,
|
||||||
|
})
|
||||||
|
return result
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _read_shared_strings(workbook):
|
def _read_shared_strings(workbook):
|
||||||
|
|||||||
@@ -4,6 +4,7 @@
|
|||||||
import datetime
|
import datetime
|
||||||
from decimal import Decimal
|
from decimal import Decimal
|
||||||
from unittest.mock import Mock, patch
|
from unittest.mock import Mock, patch
|
||||||
|
from xml.etree import ElementTree
|
||||||
|
|
||||||
from trytond.pool import Pool
|
from trytond.pool import Pool
|
||||||
from trytond.pyson import Eval
|
from trytond.pyson import Eval
|
||||||
@@ -2058,6 +2059,37 @@ class PurchaseTradeTestCase(ModuleTestCase):
|
|||||||
self.assertTrue(Trigger.default_average())
|
self.assertTrue(Trigger.default_average())
|
||||||
self.assertEqual(Trigger.last.string, 'Latest')
|
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('''
|
||||||
|
<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">
|
||||||
|
<sheetData>
|
||||||
|
<row r="1">
|
||||||
|
<c r="A1" t="inlineStr"><is><t>Price Index</t></is></c>
|
||||||
|
<c r="B1" t="inlineStr"><is><t>Price Date</t></is></c>
|
||||||
|
<c r="C1" t="inlineStr"><is><t>2026-03</t></is></c>
|
||||||
|
</row>
|
||||||
|
<row r="2">
|
||||||
|
<c r="A2" t="inlineStr"><is><t>ICE Cotton</t></is></c>
|
||||||
|
<c r="B2" t="inlineStr"><is><t>2026-03-01</t></is></c>
|
||||||
|
<c r="C2"><v>75.25</v></c>
|
||||||
|
</row>
|
||||||
|
</sheetData>
|
||||||
|
</worksheet>
|
||||||
|
''')
|
||||||
|
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):
|
def test_pricing_component_matrix_returns_generic_line_price(self):
|
||||||
'matrix pricing can use an unconditional matrix line as a component price'
|
'matrix pricing can use an unconditional matrix line as a component price'
|
||||||
Component = Pool().get('pricing.component')
|
Component = Pool().get('pricing.component')
|
||||||
|
|||||||
83
notes/purchase_trade_import_prices_backlog.md
Normal file
83
notes/purchase_trade_import_prices_backlog.md
Normal file
@@ -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`.
|
||||||
Reference in New Issue
Block a user