Skip to content

Commit

Permalink
Merge branch 'develop' into feature/learning-analytics-dashboard
Browse files Browse the repository at this point in the history
  • Loading branch information
JohannesWt authored Nov 2, 2024
2 parents b9f633a + 4954074 commit f69cdf7
Show file tree
Hide file tree
Showing 143 changed files with 1,027 additions and 5,137 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ jacocoTestCoverageVerification {
counter = "INSTRUCTION"
value = "COVEREDRATIO"
// TODO: in the future the following value should become higher than 0.92
minimum = 0.894
minimum = 0.892
}
limit {
counter = "CLASS"
Expand Down
3 changes: 2 additions & 1 deletion docs/admin/setup/distributed.rst
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,6 @@ These credentials are used to clone repositories via HTTPS. You must also add th
specify-concurrent-builds: true # Set to false, if the number of concurrent build jobs should be chosen automatically based on system resources
concurrent-build-size: 1 # If previous value is true: Set to desired value but keep available system resources in mind
asynchronous: true
timeout-seconds: 240 # Time limit of a build before it will be cancelled
build-container-prefix: local-ci-
image-cleanup:
enabled: true # If set to true (recommended), old Docker images will be deleted on a schedule.
Expand All @@ -620,6 +619,8 @@ These credentials are used to clone repositories via HTTPS. You must also add th
build-agent:
short-name: "artemis-build-agent-X" # Short name of the build agent. This should be unique for each build agent. Only lowercase letters, numbers and hyphens are allowed.
display-name: "Artemis Build Agent X" # This value is optional. If omitted, the short name will be used as display name. Display name of the build agent. This is shown in the Artemis UI.
build-timeout-seconds:
max: 240 # (Optional, default 240) Maximum time in seconds a build job is allowed to run. If a build job exceeds this time, it will be cancelled.
Please note that ``artemis.continuous-integration.build-agent.short-name`` must be provided. Otherwise, the build agent will not start.
Expand Down
21 changes: 21 additions & 0 deletions docs/admin/setup/programming-exercises.rst
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,24 @@ Adjust ``dockerFlags`` and ``mavenFlags`` only for student submissions, like thi
dockerFlags += ' --network none'
mavenFlags += ' --offline'
}
Timeout Options
^^^^^^^^^^^^^^^

You can adjust possible :ref:`timeout options<edit_build_duration>` for the build process in :ref:`Integrated Code Lifecycle Setup <Integrated Code Lifecycle Setup>`.
These values will determine what is the minimum, maximum, and default value for the build timeout in seconds that can be set in the Artemis UI.
The max value is the upper limit for the timeout, if the value is set higher than the max value, the max value will be used.

If you want to change these values, you need to change them in ``localci`` and ``buildagent`` nodes.
The corresponding configuration files are ``application-localci.yml`` and ``application-buildagent.yml``.


.. code-block:: yaml
artemis:
continuous-integration:
build-timeout-seconds:
min: <value>
max: <value>
default: <value>
2 changes: 1 addition & 1 deletion docs/dev/setup.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ following dependencies/tools on your machine:
2. `MySQL Database Server 9 <https://dev.mysql.com/downloads/mysql>`__, or `PostgreSQL 17 <https://www.postgresql.org/>`_:
Artemis uses Hibernate to store entities in an SQL database and Liquibase to
automatically apply schema transformations when updating Artemis.
3. `Node.js <https://nodejs.org/en/download>`__: We use Node LTS (>=20.16.0 < 21) to compile
3. `Node.js <https://nodejs.org/en/download>`__: We use Node LTS (>=22.10.0 < 23) to compile
and run the client Angular application. Depending on your system, you
can install Node either from source or as a pre-packaged bundle.
4. `Npm <https://nodejs.org/en/download>`__: We use Npm (>=10.8.0) to
Expand Down
4 changes: 3 additions & 1 deletion docs/user/exercises/programming-exercise-setup.inc
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ You must then change the paths in the build script if necessary. Please refer to
- Changing the checkout paths can lead to build errors if the build script is not adapted accordingly.
- For C programming exercises, if used with the default docker image, changing the checkout paths will lead to build errors. The default docker image is configured to work with the default checkout paths.

.. _configure_static_code_analysis_tools:
.. _edit_build_duration:

Edit Maximum Build Duration
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -412,6 +412,8 @@ You can change the maximum build duration by using the slider.
.. figure:: programming/timeout-slider.png
:align: center

.. _configure_static_code_analysis_tools:

