Refactor models and remove some unused things, add resource groups th…#116
Refactor models and remove some unused things, add resource groups th…#116
Conversation
…at can make resource selection mandatory and unique, add allocation trading and approval process
There was a problem hiding this comment.
Pull request overview
This pull request implements a comprehensive allocation trading and approval system for the timetable application. The changes enable teachers to request time slot exchanges with colleagues, with an administrative approval workflow for managing these requests.
Key Changes:
- Added trade request and approval workflow with multiple status states (OPEN, MATCHED, PENDING_APPROVAL, APPROVED, REJECTED, CANCELLED, EXPIRED)
- Implemented views for creating, listing, viewing, canceling, and responding to trade requests
- Added comprehensive test coverage for trade request functionality
- Created template filters for formatting time, duration, and status displays
- Uncommented
SITE_ID = 1in settings to enable multi-site functionality
Reviewed changes
Copilot reviewed 49 out of 52 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Complete dependency lock file added (1489 lines) for Python 3.13+ |
| urnik_fri/settings_common.py | Uncommented SITE_ID setting to enable Django sites framework |
| timetable/views.py | Added 11 new views for trade request lifecycle management (736 lines added) |
| timetable/urls.py | Removed deprecated teacher preferences URL, added 8 trade request URLs |
| timetable/tests.py | Added comprehensive test suite with 609 lines covering models, forms, and views |
| timetable/templatetags/timetable_extras.py | Added 8 utility filters for formatting and display |
| timetable/templates/timetable/trade_requests/*.html | Added 3 templates for rejection confirmation, request listing, and detail views |
| timetable/models/tags.py | New Tag model for categorizing teachers, activities, and groups |
| timetable/models/classrooms.py | New classroom-related models extracted to separate file |
| timetable/management/commands/init.py | Empty init file for management commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # class ActivityPercentage(models.Model): | ||
| # teacher = models.ForeignKey('Teacher') | ||
| # activity = models.ForeignKey('Activity') |
There was a problem hiding this comment.
This comment appears to contain commented-out code.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| from timetable.forms import TradeRequestForm | ||
|
|
||
| # Create another allocation for the same teacher at different time | ||
| other_allocation = Allocation.objects.create( |
There was a problem hiding this comment.
Variable other_allocation is not used.
| # Get timetable if slug provided (for context) | ||
| timetable = None | ||
| if timetable_slug: | ||
| timetable = get_object_or_404(Timetable, slug=timetable_slug) |
There was a problem hiding this comment.
Variable timetable is not used.
| # Get timetable if slug provided (for redirect context) | ||
| timetable = None | ||
| if timetable_slug: | ||
| timetable = get_object_or_404(Timetable, slug=timetable_slug) |
There was a problem hiding this comment.
Variable timetable is not used.
|
|
||
| # Create a match between the two requests | ||
| try: | ||
| trade_match = original_request.create_match(counter_request) |
There was a problem hiding this comment.
Variable trade_match is not used.
| from timetable.views import ( | ||
| trade_request_list, | ||
| my_trade_requests, | ||
| create_trade_request, | ||
| trade_request_detail, | ||
| cancel_trade_request, | ||
| respond_to_trade_request, | ||
| reject_trade_request, | ||
| trade_match_queue, | ||
| ) |
There was a problem hiding this comment.
Import of 'trade_request_list' is not used.
Import of 'my_trade_requests' is not used.
Import of 'create_trade_request' is not used.
Import of 'trade_request_detail' is not used.
Import of 'cancel_trade_request' is not used.
Import of 'respond_to_trade_request' is not used.
Import of 'reject_trade_request' is not used.
Import of 'trade_match_queue' is not used.
| from timetable.views import ( | |
| trade_request_list, | |
| my_trade_requests, | |
| create_trade_request, | |
| trade_request_detail, | |
| cancel_trade_request, | |
| respond_to_trade_request, | |
| reject_trade_request, | |
| trade_match_queue, | |
| ) |
| try: | ||
| trade_request = TradeRequest.objects.get(pk=request_id) | ||
| timetable_slug = trade_request.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| try: | ||
| trade_match = TradeMatch.objects.get(pk=match_id) | ||
| timetable_slug = trade_match.request_1.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| try: | ||
| trade_request = TradeRequest.objects.get(pk=request_id) | ||
| timetable_slug = trade_request.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| try: | ||
| trade_match = TradeMatch.objects.get(pk=match_id) | ||
| timetable_slug = trade_match.request_1.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 52 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| trade_request = TradeRequest.objects.get(pk=request_id) | ||
| timetable_slug = trade_request.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| try: | ||
| trade_match = TradeMatch.objects.get(pk=match_id) | ||
| timetable_slug = trade_match.request_1.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| try: | ||
| trade_request = TradeRequest.objects.get(pk=request_id) | ||
| timetable_slug = trade_request.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| try: | ||
| trade_match = TradeMatch.objects.get(pk=match_id) | ||
| timetable_slug = trade_match.request_1.offered_allocation.timetable.slug | ||
| except: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
…at can make resource selection mandatory and unique, add allocation trading and approval process