Dev#196
Merged
Merged
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…etti-OJ/back_end into feat/problem-checker-field
Adds custom checker support to problem settings
feat:api authentic方式改全域
feat: Adds database seeding management command for demo data
…tion for test cases
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
feat: Adds database seeding management command with demo data
…back_end into feat/add-mock-data
feat: 移除 permission
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Feat/add username to email
…ons-assignments fix: remove per-view IsAuthenticated to enforce email verification
…n_classes([permissions.IsAuthenticated]).
Fix quota enforcement race conditions and transaction safety
Refines user submission quota enforcement logic
feat:alloewdNetwork的欄位
feat:支援localhost
… into helper functions Co-authored-by: happylittle7 <7501374+happylittle7@users.noreply.github.com>
Refactor: Extract duplicated static analysis and network config logic
Feat/add some data for submit
feat:like會回傳course_name
fix: align homework scoreboard response keys with frontend
…置的 authentication_classes (auths, api_tokens, copycat)\n2. 實現 copycat 管理員特權 - 允許 superuser 訪問所有課程報告\n3. 修復所有 RBAC 測試：\n - 使用 JWT token 認證替代 force_authenticate\n - 正確創建 UserProfile 並設置 email_verified\n - 修正測試數據欄位名稱和值\n\n測試結果：\n- auths: 9/9 通過 (100%)\n- copycat: 13/13 通過 (100%)\n- 整體：28/34 通過 (82.4%，剩餘為 api-tokens 已知問題)
…eld 改為 IntegerField\n- 修正 homework scoreboard 欄位命名\n- 添加 Course_members import 修復 NameError
fix/rbac authentication issues
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces several new features and improvements across the codebase, with a focus on assignments, announcements, submissions, and enhanced test coverage. Key highlights include:
- Added comprehensive test coverage for RBAC, API tokens, submissions, and copycat detection
- Implemented IP whitelist support and submission quota enforcement
- Enhanced submission callback handling with better error diagnostics
- Added database seeding command for development and testing
- Improved static analysis and custom checker settings for problems
- Enhanced API documentation with updated field descriptions
Reviewed changes
Copilot reviewed 67 out of 68 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
user/management/commands/seed_data.py |
New Django management command for seeding test data with demo users, courses, problems, and submissions |
tests/rbac_tests/*.py |
New comprehensive RBAC test suites for authentication, API tokens, and copycat detection |
submissions/views.py |
Enhanced with IP filtering, rate limiting, quota enforcement, and improved callback handling |
submissions/test_file/*.py |
New and updated test files for submission flows, callbacks, and sandbox integration |
submissions/models.py |
Updated SubmissionResult with composite primary key and compile_error status support |
submissions/serializers.py |
Simplified Editorial serializers by removing title and other fields |
submissions/sandbox_client.py |
Enhanced with static analysis and network configuration support |
problems/views/api.py |
Added testcase hash support, improved meta.json generation, and enhanced permission handling |
problems/models.py |
Added custom checker, static analysis rules, and network whitelist support |
problems/migrations/*.py |
Database migrations for new problem features |
problems/tests/test_api.py |
Extended tests for custom checkers, static analysis, and testcase uploads |
docs/*.MD |
Updated documentation for user authentication and submission APIs |
courses/views/*.py |
Removed redundant permission_classes declarations to use global defaults |
search/tests/test_search_api.py |
New API tests for search functionality |
profiles/tests/test_profiles_api.py |
New API tests for profile views |
Comments suppressed due to low confidence (9)
submissions/views.py:616
- This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
submissions/views.py:771 - This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
submissions/views.py:1390 - This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
submissions/views.py:1394 - This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
submissions/views.py:1586 - This import of module logging is redundant, as it was previously imported on line 15.
submissions/views.py:1769 - This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
submissions/views.py:1905 - This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
submissions/views.py:1954 - This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
submissions/views.py:2209 - This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| url = reverse('announcements:course', kwargs={'course_id': self.course.id}) | ||
| response = self.client.get(url) | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertTrue(len(response.data['data']) >= 1) |
There was a problem hiding this comment.
assertTrue(a >= b) cannot provide an informative message. Using assertGreaterEqual(a, b) instead will give more informative messages.
Suggested change
| self.assertTrue(len(response.data['data']) >= 1) | |
| self.assertGreaterEqual(len(response.data['data']), 1) |
| # Check if wrapped in api_response or similar | ||
| # If response.data is list or dict with data | ||
| data = response.data.get('data', response.data) | ||
| self.assertTrue(len(data) > 0) |
There was a problem hiding this comment.
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
Suggested change
| self.assertTrue(len(data) > 0) | |
| self.assertGreater(len(data), 0) |
| results = data['results'] | ||
| else: | ||
| results = data | ||
| self.assertTrue(len(results) > 0) |
There was a problem hiding this comment.
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
| # 確認 Django 伺服器正在運行 | ||
| print_section("檢查 Django 伺服器") | ||
| try: | ||
| response = requests.get(f"{BACKEND_URL}/", timeout=5) |
There was a problem hiding this comment.
Variable response is not used.
| except requests.RequestException as e: | ||
| print_error(f"無法連接到 Django Server: {str(e)}") | ||
| print_warning("請先啟動 Django Server: python manage.py runserver") | ||
| response = requests.get(f"{BACKEND_URL}/", timeout=5) |
There was a problem hiding this comment.
Variable response is not used.
| except Exception as e: | ||
| # 其他系統錯誤 | ||
| return api_response(data=None, message="invalid data!", status_code=status.HTTP_400_BAD_REQUEST) | ||
| import logging |
There was a problem hiding this comment.
This import of module logging is redundant, as it was previously imported on line 15.
This import of module logging is redundant, as it was previously imported on line 1586.
|
|
||
| try: | ||
| submit_to_sandbox(mock_submission) | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| try: | ||
| profile = user.userprofile | ||
| avatar_url = profile.avatar.url if profile.avatar else None | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| try: | ||
| if _storage.exists(rel): | ||
| _storage.delete(rel) | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except (ValueError, TypeError) as e: | ||
| logger = logging.getLogger(__name__) | ||
| logger.warning(f'Invalid ip_prefix parameter: {ip_prefix}, error: {e}') | ||
| pass # 忽略無效的 IP 前綴 |
There was a problem hiding this comment.
Unnecessary 'pass' statement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several new features and improvements across the codebase, with a focus on assignments and announcements functionality. Key highlights include the addition of IP whitelist support for assignments, expanded serializer and API test coverage, and adjustments to authentication handling for consistency. Below are the most important changes grouped by theme:
Assignments: IP Whitelist & Serializer Enhancements
ip_whitelistfield to theAssignmentsmodel, including a customvalidate_cidr_whitelistvalidator and corresponding database migration. This allows specifying allowed IP ranges for assignment access. [1] [2] [3]max_attemptswith validation, improved scoreboard and stats serializers, and fixed several field types for better API consistency. [1] [2] [3] [4] [5] [6] [7] [8]assignments/tests/test_assignments_api.pywith tests for creating, retrieving, updating, and listing assignments, improving API reliability.Announcements: API Test Coverage & Permissions
announcements/tests/test_announcements_api.pywith comprehensive tests for listing, retrieving, creating, updating, and deleting announcements, including permission checks for teacher and student roles.permission_classesdeclarations from announcement views to rely on global defaults, reducing code duplication and centralizing permission logic. [1] [2]API Authentication & User Serializers
authentication_classesandpermission_classesin API token and user activity views to use global default authentication, simplifying configuration and ensuring consistency. [1] [2] [3] [4]MeSerializerto include anaccess_coursefield, listing all courses where the user is a TA or teacher, improving user context in API responses. [1] [2]Miscellaneous Improvements
ForgotPasswordSerializerto require an email field, strengthening password reset security..github/workflows/deploy.ymlto reflect recent branch naming changes.