Configure static code analysis
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ rootProject.name=Artemis
profile=dev

# Build properties
node_version=20.16.0
node_version=22.10.0
npm_version=10.8.0

# Dependency versions
Expand Down
8 changes: 4 additions & 4 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ module.exports = {
coverageThreshold: {
global: {
// TODO: in the future, the following values should increase to at least 90%
statements: 87.52,
branches: 73.62,
functions: 82.12,
lines: 87.57,
statements: 87.46,
branches: 73.56,
functions: 82.04,
lines: 87.52,
},
},
coverageReporters: ['clover', 'json', 'lcov', 'text-summary'],
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
"weak-napi": "2.0.2"
},
"engines": {
"node": ">=20.16.0"
"node": ">=22.10.0"
},
"scripts": {
"build": "npm run webapp:prod --",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
import java.util.Optional;

import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;
import org.springframework.transaction.annotation.Transactional;

import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText;
import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository;
Expand Down Expand Up @@ -42,6 +44,14 @@ public interface LongFeedbackTextRepository extends ArtemisJpaRepository<LongFee
""")
Optional<LongFeedbackText> findWithFeedbackAndResultAndParticipationByFeedbackId(@Param("feedbackId") final Long feedbackId);

@Modifying
@Transactional
@Query("""
DELETE FROM LongFeedbackText longFeedback
WHERE longFeedback.feedback.id IN :feedbackIds
""")
void deleteByFeedbackIds(@Param("feedbackIds") List<Long> feedbackIds);

default LongFeedbackText findByFeedbackIdWithFeedbackAndResultAndParticipationElseThrow(final Long feedbackId) {
return getValueElseThrow(findWithFeedbackAndResultAndParticipationByFeedbackId(feedbackId), feedbackId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import de.tum.cit.aet.artemis.assessment.dto.AssessmentUpdateBaseDTO;
import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository;
import de.tum.cit.aet.artemis.assessment.repository.FeedbackRepository;
import de.tum.cit.aet.artemis.assessment.repository.LongFeedbackTextRepository;
import de.tum.cit.aet.artemis.assessment.repository.ResultRepository;
import de.tum.cit.aet.artemis.assessment.web.ResultWebsocketService;
import de.tum.cit.aet.artemis.communication.service.notifications.SingleUserNotificationService;
Expand Down Expand Up @@ -67,10 +68,12 @@ public class AssessmentService {

protected final ResultWebsocketService resultWebsocketService;

private final LongFeedbackTextRepository longFeedbackTextRepository;

public AssessmentService(ComplaintResponseService complaintResponseService, ComplaintRepository complaintRepository, FeedbackRepository feedbackRepository,
ResultRepository resultRepository, StudentParticipationRepository studentParticipationRepository, ResultService resultService, SubmissionService submissionService,
SubmissionRepository submissionRepository, ExamDateService examDateService, UserRepository userRepository, Optional<LtiNewResultService> ltiNewResultService,
SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService) {
SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService, LongFeedbackTextRepository longFeedbackTextRepository) {
this.complaintResponseService = complaintResponseService;
this.complaintRepository = complaintRepository;
this.feedbackRepository = feedbackRepository;
Expand All @@ -84,6 +87,7 @@ public AssessmentService(ComplaintResponseService complaintResponseService, Comp
this.ltiNewResultService = ltiNewResultService;
this.singleUserNotificationService = singleUserNotificationService;
this.resultWebsocketService = resultWebsocketService;
this.longFeedbackTextRepository = longFeedbackTextRepository;
}

/**
Expand Down Expand Up @@ -283,6 +287,9 @@ public Result saveManualAssessment(final Submission submission, final List<Feedb
User user = userRepository.getUser();
result.setAssessor(user);

// long feedback text is deleted as it otherwise causes duplicate entries errors and will be saved again with {@link resultRepository.save}
resultService.deleteLongFeedback(feedbackList, result);

result.updateAllFeedbackItems(feedbackList, false);
result.determineAssessmentType();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

import jakarta.annotation.Nullable;
Expand Down Expand Up @@ -494,45 +495,49 @@ public Map<Long, String> getLogsAvailabilityForResults(List<Result> results, Par

@NotNull
private List<Feedback> saveFeedbackWithHibernateWorkaround(@NotNull Result result, List<Feedback> feedbackList) {
// Avoid hibernate exception
List<Feedback> savedFeedbacks = new ArrayList<>();
// Collect ids of feedbacks that have long feedback.
List<Long> feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId)
.toList();
// Get long feedback list from the database.
List<LongFeedbackText> longFeedbackTextList = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongFeedback);

// Convert list to map for accessing later.
Map<Long, LongFeedbackText> longLongFeedbackTextMap = longFeedbackTextList.stream()
.collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), longFeedbackText -> longFeedbackText));
feedbackList.forEach(feedback -> {
// cut association to parent object
feedback.setResult(null);
LongFeedbackText longFeedback = null;
// look for long feedback that parent feedback has and cut the association between them.
if (feedback.getId() != null && feedback.getHasLongFeedbackText()) {
longFeedback = longLongFeedbackTextMap.get(feedback.getId());
if (longFeedback != null) {
feedback.clearLongFeedback();
}
}
// persist the child object without an association to the parent object.
feedback = feedbackRepository.saveAndFlush(feedback);
// restore the association to the parent object
feedback.setResult(result);
// Fetch long feedback texts associated with the provided feedback list
Map<Long, LongFeedbackText> longFeedbackTextMap = longFeedbackTextRepository
.findByFeedbackIds(feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId).toList()).stream()
.collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), Function.identity()));

// restore the association of the long feedback to the parent feedback
if (longFeedback != null) {
feedback.setDetailText(longFeedback.getText());
}
feedbackList.forEach(feedback -> {
handleFeedbackPersistence(feedback, result, longFeedbackTextMap);
savedFeedbacks.add(feedback);
});

return savedFeedbacks;
}

private void handleFeedbackPersistence(Feedback feedback, Result result, Map<Long, LongFeedbackText> longFeedbackTextMap) {
// Temporarily detach feedback from the parent result to avoid Hibernate issues
feedback.setResult(null);

// Clear the long feedback if it exists in the map
if (feedback.getId() != null && feedback.getHasLongFeedbackText()) {
LongFeedbackText longFeedback = longFeedbackTextMap.get(feedback.getId());
if (longFeedback != null) {
feedback.clearLongFeedback();
}
}

// Persist the feedback entity without the parent association
feedback = feedbackRepository.saveAndFlush(feedback);

// Restore associations to the result and long feedback after persistence
feedback.setResult(result);
LongFeedbackText longFeedback = longFeedbackTextMap.get(feedback.getId());
if (longFeedback != null) {
feedback.setDetailText(longFeedback.getText());
}
}

@NotNull
private Result shouldSaveResult(@NotNull Result result, boolean shouldSave) {
if (shouldSave) {
// long feedback text is deleted as it otherwise causes duplicate entries errors and will be saved again with {@link resultRepository.save}
deleteLongFeedback(result.getFeedbacks(), result);
// Note: This also saves the feedback objects in the database because of the 'cascade = CascadeType.ALL' option.
return resultRepository.save(result);
}
Expand Down Expand Up @@ -623,4 +628,32 @@ public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, Fee
public long getMaxCountForExercise(long exerciseId) {
return studentParticipationRepository.findMaxCountForExercise(exerciseId);
}

/**
* Deletes long feedback texts for the provided list of feedback items to prevent duplicate entries in the {@link LongFeedbackTextRepository}.
* <br>
* This method processes the provided list of feedback items, identifies those with associated long feedback texts, and removes them in bulk
* from the repository to avoid potential duplicate entry errors when saving new feedback entries.
* <p>
* Primarily used to ensure data consistency in the {@link LongFeedbackTextRepository}, especially during operations where feedback entries are
* overridden or updated. The deletion is performed only for feedback items with a non-null ID and an associated long feedback text.
* <p>
* This approach reduces the need for individual deletion calls and performs batch deletion in a single database operation.
*
* @param feedbackList The list of {@link Feedback} objects for which the long feedback texts are to be deleted. Only feedback items that have long feedback texts and a
* non-null ID will be processed.
* @param result The {@link Result} object associated with the feedback items, used to update feedback list before processing.
*/
public void deleteLongFeedback(List<Feedback> feedbackList, Result result) {
if (feedbackList == null) {
return;
}

List<Long> feedbackIdsWithLongText = feedbackList.stream().filter(feedback -> feedback.getHasLongFeedbackText() && feedback.getId() != null).map(Feedback::getId).toList();

longFeedbackTextRepository.deleteByFeedbackIds(feedbackIdsWithLongText);

List<Feedback> feedbacks = new ArrayList<>(feedbackList);
result.updateAllFeedbackItems(feedbacks, false);
}
}

This file was deleted.

Loading

0 comments on commit f69cdf7

Please sign in to comment.