Refactored and eliminated Design smells on teammates.logic.core package

build.gradle

@@ -41,6 +41,7 @@ def objectify = "com.googlecode.objectify:objectify:6.0.7"
 def testng = "org.testng:testng:7.6.1"
 
 dependencies {
+    testImplementation 'org.junit.jupiter:junit-jupiter:5.8.1'
     staticAnalysis("com.puppycrawl.tools:checkstyle:${checkstyleVersion}")
     staticAnalysis("net.sourceforge.pmd:pmd-java:${pmdVersion}")
     staticAnalysis("com.github.spotbugs:spotbugs:${spotbugsVersion}")
@@ -79,6 +80,7 @@ dependencies {
     testImplementation("org.seleniumhq.selenium:selenium-java:4.3.0")
     testImplementation("com.deque.html.axe-core:selenium:4.6.0")
     testImplementation(testng)
+    testImplementation("org.mockito:mockito-core:3.11.2")
     // For supporting authorization code flow locally
     testImplementation("com.google.oauth-client:google-oauth-client-jetty:1.34.1")
     // For using Gmail API

src/main/java/teammates/logic/api/EmailGenerator.java

@@ -25,6 +25,7 @@
 import teammates.common.util.Templates.EmailTemplates;
 import teammates.common.util.TimeHelper;
 import teammates.logic.core.CoursesLogic;
+import teammates.logic.core.FeedbackQuestionsLogic;
 import teammates.logic.core.FeedbackSessionsLogic;
 import teammates.logic.core.InstructorsLogic;
 import teammates.logic.core.StudentsLogic;
@@ -61,6 +62,8 @@ public final class EmailGenerator {
 
     private final CoursesLogic coursesLogic = CoursesLogic.inst();
     private final FeedbackSessionsLogic fsLogic = FeedbackSessionsLogic.inst();
+    private final FeedbackQuestionsLogic fqLogic = FeedbackQuestionsLogic.inst();
+
     private final InstructorsLogic instructorsLogic = InstructorsLogic.inst();
     private final StudentsLogic studentsLogic = StudentsLogic.inst();
 
@@ -82,8 +85,8 @@ public List<EmailWrapper> generateFeedbackSessionOpeningEmails(FeedbackSessionAt
     private List<EmailWrapper> generateFeedbackSessionOpeningOrClosingEmails(
             FeedbackSessionAttributes session, EmailType emailType) {
         CourseAttributes course = coursesLogic.getCourse(session.getCourseId());
-        boolean isEmailNeededForStudents = fsLogic.isFeedbackSessionForUserTypeToAnswer(session, false);
-        boolean isEmailNeededForInstructors = fsLogic.isFeedbackSessionForUserTypeToAnswer(session, true);
+        boolean isEmailNeededForStudents = isFeedbackSessionForUserTypeToAnswer(session, false);
+        boolean isEmailNeededForInstructors = isFeedbackSessionForUserTypeToAnswer(session, true);
         List<InstructorAttributes> instructorsToNotify = isEmailNeededForStudents
                 ? instructorsLogic.getCoOwnersForCourse(session.getCourseId())
                 : new ArrayList<>();
@@ -501,9 +504,9 @@ public List<EmailWrapper> generateFeedbackSessionClosingWithExtensionEmails(
                 deadlineExtensions.stream().filter(x -> x.getIsInstructor()).collect(Collectors.toList());
 
         boolean isEmailNeededForStudents =
-                !studentDeadlines.isEmpty() && fsLogic.isFeedbackSessionForUserTypeToAnswer(session, false);
+                !studentDeadlines.isEmpty() && isFeedbackSessionForUserTypeToAnswer(session, false);
         boolean isEmailNeededForInstructors =
-                !instructorDeadlines.isEmpty() && fsLogic.isFeedbackSessionForUserTypeToAnswer(session, true);
+                !instructorDeadlines.isEmpty() && isFeedbackSessionForUserTypeToAnswer(session, true);
 
         List<StudentAttributes> students = new ArrayList<>();
         if (isEmailNeededForStudents) {
@@ -1078,4 +1081,17 @@ private String getAdditionalContactInformationFragment(CourseAttributes course,
                 "${coOwnersEmails}", generateCoOwnersEmailsLine(course.getId()),
                 "${supportEmail}", Config.SUPPORT_EMAIL);
     }
+
+    /**
+     * Returns true if there are any questions for the specified user type (students/instructors) to answer.
+     */
+    public boolean isFeedbackSessionForUserTypeToAnswer(FeedbackSessionAttributes session, boolean isInstructor) {
+        if (!session.isVisible()) {
+            return false;
+        }
+
+        return isInstructor
+                ? fqLogic.hasFeedbackQuestionsForInstructors(session, false)
+                : fqLogic.hasFeedbackQuestionsForStudents(session);
+    }
 }

src/main/java/teammates/logic/core/FeedbackResponsesLogic.java

@@ -1,27 +1,7 @@
 package teammates.logic.core;
 
-import java.time.Instant;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
-import javax.annotation.Nullable;
-
-import teammates.common.datatransfer.AttributesDeletionQuery;
-import teammates.common.datatransfer.CourseRoster;
-import teammates.common.datatransfer.FeedbackParticipantType;
-import teammates.common.datatransfer.FeedbackResultFetchType;
-import teammates.common.datatransfer.SessionResultsBundle;
-import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
-import teammates.common.datatransfer.attributes.FeedbackResponseAttributes;
-import teammates.common.datatransfer.attributes.FeedbackResponseCommentAttributes;
-import teammates.common.datatransfer.attributes.InstructorAttributes;
-import teammates.common.datatransfer.attributes.StudentAttributes;
+import teammates.common.datatransfer.*;
+import teammates.common.datatransfer.attributes.*;
 import teammates.common.datatransfer.questions.FeedbackQuestionType;
 import teammates.common.datatransfer.questions.FeedbackRankRecipientsResponseDetails;
 import teammates.common.datatransfer.questions.FeedbackTextResponseDetails;
@@ -32,6 +12,10 @@
 import teammates.common.util.RequestTracer;
 import teammates.storage.api.FeedbackResponsesDb;
 
+import javax.annotation.Nullable;
+import java.time.Instant;
+import java.util.*;
+
 /**
  * Handles operations related to feedback responses.
  *
@@ -368,7 +352,7 @@ private SessionResultsBundle buildResultsBundle(
         // related questions, responses, and comment
         Map<String, FeedbackQuestionAttributes> relatedQuestionsMap = new HashMap<>();
         Map<String, FeedbackQuestionAttributes> relatedQuestionsNotVisibleForPreviewMap = new HashMap<>();
-        Set<String> relatedQuestionsWithCommentNotVisibleForPreview = new HashSet<>();
+        Set<String> unseenCommentQuestions = new HashSet<>();
         Map<String, FeedbackResponseAttributes> relatedResponsesMap = new HashMap<>();
         Map<String, List<FeedbackResponseCommentAttributes>> relatedCommentsMap = new HashMap<>();
         if (isCourseWide) {
@@ -447,7 +431,7 @@ private SessionResultsBundle buildResultsBundle(
             // if previewing results and the comment should not be visible to instructors,
             // note down the corresponding question and do not add the comment
             if (isPreviewResults && !canInstructorsSeeComment(frc)) {
-                relatedQuestionsWithCommentNotVisibleForPreview.add(frc.getFeedbackQuestionId());
+                unseenCommentQuestions.add(frc.getFeedbackQuestionId());
                 continue;
             }
 
@@ -467,7 +451,7 @@ private SessionResultsBundle buildResultsBundle(
         RequestTracer.checkRemainingTime();
 
         return new SessionResultsBundle(relatedQuestionsMap, relatedQuestionsNotVisibleForPreviewMap,
-                relatedQuestionsWithCommentNotVisibleForPreview,
+                unseenCommentQuestions,
                 existingResponses, missingResponses, responseGiverVisibilityTable, responseRecipientVisibilityTable,
                 relatedCommentsMap, commentVisibilityTable, roster);
     }

src/main/java/teammates/logic/core/FeedbackSessionsLogic.java

@@ -19,8 +19,16 @@
 import teammates.common.util.Const;
 import teammates.common.util.Logger;
 import teammates.common.util.TimeHelper;
+import teammates.logic.api.EmailGenerator;
 import teammates.storage.api.FeedbackSessionsDb;
 
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+
 /**
  * Handles operations related to feedback sessions.
  *
@@ -52,6 +60,7 @@ public final class FeedbackSessionsLogic {
     private InstructorsLogic instructorsLogic;
     private StudentsLogic studentsLogic;
     private DeadlineExtensionsLogic deLogic;
+    private EmailGenerator emailGenerator;
 
     private FeedbackSessionsLogic() {
         // prevent initialization
@@ -575,7 +584,7 @@ private List<FeedbackSessionAttributes> getSoftDeletedFeedbackSessionsListForCou
      */
     public boolean isFeedbackSessionViewableToUserType(FeedbackSessionAttributes session, boolean isInstructor) {
         // Allow user to view the feedback session if there are questions for them
-        if (isFeedbackSessionForUserTypeToAnswer(session, isInstructor)) {
+        if (emailGenerator.isFeedbackSessionForUserTypeToAnswer(session, isInstructor)) {
             return true;
         }
 
@@ -595,19 +604,6 @@ public boolean isFeedbackSessionViewableToUserType(FeedbackSessionAttributes ses
         return session.isVisible() && !questionsWithVisibleResponses.isEmpty();
     }
 
-    /**
-     * Returns true if there are any questions for the specified user type (students/instructors) to answer.
-     */
-    public boolean isFeedbackSessionForUserTypeToAnswer(FeedbackSessionAttributes session, boolean isInstructor) {
-        if (!session.isVisible()) {
-            return false;
-        }
-
-        return isInstructor
-                ? fqLogic.hasFeedbackQuestionsForInstructors(session, false)
-                : fqLogic.hasFeedbackQuestionsForStudents(session);
-    }
-
     private void updateFeedbackSessionsDeadlinesWithNewEmail(String courseId, String oldEmailAddress,
             String newEmailAddress, boolean isInstructor) {
         if (oldEmailAddress.equals(newEmailAddress)) {

src/main/java/teammates/logic/core/FeedbackVariables.java

@@ -0,0 +1,17 @@
+package teammates.logic.core;
+
+public class FeedbackVariables {
+    protected FeedbackResponsesLogic frLogic;
+    protected FeedbackSessionsLogic fsLogic;
+    protected DeadlineExtensionsLogic deLogic;
+    protected FeedbackResponseCommentsLogic frcLogic;
+    public FeedbackQuestionsLogic fqLogic;
+
+    void initLogicDependencies() {
+        frLogic = FeedbackResponsesLogic.inst();
+        fsLogic = FeedbackSessionsLogic.inst();
+        deLogic = DeadlineExtensionsLogic.inst();
+        fqLogic = FeedbackQuestionsLogic.inst();
+        frcLogic = FeedbackResponseCommentsLogic.inst();
+    }
+}

src/main/java/teammates/logic/core/GiverTypeHandler.java

@@ -0,0 +1,9 @@
+package teammates.logic.core;
+
+import java.util.*;
+import teammates.common.datatransfer.CourseRoster;
+import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
+
+public interface GiverTypeHandler {
+    List<String> getPossibleGivers(FeedbackQuestionAttributes fqa, CourseRoster courseRoster, FeedbackSessionsLogic fsLogic);
+}

src/main/java/teammates/logic/core/GiversLogic.java

@@ -0,0 +1,35 @@
+package teammates.logic.core;
+
+import java.util.*;
+import java.util.stream.Collectors;
+
+import teammates.common.datatransfer.CourseRoster;
+import teammates.common.datatransfer.FeedbackParticipantType;
+import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
+import teammates.common.util.Logger;
+
+public class GiversLogic {
+    private Map<FeedbackParticipantType, GiverTypeHandler> giverTypeHandlerMap;
+    private static final Logger log = Logger.getLogger();
+
+    public GiversLogic() {
+        giverTypeHandlerMap = new HashMap<>();
+        giverTypeHandlerMap.put(FeedbackParticipantType.STUDENTS, new StudentsGiverTypeHandler());
+        giverTypeHandlerMap.put(FeedbackParticipantType.INSTRUCTORS, new InstructorsGiverTypeHandler());
+        giverTypeHandlerMap.put(FeedbackParticipantType.TEAMS, new TeamsGiverTypeHandler());
+        giverTypeHandlerMap.put(FeedbackParticipantType.SELF, new SelfGiverTypeHandler());
+    }
+
+    public List<String> getPossibleGivers(
+            FeedbackQuestionAttributes fqa,CourseRoster courseRoster, FeedbackSessionsLogic fsLogic) {
+        FeedbackParticipantType giverType = fqa.getGiverType();
+        GiverTypeHandler handler = giverTypeHandlerMap.get(giverType);
+
+        if (handler != null) {
+            return handler.getPossibleGivers(fqa, courseRoster, fsLogic);
+        } else {
+            log.severe("Invalid giver type specified");
+            return Collections.emptyList();
+        }
+    }
+}

src/main/java/teammates/logic/core/InstructorsGiverTypeHandler.java

@@ -0,0 +1,18 @@
+package teammates.logic.core;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import teammates.common.datatransfer.CourseRoster;
+import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
+import teammates.common.datatransfer.attributes.InstructorAttributes;
+
+public class InstructorsGiverTypeHandler implements GiverTypeHandler {
+    @Override
+    public List<String> getPossibleGivers(FeedbackQuestionAttributes fqa, CourseRoster courseRoster, FeedbackSessionsLogic fsLogic) {
+        return courseRoster.getInstructors()
+                .stream()
+                .map(InstructorAttributes::getEmail)
+                .collect(Collectors.toList());
+    }
+}

src/main/java/teammates/logic/core/InstructorsLogic.java

@@ -26,20 +26,14 @@
  * @see InstructorAttributes
  * @see InstructorsDb
  */
-public final class InstructorsLogic {
+public final class InstructorsLogic extends FeedbackVariables {
 
     private static final Logger log = Logger.getLogger();
 
     private static final InstructorsLogic instance = new InstructorsLogic();
 
     private final InstructorsDb instructorsDb = InstructorsDb.inst();
 
-    private FeedbackResponsesLogic frLogic;
-    private FeedbackResponseCommentsLogic frcLogic;
-    private FeedbackQuestionsLogic fqLogic;
-    private FeedbackSessionsLogic fsLogic;
-    private DeadlineExtensionsLogic deLogic;
-
     private InstructorsLogic() {
         // prevent initialization
     }
@@ -48,14 +42,6 @@ public static InstructorsLogic inst() {
         return instance;
     }
 
-    void initLogicDependencies() {
-        fqLogic = FeedbackQuestionsLogic.inst();
-        frLogic = FeedbackResponsesLogic.inst();
-        frcLogic = FeedbackResponseCommentsLogic.inst();
-        fsLogic = FeedbackSessionsLogic.inst();
-        deLogic = DeadlineExtensionsLogic.inst();
-    }
-
     /**
      * Creates or updates search document for the given instructor.
      *

src/main/java/teammates/logic/core/SelfGiverTypeHandler.java

@@ -0,0 +1,17 @@
+package teammates.logic.core;
+
+import java.util.Collections;
+import java.util.List;
+
+import teammates.common.datatransfer.CourseRoster;
+import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
+import teammates.common.datatransfer.attributes.FeedbackSessionAttributes;
+
+public class SelfGiverTypeHandler implements GiverTypeHandler {
+    @Override
+    public List<String> getPossibleGivers(FeedbackQuestionAttributes fqa, CourseRoster courseRoster, FeedbackSessionsLogic fsLogic) {
+        FeedbackSessionAttributes feedbackSession =
+                fsLogic.getFeedbackSession(fqa.getFeedbackSessionName(), fqa.getCourseId());
+        return Collections.singletonList(feedbackSession.getCreatorEmail());
+    }
+}

src/main/java/teammates/logic/core/StudentsGiverTypeHandler.java

@@ -0,0 +1,18 @@
+package teammates.logic.core;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import teammates.common.datatransfer.CourseRoster;
+import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
+import teammates.common.datatransfer.attributes.StudentAttributes;
+
+public class StudentsGiverTypeHandler implements GiverTypeHandler {
+    @Override
+    public List<String> getPossibleGivers(FeedbackQuestionAttributes fqa, CourseRoster courseRoster, FeedbackSessionsLogic fsLogic) {
+        return courseRoster.getStudents()
+                .stream()
+                .map(StudentAttributes::getEmail)
+                .collect(Collectors.toList());
+    }
+}

src/main/java/teammates/logic/core/StudentsLogic.java

@@ -24,7 +24,7 @@
  * @see StudentAttributes
  * @see StudentsDb
  */
-public final class StudentsLogic {
+public final class StudentsLogic extends FeedbackVariables {
 
     static final String ERROR_INVALID_TEAM_NAME =
             "Team \"%s\" is detected in both Section \"%s\" and Section \"%s\".";
@@ -39,9 +39,6 @@ public final class StudentsLogic {
 
     private final StudentsDb studentsDb = StudentsDb.inst();
 
-    private FeedbackResponsesLogic frLogic;
-    private FeedbackSessionsLogic fsLogic;
-    private DeadlineExtensionsLogic deLogic;
 
     private StudentsLogic() {
         // prevent initialization
@@ -51,12 +48,6 @@ public static StudentsLogic inst() {
         return instance;
     }
 
-    void initLogicDependencies() {
-        frLogic = FeedbackResponsesLogic.inst();
-        fsLogic = FeedbackSessionsLogic.inst();
-        deLogic = DeadlineExtensionsLogic.inst();
-    }
-
     /**
      * Creates a student.
      *