diff --git a/modules/purchase_trade/pricing.py b/modules/purchase_trade/pricing.py index 6c9f74e..c25174d 100755 --- a/modules/purchase_trade/pricing.py +++ b/modules/purchase_trade/pricing.py @@ -117,9 +117,16 @@ class ImportPrices(Wizard): create_missing_price_index=( self.start.create_missing_price_index), overwrite_existing_price=self.start.overwrite_existing_price) - self.result.message = self._format_result(stats) + self._result_message = self._format_result(stats) return 'result' + def default_result(self, fields): + return { + 'message': getattr( + self, '_result_message', + 'No import result was produced.'), + } + @classmethod def _import_rows( cls, rows, create_missing_price_index=False, @@ -127,64 +134,87 @@ class ImportPrices(Wizard): Price = Pool().get('price.price') PriceValue = Pool().get('price.price_value') stats = { - 'created_indexes': 0, - 'imported': 0, - 'updated': 0, + 'created_indexes': [], + 'imported': [], + 'updated': [], 'skipped': [], + 'errors': [], } for row_number, row in enumerate(rows, start=2): price_index = (row.get('price_index') or '').strip() - price_date = row.get('price_date') - if not price_index: - stats['skipped'].append( - cls._skip(row_number, '', 'missing price_index')) - continue - if not price_date: - stats['skipped'].append( - cls._skip(row_number, price_index, 'missing price_date')) - continue + try: + price_date = cls._as_date(row.get('price_date')) + if not price_index: + stats['skipped'].append( + cls._result_line( + row_number, '', None, 'missing price_index')) + continue + if not price_date: + stats['skipped'].append( + cls._result_line( + row_number, price_index, None, + 'missing price_date')) + continue - prices = Price.search([('price_index', '=', price_index)], limit=1) - if prices: - price = prices[0] - elif create_missing_price_index: - price, = Price.create([{ - 'price_index': price_index, - 'price_desc': price_index, - }]) - stats['created_indexes'] += 1 - else: - stats['skipped'].append( - cls._skip(row_number, price_index, 'price_index missing')) - continue - - values = cls._price_value_values(price, row) - existing = PriceValue.search([ - ('price', '=', price.id), - ('price_date', '=', price_date), - ], limit=1) - - if existing: - if overwrite_existing_price: - PriceValue.write(existing, values) - stats['updated'] += 1 + prices = Price.search( + [('price_index', '=', price_index)], limit=1) + if prices: + price = prices[0] + elif create_missing_price_index: + price, = Price.create([{ + 'price_index': price_index, + 'price_desc': price_index, + }]) + stats['created_indexes'].append( + cls._result_line( + row_number, price_index, None, + 'price index created')) else: - stats['skipped'].append(cls._skip( - row_number, price_index, - 'price_date already exists')) - continue + stats['skipped'].append( + cls._result_line( + row_number, price_index, None, + 'price_index missing')) + continue - PriceValue.create([values]) - stats['imported'] += 1 + values = cls._price_value_values(price, row, price_date) + existing = PriceValue.search([ + ('price', '=', price.id), + ('price_date', '=', price_date), + ], limit=1) + + if existing: + if overwrite_existing_price: + PriceValue.write(existing, values) + stats['updated'].append( + cls._result_line( + row_number, price_index, price_date, + cls._price_summary(values))) + else: + stats['skipped'].append(cls._result_line( + row_number, price_index, price_date, + 'price_date already exists')) + continue + + PriceValue.create([values]) + stats['imported'].append( + cls._result_line( + row_number, price_index, price_date, + cls._price_summary(values))) + except Exception as exception: + stats['errors'].append( + cls._result_line( + row_number, price_index, row.get('price_date'), + str(exception))) + continue return stats @classmethod - def _price_value_values(cls, price, row): + def _price_value_values(cls, price, row, price_date): return { 'price': price.id, - 'price_date': row['price_date'], + 'price_date': price_date, 'high_price': cls._as_float(row.get('high_price')), 'low_price': cls._as_float(row.get('low_price')), 'open_price': cls._as_float(row.get('open_price')), @@ -192,31 +222,61 @@ class ImportPrices(Wizard): } @staticmethod - def _skip(row_number, price_index, reason): + def _result_line(row_number, price_index, price_date, detail): return { 'row': row_number, 'price_index': price_index, - 'reason': reason, + 'price_date': price_date, + 'detail': detail, } @classmethod def _format_result(cls, stats): lines = [ 'Import completed.', - f"Created price indexes: {stats['created_indexes']}", - f"Imported prices: {stats['imported']}", - f"Updated prices: {stats['updated']}", + f"Created price indexes: {len(stats['created_indexes'])}", + f"Imported prices: {len(stats['imported'])}", + f"Updated prices: {len(stats['updated'])}", f"Skipped rows: {len(stats['skipped'])}", + f"Errors: {len(stats['errors'])}", ] - if stats['skipped']: + + sections = [ + ('Created price indexes', stats['created_indexes']), + ('Successfully imported prices', stats['imported']), + ('Updated existing prices', stats['updated']), + ('Skipped records', stats['skipped']), + ('Errors', stats['errors']), + ] + for title, items in sections: lines.append('') - lines.append('Skipped details:') - for skip in stats['skipped']: - label = skip['price_index'] or '' - lines.append( - f"Row {skip['row']} - {label}: {skip['reason']}") + lines.append('%s:' % title) + if not items: + lines.append('- None') + continue + for item in items: + lines.append('- %s' % cls._format_result_line(item)) return '\n'.join(lines) + @staticmethod + def _format_result_line(item): + label = item['price_index'] or '' + price_date = item['price_date'] + if isinstance(price_date, datetime.date): + price_date = price_date.isoformat() + if price_date: + label = '%s / %s' % (label, price_date) + return 'Row %(row)s - ' % item + '%s: %s' % (label, item['detail']) + + @staticmethod + def _price_summary(values): + parts = [] + for name in ('price_value', 'open_price', 'low_price', 'high_price'): + value = values.get(name) + if value is not None: + parts.append('%s=%s' % (name, value)) + return ', '.join(parts) or 'price imported' + @classmethod def _read_xlsx(cls, data): try: @@ -255,7 +315,6 @@ class ImportPrices(Wizard): if field: values[field] = cls._cell_value(cell, shared_strings) if any(v not in (None, '') for v in values.values()): - values['price_date'] = cls._as_date(values.get('price_date')) result.append(values) return result diff --git a/modules/purchase_trade/tests/test_module.py b/modules/purchase_trade/tests/test_module.py index 1592ece..734d2ae 100644 --- a/modules/purchase_trade/tests/test_module.py +++ b/modules/purchase_trade/tests/test_module.py @@ -93,9 +93,9 @@ class PurchaseTradeTestCase(ModuleTestCase): 'price_value': '98', }]) - self.assertEqual(stats['imported'], 0) + self.assertEqual(len(stats['imported']), 0) self.assertEqual(len(stats['skipped']), 1) - self.assertEqual(stats['skipped'][0]['reason'], 'price_index missing') + self.assertEqual(stats['skipped'][0]['detail'], 'price_index missing') def test_import_prices_creates_index_and_imports_new_price(self): 'import prices creates missing index when requested' @@ -117,8 +117,8 @@ class PurchaseTradeTestCase(ModuleTestCase): 'price_value': '98', }], create_missing_price_index=True) - self.assertEqual(stats['created_indexes'], 1) - self.assertEqual(stats['imported'], 1) + self.assertEqual(len(stats['created_indexes']), 1) + self.assertEqual(len(stats['imported']), 1) self.assertEqual(price_model.records[0].price_index, 'LME Copper') self.assertEqual(price_value_model.records[0].price_value, 98.0) @@ -143,10 +143,10 @@ class PurchaseTradeTestCase(ModuleTestCase): 'price_value': '98', }]) - self.assertEqual(stats['updated'], 0) + self.assertEqual(len(stats['updated']), 0) self.assertEqual(price_value_model.records[0].price_value, 97.0) self.assertEqual( - stats['skipped'][0]['reason'], 'price_date already exists') + stats['skipped'][0]['detail'], 'price_date already exists') def test_import_prices_overwrites_existing_date_when_requested(self): 'import prices updates existing price dates when requested' @@ -169,9 +169,66 @@ class PurchaseTradeTestCase(ModuleTestCase): 'price_value': '98', }], overwrite_existing_price=True) - self.assertEqual(stats['updated'], 1) + self.assertEqual(len(stats['updated']), 1) self.assertEqual(price_value_model.records[0].price_value, 98.0) + def test_import_prices_collects_row_errors(self): + 'import prices reports invalid row values without hiding the result' + price = SimpleNamespace(id=1, price_index='LME Copper') + price_model = _FakePriceModel([price]) + price_value_model = _FakePriceValueModel() + + with patch('trytond.modules.purchase_trade.pricing.Pool') as pool: + pool.return_value.get.side_effect = { + 'price.price': price_model, + 'price.price_value': price_value_model, + }.__getitem__ + + stats = ImportPrices._import_rows([{ + 'price_index': 'LME Copper', + 'price_date': date(2026, 3, 27), + 'price_value': 'not-a-number', + }]) + + self.assertEqual(len(stats['errors']), 1) + self.assertIn('not-a-number', stats['errors'][0]['detail']) + + def test_import_prices_result_lists_all_outcome_sections(self): + 'import prices formats detailed result sections' + message = ImportPrices._format_result({ + 'created_indexes': [{ + 'row': 2, + 'price_index': 'LME Copper', + 'price_date': None, + 'detail': 'price index created', + }], + 'imported': [{ + 'row': 2, + 'price_index': 'LME Copper', + 'price_date': date(2026, 3, 27), + 'detail': 'price_value=98.0', + }], + 'updated': [], + 'skipped': [{ + 'row': 3, + 'price_index': 'LME Copper', + 'price_date': date(2026, 3, 27), + 'detail': 'price_date already exists', + }], + 'errors': [{ + 'row': 4, + 'price_index': 'LME Zinc', + 'price_date': 'bad-date', + 'detail': 'Invalid price_date: bad-date', + }], + }) + + self.assertIn('Created price indexes:', message) + self.assertIn('Successfully imported prices:', message) + self.assertIn('Skipped records:', message) + self.assertIn('Errors:', message) + self.assertIn('Row 2 - LME Copper / 2026-03-27', message) + class _FakePriceModel: