[#11585] Fix unnecessary data read in expected submission count

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

@@ -144,16 +144,16 @@ private boolean areQuestionNumbersConsistent(List<FeedbackQuestionAttributes> qu
      * Checks if there are any questions for the given session that instructors can view/submit.
      */
     public boolean hasFeedbackQuestionsForInstructors(
-            String feedbackSessionName, String courseId, String userEmail) {
+            FeedbackSessionAttributes fsa, boolean isCreator) {
         boolean hasQuestions = fqDb.hasFeedbackQuestionsForGiverType(
-                feedbackSessionName, courseId, FeedbackParticipantType.INSTRUCTORS);
+                fsa.getFeedbackSessionName(), fsa.getCourseId(), FeedbackParticipantType.INSTRUCTORS);
         if (hasQuestions) {
             return true;
         }
 
-        if (userEmail != null && fsLogic.isCreatorOfSession(feedbackSessionName, courseId, userEmail)) {
+        if (isCreator) {
             hasQuestions = fqDb.hasFeedbackQuestionsForGiverType(
-                    feedbackSessionName, courseId, FeedbackParticipantType.SELF);
+                    fsa.getFeedbackSessionName(), fsa.getCourseId(), FeedbackParticipantType.SELF);
         }
 
         return hasQuestions;
@@ -202,10 +202,11 @@ public List<FeedbackQuestionAttributes> getFeedbackQuestionsForInstructors(
     /**
      * Checks if there are any questions for the given session that students can view/submit.
      */
-    public boolean hasFeedbackQuestionsForStudents(
-            String feedbackSessionName, String courseId) {
-        return fqDb.hasFeedbackQuestionsForGiverType(feedbackSessionName, courseId, FeedbackParticipantType.STUDENTS)
-                || fqDb.hasFeedbackQuestionsForGiverType(feedbackSessionName, courseId, FeedbackParticipantType.TEAMS);
+    public boolean hasFeedbackQuestionsForStudents(FeedbackSessionAttributes fsa) {
+        return fqDb.hasFeedbackQuestionsForGiverType(
+                fsa.getFeedbackSessionName(), fsa.getCourseId(), FeedbackParticipantType.STUDENTS)
+                || fqDb.hasFeedbackQuestionsForGiverType(
+                        fsa.getFeedbackSessionName(), fsa.getCourseId(), FeedbackParticipantType.TEAMS);
     }
 
     /**

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

@@ -10,7 +10,6 @@
 import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
 import teammates.common.datatransfer.attributes.FeedbackSessionAttributes;
 import teammates.common.datatransfer.attributes.InstructorAttributes;
-import teammates.common.datatransfer.attributes.StudentAttributes;
 import teammates.common.exception.EntityAlreadyExistsException;
 import teammates.common.exception.EntityDoesNotExistException;
 import teammates.common.exception.InvalidParametersException;
@@ -249,12 +248,8 @@ public boolean isFeedbackSessionAttemptedByInstructor(FeedbackSessionAttributes
             return true;
         }
 
-        String feedbackSessionName = fsa.getFeedbackSessionName();
-        String courseId = fsa.getCourseId();
-        List<FeedbackQuestionAttributes> allQuestions =
-                fqLogic.getFeedbackQuestionsForInstructors(feedbackSessionName, courseId, userEmail);
         // if there is no question for instructor, session is attempted
-        return allQuestions.isEmpty();
+        return !fqLogic.hasFeedbackQuestionsForInstructors(fsa, fsa.isCreator(userEmail));
     }
 
     /**
@@ -492,24 +487,31 @@ public void restoreFeedbackSessionFromRecycleBin(String feedbackSessionName, Str
      * Gets the expected number of submissions for a feedback session.
      */
     public int getExpectedTotalSubmission(FeedbackSessionAttributes fsa) {
-        List<StudentAttributes> students = studentsLogic.getStudentsForCourse(fsa.getCourseId());
-        List<InstructorAttributes> instructors = instructorsLogic.getInstructorsForCourse(fsa.getCourseId());
-        List<FeedbackQuestionAttributes> questions =
-                fqLogic.getFeedbackQuestionsForSession(fsa.getFeedbackSessionName(), fsa.getCourseId());
-        List<FeedbackQuestionAttributes> studentQns = fqLogic.getFeedbackQuestionsForStudents(questions);
-
         int expectedTotal = 0;
 
-        if (!studentQns.isEmpty()) {
-            expectedTotal += students.size();
+        if (fqLogic.hasFeedbackQuestionsForStudents(fsa)) {
+            expectedTotal += studentsLogic.getNumberOfStudentsForCourse(fsa.getCourseId());
         }
 
-        for (InstructorAttributes instructor : instructors) {
-            List<FeedbackQuestionAttributes> instructorQns =
-                    fqLogic.getFeedbackQuestionsForInstructors(questions, fsa.isCreator(instructor.getEmail()));
-            if (!instructorQns.isEmpty()) {
-                expectedTotal += 1;
-            }
+        // Pre-flight check to ensure there are questions for instructors.
+        if (!fqLogic.hasFeedbackQuestionsForInstructors(fsa, true)) {
+            return expectedTotal;
+        }
+
+        List<String> instructorEmails = instructorsLogic.getInstructorEmailsForCourse(fsa.getCourseId());
+        if (instructorEmails.isEmpty()) {
+            return expectedTotal;
+        }
+
+        // Check presence of questions for instructors.
+        if (fqLogic.hasFeedbackQuestionsForInstructors(fsa, false)) {
+            expectedTotal += instructorEmails.size();
+        } else {
+            // No questions for instructors. There must be questions for creator.
+            List<String> creatorEmails = instructorEmails.stream()
+                    .filter(fsa::isCreator)
+                    .collect(Collectors.toList());
+            expectedTotal += creatorEmails.size();
         }
 
         return expectedTotal;
@@ -566,8 +568,8 @@ public boolean isFeedbackSessionForUserTypeToAnswer(FeedbackSessionAttributes se
         }
 
         return isInstructor
-                ? fqLogic.hasFeedbackQuestionsForInstructors(session.getFeedbackSessionName(), session.getCourseId(), null)
-                : fqLogic.hasFeedbackQuestionsForStudents(session.getFeedbackSessionName(), session.getCourseId());
+                ? fqLogic.hasFeedbackQuestionsForInstructors(session, false)
+                : fqLogic.hasFeedbackQuestionsForStudents(session);
     }
 
 }

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

@@ -2,6 +2,7 @@
 
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.List;
 
 import teammates.common.datatransfer.AttributesDeletionQuery;
@@ -123,6 +124,16 @@ public InstructorAttributes getInstructorForRegistrationKey(String registrationK
         return instructorsDb.getInstructorForRegistrationKey(registrationKey);
     }
 
+    /**
+     * Gets emails of all instructors of a course.
+     */
+    public List<String> getInstructorEmailsForCourse(String courseId) {
+        List<String> instructorEmails = instructorsDb.getInstructorEmailsForCourse(courseId);
+        instructorEmails.sort(Comparator.naturalOrder());
+
+        return instructorEmails;
+    }
+
     /**
      * Gets all instructors of a course.
      */

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

@@ -99,6 +99,13 @@ public List<StudentAttributes> getStudentsForGoogleId(String googleId) {
         return studentsDb.getStudentsForGoogleId(googleId);
     }
 
+    /**
+     * Gets the total number of students of a course.
+     */
+    public int getNumberOfStudentsForCourse(String courseId) {
+        return studentsDb.getNumberOfStudentsForCourse(courseId);
+    }
+
     /**
      * Gets all students of a course.
      */

src/main/java/teammates/storage/api/InstructorsDb.java

@@ -147,6 +147,21 @@ public List<InstructorAttributes> getInstructorsForGoogleId(String googleId, boo
         return makeAttributes(getInstructorEntitiesForGoogleId(googleId, omitArchived));
     }
 
+    /**
+     * Gets the emails of all instructors of a course.
+     */
+    public List<String> getInstructorEmailsForCourse(String courseId) {
+        assert courseId != null;
+
+        return load()
+                .filter("courseId =", courseId)
+                .project("email")
+                .list()
+                .stream()
+                .map(Instructor::getEmail)
+                .collect(Collectors.toList());
+    }
+
     /**
      * Gets all instructors of a course.
      */

src/main/java/teammates/storage/api/StudentsDb.java

@@ -167,6 +167,15 @@ public List<StudentAttributes> getStudentsForGoogleId(String googleId) {
         return makeAttributes(getCourseStudentEntitiesForGoogleId(googleId));
     }
 
+    /**
+     * Gets the total number of students of a course.
+     */
+    public int getNumberOfStudentsForCourse(String courseId) {
+        assert courseId != null;
+
+        return getCourseStudentsForCourseQuery(courseId).count();
+    }
+
     /**
      * Gets all students of a course.
      */

src/test/java/teammates/logic/core/FeedbackQuestionsLogicTest.java

@@ -909,16 +909,16 @@ public void testBuildCompleteGiverRecipientMap_teamQuestion_shouldBuildMapCorrec
 
     private void testHasFeedbackQuestionsForInstructor() {
         ______TS("Valid session valid instructor should have questions");
-        assertTrue(fqLogic.hasFeedbackQuestionsForInstructors(
-                "First feedback session", "idOfTypicalCourse1", "instructor1@course1.tmt"));
+        FeedbackSessionAttributes fsa = fsLogic.getFeedbackSession("First feedback session", "idOfTypicalCourse1");
+        assertTrue(fqLogic.hasFeedbackQuestionsForInstructors(fsa, false));
 
         ______TS("Valid session without questions for instructor should return false");
-        assertFalse(fqLogic.hasFeedbackQuestionsForInstructors(
-                "session without instructor questions", "idOfTestingSanitizationCourse", "instructor1@course1.tmt"));
+        fsa = fsLogic.getFeedbackSession("session without instructor questions", "idOfArchivedCourse");
+        assertFalse(fqLogic.hasFeedbackQuestionsForInstructors(fsa, false));
 
         ______TS("Invalid session should not have questions");
-        assertFalse(fqLogic.hasFeedbackQuestionsForInstructors(
-                "Zeroth feedback session", "idOfTypicalCourse1", "nonexistent@course1.tmt"));
+        fsa.setFeedbackSessionName("non-existent session");
+        assertFalse(fqLogic.hasFeedbackQuestionsForInstructors(fsa, false));
     }
 
     private void testGetFeedbackQuestionsForInstructor() {
@@ -981,16 +981,16 @@ private void testGetFeedbackQuestionsForInstructor() {
 
     private void testHasFeedbackQuestionsForStudents() {
         ______TS("Valid session should have questions");
-        assertTrue(fqLogic.hasFeedbackQuestionsForStudents(
-                "First feedback session", "idOfTypicalCourse1"));
+        FeedbackSessionAttributes fsa = fsLogic.getFeedbackSession("First feedback session", "idOfTypicalCourse1");
+        assertTrue(fqLogic.hasFeedbackQuestionsForStudents(fsa));
 
         ______TS("Valid session without questions for students should return false");
-        assertFalse(fqLogic.hasFeedbackQuestionsForStudents(
-                "session without student questions", "idOfArchivedCourse"));
+        fsa = fsLogic.getFeedbackSession("session without student questions", "idOfArchivedCourse");
+        assertFalse(fqLogic.hasFeedbackQuestionsForStudents(fsa));
 
         ______TS("Invalid session should not have questions");
-        assertFalse(fqLogic.hasFeedbackQuestionsForStudents(
-                "Zeroth feedback session", "idOfTypicalCourse1"));
+        fsa.setFeedbackSessionName("non-existent session");
+        assertFalse(fqLogic.hasFeedbackQuestionsForStudents(fsa));
     }
 
     private void testGetFeedbackQuestionsForStudents() {

src/test/java/teammates/logic/core/StudentsLogicTest.java

@@ -51,6 +51,7 @@ public void testAll() throws Exception {
         testGetStudentForRegistrationKey();
         testGetStudentsForGoogleId();
         testGetStudentForCourseIdAndGoogleId();
+        testGetNumberOfStudentsForCourse();
         testGetStudentsForCourse();
         testIsStudentInAnyCourse();
         testIsStudentInTeam();
@@ -411,6 +412,31 @@ private void testGetStudentForCourseIdAndGoogleId() {
                 () -> studentsLogic.getStudentForCourseIdAndGoogleId("valid.course", null));
     }
 
+    private void testGetNumberOfStudentsForCourse() {
+
+        ______TS("course with multiple students");
+
+        CourseAttributes course1OfInstructor1 = dataBundle.courses.get("typicalCourse1");
+        int numOfStudents = studentsLogic.getNumberOfStudentsForCourse(course1OfInstructor1.getId());
+        assertEquals(5, numOfStudents);
+
+        ______TS("course with 0 students");
+
+        CourseAttributes course2OfInstructor1 = dataBundle.courses.get("courseNoEvals");
+        numOfStudents = studentsLogic.getNumberOfStudentsForCourse(course2OfInstructor1.getId());
+        assertEquals(0, numOfStudents);
+
+        ______TS("null parameter");
+
+        assertThrows(AssertionError.class, () -> studentsLogic.getNumberOfStudentsForCourse(null));
+
+        ______TS("non-existent course");
+
+        numOfStudents = studentsLogic.getNumberOfStudentsForCourse("non-existent");
+        assertEquals(0, numOfStudents);
+
+    }
+
     private void testGetStudentsForCourse() {
 
         ______TS("course with multiple students");

src/test/java/teammates/storage/api/InstructorsDbTest.java

@@ -210,6 +210,31 @@ public void testGetInstructorsForGoogleId() throws Exception {
 
     }
 
+    @Test
+    public void testGetInstructorEmailsForCourse() {
+
+        ______TS("Success: get instructors of a specific course");
+
+        String courseId = "idOfTypicalCourse1";
+
+        List<String> emails = instructorsDb.getInstructorEmailsForCourse(courseId);
+        List<InstructorAttributes> instructors = instructorsDb.getInstructorsForCourse(courseId);
+        assertEquals(5, emails.size());
+        assertEquals(5, instructors.size());
+        for (var instructor : instructors) {
+            assertTrue(emails.contains(instructor.getEmail()));
+        }
+
+        ______TS("Failure: no instructors for a course");
+
+        emails = instructorsDb.getInstructorEmailsForCourse("non-exist-course");
+        assertEquals(0, emails.size());
+
+        ______TS("Failure: null parameters");
+
+        assertThrows(AssertionError.class, () -> instructorsDb.getInstructorEmailsForCourse(null));
+    }
+
     @Test
     public void testGetInstructorsForCourse() {
 

src/test/java/teammates/storage/api/StudentsDbTest.java

@@ -344,7 +344,7 @@ public void testDeleteStudent() throws Exception {
                         .build());
 
         // should pass, others students remain
-        assertEquals(1, studentsDb.getStudentsForCourse(s.getCourse()).size());
+        assertEquals(1, studentsDb.getNumberOfStudentsForCourse(s.getCourse()));
 
         // delete all students in a course
 
@@ -361,16 +361,16 @@ public void testDeleteStudent() throws Exception {
         assertNotNull(studentsDb.getStudentForEmail(anotherStudent.getCourse(), anotherStudent.getEmail()));
 
         // there are students in the course
-        assertFalse(studentsDb.getStudentsForCourse(s.getCourse()).isEmpty());
+        assertNotEquals(0, studentsDb.getNumberOfStudentsForCourse(s.getCourse()));
 
         studentsDb.deleteStudents(
                 AttributesDeletionQuery.builder()
                         .withCourseId(s.getCourse())
                         .build());
 
-        assertEquals(0, studentsDb.getStudentsForCourse(s.getCourse()).size());
+        assertEquals(0, studentsDb.getNumberOfStudentsForCourse(s.getCourse()));
         // other course should remain
-        assertEquals(1, studentsDb.getStudentsForCourse(anotherStudent.getCourse()).size());
+        assertEquals(1, studentsDb.getNumberOfStudentsForCourse(anotherStudent.getCourse()));
 
         // clean up
         studentsDb.deleteStudent(anotherStudent.getCourse(), anotherStudent.getEmail());