From 73307096c36f4a012ff0325891a5e1700b6b3a74 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Wed, 20 Nov 2024 12:56:21 +0100
Subject: [PATCH] Fix some remaining issues from the Spring migration (#20)
A few missing `@Bean` definitions were discovered.
Spring also has a much stricter requirement about requiring `@Transactional` for every database change.
## How to test `@Bean GroupFacadeImpl`
1. Log in as a supervisor
2. Go to "My groups" and create a group
3. Go back to "My groups" and try to open the group
## How to test `@Bean IdeaFacadae`
1. Create an application period that is open ("Admin" / "Match" / "Application periods")
2. Log in as an author
3. Go to "Ideas" / "My ideas"
4. Click "Select from available ideas" in the period created in step 1
## How to test missing `@Transactional`
1. Log in as a user with notifications (or generate some by for example writing in the forum)
2. Go to "Notifications" in the top right
3. Click on any notification subject to navigate to it
## How to test crash while trying to schedule final seminar
1. Log in as a supervisor
2. Open a project and attempt to schedule a final seminar
## How to test crash while trying to upload final thesis as supervisor
1. Log in as supervisor
2. Note down the supervisors username
3. Open a project that has had a final seminar
4. Go to the "Finishing up" tab
5. Submit the supervisors username as the custom principal
6. Try to upload a final thesis
## How to test removal of approved final thesis
1. Log in as supervisor
2. Note down the supervisors username
3. Open a project that has had a final seminar
4. Go to the "Finishing up" tab
5. Submit the supervisors username as the custom principal
6. Upload a final thesis
7. Approve the final thesis
8. Remove the approval
## How to test crash while trying to schedule first meeting
1. Log in as supervisor
2. Open a project
3. Go to the "First meeting tab"
4. Try to submit
## How to test crash while trying to unselect an idea as an author
1. Create an application period that is open ("Admin" / "Match" / "Application periods")
2. Log in as an author
3. Go to "Ideas" / "My ideas"
4. Click "Select from available ideas" in the period created in step 1
5. Select any available supervisor idea
6. Go back to "Ideas" / "My ideas"
7. Open the selected idea
8. Hit unselect at the bottom
## How to test crash while trying to toggle milestone
1. Log in as supervisor
2. Open any project
3. Go to "Milestones" tab
4. Attempt to toggle both individual and project milestones
## How to test crash while trying to get user's note
1. Find a user without a row in the `note` table
2. Log in as that user
3. Click "My notes" in the top right
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/20
Reviewed-by: Nico Athanassiadis <nico@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
---
core/src/main/java/se/su/dsv/scipro/CoreConfig.java | 12 ++++++++++++
.../FinalSeminarRespondentServiceImpl.java | 2 ++
.../scipro/finalthesis/FinalThesisServiceImpl.java | 2 ++
.../scipro/firstmeeting/FirstMeetingServiceImpl.java | 2 ++
.../java/se/su/dsv/scipro/match/IdeaServiceImpl.java | 3 +--
.../service/impl/MilestoneServiceImpl.java | 3 +++
.../java/se/su/dsv/scipro/notes/NoteServiceImpl.java | 2 ++
.../notifications/NotificationServiceImpl.java | 2 ++
.../dsv/scipro/report/GradingReportServiceImpl.java | 1 +
.../se/su/dsv/scipro/report/ReportServiceImpl.java | 2 ++
.../reviewing/ReviewerInteractionServiceImpl.java | 1 +
.../dsv/scipro/reviewing/ReviewingServiceImpl.java | 3 +++
.../serviceimpls/UserProfileServiceImpl.java | 1 +
.../dsv/scipro/system/ResearchAreaServiceImpl.java | 3 +++
14 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
index d9245d2155..a6ec1b8ffe 100644
--- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
+++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
@@ -65,6 +65,7 @@ import se.su.dsv.scipro.grading.NationalSubjectCategoryServiceImpl;
import se.su.dsv.scipro.grading.PublicationMetadataRepository;
import se.su.dsv.scipro.grading.PublicationMetadataServiceImpl;
import se.su.dsv.scipro.grading.ThesisSubmissionHistoryService;
+import se.su.dsv.scipro.group.GroupFacadeImpl;
import se.su.dsv.scipro.group.GroupService;
import se.su.dsv.scipro.group.GroupServiceImpl;
import se.su.dsv.scipro.integration.activityfinalseminar.ActivityFinalSeminarRepository;
@@ -79,6 +80,7 @@ import se.su.dsv.scipro.match.ApplicationPeriodProjectTypeServiceImpl;
import se.su.dsv.scipro.match.ApplicationPeriodService;
import se.su.dsv.scipro.match.ApplicationPeriodServiceImpl;
import se.su.dsv.scipro.match.FirstMeetingRepository;
+import se.su.dsv.scipro.match.IdeaFacade;
import se.su.dsv.scipro.match.IdeaRepository;
import se.su.dsv.scipro.match.IdeaService;
import se.su.dsv.scipro.match.IdeaServiceImpl;
@@ -1009,4 +1011,14 @@ public class CoreConfig {
public NotificationEventServiceImpl notificationEventService(NotificationEventRepository notificationEventRepository) {
return new NotificationEventServiceImpl(notificationEventRepository);
}
+
+ @Bean
+ public IdeaFacade ideaFacade() {
+ return new IdeaFacade();
+ }
+
+ @Bean
+ public GroupFacadeImpl groupFacade() {
+ return new GroupFacadeImpl();
+ }
}
diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarRespondentServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarRespondentServiceImpl.java
index ee7478c33c..f532f0289d 100644
--- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarRespondentServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarRespondentServiceImpl.java
@@ -1,5 +1,6 @@
package se.su.dsv.scipro.finalseminar;
+import jakarta.transaction.Transactional;
import se.su.dsv.scipro.system.AbstractServiceImpl;
import se.su.dsv.scipro.system.User;
@@ -33,6 +34,7 @@ public class FinalSeminarRespondentServiceImpl extends AbstractServiceImpl<Final
}
@Override
+ @Transactional
public List<FinalSeminarRespondent> findOrCreate(FinalSeminar finalSeminar) {
if(finalSeminar.getId() == null) {
return new ArrayList<>();
diff --git a/core/src/main/java/se/su/dsv/scipro/finalthesis/FinalThesisServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/finalthesis/FinalThesisServiceImpl.java
index 41251d1700..f505a3d189 100644
--- a/core/src/main/java/se/su/dsv/scipro/finalthesis/FinalThesisServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/finalthesis/FinalThesisServiceImpl.java
@@ -56,6 +56,7 @@ public class FinalThesisServiceImpl extends AbstractServiceImpl<FinalThesis, Lon
}
@Override
+ @Transactional
public FinalThesis upload(ProjectFileUpload fileUpload, String englishTitle, String swedishTitle) {
ProjectFile fileDescription = storeFinalThesisFile(fileUpload);
final FileReference reference = fileService.createReference(fileDescription.getFileDescription());
@@ -133,6 +134,7 @@ public class FinalThesisServiceImpl extends AbstractServiceImpl<FinalThesis, Lon
}
@Override
+ @Transactional
public void removeApproval(Project project) {
setStatus(project, FinalThesis.Status.NO_DECISION);
}
diff --git a/core/src/main/java/se/su/dsv/scipro/firstmeeting/FirstMeetingServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/firstmeeting/FirstMeetingServiceImpl.java
index 23d1832201..1ab1e575a8 100644
--- a/core/src/main/java/se/su/dsv/scipro/firstmeeting/FirstMeetingServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/firstmeeting/FirstMeetingServiceImpl.java
@@ -1,5 +1,6 @@
package se.su.dsv.scipro.firstmeeting;
+import jakarta.transaction.Transactional;
import se.su.dsv.scipro.activityplan.Activity;
import se.su.dsv.scipro.activityplan.ActivityPlanFacade;
import se.su.dsv.scipro.project.Project;
@@ -26,6 +27,7 @@ public class FirstMeetingServiceImpl extends AbstractServiceImpl<ProjectFirstMee
}
@Override
+ @Transactional
public ProjectFirstMeeting schedule(final Project project, final Date date, final String room, final String description) {
final Optional<ProjectFirstMeeting> optFirstMeeting = findByProject(project);
final ProjectFirstMeeting firstMeeting = optFirstMeeting
diff --git a/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java
index d3c410eb05..e6be18e1c3 100755
--- a/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java
@@ -358,7 +358,6 @@ public class IdeaServiceImpl extends AbstractServiceImpl<Idea, Long> implements
}
}
- @Transactional
private Idea acceptIdea0(Idea idea, User creator, Program accepteeProgram, User coAuthor, final ApplicationPeriod applicationPeriod) {
Idea localIdea = idea;
@@ -442,6 +441,7 @@ public class IdeaServiceImpl extends AbstractServiceImpl<Idea, Long> implements
}
@Override
+ @Transactional
public void studentUnselect(final Idea idea, final User student) {
unmatch(idea, student, new NotificationSource());
}
@@ -490,7 +490,6 @@ public class IdeaServiceImpl extends AbstractServiceImpl<Idea, Long> implements
return returnValue;
}
- @Transactional
private Idea addMatch(User creator, User supervisor, Idea idea, NotificationSource notificationSource) {
Match match = new Match();
match.setIdea(idea);
diff --git a/core/src/main/java/se/su/dsv/scipro/milestones/service/impl/MilestoneServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/milestones/service/impl/MilestoneServiceImpl.java
index 70c69ddef2..f7b5b8c447 100644
--- a/core/src/main/java/se/su/dsv/scipro/milestones/service/impl/MilestoneServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/milestones/service/impl/MilestoneServiceImpl.java
@@ -4,6 +4,7 @@ import com.querydsl.core.BooleanBuilder;
import com.querydsl.core.types.Predicate;
import com.querydsl.core.types.dsl.BooleanExpression;
import com.querydsl.jpa.impl.JPAQuery;
+import jakarta.transaction.Transactional;
import se.su.dsv.scipro.system.Pageable;
import se.su.dsv.scipro.milestones.dataobjects.Milestone;
import se.su.dsv.scipro.milestones.dataobjects.MilestoneActivityTemplate;
@@ -54,6 +55,7 @@ public class MilestoneServiceImpl extends AbstractServiceImpl<Milestone, Long> i
}
@Override
+ @Transactional
public void setConfirmed(Project project, MilestoneActivityTemplate activity, boolean confirmed) {
if (confirmed) {
for (MilestoneActivityTemplate earlierActivity : getEarlierIncompleteProjectMilestoneActivities(project, activity)) {
@@ -84,6 +86,7 @@ public class MilestoneServiceImpl extends AbstractServiceImpl<Milestone, Long> i
}
@Override
+ @Transactional
public void setConfirmed(Project project, User student, MilestoneActivityTemplate activity, boolean confirmed) {
if (!project.hasModule(ProjectModule.MILESTONES)) {
diff --git a/core/src/main/java/se/su/dsv/scipro/notes/NoteServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/notes/NoteServiceImpl.java
index 126470035e..58cb33dbc1 100755
--- a/core/src/main/java/se/su/dsv/scipro/notes/NoteServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/notes/NoteServiceImpl.java
@@ -1,6 +1,7 @@
package se.su.dsv.scipro.notes;
import com.querydsl.core.types.dsl.BooleanExpression;
+import jakarta.transaction.Transactional;
import se.su.dsv.scipro.system.AbstractServiceImpl;
import se.su.dsv.scipro.system.User;
@@ -20,6 +21,7 @@ public class NoteServiceImpl extends AbstractServiceImpl<Note, Long> implements
}
@Override
+ @Transactional
public Note getNote(User user) {
Note note = findOne(hasUser(user));
if (note == null){
diff --git a/core/src/main/java/se/su/dsv/scipro/notifications/NotificationServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/notifications/NotificationServiceImpl.java
index 6c460a0741..5bbe68093e 100755
--- a/core/src/main/java/se/su/dsv/scipro/notifications/NotificationServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/notifications/NotificationServiceImpl.java
@@ -50,6 +50,7 @@ public class NotificationServiceImpl extends AbstractServiceImpl<Notification,Lo
}
@Override
+ @Transactional
public void setRead(User user, NotificationEvent notificationEvent, boolean read) {
Iterable<Notification> notifications = findAll(
QNotification.notification.notificationEvent.eq(notificationEvent)
@@ -62,6 +63,7 @@ public class NotificationServiceImpl extends AbstractServiceImpl<Notification,Lo
}
@Override
+ @Transactional
@SuppressWarnings("unchecked")
// This looks so wrong but a List<Notification> is what we actually get
// during runtime, see SCIPRO-167935. Reflection so awesome...
diff --git a/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java
index 1dd53e1665..876d3a4010 100644
--- a/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java
@@ -48,6 +48,7 @@ public class GradingReportServiceImpl implements GradingReportTemplateService, G
}
@Override
+ @Transactional
public boolean updateOppositionCriteria(SupervisorGradingReport report, FinalSeminarOpposition opposition) {
for (GradingCriterion gradingCriterion : report.getIndividualCriteria()) {
boolean isOppositionCriterion = gradingCriterion.getFlag() == GradingCriterion.Flag.OPPOSITION;
diff --git a/core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java
index 44d938d819..a0f324029f 100644
--- a/core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java
@@ -22,12 +22,14 @@ public class ReportServiceImpl extends AbstractServiceImpl<Report, Long> impleme
}
@Override
+ @Transactional
public AttachmentReport submit(AttachmentReport report) {
report.submit();
return save(report);
}
@Override
+ @Transactional
public void save(AttachmentReport report, Optional<FileUpload> fileUpload) {
storeReportFile(report, fileUpload);
save(report);
diff --git a/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewerInteractionServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewerInteractionServiceImpl.java
index a08f5ffcbc..f0e62aacc6 100644
--- a/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewerInteractionServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewerInteractionServiceImpl.java
@@ -49,6 +49,7 @@ public class ReviewerInteractionServiceImpl implements ReviewerInteractionServic
}
@Override
+ @Transactional
public ForumPost reply(final Project project, final User user, final String content, final Set<Attachment> attachments) {
ReviewerThread reviewerThread = getReviewerThread(project);
ForumPost reply = forumService.createReply(reviewerThread.getForumThread(), user, content, attachments);
diff --git a/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewingServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewingServiceImpl.java
index 9087510e37..b56e733c68 100644
--- a/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewingServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/reviewing/ReviewingServiceImpl.java
@@ -7,6 +7,7 @@ import com.querydsl.core.types.Predicate;
import com.querydsl.core.types.dsl.BooleanExpression;
import com.querydsl.core.types.dsl.LiteralExpression;
import com.querydsl.jpa.impl.JPAQuery;
+import jakarta.transaction.Transactional;
import se.su.dsv.scipro.file.FileReference;
import se.su.dsv.scipro.file.FileService;
import se.su.dsv.scipro.file.FileUpload;
@@ -92,6 +93,7 @@ public class ReviewingServiceImpl extends AbstractServiceImpl<ReviewerApproval,
}
@Override
+ @Transactional
public void reject(final ReviewerApproval reviewerApproval, final String reason, final Optional<FileUpload> feedback) {
Optional<FileReference> feedbackFile = store(feedback);
reviewerApproval.reject(reason, feedbackFile);
@@ -106,6 +108,7 @@ public class ReviewingServiceImpl extends AbstractServiceImpl<ReviewerApproval,
}
@Override
+ @Transactional
public void approve(final ReviewerApproval process, final String reason, final Optional<FileUpload> feedback) {
Optional<FileReference> feedbackFile = store(feedback);
process.approve(reason, feedbackFile);
diff --git a/core/src/main/java/se/su/dsv/scipro/springdata/serviceimpls/UserProfileServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/springdata/serviceimpls/UserProfileServiceImpl.java
index 788ae44b07..992aa54888 100644
--- a/core/src/main/java/se/su/dsv/scipro/springdata/serviceimpls/UserProfileServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/springdata/serviceimpls/UserProfileServiceImpl.java
@@ -38,6 +38,7 @@ public class UserProfileServiceImpl extends AbstractServiceImpl<UserProfile, Lon
}
@Override
+ @Transactional
public Roles findSelectedRole(User user) {
return findByUser(user).getSelectedRole();
}
diff --git a/core/src/main/java/se/su/dsv/scipro/system/ResearchAreaServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/system/ResearchAreaServiceImpl.java
index 76dc29c854..3c75c66915 100755
--- a/core/src/main/java/se/su/dsv/scipro/system/ResearchAreaServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/system/ResearchAreaServiceImpl.java
@@ -3,6 +3,8 @@ package se.su.dsv.scipro.system;
import jakarta.inject.Inject;
import jakarta.inject.Provider;
import jakarta.persistence.EntityManager;
+import jakarta.transaction.Transactional;
+
import java.util.Comparator;
import java.util.List;
@@ -13,6 +15,7 @@ public class ResearchAreaServiceImpl extends AbstractServiceImpl<ResearchArea,Lo
}
@Override
+ @Transactional
public ResearchArea updateExternalResearchArea(Long identifier, String name, final boolean active) {
ResearchArea ra = new ResearchArea();
if (identifier != null) {