feat(backend): sync item history with current_location

[d-p35]

Summary

Implements automatic synchronization between ItemHistory events and CollectionItem location fields. When location-changing history events are created (INITIAL, ARRIVED, VERIFIED, LOCATION_CORRECTION), the item's current_location and is_on_floor fields are automatically updated via Django signals. Also includes a management command for rebuilding all item locations from history.

Related Issues

• Closes Implement Item History Service to Sync 'current_location' #11 (Implement Item History Service)

Changes

Auto-sync Logic:

• Added Django post_save signal on ItemHistory model to automatically trigger location updates
• Signal updates current_location and is_on_floor fields when location-changing events are created (INITIAL, ARRIVED, VERIFIED, LOCATION_CORRECTION)

Maintenance Command:

• Created rebuild_item_locations Django management command for data recovery and consistency checks
• Command supports --item-id flag to rebuild single items or processes all items when no flag provided

Testing & Code Quality:

• Added 6 comprehensive test cases covering signal behavior, location updates, and management command

How to Test

cd backend

# Run all inventory tests
python manage.py test inventory.tests

# Test the management command
python manage.py rebuild_item_locations --item-id 1
python manage.py rebuild_item_locations

[Copilot]

Pull request overview

This PR implements automatic synchronization between ItemHistory events and CollectionItem location fields through Django signals. When location-changing events (INITIAL, ARRIVED, VERIFIED, LOCATION_CORRECTION) are created, the item's current_location and is_on_floor fields are automatically updated. A management command is also provided for rebuilding item locations from history for data recovery purposes.

Key Changes:

• Added post_save signal on ItemHistory to automatically update item locations when new history events are created
• Created rebuild_item_locations management command for batch location updates with support for single-item or all-items processing
• Added 6 comprehensive test cases covering signal behavior, manual location updates, and management command functionality

Reviewed changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
backend/inventory/tests.py	Adds comprehensive test suite with 6 test cases covering location update methods, signal triggers, and management command execution
backend/inventory/models.py	Implements post_save signal handler to automatically sync item locations when history events are created, with error handling to prevent cascading failures
backend/inventory/management/commands/rebuild_item_locations.py	Provides management command for rebuilding item locations from history, supporting both single-item and bulk operations
backend/inventory/management/commands/init.py	Empty init file to make commands directory a Python package
backend/inventory/management/init.py	Empty init file to make management directory a Python package

Comments suppressed due to low confidence (2)

backend/inventory/tests.py:1

• Import of 'pytest' is not used.

import pytest

backend/inventory/tests.py:4

• Import of 'ObjectDoesNotExist' is not used.

from django.core.exceptions import ObjectDoesNotExist

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

[Copilot]

The pytest module is imported but never used in this file. All tests use Django's TestCase class. Consider removing this unused import.

Suggested change

import pytest

[Copilot]

The ObjectDoesNotExist exception is imported but never used. The management command uses CollectionItem.DoesNotExist instead. Consider removing this unused import.

Suggested change

from django.core.exceptions import ObjectDoesNotExist

[Copilot]

The logging module should be imported at the top of the file rather than inside the exception handler. Moving it to the module level follows Python conventions and improves performance by avoiding repeated imports on each exception.

Suggested change:

import logging
from django.db import models
from django.conf import settings
from django.db.models.signals import post_save
from django.dispatch import receiver

logger = logging.getLogger(__name__)

Then in the signal handler, simply use logger.error(...) without the import.

[Copilot]

The signal triggers update_location_from_history() for every newly created history event, which performs a database query even for workflow-only events (MOVE_REQUESTED, MOVE_APPROVED, etc.) that don't change location.

Consider filtering at the signal level for better performance:

@receiver(post_save, sender=ItemHistory)
def update_item_location_on_history_change(sender, instance, created, **kwargs):
    """
    Update item location when a new history event is created.
    """
    LOCATION_CHANGING_EVENTS = ["INITIAL", "ARRIVED", "VERIFIED", "LOCATION_CORRECTION"]
    
    if created and instance.event_type in LOCATION_CHANGING_EVENTS:
        try:
            item: CollectionItem = instance.item
            item.update_location_from_history()
        except Exception as e:
            logger.error(f"Failed to update item location for item {instance.item_id}: {e}")

This avoids unnecessary database queries for workflow events while maintaining the same functionality.

Suggested change

	"""
	if created: # Only run for new history events
	# Use try/except to prevent cascading failures
	try:
	item: CollectionItem = instance.item
	item.update_location_from_history()
	except Exception as e:
	# Log error but don't raise to prevent disrupting the original save
	import logging
	Only update location for events that actually change location.
	"""
	LOCATION_CHANGING_EVENTS = ["INITIAL", "ARRIVED", "VERIFIED", "LOCATION_CORRECTION"]
	if created and instance.event_type in LOCATION_CHANGING_EVENTS:
	try:
	item: CollectionItem = instance.item
	item.update_location_from_history()
	except Exception as e:
	import logging

[vivjd]

I would strongly recommend implementing copilot's suggested change for this one!

[Copilot]

Missing test coverage for the error path when a non-existent item ID is provided. The command handles this case at line 28-29 in the management command but lacks a test.

Consider adding a test case:

def test_command_nonexistent_item(self):
    """Test command with non-existent item ID."""
    out = StringIO()
    call_command("rebuild_item_locations", "--item-id", 99999, stdout=out)
    
    output = out.getvalue()
    self.assertIn("not found", output)

[Copilot]

[nitpick] The test name and comment are slightly misleading. The signal does trigger for workflow events like MOVE_REQUESTED, but the location doesn't change because get_current_location() only considers LOCATION_CHANGING_EVENTS.

Consider renaming to better reflect the actual behavior:

def test_signal_does_not_update_location_for_workflow_events(self):
    """Test that signal triggers but doesn't update location for workflow-only events."""

This clarifies that the signal fires but has no effect on the location for workflow events.

Suggested change

	def test_signal_ignores_workflow_events(self):
	"""Test that signal doesn't trigger for workflow-only events."""
	def test_signal_does_not_update_location_for_workflow_events(self):
	"""Test that signal triggers but doesn't update location for workflow-only events."""

[vivjd]

Looks good overall! Please review copilot's comments and re-request review!