1
\$\begingroup\$

I'm trying to learn Django by building an inventory management system, here's my final models.py.

I'm looking for feedback!

from django.core.exceptions import ValidationError
from django.urls import reverse
from django.db import models
from core.models import Department
from accounts.models import User


class Item(models.Model):
    description = models.CharField(
        max_length=250,
        db_index=True,
        verbose_name='Description'
    )
    quantity_in_stock = models.PositiveSmallIntegerField(
        default=0,
        blank=True,
        verbose_name='Quantity In Stock'
    )
    
    class Meta:
        db_table = 'items'
        verbose_name = 'Item'
        verbose_name_plural = 'Items'
    
    def __str__(self):
        return self.description
    
    def get_absolute_url(self):
        return reverse('item-details', args=[self.id])
    
    def increase_stock_quantity(self, amount):
        self.quantity_in_stock += amount
        self.save()

    def decrease_stock_quantity(self, amount):
        if amount > self.quantity_in_stock:
            raise ValueError('The Amount is Larger Than the Quantity in Stock')
        self.quantity_in_stock -= amount
        self.save()
        

class PurchaseEntry(models.Model):
    item = models.ForeignKey(
        Item,
        on_delete=models.CASCADE,
        verbose_name='Item'
    )
    unit_price = models.PositiveIntegerField(
        blank=True,
        null=True,
        verbose_name='Unit Price'
    )
    purchased_quantity = models.PositiveSmallIntegerField(
        default=1,
        verbose_name='Purchased Quantity'
    )
    purchase_date = models.DateField(
        blank=True,
        null=True,
        verbose_name='Purchase Date'
    )
    supplier = models.CharField(
        max_length=250,
        verbose_name='Supplier'
    )
    entry_number = models.PositiveIntegerField(
        blank=True,
        null=True,
        verbose_name='Entry Number'
    )
    entry_date = models.DateField(
        verbose_name='Entry Date'
    )
    
    class Meta:
        db_table = 'purchase_entries'
        ordering = ['-entry_date']
        verbose_name = 'Purchase Entry'
        verbose_name_plural = 'Purchase Entries'
    
    def __str__(self):
        return f'{self.entry_date} - {self.item.description}'
    
    def save(self, *args, **kwargs):
        self.item.increase_stock_quantity(self.purchased_quantity)
        super(PurchaseEntry, self).save(*args, **kwargs)
    
    def get_total_price(self):
        return self.unit_price * self.purchased_quantity

class IssuedItem(models.Model):
    item = models.ForeignKey(
        Item,
        on_delete=models.PROTECT,
        related_name='issued_items',
        verbose_name='Item'
    )
    recipient_employee = models.ForeignKey(
        User,
        on_delete=models.PROTECT,
        related_name='issued_items',
        verbose_name='Recipient Employee'
    )
    recipient_department = models.ForeignKey(
        Department,
        on_delete=models.PROTECT,
        related_name='issued_items',
        verbose_name='Recipient Department'
    )
    issue_quantity = models.PositiveSmallIntegerField(
        default=1,
        verbose_name='Issue Quantity'
    )
    issue_reason = models.CharField(
        max_length=250,
        verbose_name='Issue Reason'
    )
    issue_date = models.DateField(
        verbose_name='Issue Date'
    )
    exit_number = models.PositiveIntegerField(
        blank=True,
        null=True,
        verbose_name='Exit Number'
    )
    notes = models.TextField(
        default='',
        verbose_name='Notes'
    )
    
    class Meta:
        db_table = 'issued_items'
        verbose_name = 'Issued Item'
        verbose_name_plural = 'Issued Items'
    
    def __str__(self):
        return f'{self.issuance_date} - {self.item.description} - {self.recipient}'
    
    def save(self, *args, **kwargs):
        if self.issued_quantity > self.item.quantity_in_stock:
            raise ValidationError('Issued Quantity Exceeds Available Quantity')
        self.item.decrease_stock_quantity(self.issued_quantity)
        super(IssuedItem, self).save(*args, **kwargs)

