mirror of
				https://github.com/inventree/InvenTree.git
				synced 2025-11-04 07:05:41 +00:00 
			
		
		
		
	Bug fix for purchase order pricing (#4373)
* Account for pack size when calculating purchase_price for received line items * Clearer display of "unit pricing" when part has units * Add data migration to fix historical pricing bugs * Remove debug statement
This commit is contained in:
		@@ -501,6 +501,11 @@ class PurchaseOrder(Order):
 | 
				
			|||||||
            # Take the 'pack_size' of the SupplierPart into account
 | 
					            # Take the 'pack_size' of the SupplierPart into account
 | 
				
			||||||
            pack_quantity = Decimal(quantity) * Decimal(line.part.pack_size)
 | 
					            pack_quantity = Decimal(quantity) * Decimal(line.part.pack_size)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					            if line.purchase_price:
 | 
				
			||||||
 | 
					                unit_purchase_price = line.purchase_price / line.part.pack_size
 | 
				
			||||||
 | 
					            else:
 | 
				
			||||||
 | 
					                unit_purchase_price = None
 | 
				
			||||||
 | 
					
 | 
				
			||||||
            # Determine if we should individually serialize the items, or not
 | 
					            # Determine if we should individually serialize the items, or not
 | 
				
			||||||
            if type(serials) is list and len(serials) > 0:
 | 
					            if type(serials) is list and len(serials) > 0:
 | 
				
			||||||
                serialize = True
 | 
					                serialize = True
 | 
				
			||||||
@@ -519,7 +524,7 @@ class PurchaseOrder(Order):
 | 
				
			|||||||
                    status=status,
 | 
					                    status=status,
 | 
				
			||||||
                    batch=batch_code,
 | 
					                    batch=batch_code,
 | 
				
			||||||
                    serial=sn,
 | 
					                    serial=sn,
 | 
				
			||||||
                    purchase_price=line.purchase_price,
 | 
					                    purchase_price=unit_purchase_price,
 | 
				
			||||||
                    barcode_hash=barcode_hash
 | 
					                    barcode_hash=barcode_hash
 | 
				
			||||||
                )
 | 
					                )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -8,12 +8,14 @@ from django.contrib.auth import get_user_model
 | 
				
			|||||||
from django.contrib.auth.models import Group
 | 
					from django.contrib.auth.models import Group
 | 
				
			||||||
from django.test import TestCase
 | 
					from django.test import TestCase
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					from djmoney.money import Money
 | 
				
			||||||
 | 
					
 | 
				
			||||||
import common.models
 | 
					import common.models
 | 
				
			||||||
import order.tasks
 | 
					import order.tasks
 | 
				
			||||||
from company.models import Company, SupplierPart
 | 
					from company.models import Company, SupplierPart
 | 
				
			||||||
from InvenTree.status_codes import PurchaseOrderStatus
 | 
					from InvenTree.status_codes import PurchaseOrderStatus
 | 
				
			||||||
from part.models import Part
 | 
					from part.models import Part
 | 
				
			||||||
from stock.models import StockLocation
 | 
					from stock.models import StockItem, StockLocation
 | 
				
			||||||
from users.models import Owner
 | 
					from users.models import Owner
 | 
				
			||||||
 | 
					
 | 
				
			||||||
from .models import PurchaseOrder, PurchaseOrderLineItem
 | 
					from .models import PurchaseOrder, PurchaseOrderLineItem
 | 
				
			||||||
