Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#11015] break down question result fetch by section #11567

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
fa27241
add: add result feedback fetch type by either giver, receiver, or both
moziliar Feb 13, 2022
9dce237
add: add result fetch type to response data path
moziliar Feb 13, 2022
48e080b
update: update frontend instructor session result fetch to include br…
moziliar Feb 13, 2022
cca6c6b
update: add test to feedback result fetch type and rename enum types
moziliar Feb 14, 2022
690d1d0
refactor: format change and var renaming
moziliar Feb 14, 2022
bca98bc
update: remove additional ArrayList allocation in getFeedbackResponse…
moziliar Feb 15, 2022
75ef133
nit: fix style
moziliar Feb 15, 2022
62f7ad8
update: remove costly removeDuplicate method altogether
moziliar Feb 15, 2022
b97f606
nit: fix lint
moziliar Feb 16, 2022
9309362
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
moziliar Feb 16, 2022
52521b8
test: add test at db level for fetch response by giver and receiver
moziliar Feb 18, 2022
3da1345
test: add test at logic level for fetch response by giver and receiver
moziliar Feb 18, 2022
c715632
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
moziliar Feb 19, 2022
d75f522
test: add test at logic level for fetch response by giver or receiver…
moziliar Feb 19, 2022
333b8fa
Merge branch '11015-break-down-question-result-fetch-by-section' of g…
moziliar Feb 19, 2022
482ba09
nit: fix lint
moziliar Feb 28, 2022
dde6cd7
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
jianhandev Feb 28, 2022
531c11c
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
moziliar Mar 3, 2022
b4708eb
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
moziliar Mar 3, 2022
76f06fc
lint: fix lint
moziliar Mar 3, 2022
bc8e2e9
update: format the code to be cleaner
moziliar Mar 6, 2022
3fba458
format: add back types to variables without type hints in context
moziliar Mar 14, 2022
80e9aaf
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
moziliar Mar 14, 2022
3f7f6ea
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
moziliar Mar 16, 2022
ba1bdf6
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
madanalogy Mar 16, 2022
f3c36e0
update: update result fetch recipient type to both
moziliar Apr 15, 2022
6b142f2
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
moziliar Apr 15, 2022
b7cb620
Merge branch 'master' into 11015-break-down-question-result-fetch-by-…
wkurniawan07 Apr 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package teammates.common.datatransfer;