class ReturnedItem(models.Model):
    item = models.ForeignKey(
        Item,
        on_delete=models.PROTECT,
        related_name='returned_items',
        verbose_name='Item'
    )
    return_reason = models.TextField(
        verbose_name='Reason for Return'
    )
    return_quantity = models.PositiveSmallIntegerField(
        default=1,
        verbose_name='Return Quantity'
    )
    return_date = models.DateField(
        blank=True,
        null=True,
        verbose_name='Return Date'
    )
    
    class Meta:
        db_table = 'returned_items'
        verbose_name = 'Returned Item'
        verbose_name_plural = 'Returned Items'
    
    def __str__(self):
        return f'{self.item.description} - {self.return_date} - {self.quantity_returned} - {self.return_reason}'

\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

Avoid premature optimization

quantity_in_stock = models.PositiveSmallIntegerField(...)
purchased_quantity = models.PositiveSmallIntegerField(...)
issue_quantity = models.PositiveSmallIntegerField(...)
return_quantity = models.PositiveSmallIntegerField(...)

Don't shrink these fields to PositiveSmall unless you somehow have infrastructure that necessitates it.

Here I'd say it's not only unnecessary but even counterproductive to limit your inventory to PositiveSmall:

  1. Positive precludes negative inventory, which does happen in practice
  2. Small precludes organizations with 32,768+ inventory

Use consistent naming

The Issue and Return models are named IssuedItem and ReturnedItem, but the Purchase model is named PurchaseEntry. Unless there is a good reason I'm missing, I'd use PurchasedItem for consistency.


Avoid redundant naming

Many of the fields are prefixed with the model name. Remove the prefixes to reduce verbosity:

  • Item.quantity_in_stock -> Item.quantity
  • PurchasedItem.purchase_quantity -> PurchasedItem.quantity
  • PurchasedItem.purchase_date -> PurchasedItem.date
  • IssuedItem.issue_reason -> IssuedItem.reason
  • IssuedItem.issue_quantity -> IssuedItem.quantity
  • IssuedItem.issue_date -> IssuedItem.date
  • ReturnedItem.return_reason -> ReturnedItem.reason
  • ReturnedItem.return_quantity -> ReturnedItem.quantity
  • ReturnedItem.return_date -> ReturnedItem.date
\$\endgroup\$
3
\$\begingroup\$

This is a very naive approach to the issue and it's a race condition away from being a day+ debugging nightmare for you.

def increase_stock_quantity(self, amount):
    self.quantity_in_stock += amount
    self.save()

def decrease_stock_quantity(self, amount):
    if amount > self.quantity_in_stock:
        raise ValueError('The Amount is Larger Than the Quantity in Stock')
    self.quantity_in_stock -= amount
    self.save()
  1. use locks (.select_for_update())
  2. (or) use query set self.objects.filter(id=self.id).update(quantity_in_stock=F('quantity_in_stock') + 1)

Also make sure you wrap this inside a transaction.


def save(self, *args, **kwargs):
    if self.issued_quantity > self.item.quantity_in_stock:
        raise ValidationError('Issued Quantity Exceeds Available Quantity')
    self.item.decrease_stock_quantity(self.issued_quantity)
    super(IssuedItem, self).save(*args, **kwargs)
  • The if should be moved to clean().
  • self.item.decrease_stock_quantity(self.issued_quantity) in the save feels very, VERY wrong! Not to mention a HUGE side-effect for something that shouldn't be happening.
  • super(IssuedItem, self).save(*args, **kwargs) missing a return and remove IssuedItem, self from super().

While I'm at it, install and run ruff.

\$\endgroup\$
2
\$\begingroup\$

looking for some feedback

We have some nouns, here. I don't see any verbs. Certainly there is no test suite. It's unclear what entrypoint someone would use to exercise this codebase.

I imagine there is some business problem we wish to address. The code has not yet made that apparent, and no specification has been cited. A unit test would be a very good way of demonstrating how we expect to interact with the datastore, and what results we might hope to get back from it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.