@@ -255,7 +257,8 @@ class OrderTest(TestCase):
 | 
				
			|||||||
        line_1 = PurchaseOrderLineItem.objects.create(
 | 
					        line_1 = PurchaseOrderLineItem.objects.create(
 | 
				
			||||||
            order=po,
 | 
					            order=po,
 | 
				
			||||||
            part=sp_1,
 | 
					            part=sp_1,
 | 
				
			||||||
            quantity=3
 | 
					            quantity=3,
 | 
				
			||||||
 | 
					            purchase_price=Money(1000, 'USD'),  # "Unit price" should be $100USD
 | 
				
			||||||
        )
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        # 13 x 0.1 = 1.3
 | 
					        # 13 x 0.1 = 1.3
 | 
				
			||||||
@@ -263,6 +266,7 @@ class OrderTest(TestCase):
 | 
				
			|||||||
            order=po,
 | 
					            order=po,
 | 
				
			||||||
            part=sp_2,
 | 
					            part=sp_2,
 | 
				
			||||||
            quantity=13,
 | 
					            quantity=13,
 | 
				
			||||||
 | 
					            purchase_price=Money(10, 'USD'),  # "Unit price" should be $100USD
 | 
				
			||||||
        )
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        po.place_order()
 | 
					        po.place_order()
 | 
				
			||||||
