Exam mode: Only send student notifications when end time has changed

[aplr]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• This is a small issue that I tested locally and was confirmed by another developer on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Server

• Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
• I followed the coding and design guidelines.
• I added multiple integration tests (Spring) related to the features (with a high test coverage).
• I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
• I documented the Java code using JavaDoc style.

Motivation and Context

This PR fixes a bug introduced in #7284. When saving, the code comparing the new and old end date uses equals, which always returns false. This concern was already discussed about in the comments of the PR, however failed to address it properly, and the issue was not found in manual testing.

Description

This PR now uses the comparator, comparing dates on seconds-precision.

Steps for Testing

Case 1: No date change - no notification

Prerequisites:

• 1 Instructor
• 2 Students
• 1 Exam

1. Log in to Artemis
2. Navigate to Course Administration
3. Set the exam dates so the exam can be worked on by students
4. Go to exam edit
5. Save the exam, without changing the dates.
6. The students should NOT receive a notification

Case 2: Date change + notification

Prerequisites:

• 1 Instructor
• 2 Students
• 1 Exam

1. Log in to Artemis
2. Navigate to Course Administration
3. Set the exam dates so the exam can be worked on by students
4. Go to exam edit
5. Change the end date of the exam
6. Save the exam, without changing the dates.
7. The students SHOULD receive a notification

Review Progress

Performance Review

• I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance
• I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

• Code Review 1
• Code Review 2

Exam Mode Test

• Test 1
• Test 2

Test Coverage

[aplr]

TODO:

• Server: Test if notifications are sent when time was changed
• Server: Test if no notification is sent when time was not changed
• Client: E2E test if confirmation pops up when time was changed
• Client: E2E test if confirmation does not pop up if no time was changed

I'd skip the E2E's as it was a server issue, and rather do them in a separate PR.

[Strohgelaender]

Please add a comment why equals cannot be used here.

[aplr]

Please add a comment why equals cannot be used here.

There's already an explanation a few lines above. Imho, this is sufficient. Tell me if you think otherwise.

Artemis/src/main/java/de/tum/in/www1/artemis/web/rest/ExamResource.java

Line 222 in 45f33b2

// We can't test dates for equality as the dates retrieved from the database lose precision. Also use instant to take timezones into account

[aplr]

Interesting thing: There is a test for the case, that a exam title is updated, without updating the dates, and no notification should be sent. However, this test just does not work, as the update data is not sent from the client, but uses the server-side dates. These dates of course match (i.e. oldDate.equals(newDate) returns true) -> the notification is never sent.

However, if the data is coming from the client, the dates do not match w/ equals. We have to verify that in the tests.

Here's the test:

Artemis/src/test/java/de/tum/in/www1/artemis/exam/ExamIntegrationTest.java

Lines 423 to 427 in 8425924

	// Update the exam -> ok
	exam1.setTitle("Best exam ever");
	var returnedExam = request.putWithResponseBody("/api/courses/" + course1.getId() + "/exams", exam1, Exam.class, HttpStatus.OK);
	assertThat(returnedExam).isEqualTo(exam1);
	verify(instanceMessageSendService, never()).sendProgrammingExerciseSchedule(any());

[aplr]

Note: this is the only change in this PR. The rest is adding & fixing tests

[aplr]

Note: In general, I've ripped some tests apart and created individual tests. This way, it is clearer what is tested, failing cases can be pinned down more easily, and when refactoring, only single test cases have to be touched. Please tell me if you explicitly want these aggregated test cases in Artemis, and this was a design decision.

[Strohgelaender]

Looks good in general, thanks for taking the time to cleanup and improve the tests. Just a few small suggestions:

[jakubriegel]

Works on ts1

[lennart-keller]

Tested in testing session

[vinceclifford]

Works as expected, tested on ts1