/**
* The FeedbackResponse result fetching type to indicate whether the fetch is for giver only, receiver only or both.
*/
public enum FeedbackResultFetchType {
/**
* Fetch by giver only.
*/
GIVER(true, false),
/**
* Fetch by receiver only.
*/
RECEIVER(false, true),
/**
* Fetch by both giver and receiver.
*/
BOTH(true, true);

private final boolean isByGiver;
private final boolean isByReceiver;

FeedbackResultFetchType(boolean isByGiver, boolean isByReceiver) {
this.isByGiver = isByGiver;
this.isByReceiver = isByReceiver;
}

/**
* Parse the input string into a {@link FeedbackResultFetchType} and default to {@link FeedbackResultFetchType}.BOTH.
*/
public static FeedbackResultFetchType parseFetchType(String typeString) {
if (typeString == null) {
return BOTH;
}

switch (typeString.toLowerCase()) {
case "giver":
return GIVER;
case "receiver":
return RECEIVER;
default:
}

return BOTH;
}

/**
* This result fetch should be by giver.
*/
public boolean shouldFetchByGiver() {
return isByGiver;
}

/**
* This result fetch should be by receiver.
*/
public boolean shouldFetchByReceiver() {
return isByReceiver;
}
}
2 changes: 2 additions & 0 deletions src/main/java/teammates/common/util/Const.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ public static class ParamsNames {

public static final String FEEDBACK_RESULTS_GROUPBYSECTION = "frgroupbysection";

public static final String FEEDBACK_RESULTS_SECTION_BY_GIVER_RECEIVER = "frsessionbygiverreceiver";

public static final String PREVIEWAS = "previewas";

public static final String STUDENT_ID = "googleid";
Expand Down
8 changes: 5 additions & 3 deletions src/main/java/teammates/logic/api/Logic.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import javax.annotation.Nullable;

import teammates.common.datatransfer.DataBundle;
import teammates.common.datatransfer.FeedbackResultFetchType;
import teammates.common.datatransfer.NotificationTargetUser;
import teammates.common.datatransfer.SessionResultsBundle;
import teammates.common.datatransfer.attributes.AccountAttributes;
Expand Down Expand Up @@ -1248,17 +1249,18 @@ public Set<String> getGiverSetThatAnswerFeedbackSession(String courseId, String
/**
* Gets the session result for a feedback session.
*
* @see FeedbackResponsesLogic#getSessionResultsForCourse(String, String, String, String, String)
* @see FeedbackResponsesLogic#getSessionResultsForCourse(
* String, String, String, String, String, FeedbackResultFetchType)
*/
public SessionResultsBundle getSessionResultsForCourse(
String feedbackSessionName, String courseId, String userEmail,
@Nullable String questionId, @Nullable String section) {
@Nullable String questionId, @Nullable String section, @Nullable FeedbackResultFetchType fetchType) {
assert feedbackSessionName != null;
assert courseId != null;
assert userEmail != null;

return feedbackResponsesLogic.getSessionResultsForCourse(
feedbackSessionName, courseId, userEmail, questionId, section);
feedbackSessionName, courseId, userEmail, questionId, section, fetchType);
}

/**
Expand Down
18 changes: 11 additions & 7 deletions src/main/java/teammates/logic/core/FeedbackResponsesLogic.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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;
Expand Down Expand Up @@ -110,14 +111,16 @@ List<FeedbackResponseAttributes> getFeedbackResponsesForSession(
* @param feedbackSessionName the name if the session
* @param courseId the course ID of the session
* @param section if null, will retrieve all responses in the session
* @param fetchType if not null, will retrieve responses by giver, receiver sections, or both
* @return a list of responses
*/
public List<FeedbackResponseAttributes> getFeedbackResponsesForSessionInSection(
String feedbackSessionName, String courseId, @Nullable String section) {
String feedbackSessionName, String courseId, @Nullable String section,
@Nullable FeedbackResultFetchType fetchType) {
if (section == null) {
return getFeedbackResponsesForSession(feedbackSessionName, courseId);
}
return frDb.getFeedbackResponsesForSessionInSection(feedbackSessionName, courseId, section);
return frDb.getFeedbackResponsesForSessionInSection(feedbackSessionName, courseId, section, fetchType);
}

/**
Expand All @@ -142,11 +145,11 @@ public boolean areThereResponsesForQuestion(String feedbackQuestionId) {
* @return a list of responses
*/
public List<FeedbackResponseAttributes> getFeedbackResponsesForQuestionInSection(
String feedbackQuestionId, @Nullable String section) {
String feedbackQuestionId, @Nullable String section, FeedbackResultFetchType fetchType) {
if (section == null) {
return getFeedbackResponsesForQuestion(feedbackQuestionId);
}
return frDb.getFeedbackResponsesForQuestionInSection(feedbackQuestionId, section);
return frDb.getFeedbackResponsesForQuestionInSection(feedbackQuestionId, section, fetchType);
}

/**
Expand Down Expand Up @@ -445,11 +448,12 @@ private SessionResultsBundle buildResultsBundle(
* @param instructorEmail the instructor viewing the feedback session
* @param questionId if not null, will only return partial bundle for the question
* @param section if not null, will only return partial bundle for the section
* @param fetchType if not null, will fetch responses by giver, receiver sections, or both
* @return the session result bundle
*/
public SessionResultsBundle getSessionResultsForCourse(
String feedbackSessionName, String courseId, String instructorEmail,
@Nullable String questionId, @Nullable String section) {
@Nullable String questionId, @Nullable String section, @Nullable FeedbackResultFetchType fetchType) {
CourseRoster roster = new CourseRoster(
studentsLogic.getStudentsForCourse(courseId),
instructorsLogic.getInstructorsForCourse(courseId));
Expand All @@ -462,9 +466,9 @@ public SessionResultsBundle getSessionResultsForCourse(
List<FeedbackResponseAttributes> allResponses;
// load all response for instructors and passively filter them later
if (questionId == null) {
allResponses = getFeedbackResponsesForSessionInSection(feedbackSessionName, courseId, section);
allResponses = getFeedbackResponsesForSessionInSection(feedbackSessionName, courseId, section, fetchType);
} else {
allResponses = getFeedbackResponsesForQuestionInSection(questionId, section);
allResponses = getFeedbackResponsesForQuestionInSection(questionId, section, fetchType);
}
RequestTracer.checkRemainingTime();

Expand Down
76 changes: 38 additions & 38 deletions src/main/java/teammates/storage/api/FeedbackResponsesDb.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static com.googlecode.objectify.ObjectifyService.ofy;

import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -16,6 +15,7 @@
import com.googlecode.objectify.cmd.Query;

import teammates.common.datatransfer.AttributesDeletionQuery;
import teammates.common.datatransfer.FeedbackResultFetchType;
import teammates.common.datatransfer.attributes.FeedbackResponseAttributes;
import teammates.common.exception.EntityAlreadyExistsException;
import teammates.common.exception.EntityDoesNotExistException;
Expand Down Expand Up @@ -96,11 +96,12 @@ public FeedbackResponseAttributes getFeedbackResponse(
* Gets all feedback responses of a question in a specific section.
*/
public List<FeedbackResponseAttributes> getFeedbackResponsesForQuestionInSection(
String feedbackQuestionId, String section) {
String feedbackQuestionId, String section, FeedbackResultFetchType fetchType) {
assert feedbackQuestionId != null;
assert section != null;
assert fetchType != null;

return makeAttributes(getFeedbackResponseEntitiesForQuestionInSection(feedbackQuestionId, section));
return makeAttributes(getFeedbackResponseEntitiesForQuestionInSection(feedbackQuestionId, section, fetchType));
}

/**
Expand Down Expand Up @@ -138,14 +139,17 @@ public List<FeedbackResponseAttributes> getFeedbackResponsesForSession(

/**
* Gets all responses given to/from a section in a feedback session in a course.
* Optionally, retrieves by either giver, receiver sections, or both.
*/
public List<FeedbackResponseAttributes> getFeedbackResponsesForSessionInSection(
String feedbackSessionName, String courseId, String section) {
String feedbackSessionName, String courseId, String section, FeedbackResultFetchType fetchType) {
assert feedbackSessionName != null;
assert courseId != null;
assert section != null;
assert fetchType != null;

return makeAttributes(getFeedbackResponseEntitiesForSessionInSection(feedbackSessionName, courseId, section));
return makeAttributes(getFeedbackResponseEntitiesForSessionInSection(
feedbackSessionName, courseId, section, fetchType));
}

/**
Expand Down Expand Up @@ -322,19 +326,21 @@ private FeedbackResponse getFeedbackResponseEntity(String feedbackResponseId) {
}

private Collection<FeedbackResponse> getFeedbackResponseEntitiesForQuestionInSection(
String feedbackQuestionId, String section) {
List<FeedbackResponse> allResponses = new ArrayList<>();
String feedbackQuestionId, String section, FeedbackResultFetchType fetchType) {
Map<String, FeedbackResponse> allResponses = new HashMap<>();

allResponses.addAll(load()
.filter("feedbackQuestionId =", feedbackQuestionId)
.filter("giverSection =", section)
.list());
allResponses.addAll(load()
.filter("feedbackQuestionId =", feedbackQuestionId)
.filter("receiverSection =", section)
.list());
if (fetchType.shouldFetchByGiver()) {
load().filter("feedbackQuestionId =", feedbackQuestionId)
.filter("giverSection =", section)
.forEach(resp -> allResponses.put(resp.getId(), resp));
}
if (fetchType.shouldFetchByReceiver()) {
load().filter("feedbackQuestionId =", feedbackQuestionId)
.filter("receiverSection =", section)
.forEach(resp -> allResponses.put(resp.getId(), resp));
}

return removeDuplicates(allResponses);
return allResponses.values();
}

private List<FeedbackResponse> getFeedbackResponseEntitiesForQuestion(String feedbackQuestionId) {
Expand All @@ -351,30 +357,24 @@ private List<FeedbackResponse> getFeedbackResponseEntitiesForSession(String feed
}

private Collection<FeedbackResponse> getFeedbackResponseEntitiesForSessionInSection(
String feedbackSessionName, String courseId, String section) {
List<FeedbackResponse> allResponse = new ArrayList<>();

allResponse.addAll(load()
.filter("feedbackSessionName =", feedbackSessionName)
.filter("courseId =", courseId)
.filter("giverSection =", section)
.list());

allResponse.addAll(load()
.filter("feedbackSessionName =", feedbackSessionName)
.filter("courseId =", courseId)
.filter("receiverSection =", section)
.list());

return removeDuplicates(allResponse);
}
String feedbackSessionName, String courseId, String section, FeedbackResultFetchType fetchType) {
Map<String, FeedbackResponse> allResponse = new HashMap<>();

if (fetchType.shouldFetchByGiver()) {
load().filter("feedbackSessionName =", feedbackSessionName)
.filter("courseId =", courseId)
.filter("giverSection =", section)
.forEach(resp -> allResponse.put(resp.getId(), resp));
}

private Collection<FeedbackResponse> removeDuplicates(Collection<FeedbackResponse> responses) {
Map<String, FeedbackResponse> uniqueResponses = new HashMap<>();
for (FeedbackResponse response : responses) {
uniqueResponses.put(response.getId(), response);
if (fetchType.shouldFetchByReceiver()) {
load().filter("feedbackSessionName =", feedbackSessionName)
.filter("courseId =", courseId)
.filter("receiverSection =", section)
.forEach(resp -> allResponse.put(resp.getId(), resp));
}
return uniqueResponses.values();

return allResponse.values();
}

private List<FeedbackResponse> getFeedbackResponseEntitiesFromGiverForQuestion(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package teammates.ui.webapi;

import teammates.common.datatransfer.FeedbackResultFetchType;
import teammates.common.datatransfer.SessionResultsBundle;
import teammates.common.datatransfer.attributes.FeedbackSessionAttributes;
import teammates.common.datatransfer.attributes.InstructorAttributes;
Expand Down Expand Up @@ -61,6 +62,8 @@ public JsonResult execute() {
// Allow additional filter by question ID (equivalent to question number) and section name
String questionId = getRequestParamValue(Const.ParamsNames.FEEDBACK_QUESTION_ID);
String selectedSection = getRequestParamValue(Const.ParamsNames.FEEDBACK_RESULTS_GROUPBYSECTION);
FeedbackResultFetchType fetchType = FeedbackResultFetchType.parseFetchType(
getRequestParamValue(Const.ParamsNames.FEEDBACK_RESULTS_SECTION_BY_GIVER_RECEIVER));

SessionResultsBundle bundle;
InstructorAttributes instructor;
Expand All @@ -71,7 +74,7 @@ public JsonResult execute() {
instructor = logic.getInstructorForGoogleId(courseId, userInfo.id);

bundle = logic.getSessionResultsForCourse(feedbackSessionName, courseId, instructor.getEmail(),
questionId, selectedSection);
questionId, selectedSection, fetchType);

return new JsonResult(SessionResultsData.initForInstructor(bundle));
case INSTRUCTOR_RESULT:
Expand Down
Loading