Development by VelkoTgr6 · Pull Request #13 · VelkoTgr6/StudentManagementSystem

VelkoTgr6 · 2025-12-04T19:19:29Z

This pull request focuses on improving error handling consistency across the codebase by standardizing the use of KeyNotFoundException for not-found scenarios, updating related unit tests, and making minor improvements to date handling and repository usage. The changes increase clarity, reliability, and maintainability of both the application code and the test suite.

Error handling standardization:

Replaced ArgumentException with KeyNotFoundException for entities not found in service methods such as editing or retrieving remarks, courses, students, and course schedules (AdminStudentService.cs, AdminCourseServiceTests.cs, AdminScheduleServiceTests.cs, AdminStudentServiceTests.cs).
Updated unit tests to expect KeyNotFoundException instead of ArgumentException or generic exceptions, and adjusted test names and assertions accordingly throughout the test files.
Repository and data handling improvements:
Ensured that repository mocks for AllAsReadOnly return correctly configured async queryables in unit tests, improving test reliability for EF Core async extensions (AdminClassServiceTests.cs, AdminCourseServiceTests.cs, AdminScheduleServiceTests.cs).
Changed repository usage in absence retrieval to use AllAsReadOnly for consistency and potentially improved performance (TeacherService.cs).

Date handling consistency:

Removed unnecessary .Date property access when specifying DateTimeKind.Utc for absence dates, ensuring the full date and time are preserved (TeacherService.cs).
Test suite cleanup:
Removed redundant or obsolete test cases, such as those checking for no-ops on already deleted entities or not found scenarios now covered by exception handling (AdminCourseServiceTests.cs, AdminScheduleServiceTests.cs, AdminStudentServiceTests.cs).
Miscellaneous:
Minor code cleanup and organization in test files, including import adjustments and whitespace fixes (AdminCourseServiceTests.cs, AdminStudentServiceTests.cs).

Updated exception types in `AdminStudentService.cs` to use `KeyNotFoundException` for missing remarks, improving semantic accuracy. Refined absence date handling in `TeacherService.cs` to preserve full `DateTime` values, ensuring proper handling of time components. Removed redundant `GradeType` assignment and switched to `AllAsReadOnly` for fetching absences to improve performance and immutability. Enhanced logging for better debugging and maintainability.

Standardized exception handling by replacing `ArgumentException` and `ArgumentNullException` with `KeyNotFoundException` across test cases. Updated test method names to reflect the new exception type. Enhanced mock setups to ensure compatibility with asynchronous LINQ operations. Removed redundant tests and consolidated similar cases to reduce duplication. Added logger dependencies (`ILogger<T>`) to services and their test setups. Improved test assertions to validate exception messages for better accuracy. Standardized mock data for entities like `Student`, `Course`, `Grade`, and `Remark`. Refactored test cases for improved readability and maintainability. Updated return types in tests where appropriate. Added new test cases to handle missing entity scenarios. Performed general code cleanup, including removing unused imports and ensuring consistent formatting.

