-
Notifications
You must be signed in to change notification settings - Fork 0
feat(backend): sync item history with current_location #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """ | ||
| Constants for the inventory app. | ||
| """ | ||
|
|
||
| # Events that actually change the physical location of an item | ||
| LOCATION_CHANGING_EVENTS = ["INITIAL", "ARRIVED", "VERIFIED", "LOCATION_CORRECTION"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| """ | ||
| Simple management command to rebuild item locations from history. | ||
| """ | ||
|
|
||
| from django.core.management.base import BaseCommand | ||
| from inventory.models import CollectionItem | ||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = "Rebuild current_location and is_on_floor for all CollectionItems based on their history" | ||
|
|
||
| def add_arguments(self, parser): | ||
| parser.add_argument( | ||
| "--item-id", | ||
| type=int, | ||
| help="Update only the specified item ID", | ||
| ) | ||
|
|
||
| def handle(self, *args, **options): | ||
| item_id = options.get("item_id") | ||
|
|
||
| if item_id: | ||
| # Single item | ||
| try: | ||
| item = CollectionItem.objects.get(id=item_id) | ||
| item.update_location_from_history() | ||
| self.stdout.write(self.style.SUCCESS(f"Updated item {item_id}")) | ||
| except CollectionItem.DoesNotExist: | ||
| self.stdout.write(self.style.ERROR(f"Item {item_id} not found")) | ||
| else: | ||
| # All items | ||
| items = CollectionItem.objects.all() | ||
| updated_count = 0 | ||
|
|
||
| for item in items: | ||
| try: | ||
| item.update_location_from_history() | ||
| updated_count += 1 | ||
| except Exception as e: | ||
| self.stdout.write(self.style.ERROR(f"Error updating item {item.id}: {e}")) | ||
|
|
||
| self.stdout.write(self.style.SUCCESS(f"Updated {updated_count} items")) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,12 @@ | ||
| from django.db import models | ||
| from django.conf import settings | ||
| from django.db.models.signals import post_save | ||
| from django.dispatch import receiver | ||
| import logging | ||
|
|
||
| from .constants import LOCATION_CHANGING_EVENTS | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class Location(models.Model): | ||
|
|
@@ -143,3 +150,19 @@ class Meta: | |
|
|
||
| def __str__(self): | ||
| return f"{self.item.item_code} - {self.get_event_type_display()} at {self.created_at}" | ||
|
|
||
|
|
||
| @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. | ||
| Only triggers for location-changing events (INITIAL, ARRIVED, VERIFIED, LOCATION_CORRECTION). | ||
| """ | ||
| if created and instance.event_type in LOCATION_CHANGING_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 | ||
| logger.error(f"Failed to update item location for item {instance.item_id}: {e}") | ||
|
Comment on lines
+163
to
+168
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,97 @@ | ||||||
| from django.test import TestCase | ||||||
| from django.core.management import call_command | ||||||
| from io import StringIO | ||||||
|
|
||||||
| # Create your tests here. | ||||||
| from .models import CollectionItem, Location, ItemHistory | ||||||
| from users.models import User | ||||||
|
|
||||||
|
|
||||||
| class ItemLocationTest(TestCase): | ||||||
| """Test the item location functionality.""" | ||||||
|
|
||||||
| def setUp(self): | ||||||
| self.user = User.objects.create_user(email="test@example.com", name="Test User", password="testpass", role="VOLUNTEER") | ||||||
| self.location_storage = Location.objects.create(name="Storage A", location_type="STORAGE") | ||||||
| self.location_floor = Location.objects.create(name="Main Floor", location_type="FLOOR") | ||||||
| self.item = CollectionItem.objects.create( | ||||||
| item_code="TEST001", title="Test Item", current_location=self.location_storage | ||||||
| ) | ||||||
|
|
||||||
| def test_update_location_from_history_initial_event(self): | ||||||
| """Test location update with INITIAL event.""" | ||||||
| # Create INITIAL history event | ||||||
| ItemHistory.objects.create(item=self.item, event_type="INITIAL", to_location=self.location_storage, acted_by=self.user) | ||||||
|
|
||||||
| # Update location using model method | ||||||
| self.item.update_location_from_history() | ||||||
|
|
||||||
| self.assertEqual(self.item.current_location, self.location_storage) | ||||||
| self.assertFalse(self.item.is_on_floor) | ||||||
|
|
||||||
| def test_update_location_from_history_floor_move(self): | ||||||
| """Test location update when item moves to floor.""" | ||||||
| # Create events | ||||||
| ItemHistory.objects.create(item=self.item, event_type="INITIAL", to_location=self.location_storage) | ||||||
| ItemHistory.objects.create( | ||||||
| item=self.item, event_type="ARRIVED", from_location=self.location_storage, to_location=self.location_floor | ||||||
| ) | ||||||
|
|
||||||
| self.item.update_location_from_history() | ||||||
|
|
||||||
| self.assertEqual(self.item.current_location, self.location_floor) | ||||||
| self.assertTrue(self.item.is_on_floor) | ||||||
|
|
||||||
| def test_signal_triggers_on_location_changing_event(self): | ||||||
| """Test that signal updates item location for location-changing events.""" | ||||||
| # Create ARRIVED event - should trigger signal | ||||||
| ItemHistory.objects.create( | ||||||
| item=self.item, event_type="ARRIVED", to_location=self.location_floor, from_location=self.location_storage | ||||||
| ) | ||||||
|
|
||||||
| # Refresh item from database | ||||||
| self.item.refresh_from_db() | ||||||
|
|
||||||
| # Verify location was updated by signal | ||||||
| self.assertEqual(self.item.current_location, self.location_floor) | ||||||
| self.assertTrue(self.item.is_on_floor) | ||||||
|
|
||||||
| def test_signal_does_not_update_location_for_workflow_events(self): | ||||||
| """Test that signal triggers but doesn't update location for workflow-only events.""" | ||||||
| original_location = self.item.current_location | ||||||
| original_is_on_floor = self.item.is_on_floor | ||||||
|
|
||||||
| # Create MOVE_REQUESTED event - should NOT trigger signal | ||||||
|
||||||
| # Create MOVE_REQUESTED event - should NOT trigger signal | |
| # Create MOVE_REQUESTED event - should NOT update location |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test coverage is missing for VERIFIED and LOCATION_CORRECTION event types, which are defined as location-changing events in LOCATION_CHANGING_EVENTS. Consider adding test cases to verify that these event types also trigger the signal and correctly update the item's location.
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the signal's error handling logic. The signal handler in models.py (lines 156-165) catches exceptions and logs errors, but there's no test case verifying this error handling behavior. Consider adding a test that triggers an error condition to ensure errors are properly caught and logged without disrupting the history event creation.
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for VERIFIED and LOCATION_CORRECTION event types. According to the utils.py get_current_location function, both VERIFIED and LOCATION_CORRECTION are location-changing events that should trigger location updates. The tests only cover INITIAL and ARRIVED event types, leaving these two event types untested in the signal behavior.
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test only verifies that the success message is printed but doesn't verify that the item's location fields were actually updated correctly. Consider adding assertions to check that current_location and is_on_floor were updated based on the item's history.
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the error case when rebuild_item_locations is called with a non-existent item ID. The management command handles CollectionItem.DoesNotExist (line 28 in rebuild_item_locations.py), but there's no test verifying this error message is properly displayed.
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the error handling in the "all items" path. The management command catches exceptions when updating each item (lines 36-40), but there's no test verifying that the command continues processing other items after encountering an error, or that the error message is properly displayed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command tracks updated_count for successful updates but doesn't track or report the number of failures. Consider adding a failed_count variable and including it in the final summary message to give users complete visibility into the rebuild operation results.