@@ -299,6 +303,23 @@ class OrderTest(TestCase):
 | 
				
			|||||||
            round(in_stock + Decimal(10.5), 1)
 | 
					            round(in_stock + Decimal(10.5), 1)
 | 
				
			||||||
        )
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        # Check that the unit purchase price value has been updated correctly
 | 
				
			||||||
 | 
					        si = StockItem.objects.filter(supplier_part=sp_1)
 | 
				
			||||||
 | 
					        self.assertEqual(si.count(), 1)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        # Ensure that received quantity and unit purchase price data are correct
 | 
				
			||||||
 | 
					        si = si.first()
 | 
				
			||||||
 | 
					        self.assertEqual(si.quantity, 10)
 | 
				
			||||||
 | 
					        self.assertEqual(si.purchase_price, Money(100, 'USD'))
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        si = StockItem.objects.filter(supplier_part=sp_2)
 | 
				
			||||||
 | 
					        self.assertEqual(si.count(), 1)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        # Ensure that received quantity and unit purchase price data are correct
 | 
				
			||||||
 | 
					        si = si.first()
 | 
				
			||||||
 | 
					        self.assertEqual(si.quantity, 0.5)
 | 
				
			||||||
 | 
					        self.assertEqual(si.purchase_price, Money(100, 'USD'))
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def test_overdue_notification(self):
 | 
					    def test_overdue_notification(self):
 | 
				
			||||||
        """Test overdue purchase order notification
 | 
					        """Test overdue purchase order notification
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -334,6 +334,7 @@
 | 
				
			|||||||
                        {% else %}
 | 
					                        {% else %}
 | 
				
			||||||
                        {% render_currency pricing.overall_min %} - {% render_currency pricing.overall_max %}
 | 
					                        {% render_currency pricing.overall_min %} - {% render_currency pricing.overall_max %}
 | 
				
			||||||
                        {% endif %}
 | 
					                        {% endif %}
 | 
				
			||||||
 | 
					                        {% if part.units %}  / {{ part.units }}{% endif %}
 | 
				
			||||||
                    </td>
 | 
					                    </td>
 | 
				
			||||||
                </tr>
 | 
					                </tr>
 | 
				
			||||||
                {% endif %}
 | 
					                {% endif %}
 | 
				
			||||||
 
 | 
				
			|||||||
							
								
								
									
										79
									
								
								InvenTree/stock/migrations/0094_auto_20230220_0025.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										79
									
								
								InvenTree/stock/migrations/0094_auto_20230220_0025.py
									
									
									
									
									
										Normal file
									
								
							@@ -0,0 +1,79 @@
 | 
				
			|||||||
 | 
					# Generated by Django 3.2.18 on 2023-02-20 00:25
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					import logging
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					from django.db import migrations
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					logger = logging.getLogger('inventree')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					def fix_purchase_price(apps, schema_editor):
 | 
				
			||||||
 | 
					    """Data migration for fixing historical issue with StockItem.purchase_price field.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    Ref: https://github.com/inventree/InvenTree/pull/4373
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    Due to an existing bug, if a PurchaseOrderLineItem was received,
 | 
				
			||||||
 | 
					    which had:
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    a) A SupplierPart with a non-unity pack size
 | 
				
			||||||
 | 
					    b) A defined purchase_price
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    then the StockItem.purchase_price was not calculated correctly!
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    Specifically, the purchase_price was not divided through by the pack_size attribute.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    This migration fixes this by looking through all stock items which:
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    - Is linked to a purchase order
 | 
				
			||||||
 | 
					    - Have a purchase_price field
 | 
				
			||||||
 | 
					    - Are linked to a supplier_part
 | 
				
			||||||
 | 
					    - We can determine correctly that the calculation was misapplied
 | 
				
			||||||
 | 
					    """
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    StockItem = apps.get_model('stock', 'stockitem')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    items = StockItem.objects.exclude(
 | 
				
			||||||
 | 
					        purchase_order=None
 | 
				
			||||||
 | 
					    ).exclude(
 | 
				
			||||||
 | 
					        supplier_part=None
 | 
				
			||||||
 | 
					    ).exclude(
 | 
				
			||||||
 | 
					        purchase_price=None
 | 
				
			||||||
 | 
					    ).exclude(
 | 
				
			||||||
 | 
					        supplier_part__pack_size=1
 | 
				
			||||||
 | 
					    )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    n_updated = 0
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    for item in items:
 | 
				
			||||||
 | 
					        # Grab a reference to the associated PurchaseOrder
 | 
				
			||||||
 | 
					        # Trying to find an absolute match between this StockItem and an associated PurchaseOrderLineItem
 | 
				
			||||||
 | 
					        po = item.purchase_order
 | 
				
			||||||
 | 
					        for line in po.lines.all():
 | 
				
			||||||
 | 
					            # SupplierPart match
 | 
				
			||||||
 | 
					            if line.part == item.supplier_part:
 | 
				
			||||||
 | 
					                # Unit price matches original PurchaseOrder (and is thus incorrect)
 | 
				
			||||||
 | 
					                if item.purchase_price == line.purchase_price:
 | 
				
			||||||
 | 
					                    item.purchase_price /= item.supplier_part.pack_size
 | 
				
			||||||
 | 
					                    item.save()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					                    n_updated += 1
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    if n_updated > 0:
 | 
				
			||||||
 | 
					        logger.info(f"Corrected purchase_price field for {n_updated} stock items.")
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					def reverse(apps, schema_editor):  # pragmae: no cover
 | 
				
			||||||
 | 
					    pass
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					class Migration(migrations.Migration):
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    dependencies = [
 | 
				
			||||||
 | 
					        ('stock', '0093_auto_20230217_2140'),
 | 
				
			||||||
 | 
					    ]
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    operations = [
 | 
				
			||||||
 | 
					        migrations.RunPython(
 | 
				
			||||||
 | 
					            fix_purchase_price,
 | 
				
			||||||
 | 
					            reverse_code=reverse,
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					    ]
 | 
				
			||||||
@@ -187,7 +187,10 @@
 | 
				
			|||||||
    <tr>
 | 
					    <tr>
 | 
				
			||||||
        <td><span class='fas fa-dollar-sign'></span></td>
 | 
					        <td><span class='fas fa-dollar-sign'></span></td>
 | 
				
			||||||
        <td>{% trans "Purchase Price" %}</td>
 | 
					        <td>{% trans "Purchase Price" %}</td>
 | 
				
			||||||
        <td>{% include "price_data.html" with price=item.purchase_price %}</td>
 | 
					        <td>
 | 
				
			||||||
 | 
					            {% include "price_data.html" with price=item.purchase_price %}
 | 
				
			||||||
 | 
					            {% if item.part.units %} / {{ item.part.units }}{% endif %}
 | 
				
			||||||
 | 
					        </td>
 | 
				
			||||||
    </tr>
 | 
					    </tr>
 | 
				
			||||||
    {% endif %}
 | 
					    {% endif %}
 | 
				
			||||||
    {% if item.parent %}
 | 
					    {% if item.parent %}
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user