Switched the deployment branch for the `student-management-system` service from `master` to `render-deployment

Copilot

Pull request overview

This pull request standardizes error handling across the codebase by replacing ArgumentException with KeyNotFoundException for entity not-found scenarios. Additionally, it improves test reliability by ensuring repository mocks properly support EF Core async extensions, updates date handling to preserve full DateTime values, and includes minor code cleanup. A deployment configuration change is also present but not documented in the description.

Key changes:

Standardized exception types: Replaced ArgumentException with KeyNotFoundException across service methods and updated ~30+ unit tests to expect the correct exception type
Improved test infrastructure: Added BuildMock() calls to repository mock setups to support EF Core async query extensions
Date handling improvements: Removed unnecessary .Date property access when specifying DateTimeKind.Utc to preserve full date-time values

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
render.yaml	Changes deployment branch from master to render-deployment (undocumented change)
StudentServiceTests/TeacherServiceTests.cs	Updates 12 test methods to expect KeyNotFoundException instead of ArgumentException/ArgumentNullException; fixes test assertion patterns
StudentServiceTests/StudentServiceTests.cs	Updates GetStudentIdAsync test to expect KeyNotFoundException and adds student existence validation
StudentServiceTests/Admin/AdminUserManageControllerTests.cs	Adds logger mock and fixes DeleteRole test to expect NotFoundResult instead of BadRequestResult
StudentServiceTests/Admin/AdminTeacherServiceTests.cs	Adds logger mock and updates test to expect KeyNotFoundException
StudentServiceTests/Admin/AdminStudentServiceTests.cs	Updates 15+ tests to expect KeyNotFoundException; removes obsolete tests; improves test setup with student existence checks
StudentServiceTests/Admin/AdminScheduleServiceTests.cs	Adds BuildMock() for async support; removes obsolete deletion test; updates exception expectations
StudentServiceTests/Admin/AdminCourseServiceTests.cs	Updates 8 tests to expect KeyNotFoundException; removes redundant test for already-deleted courses; adds BuildMock() support
StudentServiceTests/Admin/AdminClassServiceTests.cs	Adds BuildMock() setup for AllAsReadOnly to support async queries
StudentManagementSystem.Core/Services/TeacherService.cs	Changes repository method from All to AllAsReadOnly for absence retrieval; removes unnecessary .Date property access
StudentManagementSystem.Core/Services/Admin/AdminStudentService.cs	Replaces ArgumentException with KeyNotFoundException for remark not found scenarios

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

Copilot · 2025-12-04T19:23:27Z

StudentServiceTests/Admin/AdminStudentServiceTests.cs

 using MockQueryable;
 using MockQueryable.Moq;
 using Moq;
+using NuGet.Protocol.Core.Types;


Unnecessary import added. NuGet.Protocol.Core.Types is imported but not used anywhere in this test file. This should be removed to keep the imports clean.

Suggested change

using NuGet.Protocol.Core.Types;

Copilot · 2025-12-04T19:23:28Z

StudentServiceTests/Admin/AdminStudentServiceTests.cs

-            Assert.That(result, Is.False);
+            // Assert & Act
+            var ex = Assert.ThrowsAsync<KeyNotFoundException>(() => adminStudentService.ExistAsync(studentId));
+            Assert.That(ex.Message, Is.EqualTo("Student with ID: 1 not found"));


Missing '.' at the end of the exception message. For consistency with other error messages in the codebase (e.g., line 186, 314, 361, etc.), this message should end with a period.

Suggested change

Assert.That(ex.Message, Is.EqualTo("Student with ID: 1 not found"));

Assert.That(ex.Message, Is.EqualTo("Student with ID: 1 not found."));

Copilot · 2025-12-04T19:23:28Z

StudentManagementSystem.Core/Services/TeacherService.cs

        public async Task<AbsenceFormViewModel> GetAbsenceByIdAsync(int absenceId)
        {
-            var absence =await repository.All<Absence>()
+            var absence =await repository.AllAsReadOnly<Absence>()


Missing space after the equals sign. The code has var absence =await repository.AllAsReadOnly<Absence>() but should be var absence = await repository.AllAsReadOnly<Absence>() for proper spacing.

Suggested change

var absence =await repository.AllAsReadOnly<Absence>()

var absence = await repository.AllAsReadOnly<Absence>()

Copilot · 2025-12-04T19:23:28Z

StudentServiceTests/StudentServiceTests.cs

 using MockQueryable;
 using MockQueryable.Moq;
 using Moq;
+using NUnit.Framework;


Unused import added. The NUnit.Framework namespace is added but is already available through other test infrastructure in this file. This explicit import is redundant and should be removed unless it's specifically needed for a new feature not visible in this diff.

Suggested change

using NUnit.Framework;

Copilot · 2025-12-04T19:23:29Z

render.yaml

    plan: free
    region: frankfurt
-    branch: master
+    branch: render-deployment


This deployment configuration change from master to render-deployment is not mentioned in the PR description. The PR description focuses on error handling standardization, but this change modifies which branch gets deployed. If this change is intentional, it should be documented in the PR description. If this is an accidental inclusion, it should be removed from this PR and handled separately.

Suggested change

branch: render-deployment

branch: master

Copilot · 2025-12-04T19:23:29Z

StudentServiceTests/Admin/AdminTeacherServiceTests.cs

+using Microsoft.Extensions.Logging;
 using MockQueryable;
 using Moq;
+using StudentManagementSystem.Areas.Admin.Controllers;


Unused import added. The StudentManagementSystem.Areas.Admin.Controllers import is added but doesn't appear to be used in the visible portions of this test file. This should be removed to keep imports clean.

Suggested change

using StudentManagementSystem.Areas.Admin.Controllers;

Copilot · 2025-12-04T19:23:30Z

StudentServiceTests/Admin/AdminCourseServiceTests.cs

 using Moq;
-using Microsoft.Extensions.Logging;
 using StudentManagementSystem.Core.Models.Admin.Course;
+using StudentManagementSystem.Core.Services;


Unused import added. The StudentManagementSystem.Core.Services import is added but doesn't appear to be used in this test file based on the visible changes. This should be removed unless it's needed for functionality not shown in this diff.

Suggested change

using StudentManagementSystem.Core.Services;

Copilot · 2025-12-04T19:23:30Z

StudentServiceTests/Admin/AdminStudentServiceTests.cs

+                new Absence { Id = 1, StudentId = studentId, Course = new Course { Name = "Math" }, Date = new DateTime(2024,12,1), IsDeleted = false },
+                new Absence { Id = 2, StudentId = studentId, Course = new Course { Name = "Science" }, Date = new DateTime(2024,12,2), IsDeleted = false }


Inconsistent date formatting. This line uses new DateTime(2024,12,1) without spaces after commas, while line 645 uses new DateTime(2024,12,2). For consistency and readability, use spaces after commas: new DateTime(2024, 12, 1).

Suggested change

new Absence { Id = 1, StudentId = studentId, Course = new Course { Name = "Math" }, Date = new DateTime(2024,12,1), IsDeleted = false },

new Absence { Id = 2, StudentId = studentId, Course = new Course { Name = "Science" }, Date = new DateTime(2024,12,2), IsDeleted = false }

new Absence { Id = 1, StudentId = studentId, Course = new Course { Name = "Math" }, Date = new DateTime(2024, 12, 1), IsDeleted = false },

new Absence { Id = 2, StudentId = studentId, Course = new Course { Name = "Science" }, Date = new DateTime(2024, 12, 2), IsDeleted = false }

Copilot · 2025-12-04T19:23:30Z

StudentServiceTests/Admin/AdminStudentServiceTests.cs

+                new Absence { Id = 1, StudentId = studentId, Course = new Course { Name = "Math" }, Date = new DateTime(2024,12,1), IsDeleted = false },
+                new Absence { Id = 2, StudentId = studentId, Course = new Course { Name = "Science" }, Date = new DateTime(2024,12,2), IsDeleted = false }


Inconsistent date formatting. This line uses new DateTime(2024,12,2) without spaces after commas. For consistency with the rest of the codebase, use spaces after commas: new DateTime(2024, 12, 2).

Suggested change

new Absence { Id = 1, StudentId = studentId, Course = new Course { Name = "Math" }, Date = new DateTime(2024,12,1), IsDeleted = false },

new Absence { Id = 2, StudentId = studentId, Course = new Course { Name = "Science" }, Date = new DateTime(2024,12,2), IsDeleted = false }

new Absence { Id = 1, StudentId = studentId, Course = new Course { Name = "Math" }, Date = new DateTime(2024, 12, 1), IsDeleted = false },

new Absence { Id = 2, StudentId = studentId, Course = new Course { Name = "Science" }, Date = new DateTime(2024, 12, 2), IsDeleted = false }

Copilot · 2025-12-04T19:23:31Z

StudentManagementSystem.Core/Services/TeacherService.cs

-            var absence =await repository.All<Absence>()
+            var absence =await repository.AllAsReadOnly<Absence>()
                .Where(a => a.Id == absenceId && !a.IsDeleted)
                .Select(a=> new AbsenceFormViewModel


Inconsistent lambda syntax in Select projection. The lambda uses a=> without spaces, while the codebase typically uses spaces around operators. For consistency, change to a => new AbsenceFormViewModel.

Suggested change

.Select(a=> new AbsenceFormViewModel

.Select(a => new AbsenceFormViewModel

VelkoTgr6 added 3 commits December 4, 2025 21:15

Update render.yaml for deployment configuration

94aa85b

Switched the deployment branch for the `student-management-system` service from `master` to `render-deployment

