Skip to content

Commit

Permalink
Merge branch 'master' into 6238-appveyor
Browse files Browse the repository at this point in the history
  • Loading branch information
@wkurniawan07
wkurniawan07 committed Jan 31, 2017
2 parents 4d5ef12 + 2d5e13f commit 4574aab
Show file tree
Hide file tree
Showing 85 changed files with 655 additions and 1,307 deletions.
2 changes: 1 addition & 1 deletion src/main/java/teammates/common/datatransfer/FeedbackQuestionAttributes.java
View file
Expand Up @@ -287,7 +287,7 @@ public boolean isResponseVisibleTo(FeedbackParticipantType userType) {
* @param newAttributes
* @return
*/
public boolean isChangesRequiresResponseDeletion(FeedbackQuestionAttributes newAttributes) {
public boolean areResponseDeletionsRequiredForChanges(FeedbackQuestionAttributes newAttributes) {
if (!newAttributes.giverType.equals(this.giverType)
|| !newAttributes.recipientType.equals(this.recipientType)) {
return true;
Expand Down
26 changes: 12 additions & 14 deletions src/main/java/teammates/common/util/ActivityLogEntry.java
View file
Expand Up @@ -12,7 +12,6 @@
import teammates.common.datatransfer.StudentAttributes;
import teammates.common.datatransfer.UserType;
import teammates.common.exception.TeammatesException;
import teammates.logic.api.GateKeeper;

import com.google.appengine.api.log.AppLogLine;

Expand All @@ -39,7 +38,7 @@ public class ActivityLogEntry {
private static final int TIME_TAKEN_DANGER_UPPER_RANGE = 60000;

private static final Logger log = Logger.getLogger();

private long time;
private String servletName;
private String action; //TODO: remove if not needed (and rename servletName to action)
Expand All @@ -56,7 +55,6 @@ public class ActivityLogEntry {
// or <studentemail>%<courseId>%<time> (for unregistered students)
// e.g. bamboo@gmail.tmt%instructor.ema-demo%20151103170618465
private String id;

private boolean isFirstRow;

@SuppressWarnings("unused") // used by js
Expand Down Expand Up @@ -107,18 +105,19 @@ public ActivityLogEntry(AppLogLine appLog) {
* Constructor that creates an ActivityLog object from scratch
* Used in the various servlets in the application
*/
public ActivityLogEntry(String servlet, String act, AccountAttributes acc, String params, String link) {
this(servlet, act, acc, params, link, null, null);
public ActivityLogEntry(String servlet, String act, AccountAttributes acc, String params, String link,
UserType userType) {
this(servlet, act, acc, params, link, null, null, userType);
}

/**
* Constructs a ActivityLogEntry.
* The googleId in the log will be based on the {@code acc} passed in, otherwise it is obtained from the GateKeeper.
* The googleId in the log will be based on the {@code acc} or {@code userType} passed in
* For the log id, if the googleId is unknown, the {@code unregisteredUserCourse} and {@code unregisteredUserEmail}
* will be used to construct the id.
*/
public ActivityLogEntry(String servlet, String act, AccountAttributes acc, String params, String link,
String unregisteredUserCourse, String unregisteredUserEmail) {
String unregisteredUserCourse, String unregisteredUserEmail, UserType userType) {
time = System.currentTimeMillis();
servletName = servlet;
action = act;
Expand All @@ -130,8 +129,6 @@ public ActivityLogEntry(String servlet, String act, AccountAttributes acc, Strin
role = "Unknown";
name = "Unknown";
email = "Unknown";

UserType userType = new GateKeeper().getCurrentUser();
googleId = userType == null ? "Unknown" : userType.id;

} else {
Expand Down Expand Up @@ -491,7 +488,7 @@ public String generateLogId(String googleId, String email, String course, long t
: googleId + "%" + formatTimeForId(new Date(time));
}

public static String generateServletActionFailureLogMessage(HttpServletRequest req, Exception e) {
public static String generateServletActionFailureLogMessage(HttpServletRequest req, Exception e, UserType userType) {
String[] actionTaken = req.getServletPath().split("/");
String action = req.getServletPath();
if (actionTaken.length > 0) {
Expand All @@ -506,12 +503,13 @@ public static String generateServletActionFailureLogMessage(HttpServletRequest r
String courseId = HttpRequestHelper.getValueFromRequestParameterMap(req, Const.ParamsNames.COURSE_ID);
String studentEmail = HttpRequestHelper.getValueFromRequestParameterMap(req, Const.ParamsNames.STUDENT_EMAIL);
ActivityLogEntry exceptionLog = new ActivityLogEntry(action, Const.ACTION_RESULT_FAILURE, null, message,
url, courseId, studentEmail);
url, courseId, studentEmail, userType);

return exceptionLog.generateLogMessage();
}

public static String generateSystemErrorReportLogMessage(HttpServletRequest req, EmailWrapper errorEmail) {
public static String generateSystemErrorReportLogMessage(HttpServletRequest req, EmailWrapper errorEmail,
UserType userType) {
String[] actionTaken = req.getServletPath().split("/");
String action = req.getServletPath();
if (actionTaken.length > 0) {
Expand Down Expand Up @@ -540,7 +538,7 @@ public static String generateSystemErrorReportLogMessage(HttpServletRequest req,
String studentEmail = HttpRequestHelper.getValueFromRequestParameterMap(req, Const.ParamsNames.STUDENT_EMAIL);

ActivityLogEntry emailReportLog = new ActivityLogEntry(action, Const.ACTION_RESULT_SYSTEM_ERROR_REPORT, null,
message, url, courseId, studentEmail);
message, url, courseId, studentEmail, userType);

return emailReportLog.generateLogMessage();
}
Expand Down Expand Up @@ -628,6 +626,6 @@ public void setFirstRow() {

public boolean isTestingData() {
return email.endsWith(".tmt");

}

}
4 changes: 2 additions & 2 deletions src/main/java/teammates/logic/api/Logic.java
View file
Expand Up @@ -1591,8 +1591,8 @@ public void deleteFeedbackQuestion(String questionId) {
* * All parameters are non-null.
*/

public boolean isQuestionHasResponses(String feedbackQuestionId) {
return feedbackQuestionsLogic.isQuestionHasResponses(feedbackQuestionId);
public boolean areThereResponsesForQuestion(String feedbackQuestionId) {
return feedbackQuestionsLogic.areThereResponsesForQuestion(feedbackQuestionId);
}

/**
Expand Down
86 changes: 37 additions & 49 deletions src/main/java/teammates/logic/core/FeedbackQuestionsLogic.java
View file
Expand Up @@ -283,8 +283,8 @@ public List<FeedbackQuestionAttributes> getFeedbackQuestionsForCreatorInstructor
}

/**
* Gets a {@code List} of all questions for the list of questions that an
* instructor can view/submit
* Filters through the given list of questions and returns a {@code List} of
* questions that an instructor can view/submit
*/
public List<FeedbackQuestionAttributes> getFeedbackQuestionsForInstructor(
List<FeedbackQuestionAttributes> allQuestions, boolean isCreator) {
Expand Down Expand Up @@ -327,8 +327,8 @@ public List<FeedbackQuestionAttributes> getFeedbackQuestionsForStudents(


/**
* Gets a {@code List} of all questions from the given list of questions
* that students can view/submit
* Filters through the given list of questions and returns a {@code List} of
* questions that students can view/submit
*/
public List<FeedbackQuestionAttributes> getFeedbackQuestionsForStudents(
List<FeedbackQuestionAttributes> allQuestions) {
Expand Down Expand Up @@ -364,17 +364,7 @@ public Map<String, String> getRecipientsForQuestion(

FeedbackParticipantType recipientType = question.recipientType;

String giverTeam = null;

boolean isStudentGiver = studentGiver != null;
boolean isInstructorGiver = instructorGiver != null;
if (isStudentGiver) {
giverTeam = studentGiver.team;
} else if (isInstructorGiver) {
giverTeam = Const.USER_TEAM_FOR_INSTRUCTOR;
} else {
giverTeam = giver;
}
String giverTeam = getGiverTeam(giver, instructorGiver, studentGiver);

switch (recipientType) {
case SELF:
Expand Down Expand Up @@ -438,8 +428,21 @@ public Map<String, String> getRecipientsForQuestion(
}
return recipients;
}

private String getGiverTeam(String defaultTeam, InstructorAttributes instructorGiver,
StudentAttributes studentGiver) {
String giverTeam = defaultTeam;
boolean isStudentGiver = studentGiver != null;
boolean isInstructorGiver = instructorGiver != null;
if (isStudentGiver) {
giverTeam = studentGiver.team;
} else if (isInstructorGiver) {
giverTeam = Const.USER_TEAM_FOR_INSTRUCTOR;
}
return giverTeam;
}

public boolean isQuestionHasResponses(String feedbackQuestionId) {
public boolean areThereResponsesForQuestion(String feedbackQuestionId) {
return !frLogic.getFeedbackResponsesForQuestionWithinRange(feedbackQuestionId, 1)
.isEmpty();
}
Expand All @@ -456,7 +459,7 @@ public boolean isQuestionFullyAnsweredByUser(FeedbackQuestionAttributes question
numberOfResponsesNeeded = getRecipientsForQuestion(question, email).size();
}

return numberOfResponsesGiven >= numberOfResponsesNeeded ? true : false;
return numberOfResponsesGiven >= numberOfResponsesNeeded;
}

/**
Expand All @@ -472,23 +475,23 @@ public boolean isQuestionFullyAnsweredByTeam(FeedbackQuestionAttributes question
List<StudentAttributes> studentsInTeam =
studentsLogic.getStudentsForTeam(question.courseId, teamName);

int numberOfResponsesNeeded =
int numberOfPendingResponses =
question.numberOfEntitiesToGiveFeedbackTo;

if (numberOfResponsesNeeded == Const.MAX_POSSIBLE_RECIPIENTS) {
numberOfResponsesNeeded = getRecipientsForQuestion(question, teamName).size();
if (numberOfPendingResponses == Const.MAX_POSSIBLE_RECIPIENTS) {
numberOfPendingResponses = getRecipientsForQuestion(question, teamName).size();
}

for (StudentAttributes student : studentsInTeam) {
List<FeedbackResponseAttributes> responses =
frLogic.getFeedbackResponsesFromGiverForQuestion(question.getId(), student.email);
for (FeedbackResponseAttributes response : responses) {
if (response.giver.equals(student.email)) {
numberOfResponsesNeeded -= 1;
numberOfPendingResponses -= 1;
}
}
}
return numberOfResponsesNeeded <= 0 ? true : false;
return numberOfPendingResponses <= 0;
}


Expand Down Expand Up @@ -533,30 +536,17 @@ public void updateFeedbackQuestionNumber(FeedbackQuestionAttributes newQuestion)
*/
private void adjustQuestionNumbers(int oldQuestionNumber,
int newQuestionNumber, List<FeedbackQuestionAttributes> questions) {

if (oldQuestionNumber > newQuestionNumber && oldQuestionNumber >= 1) {
for (int i = oldQuestionNumber - 1; i >= newQuestionNumber; i--) {
FeedbackQuestionAttributes question = questions.get(i - 1);
question.questionNumber += 1;
try {
updateFeedbackQuestionWithoutResponseRateUpdate(question);
} catch (InvalidParametersException e) {
Assumption.fail("Invalid question.");
} catch (EntityDoesNotExistException e) {
Assumption.fail("Question disappeared.");
}
updateFeedbackQuestionWithoutResponseRateUpdate(question);
}
} else if (oldQuestionNumber < newQuestionNumber && oldQuestionNumber < questions.size()) {
for (int i = oldQuestionNumber + 1; i <= newQuestionNumber; i++) {
FeedbackQuestionAttributes question = questions.get(i - 1);
question.questionNumber -= 1;
try {
updateFeedbackQuestionWithoutResponseRateUpdate(question);
} catch (InvalidParametersException e) {
Assumption.fail("Invalid question.");
} catch (EntityDoesNotExistException e) {
Assumption.fail("Question disappeared.");
}
updateFeedbackQuestionWithoutResponseRateUpdate(question);
}
}
}
Expand All @@ -571,10 +561,14 @@ private void adjustQuestionNumbers(int oldQuestionNumber,
* Precondition: <br>
* {@code newAttributes} is not {@code null}
*/
private void updateFeedbackQuestionWithoutResponseRateUpdate(FeedbackQuestionAttributes newAttributes)
throws InvalidParametersException, EntityDoesNotExistException {

updateFeedbackQuestion(newAttributes, false);
private void updateFeedbackQuestionWithoutResponseRateUpdate(FeedbackQuestionAttributes newAttributes) {
try {
updateFeedbackQuestion(newAttributes, false);
} catch (InvalidParametersException e) {
Assumption.fail("Invalid question.");
} catch (EntityDoesNotExistException e) {
Assumption.fail("Question disappeared.");
}
}

/**
Expand Down Expand Up @@ -608,7 +602,7 @@ private void updateFeedbackQuestion(FeedbackQuestionAttributes newAttributes, bo
"Trying to update a feedback question that does not exist.");
}

if (oldQuestion.isChangesRequiresResponseDeletion(newAttributes)) {
if (oldQuestion.areResponseDeletionsRequiredForChanges(newAttributes)) {
frLogic.deleteFeedbackResponsesForQuestionAndCascade(oldQuestion.getId(), hasResponseRateUpdate);
}

Expand Down Expand Up @@ -723,13 +717,7 @@ private void shiftQuestionNumbersDown(int questionNumberToShiftFrom,
for (FeedbackQuestionAttributes question : questionsToShift) {
if (question.questionNumber > questionNumberToShiftFrom) {
question.questionNumber -= 1;
try {
updateFeedbackQuestionWithoutResponseRateUpdate(question);
} catch (InvalidParametersException e) {
Assumption.fail("Invalid question.");
} catch (EntityDoesNotExistException e) {
Assumption.fail("Question disappeared.");
}
updateFeedbackQuestionWithoutResponseRateUpdate(question);
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/teammates/ui/automated/AutomatedServlet.java
View file
Expand Up @@ -5,9 +5,11 @@
import javax.servlet.http.HttpServletResponse;

import teammates.common.exception.TeammatesException;
import teammates.common.datatransfer.UserType;
import teammates.common.util.ActivityLogEntry;
import teammates.common.util.HttpRequestHelper;
import teammates.common.util.Logger;
import teammates.logic.api.GateKeeper;

/**
* Receives automated requests from the App Engine server and executes the matching automated action.
Expand All @@ -26,12 +28,13 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) {
public void doPost(HttpServletRequest req, HttpServletResponse resp) {
try {
AutomatedAction action = new AutomatedActionFactory().getAction(req, resp);
UserType userType = new GateKeeper().getCurrentUser();

String url = HttpRequestHelper.getRequestedUrl(req);
// Do not log task queue worker actions to prevent excessive logging
if (!url.startsWith("/worker/")) {
ActivityLogEntry activityLogEntry = new ActivityLogEntry(
url, action.getActionDescription(), null, action.getActionMessage(), url);
url, action.getActionDescription(), null, action.getActionMessage(), url, userType);
log.info(activityLogEntry.generateLogMessage());
}

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/teammates/ui/automated/FeedbackResponseAdjustmentWorkerAction.java
View file
Expand Up @@ -5,9 +5,11 @@
import teammates.common.datatransfer.FeedbackResponseAttributes;
import teammates.common.datatransfer.FeedbackSessionAttributes;
import teammates.common.datatransfer.StudentEnrollDetails;
import teammates.common.datatransfer.UserType;
import teammates.common.util.ActivityLogEntry;
import teammates.common.util.Assumption;
import teammates.common.util.Const.ParamsNames;
import teammates.logic.api.GateKeeper;
import teammates.common.util.JsonUtils;

import com.google.gson.reflect.TypeToken;
Expand Down Expand Up @@ -60,8 +62,9 @@ public void execute() {
try {
logic.adjustFeedbackResponseForEnrollments(enrollmentList, response);
} catch (Exception e) {
UserType userType = new GateKeeper().getCurrentUser();
log.severe(String.format(errorString, sessionName, courseId, e.getMessage(),
ActivityLogEntry.generateServletActionFailureLogMessage(request, e)));
ActivityLogEntry.generateServletActionFailureLogMessage(request, e, userType)));
setForRetry();
return;
}
Expand Down
13 changes: 7 additions & 6 deletions src/main/java/teammates/ui/controller/ControllerServlet.java
View file
Expand Up @@ -46,6 +46,8 @@ public final void doGet(HttpServletRequest req, HttpServletResponse resp) throws
@Override
@SuppressWarnings("PMD.AvoidCatchingThrowable") // used as fallback
public final void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {

UserType userType = new GateKeeper().getCurrentUser();

try {
/* We are using the Template Method Design Pattern here.
Expand Down Expand Up @@ -73,22 +75,22 @@ public final void doPost(HttpServletRequest req, HttpServletResponse resp) throw
log.info(c.getLogMessage() + "|||" + timeTaken);

} catch (PageNotFoundException e) {
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e));
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e, userType));
cleanUpStatusMessageInSession(req);
resp.sendRedirect(Const.ViewURIs.ACTION_NOT_FOUND_PAGE);
} catch (EntityNotFoundException e) {
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e));
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e, userType));
cleanUpStatusMessageInSession(req);
resp.sendRedirect(Const.ViewURIs.ENTITY_NOT_FOUND_PAGE);

} catch (FeedbackSessionNotVisibleException e) {
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e));
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e, userType));
cleanUpStatusMessageInSession(req);
req.getSession().setAttribute(Const.ParamsNames.FEEDBACK_SESSION_NOT_VISIBLE, e.getStartTimeString());
resp.sendRedirect(Const.ViewURIs.FEEDBACK_SESSION_NOT_VISIBLE);

} catch (UnauthorizedAccessException e) {
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e));
log.warning(ActivityLogEntry.generateServletActionFailureLogMessage(req, e, userType));
cleanUpStatusMessageInSession(req);
resp.sendRedirect(Const.ViewURIs.UNAUTHORIZED);

Expand Down Expand Up @@ -127,14 +129,13 @@ public final void doPost(HttpServletRequest req, HttpServletResponse resp) throw
String requestPath = req.getServletPath();
String requestUrl = req.getRequestURL().toString();
String requestParams = HttpRequestHelper.printRequestParameters(req);
UserType userType = new GateKeeper().getCurrentUser();

EmailWrapper errorReport =
new EmailGenerator().generateSystemErrorEmail(requestMethod, requestUserAgent, requestPath,
requestUrl, requestParams, userType, t);
new EmailSender().sendReport(errorReport);
if (errorReport != null) {
log.severe(ActivityLogEntry.generateSystemErrorReportLogMessage(req, errorReport));
log.severe(ActivityLogEntry.generateSystemErrorReportLogMessage(req, errorReport, userType));
}

cleanUpStatusMessageInSession(req);
Expand Down

0 comments on commit 4574aab

Please sign in to comment.