VelkoTgr6 requested a review from Copilot December 4, 2025 19:19

VelkoTgr6 merged commit 2c22f9a into render-deployment Dec 4, 2025
2 checks passed

Copilot started reviewing on behalf of VelkoTgr6 December 4, 2025 19:19 View session

Copilot finished reviewing on behalf of VelkoTgr6 December 4, 2025 19:22

Copilot AI reviewed Dec 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Development#13

Development#13
VelkoTgr6 merged 3 commits intorender-deploymentfrom
development

VelkoTgr6 commented Dec 4, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Assert.That(ex.Message, Is.EqualTo("Student with ID: 1 not found"));
	Assert.That(ex.Message, Is.EqualTo("Student with ID: 1 not found."));

	var absence =await repository.AllAsReadOnly<Absence>()
	var absence = await repository.AllAsReadOnly<Absence>()

		new Absence { Id = 1, StudentId = studentId, Course = new Course { Name = "Math" }, Date = new DateTime(2024,12,1), IsDeleted = false },
		new Absence { Id = 2, StudentId = studentId, Course = new Course { Name = "Science" }, Date = new DateTime(2024,12,2), IsDeleted = false }

	.Select(a=> new AbsenceFormViewModel
	.Select(a => new AbsenceFormViewModel

Conversation

VelkoTgr6 commented Dec 4, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants